Skip to content

Commit 56467b5

Browse files
committed
fix: allow retrying eval on serve testing timeouts
1 parent c96aa9b commit 56467b5

2 files changed

Lines changed: 62 additions & 70 deletions

File tree

runner/orchestration/serve-testing-worker.ts

Lines changed: 59 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -31,84 +31,74 @@ export async function serveAndTestApp(
3131

3232
progress.log(rootPromptDef, 'serve-testing', `Validating the running app`);
3333

34-
try {
35-
const result = await env.executor.serveWebApplication(
36-
evalID,
37-
appDirectoryPath,
38-
rootPromptDef,
39-
progress,
40-
abortSignal,
41-
async serveUrl => {
42-
progress.log(
43-
rootPromptDef,
44-
'serve-testing',
45-
`Validating the running app (URL: ${serveUrl})`,
46-
);
47-
const serveParams: ServeTestingWorkerMessage = {
48-
serveUrl,
49-
appName: rootPromptDef.name,
50-
enableAutoCsp: !!config.enableAutoCsp,
51-
includeAxeTesting: config.skipAxeTesting === false,
52-
takeScreenshots: config.skipScreenshots === false,
53-
includeLighthouseData: config.skipLighthouse !== true,
54-
userJourneyAgentTaskInput,
55-
};
34+
const result = await env.executor.serveWebApplication(
35+
evalID,
36+
appDirectoryPath,
37+
rootPromptDef,
38+
progress,
39+
abortSignal,
40+
async serveUrl => {
41+
progress.log(rootPromptDef, 'serve-testing', `Validating the running app (URL: ${serveUrl})`);
42+
const serveParams: ServeTestingWorkerMessage = {
43+
serveUrl,
44+
appName: rootPromptDef.name,
45+
enableAutoCsp: !!config.enableAutoCsp,
46+
includeAxeTesting: config.skipAxeTesting === false,
47+
takeScreenshots: config.skipScreenshots === false,
48+
includeLighthouseData: config.skipLighthouse !== true,
49+
userJourneyAgentTaskInput,
50+
};
5651

57-
return await workerConcurrencyQueue.add(
58-
() =>
59-
new Promise<ServeTestingResult>((resolve, reject) => {
60-
const child: ChildProcess = fork(
61-
path.resolve(import.meta.dirname, '../workers/serve-testing/worker.js'),
62-
{signal: abortSignal},
63-
);
64-
child.send(serveParams);
52+
return await workerConcurrencyQueue.add(
53+
() =>
54+
new Promise<ServeTestingResult>((resolve, reject) => {
55+
const child: ChildProcess = fork(
56+
path.resolve(import.meta.dirname, '../workers/serve-testing/worker.js'),
57+
{signal: abortSignal},
58+
);
59+
child.send(serveParams);
6560

66-
child.on('message', async (result: ServeTestingWorkerResponseMessage) => {
67-
if (result.type === 'result') {
68-
try {
69-
await killChildProcessWithSigterm(child);
70-
} catch (e) {
71-
progress.debugLog(`Error while killing serve testing worker: ${e}`);
72-
}
73-
resolve(result.payload);
74-
} else {
75-
progress.log(
76-
rootPromptDef,
77-
result.payload.state,
78-
result.payload.message,
79-
result.payload.details,
80-
);
81-
}
82-
});
83-
child.on('error', async err => {
61+
child.on('message', async (result: ServeTestingWorkerResponseMessage) => {
62+
if (result.type === 'result') {
8463
try {
8564
await killChildProcessWithSigterm(child);
8665
} catch (e) {
8766
progress.debugLog(`Error while killing serve testing worker: ${e}`);
8867
}
89-
reject(err);
90-
});
91-
}),
92-
);
93-
},
94-
);
95-
96-
// An executor might define `serveWebApplication` but conditionally decide
97-
// that no web application can be started/served.
98-
if (result === null) {
99-
return null;
100-
}
68+
resolve(result.payload);
69+
} else {
70+
progress.log(
71+
rootPromptDef,
72+
result.payload.state,
73+
result.payload.message,
74+
result.payload.details,
75+
);
76+
}
77+
});
78+
child.on('error', async err => {
79+
try {
80+
await killChildProcessWithSigterm(child);
81+
} catch (e) {
82+
progress.debugLog(`Error while killing serve testing worker: ${e}`);
83+
}
84+
reject(err);
85+
});
86+
}),
87+
);
88+
},
89+
);
10190

102-
if (result.errorMessage === undefined) {
103-
progress.log(rootPromptDef, 'success', 'Validation of running app is successful');
104-
} else {
105-
progress.log(rootPromptDef, 'error', 'Validation of running app failed', result.errorMessage);
106-
}
91+
// An executor might define `serveWebApplication` but conditionally decide
92+
// that no web application can be started/served.
93+
if (result === null) {
94+
return null;
95+
}
10796

108-
return result;
109-
} catch (e) {
110-
progress.log(rootPromptDef, 'error', 'Error while trying to validate running app', `${e}`);
97+
if (result.errorMessage === undefined) {
98+
progress.log(rootPromptDef, 'success', 'Validation of running app is successful');
99+
} else {
100+
progress.log(rootPromptDef, 'error', 'Validation of running app failed', result.errorMessage);
111101
}
112102

113-
return null;
103+
return result;
114104
}

runner/workers/serve-testing/serve-app.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ export async function serveApp<T>(
2424
);
2525

2626
const actualPort = await new Promise<number>((resolvePort, rejectPort) => {
27-
const serveStartTimeout = 45000; // 45s for serve to start
27+
// Timeout for server to start. CPU queueing might cause this to take longer.
28+
// It's a upper safety boundary. The overall eval timeout is the main control.
29+
const serveStartTimeout = 1000 * 60 * 5;
2830
const timeoutId = setTimeout(() => {
2931
rejectPort(
3032
new Error(

0 commit comments

Comments
 (0)