fix(pack-verify): kill the --web child on failure too (same exit-skips-finally)
Re-review found the same process.exit()-skips-finally pattern in verifyWeb: a
failure during the --web phase goes through fail() → process.exit(1), skipping
its `finally { stop() }`, so the spawned web server was orphaned — left on the
port, where a later run could fetch a stale false-200 and mask a regression.
Track the live child in a module `webChild` and have fail() SIGTERM it before
exiting, alongside the workDir cleanup; stop() clears it on the success path.
Verified: a forced web-phase failure exits 1, retains the tarball, and leaves
no listener on the port (child killed, not orphaned).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
This commit is contained in:
parent
3dead1f83d
commit
9324f9f21c
1 changed files with 12 additions and 0 deletions
|
|
@ -58,9 +58,17 @@ const TOKEN_GLOBAL = "__INSPECTOR_API_TOKEN__";
|
||||||
// dir (a full node_modules from the real install) but leave the packed tarball
|
// dir (a full node_modules from the real install) but leave the packed tarball
|
||||||
// in place for post-mortem inspection.
|
// in place for post-mortem inspection.
|
||||||
let workDir = null;
|
let workDir = null;
|
||||||
|
// The live `--web` child during verifyWeb(), if any. fail() kills it so a
|
||||||
|
// failure in the web phase doesn't orphan the server — which would linger on
|
||||||
|
// the port and could even serve a stale false-200 to a later run (verifyWeb's
|
||||||
|
// own `finally { stop() }` is skipped when fail() calls process.exit()).
|
||||||
|
let webChild = null;
|
||||||
|
|
||||||
function fail(message) {
|
function fail(message) {
|
||||||
console.error(`\npack:verify FAILED — ${message}`);
|
console.error(`\npack:verify FAILED — ${message}`);
|
||||||
|
if (webChild && webChild.exitCode === null) {
|
||||||
|
webChild.kill("SIGTERM");
|
||||||
|
}
|
||||||
if (workDir) {
|
if (workDir) {
|
||||||
rmSync(workDir, { recursive: true, force: true });
|
rmSync(workDir, { recursive: true, force: true });
|
||||||
// `tarball` is initialized before `workDir` is ever set, so this is safe.
|
// `tarball` is initialized before `workDir` is ever set, so this is safe.
|
||||||
|
|
@ -299,6 +307,9 @@ async function verifyWeb(bin, cwd) {
|
||||||
stdio: ["ignore", "inherit", "inherit"],
|
stdio: ["ignore", "inherit", "inherit"],
|
||||||
shell: process.platform === "win32",
|
shell: process.platform === "win32",
|
||||||
});
|
});
|
||||||
|
// Expose the child so fail() can kill it if a check below exits the process
|
||||||
|
// (process.exit skips the `finally { stop() }`).
|
||||||
|
webChild = child;
|
||||||
|
|
||||||
let exited = false;
|
let exited = false;
|
||||||
let exitCode = null;
|
let exitCode = null;
|
||||||
|
|
@ -308,6 +319,7 @@ async function verifyWeb(bin, cwd) {
|
||||||
});
|
});
|
||||||
const stop = () => {
|
const stop = () => {
|
||||||
if (!exited) child.kill("SIGTERM");
|
if (!exited) child.kill("SIGTERM");
|
||||||
|
webChild = null;
|
||||||
};
|
};
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue