Skip to content

Commit 0abcbaf

Browse files
runspiredrunspired
committed
refactor diagnostic for safer teardown
1 parent ca65522 commit 0abcbaf

File tree

7 files changed

+70
-29
lines changed

7 files changed

+70
-29
lines changed

‎packages/diagnostic/server/bun/fetch.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,13 @@ export function handleBunFetch(config, state, req, server) {
3131
if (INDEX_PATHS.includes(url.pathname)) {
3232
if (bId && wId) {
3333
// serve test index.html
34-
if (config.entry.indexOf('?')) {
35-
config._realEntry = config.entry.substr(0, config.entry.indexOf('?'));
34+
if (!config._realEntry && config.entry.indexOf('?')) {
35+
config._realEntry = path.join(process.cwd(), config.entry.substr(0, config.entry.indexOf('?')));
3636
}
3737
debug(`Serving entry ${config._realEntry} for browser ${bId} window ${wId}`);
38-
return new Response(Bun.file(config._realEntry));
38+
39+
const asset = Bun.file(config._realEntry);
40+
return new Response(asset);
3941
}
4042
const _bId = bId ?? state.lastBowserId ?? state.browserId;
4143
const _wId = wId ?? state.lastWindowId ?? state.windowId;

‎packages/diagnostic/server/bun/socket-handler.js

+4-12
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { watchAssets } from './watch.js';
77
export function buildHandler(config, state) {
88
const Connections = new Set();
99
if (config.serve && !config.noWatch) {
10-
watchAssets(config.assets, () => {
10+
watchAssets(state, config.assets, () => {
1111
Connections.forEach((ws) => {
1212
ws.send(JSON.stringify({ name: 'reload' }));
1313
});
@@ -18,7 +18,8 @@ export function buildHandler(config, state) {
1818
perMessageDeflate: true,
1919
async message(ws, message) {
2020
const msg = JSON.parse(message);
21-
msg.launcher = state.browsers.get(msg.browserId).launcher;
21+
msg.launcher = state.browsers.get(msg.browserId)?.launcher ?? '<unknown>';
22+
2223
info(`${chalk.green('➡')} [${chalk.cyan(msg.browserId)}/${chalk.cyan(msg.windowId)}] ${chalk.green(msg.name)}`);
2324

2425
switch (msg.name) {
@@ -53,16 +54,7 @@ export function buildHandler(config, state) {
5354
debug(`${chalk.green('✅ [All Complete]')} ${chalk.yellow('@' + sinceStart())}`);
5455

5556
if (!config.serve) {
56-
state.browsers.forEach((browser) => {
57-
browser.proc.kill();
58-
browser.proc.unref();
59-
});
60-
state.server.stop();
61-
if (config.cleanup) {
62-
debug(`Running configured cleanup hook`);
63-
await config.cleanup();
64-
debug(`Configured cleanup hook completed`);
65-
}
57+
await state.safeCleanup();
6658
debug(`\n\nExiting with code ${exitCode}`);
6759
// 1. We expect all cleanup to have happened after
6860
// config.cleanup(), so exiting here should be safe.

‎packages/diagnostic/server/bun/watch.js

+13-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { watch } from 'fs';
22

3-
export function addCloseHandler(cb) {
3+
export function addCloseHandler(state, cb) {
4+
state.closeHandlers.push(createCloseHandler(cb));
5+
}
6+
7+
function createCloseHandler(cb) {
48
let executed = false;
59

610
process.on('SIGINT', () => {
@@ -26,14 +30,20 @@ export function addCloseHandler(cb) {
2630
executed = true;
2731
cb();
2832
});
33+
34+
return () => {
35+
if (executed) return;
36+
executed = true;
37+
cb();
38+
};
2939
}
3040

31-
export function watchAssets(directory, onAssetChange) {
41+
export function watchAssets(state, directory, onAssetChange) {
3242
const watcher = watch(directory, { recursive: true }, (event, filename) => {
3343
onAssetChange(event, filename);
3444
});
3545

36-
addCloseHandler(() => {
46+
addCloseHandler(state, () => {
3747
watcher.close();
3848
});
3949
}

‎packages/diagnostic/server/default-setup.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import fs from 'fs';
33
import path from 'path';
44

55
import { getBrowser, recommendedArgs } from './browsers/index.js';
6-
import launch from './index.js';
6+
import { launch } from './index.js';
77
import DefaultReporter from './reporters/default.js';
88
import { getFlags } from './utils/get-flags.js';
99

@@ -79,6 +79,11 @@ export default async function launchDefault(overrides = {}) {
7979
debug: overrides.debug ?? false,
8080
headless: overrides.headless ?? false,
8181
useExisting: overrides.useExisting ?? false,
82+
protocol: overrides.protocol ?? 'http',
83+
key: overrides.key ?? null,
84+
cert: overrides.cert ?? null,
85+
hostname: overrides.hostname ?? 'localhost',
86+
port: overrides.port ?? null,
8287

8388
entry: overrides.entry ?? `./dist-test/tests/index.html?${TEST_PAGE_FLAGS.join('&')}`,
8489
assets: overrides.assets ?? './dist-test',

‎packages/diagnostic/server/index.js

+36-7
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@ import { launchBrowsers } from './bun/launch-browser.js';
55
import { buildHandler } from './bun/socket-handler.js';
66
import { debug, error, print } from './utils/debug.js';
77
import { getPort } from './utils/port.js';
8+
import { addCloseHandler } from './bun/watch.js';
89

910
/** @type {import('bun-types')} */
1011
const isBun = typeof Bun !== 'undefined';
1112

12-
export default async function launch(config) {
13+
export async function launch(config) {
1314
if (isBun) {
1415
debug(`Bun detected, using Bun.serve()`);
1516

@@ -34,6 +35,18 @@ export default async function launch(config) {
3435
browsers: new Map(),
3536
completed: 0,
3637
expected: config.parallel ?? 1,
38+
closeHandlers: [],
39+
};
40+
state.safeCleanup = async () => {
41+
debug(`Running close handlers`);
42+
for (const handler of state.closeHandlers) {
43+
try {
44+
await handler();
45+
} catch (e) {
46+
error(`Error in close handler: ${e?.message ?? e}`);
47+
}
48+
}
49+
debug(`All close handlers completed`);
3750
};
3851

3952
if (protocol === 'https') {
@@ -56,6 +69,16 @@ export default async function launch(config) {
5669
},
5770
websocket: buildHandler(config, state),
5871
});
72+
73+
addCloseHandler(state, () => {
74+
state.browsers?.forEach((browser) => {
75+
browser.proc.kill();
76+
browser.proc.unref();
77+
});
78+
state.server.stop();
79+
state.server.unref();
80+
});
81+
5982
print(chalk.magenta(`🚀 Serving on ${chalk.white(protocol + '://' + hostname + ':')}${chalk.magenta(port)}`));
6083
config.reporter.serverConfig = {
6184
port,
@@ -66,22 +89,28 @@ export default async function launch(config) {
6689

6790
if (config.setup) {
6891
debug(`Running configured setup hook`);
92+
6993
await config.setup({
7094
port,
7195
hostname,
7296
protocol,
7397
});
7498
debug(`Configured setup hook completed`);
7599
}
100+
if (config.cleanup) {
101+
addCloseHandler(state, async () => {
102+
debug(`Running configured cleanup hook`);
103+
await config.cleanup();
104+
debug(`Configured cleanup hook completed`);
105+
});
106+
}
76107

77-
await launchBrowsers(config, state);
108+
if (!config.noLaunch) {
109+
await launchBrowsers(config, state);
110+
}
78111
} catch (e) {
79112
error(`Error: ${e?.message ?? e}`);
80-
if (config.cleanup) {
81-
debug(`Running configured cleanup hook`);
82-
await config.cleanup();
83-
debug(`Configured cleanup hook completed`);
84-
}
113+
await state.safeCleanup();
85114
throw e;
86115
}
87116
} else {

‎packages/diagnostic/server/reporters/default.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -181,14 +181,14 @@ export default class CustomDotReporter {
181181
if (this.failedTests.length) {
182182
this.write(
183183
chalk.red(
184-
`\n\n${this.failedTests.length} Tests Failed. Complete stack traces for failures will print at the end.`
184+
`\n\n\t${this.failedTests.length} Tests Failed. Complete stack traces for failures will print at the end.`
185185
)
186186
);
187187
}
188188
if (this.globalFailures.length) {
189189
this.write(
190190
chalk.red(
191-
`\n\n${this.globalFailures.length} Global Failures were detected.. Complete stack traces for failures will print at the end.`
191+
`\n\n\t${this.globalFailures.length} Global Failures were detected.. Complete stack traces for failures will print at the end.`
192192
)
193193
);
194194
}
@@ -457,6 +457,9 @@ export default class CustomDotReporter {
457457
}
458458

459459
reportFailedTests() {
460+
if (this.failedTests.length) {
461+
this.write(chalk.red(`\n\n\tPrinting ${this.failedTests.length} Failed Tests\n\n\t====================\n\n`));
462+
}
460463
this.failedTests.forEach((failure) => {
461464
const result = failure.data;
462465
this.write(chalk.red(`\n\t💥 Failed: ${result.runDuration.toLocaleString('en-US')}ms ${result.name}\n`));

‎packages/diagnostic/server/utils/get-flags.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export function getFlags() {
2424
const filter = flags.has('--filter') || flags.has('-f');
2525
const retry = flags.has('--retry') || flags.has('-r');
2626
const headless = flags.has('--headless') || flags.has('-h');
27-
const useExisting = flags.has('--use-existing') || flags.has('-e');
27+
const useExisting = flags.has('--use-existing') || flags.has('-e') || flags.has('-b');
2828

2929
if (filter) {
3030
filtered['filter'] = true;

0 commit comments

Comments
 (0)