Skip to content

Commit f2669b1

Browse files
authored
Merge pull request #287 from ember-learn/copilot/fix-github-actions-failure
Harden run-code-block server readiness and screenshot navigation retries
2 parents 1e64dc7 + 23b07d1 commit f2669b1

2 files changed

Lines changed: 125 additions & 6 deletions

File tree

src/lib/plugins/run-code-blocks/directives/screenshot.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,24 @@ function compile(steps: string, path: `${string}.png`, args: Args): string {
5757

5858
let script = [
5959
`const puppeteer = require('puppeteer');
60+
const NAVIGATION_TIMEOUT = 180000;
61+
const MAX_NAVIGATION_RETRIES = 6;
62+
const RETRYABLE_NAVIGATION_ERRORS = [
63+
'Navigation timeout',
64+
'net::ERR_CONNECTION_REFUSED',
65+
'net::ERR_CONNECTION_RESET',
66+
'net::ERR_ABORTED',
67+
'ERR_HTTP_RESPONSE_CODE_FAILURE'
68+
];
69+
70+
function retryDelay(attempt) {
71+
return Math.min(500 * Math.pow(2, attempt), 5000);
72+
}
6073
6174
async function main() {
6275
let browser = await puppeteer.launch();
6376
let page = await browser.newPage();
64-
page.setDefaultNavigationTimeout(120000);
77+
page.setDefaultNavigationTimeout(NAVIGATION_TIMEOUT);
6578
await page.setViewport(${js(viewport)});
6679
`
6780
];
@@ -92,13 +105,24 @@ async function main() {
92105
script.push(` await page.evaluate(${js(params[0])});`);
93106
break;
94107
case 'visit':
95-
script.push(` for (let _attempt = 0; _attempt < 3; _attempt++) {`);
108+
script.push(` for (let _attempt = 0; _attempt < MAX_NAVIGATION_RETRIES; _attempt++) {`);
96109
script.push(` try {`);
97-
script.push(` await page.goto(${js(params[0])}, { waitUntil: 'domcontentloaded', timeout: 120000 });`);
110+
script.push(` await page.goto(${js(params[0])}, { waitUntil: 'domcontentloaded', timeout: NAVIGATION_TIMEOUT });`);
98111
script.push(` break;`);
99112
script.push(` } catch (e) {`);
100-
script.push(` if (_attempt === 2) throw e;`);
101-
script.push(` await new Promise(r => setTimeout(r, 2000));`);
113+
script.push(` let message = e instanceof Error ? e.message : String(e);`);
114+
script.push(` let shouldRetry = RETRYABLE_NAVIGATION_ERRORS.some(pattern => message.includes(pattern));`);
115+
script.push(` if (_attempt === MAX_NAVIGATION_RETRIES - 1 || !shouldRetry) throw e;`);
116+
script.push(` try {`);
117+
script.push(` await page.close();`);
118+
script.push(` } catch (closeError) {`);
119+
script.push(` let closeMessage = closeError instanceof Error ? closeError.message : String(closeError);`);
120+
script.push(` if (!closeMessage.includes('Target closed')) throw closeError;`);
121+
script.push(` }`);
122+
script.push(` page = await browser.newPage();`);
123+
script.push(` page.setDefaultNavigationTimeout(NAVIGATION_TIMEOUT);`);
124+
script.push(` await page.setViewport(${js(viewport)});`);
125+
script.push(` await new Promise(r => setTimeout(r, retryDelay(_attempt)));`);
102126
script.push(` }`);
103127
script.push(` }`);
104128
break;

src/lib/plugins/run-code-blocks/directives/server/start.ts

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import { Code } from 'mdast';
2+
import { createConnection } from 'net';
23
import { join } from 'path';
34
import { Option, assert } from 'ts-std';
45
import { parseCommand } from '../../commands';
56
import Options from '../../options';
67
import parseArgs, { ToBool, optional } from '../../parse-args';
78
import Servers from '../../servers';
89

10+
const DEFAULT_READY_TIMEOUT = 30000;
11+
const READY_RETRY_INTERVAL = 500;
12+
913
interface Args {
1014
id?: string;
1115
lang?: string;
@@ -17,6 +21,68 @@ interface Args {
1721
captureOutput?: boolean;
1822
}
1923

24+
function extractURL(expect: Option<string> | undefined): Option<string> {
25+
if (!expect) {
26+
return null;
27+
}
28+
29+
let match = expect.match(/https?:\/\/[^\s"']+/);
30+
31+
if (match) {
32+
try {
33+
return new URL(match[0]!).toString();
34+
} catch {
35+
return null;
36+
}
37+
} else {
38+
return null;
39+
}
40+
}
41+
42+
async function waitForServerReady(url: string, timeout: number): Promise<void> {
43+
let parsed = new URL(url);
44+
let host = parsed.hostname;
45+
let port = parsed.port ? Number(parsed.port) : (parsed.protocol === 'https:' ? 443 : 80);
46+
let startedAt = Date.now();
47+
let lastError: Option<Error> = null;
48+
49+
while (Date.now() - startedAt < timeout) {
50+
try {
51+
await new Promise<void>((resolve, reject) => {
52+
let socket = createConnection({ host, port });
53+
54+
socket.once('connect', () => {
55+
socket.end();
56+
resolve();
57+
});
58+
59+
socket.once('error', e => {
60+
socket.destroy();
61+
62+
if (e instanceof Error) {
63+
reject(e);
64+
} else {
65+
reject(new Error(String(e)));
66+
}
67+
});
68+
});
69+
70+
return;
71+
} catch (e) {
72+
if (e instanceof Error) {
73+
lastError = e;
74+
} else {
75+
lastError = new Error(String(e));
76+
}
77+
}
78+
79+
await new Promise(resolve => setTimeout(resolve, READY_RETRY_INTERVAL));
80+
}
81+
82+
let details = lastError ? ` Last error: ${lastError.message}` : '';
83+
throw new Error(`Timed out while waiting for ${url} to accept connections after ${timeout}ms.${details}`);
84+
}
85+
2086
export default async function startServer(node: Code, options: Options, servers: Servers): Promise<Option<Code>> {
2187
let args = parseArgs<Args>(node, [
2288
optional('id', String),
@@ -72,7 +138,36 @@ export default async function startServer(node: Code, options: Options, servers:
72138
output.push(`$ ${display}`);
73139
}
74140

75-
let stdout = await server.start(args.expect);
141+
let stdout = await server.start(args.expect, args.timeout);
142+
143+
let readyURL = extractURL(args.expect);
144+
145+
if (readyURL) {
146+
let timeout = args.timeout || DEFAULT_READY_TIMEOUT;
147+
148+
try {
149+
await waitForServerReady(readyURL, timeout);
150+
} catch (e) {
151+
await server.kill();
152+
153+
let message = (e instanceof Error) ? e.message : String(e);
154+
155+
throw new Error(
156+
`${message}
157+
158+
====== STDOUT ======
159+
160+
${server.stdout || '(No output)'}
161+
162+
====== STDERR ======
163+
164+
${server.stderr || '(No output)'}
165+
166+
====================
167+
`
168+
);
169+
}
170+
}
76171

77172
if (args.captureOutput && stdout) {
78173
output.push(stdout);

0 commit comments

Comments
 (0)