Skip to content

Commit 11123c3

Browse files
devversiondevversion
committed
fix: do not silently fail puppeteer testing
We currently catch errors for Puppeteer logic (screenshot, runtime error collection, CSP testing etc.). The error message logged is actually wrongly saying something about screenshots. In addition, we don't even report the error back to the worker "host". This means that the error is simply swallowed and there is empty serve testing result. We should surface the error properly by removing the `try/catch` and letting the try/catch in `worker.ts` catch the error --> so that it sets the `serveTestingResult#errorMessage`.
1 parent ea4be92 commit 11123c3

File tree

1 file changed

+119
-126
lines changed

1 file changed

+119
-126
lines changed

‎runner/workers/serve-testing/puppeteer.ts‎

Lines changed: 119 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -28,149 +28,142 @@ export async function runAppInPuppeteer(
2828
let screenshotBase64Data: string | undefined;
2929
let axeViolations: AxeResult[] | undefined;
3030
let lighthouseResult: LighthouseResult | undefined;
31+
let unexpectedErrorMessage: string | undefined;
3132

32-
try {
33-
const browser = await puppeteer.launch({
34-
headless: true,
35-
args: [
36-
'--no-sandbox',
37-
'--disable-setuid-sandbox',
38-
'--disable-dev-shm-usage',
39-
'--disable-gpu',
40-
],
41-
});
42-
const page = await browser.newPage();
33+
const browser = await puppeteer.launch({
34+
headless: true,
35+
args: ['--no-sandbox', '--disable-setuid-sandbox', '--disable-dev-shm-usage', '--disable-gpu'],
36+
});
37+
const page = await browser.newPage();
4338

44-
page.on('console', async message => {
45-
if (message.type() !== 'error') return;
39+
page.on('console', async message => {
40+
if (message.type() !== 'error') return;
4641

47-
if (!message.text().includes('JSHandle@error')) {
48-
progressLog(
49-
'error',
50-
`Runtime Error: ${message.type().substring(0, 3).toUpperCase()} ${message.text()}`,
51-
);
42+
if (!message.text().includes('JSHandle@error')) {
43+
progressLog(
44+
'error',
45+
`Runtime Error: ${message.type().substring(0, 3).toUpperCase()} ${message.text()}`,
46+
);
47+
return;
48+
}
49+
const messages = await Promise.all(
50+
message.args().map(async arg => {
51+
const [message, stack] = await Promise.all([
52+
arg.getProperty('message'),
53+
arg.getProperty('stack'),
54+
]);
55+
56+
let result = '';
57+
if (message) {
58+
result += message;
59+
}
60+
if (stack) {
61+
result += (result.length ? '\n\n' : '') + stack;
62+
}
63+
return result;
64+
}),
65+
);
66+
runtimeErrors.push(messages.filter(Boolean).join('\n'));
67+
});
68+
69+
page.on('pageerror', error => {
70+
progressLog('error', 'Page error', error.message);
71+
runtimeErrors.push(error.toString());
72+
});
73+
74+
await page.setViewport({width: 1280, height: 720});
75+
76+
// Set up auto-CSP handling if enabled for the environment.
77+
if (enableAutoCsp) {
78+
const autoCsp = new AutoCsp();
79+
await autoCsp.connectToDevTools(page);
80+
await page.setRequestInterception(true);
81+
page.on('request', async request => {
82+
if (request.isInterceptResolutionHandled()) {
5283
return;
5384
}
54-
const messages = await Promise.all(
55-
message.args().map(async arg => {
56-
const [message, stack] = await Promise.all([
57-
arg.getProperty('message'),
58-
arg.getProperty('stack'),
59-
]);
60-
61-
let result = '';
62-
if (message) {
63-
result += message;
64-
}
65-
if (stack) {
66-
result += (result.length ? '\n\n' : '') + stack;
67-
}
68-
return result;
69-
}),
70-
);
71-
runtimeErrors.push(messages.filter(Boolean).join('\n'));
72-
});
7385

74-
page.on('pageerror', error => {
75-
progressLog('error', 'Page error', error.message);
76-
runtimeErrors.push(error.toString());
86+
// Delegate CSP-related requests to the AutoCsp class
87+
const handled = await autoCsp.handleRequest(request);
88+
if (!handled) {
89+
// Other requests (CSS, JS, images) pass through
90+
await request.continue();
91+
}
7792
});
7893

79-
await page.setViewport({width: 1280, height: 720});
80-
81-
// Set up auto-CSP handling if enabled for the environment.
82-
if (enableAutoCsp) {
83-
const autoCsp = new AutoCsp();
84-
await autoCsp.connectToDevTools(page);
85-
await page.setRequestInterception(true);
86-
page.on('request', async request => {
87-
if (request.isInterceptResolutionHandled()) {
88-
return;
89-
}
94+
await page.goto(hostUrl, {
95+
waitUntil: 'networkidle0',
96+
timeout: 30000,
97+
});
9098

91-
// Delegate CSP-related requests to the AutoCsp class
92-
const handled = await autoCsp.handleRequest(request);
93-
if (!handled) {
94-
// Other requests (CSS, JS, images) pass through
95-
await request.continue();
96-
}
97-
});
98-
99-
await page.goto(hostUrl, {
100-
waitUntil: 'networkidle0',
101-
timeout: 30000,
102-
});
103-
104-
// Now that the page is loaded, process the collected CSP reports.
105-
autoCsp.processViolations();
106-
cspViolations = autoCsp.violations;
107-
} else {
108-
// If CSP is not enabled, just navigate to the page directly.
109-
await page.goto(hostUrl, {
110-
waitUntil: 'networkidle0',
111-
timeout: 30000,
112-
});
113-
}
99+
// Now that the page is loaded, process the collected CSP reports.
100+
autoCsp.processViolations();
101+
cspViolations = autoCsp.violations;
102+
} else {
103+
// If CSP is not enabled, just navigate to the page directly.
104+
await page.goto(hostUrl, {
105+
waitUntil: 'networkidle0',
106+
timeout: 30000,
107+
});
108+
}
114109

115-
// Perform Axe Testing
116-
if (includeAxeTesting) {
117-
try {
118-
progressLog('eval', `Running Axe accessibility test from ${hostUrl}`);
119-
const axeResults = await new AxePuppeteer(page).analyze();
120-
axeViolations = axeResults.violations;
121-
progressLog('success', `Axe accessibility test completed.`);
122-
123-
if (axeViolations.length > 0) {
124-
progressLog('error', `Found ${axeViolations.length} Axe violations.`);
125-
} else {
126-
progressLog('success', `No Axe violations found.`);
127-
}
128-
} catch (axeError: any) {
129-
progressLog('error', 'Could not perform Axe accessibility test', axeError.message);
110+
// Perform Axe Testing
111+
if (includeAxeTesting) {
112+
try {
113+
progressLog('eval', `Running Axe accessibility test from ${hostUrl}`);
114+
const axeResults = await new AxePuppeteer(page).analyze();
115+
axeViolations = axeResults.violations;
116+
progressLog('success', `Axe accessibility test completed.`);
117+
118+
if (axeViolations.length > 0) {
119+
progressLog('error', `Found ${axeViolations.length} Axe violations.`);
120+
} else {
121+
progressLog('success', `No Axe violations found.`);
130122
}
123+
} catch (axeError: any) {
124+
progressLog('error', 'Could not perform Axe accessibility test', axeError.message);
131125
}
126+
}
132127

133-
if (takeScreenshots) {
134-
progressLog('eval', `Taking screenshot from ${hostUrl}`);
135-
136-
screenshotBase64Data = await callWithTimeout(
137-
`Taking screenshot for ${appName}`,
138-
() =>
139-
page.screenshot({
140-
type: 'png',
141-
fullPage: true,
142-
encoding: 'base64',
143-
}),
144-
1, // 1 minute
145-
);
146-
progressLog('success', 'Screenshot captured and encoded');
147-
}
148-
149-
if (includeLighthouseData) {
150-
try {
151-
progressLog('eval', `Gathering Lighthouse data from ${hostUrl}`);
152-
lighthouseResult = await getLighthouseData(hostUrl, page);
128+
if (takeScreenshots) {
129+
progressLog('eval', `Taking screenshot from ${hostUrl}`);
153130

154-
if (lighthouseResult) {
155-
progressLog('success', 'Lighthouse data has been collected');
156-
} else {
157-
progressLog('error', 'Lighthouse did not produce usable data');
158-
}
159-
} catch (lighthouseError: any) {
160-
progressLog('error', 'Could not gather Lighthouse data', lighthouseError.message);
161-
}
162-
}
131+
screenshotBase64Data = await callWithTimeout(
132+
`Taking screenshot for ${appName}`,
133+
() =>
134+
page.screenshot({
135+
type: 'png',
136+
fullPage: true,
137+
encoding: 'base64',
138+
}),
139+
1, // 1 minute
140+
);
141+
progressLog('success', 'Screenshot captured and encoded');
142+
}
163143

164-
await browser.close();
165-
} catch (screenshotError: any) {
166-
let details: string = screenshotError.message;
144+
if (includeLighthouseData) {
145+
try {
146+
progressLog('eval', `Gathering Lighthouse data from ${hostUrl}`);
147+
lighthouseResult = await getLighthouseData(hostUrl, page);
167148

168-
if (screenshotError.stack) {
169-
details += '\n' + screenshotError.stack;
149+
if (lighthouseResult) {
150+
progressLog('success', 'Lighthouse data has been collected');
151+
} else {
152+
progressLog('error', 'Lighthouse did not produce usable data');
153+
}
154+
} catch (lighthouseError: any) {
155+
progressLog('error', 'Could not gather Lighthouse data', lighthouseError.message);
170156
}
171-
172-
progressLog('error', 'Could not take screenshot', details);
173157
}
174158

175-
return {screenshotBase64Data, runtimeErrors, axeViolations, cspViolations, lighthouseResult};
159+
await browser.close();
160+
161+
return {
162+
screenshotBase64Data,
163+
runtimeErrors,
164+
axeViolations,
165+
cspViolations,
166+
lighthouseResult,
167+
unexpectedErrorMessage,
168+
};
176169
}

0 commit comments

Comments
 (0)