Skip to content

fix(launcher): fail fast when the spawned proxy child exits pre-bind - #481

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-03_launcher-fast-fail-child-exit
Sep 7, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-03_launcher-fast-fail-child-exit

Conversation

@ranxianglei

@ranxianglei ranxianglei commented Sep 3, 2026 •

Copy link
Copy Markdown
Owner

Closes #596 — tracking issue for this PR.

Context

Salvages the one non-duplicated improvement from PR #480 ("child-reported handshake bind; stop forcing --debug").

While reviewing #480 I found that two of its three goals are already merged into master via an independent, more complete implementation — commit c2567cc "instance governance … (#417)":

  • The port TOCTOU is already fixed: the child self-binds and retries on EADDRINUSE (≤17 attempts), reporting its real origin through the instance file matched by BILI_LAUNCH_TOKEN (src/server.ts, and ensureProxyRunning in src/launcher.ts).
  • The forced debug: true is already removed (proxyStartArgs uses opts.debug).

Merging #480 as-is would add a second, competing handshake mechanism (a per-launch --handshake-file) and conflicts with master in src/launcher.ts, src/server.ts, and tests/launcher.test.ts (all changed-in-both since #480's base). So #480 should be closed as superseded by #417.

The single piece of #480 that master still lacks is fast-fail on child exit: if the spawned proxy dies before becoming healthy (bad config, missing upstream, startup crash), master's spawn path waits the full SPAWN_WAIT_MS = 20000 before erroring — there is no child.on("exit") watch inside ensureProxyRunning (the only exit handler lives in runClient).

Change

  • src/launcher.ts ensureProxyRunning: register an exit listener on the spawned child; break the health-poll loop as soon as it fires; throw a specific error carrying the exit code/signal and pointing at the per-launch log instead of the generic 20s timeout message.
  • tests/launcher.test.ts: new test — the child fires exit(1) right after spawn → ensureProxyRunning rejects in well under 5s with exited before becoming healthy (code 1) plus the log path.

The success path is untouched: a live child never emits exit during polling, so normal startup behavior is unchanged.

Verification

  • npm run typecheck — clean
  • npm test — 947/948 pass; the single failure (resolveClientCommand: codex/claude resolve to themselves) is the pre-existing env-dependent one (this sandbox has /usr/bin/codex), reproduces identically on pristine master, unrelated to this change
  • npm run build — clean

Partially supersedes #480; see that thread for the full duplicate analysis.

Salvages the one non-duplicated improvement from superseded PR #480 (its handshake-file TOCTOU fix and --debug removal are already in master via #417 instance governance). ensureProxyRunning now watches the spawned child's exit event and breaks the health-poll loop early, so a startup crash reports in ms instead of burning the full SPAWN_WAIT_MS (20s) window. Error carries the exit code/signal and points at the per-launch log.
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-03_launcher-fast-fail-child-exit (eb6ad47)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-481

Each push to this PR publishes a new version under the pr-481 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr481.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Picked up. Verified the premise on master: ensureProxyRunning (src/launcher.ts:1633) has no exit watch — the only exit handler in the file is in runClient (src/launcher.ts:1747), so a pre-bind child death burns the full SPAWN_WAIT_MS = 20000 and reports the generic timeout. Implementing the scoped fix now (exit listener + early loop break + specific error with code/signal + log path, plus the fast-fail test). Will follow up with the PR link.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Done — PR: #482

What changed (branch 2026-09-03_launcher-fail-fast-child-exit, off master 09ef208):

  • src/launcher.ts ensureProxyRunning: registers an exit listener on the spawned child right after spawn/unref; the health-poll loop condition becomes !childExit && now() < deadline so it breaks immediately once the child dies; post-loop it throws bili: proxy child exited before becoming healthy (code N) — or (signal SIGTERM) / (code N, signal SIGTERM) when applicable — with (log: <per-launch log path>), instead of the generic 20s timeout message. The timeout path is preserved verbatim for the no-exit case.
  • tests/launcher.test.ts: new test ensureProxyRunning: fails fast when the child exits before becoming healthy (#481) — fake child fires exit(1, null) via setImmediate right after spawn; asserts rejection matches exited before becoming healthy (code 1), includes the bili-proxy-8787.log path, and lands in well under 5s of wall-clock (real timers, ~200ms actual).

Verification (matches your stated bar):

  • npm run typecheck — clean
  • npm test — 947/948 pass; sole failure is resolveClientCommand: codex/claude resolve to themselves, the pre-existing env-dependent one (this sandbox has /usr/bin/codex)
  • npm run build — clean

Success path untouched: a live child never emits exit during polling. Ready for review — merge stays with you.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Cross-reference: a second, independent implementation of this same fix landed as PR #482 (branch 2026-09-03_launcher-fail-fast-child-exit, a027923) ~5 minutes after this one. I reviewed and verified both branches locally (typecheck clean; 947/948 on each — the single failure is the pre-existing env-dependent resolveClientCommand test, reproduces on pristine master; build clean; fail-fast test rejects in ~205ms vs the old 20s window). The trees differ only in style (typed {code,signal} vs [unknown,unknown]; test placement/timing strategy).

Per duplicate screening I closed #482 as duplicate in favor of this PR — it's first-in-time and its body carries the fuller #480 salvage analysis. Full comparison table and reasoning are in the #482 thread. If you'd rather merge the #482 tree instead, reopen it and close this one — both are verified merge-ready. One suggested tweak before merge: the code comment cites #401/#480, but #401 is the unrelated sessions-storage bloat issue; #480 is the correct source ref. Merge stays yours.

@ranxianglei

Copy link
Copy Markdown
Owner Author

Review vs master 3cd85b6: ✅ merge-ready.

  • Verified against master's ensureProxyRunning (src/launcher.ts:1474): the patch context (child.unref?.() → poll loop → timeout throw) is unchanged; applies clean.
  • SpawnChild.on?: exists in the interface (src/launcher.ts:100), so the exit listener attach type-checks; attaching synchronously right after spawn is safe (Node emits exit asynchronously).
  • if (childExit) break at loop top + post-loop detail throw is correct; the pre-existing "stale record / live record" fallback paths are untouched.
  • Test: mock child fires exit(1, null) via setImmediate — deterministic, <5s assertion is meaningful against SPAWN_WAIT_MS = 20000.

One nit (non-blocking): the fast-fail error could also mention the child command (process.execPath + script) — the log path already points at the captured stderr, so current form is fine.

(tracking: #596)

@ranxianglei
ranxianglei merged commit d21e20d into master Sep 7, 2026
5 of 6 checks passed
@ranxianglei ranxianglei mentioned this pull request Sep 7, 2026
@ranxianglei
ranxianglei deleted the 2026-09-03_launcher-fast-fail-child-exit branch October 15, 2026 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

launcher: spawned proxy child exiting before bind leaves the client launch hanging

1 participant