fix(bundler): restart the esbuild service after an unexpected child death instead of poisoning the process - #3718
Conversation
…eath instead of poisoning the process An OOM-kill or external signal that took down the managed esbuild service child used to latch the permanent ownership error, failing every later transform/bundle for the life of the process and streaming per-request 'Critical page module(s) failed to load' events to Sentry until the pod restarted. The close handler now records the death as a recoverable loss; the next bundler operation resets esbuild's module state (its stop() clears the cached dead service) and lets a fresh child spawn, bounded by a budget of MAX_SERVICE_RESTARTS before the sticky ownership error still latches. Genuinely foreign services - started while the adapter's spawn interceptor was not installed - are still rejected permanently, and a clean stop() clears any pending crash state. Fixes VERYFRONT-SERVER-V
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
A managed esbuild child can die while the shared esbuild module remains cached. If raw esbuild starts a replacement service before adapter recovery, module-wide stop would otherwise stop or adopt that foreign service. The adapter now guards the loss window, records any service spawn outside the adapter, and fails closed before recovery or shutdown can touch it. Constraint: esbuild exposes only module-wide service state, not the current child owner Rejected: Always call esbuild.stop() after a managed child death | can stop a raw service started after the managed child died Confidence: high Scope-risk: narrow Directive: Do not reset a lost esbuild service through module-wide stop after observing a foreign service spawn Tested: deno fmt --check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno lint extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src Not-tested: full repository test suite
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81fe3d7ec8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A rejected crash recovery previously removed the temporary spawn guard while leaving the lost-service state active. That let a later shutdown enter esbuild's module-wide stop path even though a raw foreign service had replaced the killed managed child. The foreign replacement marker now survives guard removal so every later recovery or shutdown rejects before touching the module-wide service. Constraint: esbuild exposes module-wide service state without an owner token Rejected: Clear esbuildServiceLost after the first foreign rejection | later operations would lose the reason they must avoid module-wide stop Confidence: high Scope-risk: narrow Directive: Foreign replacement state must outlive the temporary spawn guard until explicit test reset or process restart Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src Tested: deno fmt --check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno lint extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: git diff --check Not-tested: full pre-push hook pending at commit time
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3320dcb447
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A stop requested while an operation was waiting for lost-service recovery could finish before that operation was admitted, leaving the operation to spawn a fresh service after shutdown. Captured build contexts also retained the pre-crash esbuild context and failed after recovery. Non-reentrant operations now hold a stop barrier across pre-admission recovery, and build contexts refresh themselves when the managed service generation advances. Constraint: esbuild exposes one module-wide service while the Bundler API exposes reusable contexts Rejected: Fail captured contexts permanently after recovery | recreating from the saved mapped options preserves the public context contract Confidence: high Scope-risk: moderate Directive: Do not let stop complete while an operation is between crash recovery and admission Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src Tested: deno fmt --check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno lint extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: git diff --check Not-tested: full pre-push hook pending at commit time
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 679916c8d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Node emits child exit before close, so a managed child can already be non-live while the adapter has not yet marked the loss recoverable. The adapter now records recoverable managed loss at exit, keeps close as the completion signal, forces stale context plugin disposals before recreating a recovered context, and latches restart-budget exhaustion even after the dead child has been cleared from esbuildService. Constraint: esbuild exposes only module-wide service state and does not dispose plugin callbacks when its service dies Rejected: Treat exit-before-close as foreign ownership | the child is still the adapter-owned managed service Confidence: high Scope-risk: moderate Directive: Managed child exit is the state transition for recovery; close is only the completion signal Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src Tested: deno fmt --check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno lint extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: deno check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: rg -n "—|–" extensions/ext-bundler-esbuild/src/esbuild-bundler.ts Tested: git diff --check Not-tested: full pre-push hook pending at commit time
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e04a897ab9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The adapter can observe a managed esbuild child death through paths that do not rebuild the stale context or that call stop before the next operation. Those paths now force plugin cleanup for the affected generation, treat an already-dead tracked child as a managed loss even before Node emits exit, and spend restart budget when stop resets a lost service.
Constraint: esbuild does not run plugin onDispose callbacks when its service dies underneath a context or request
Rejected: Force plugin cleanup on every build error | ordinary esbuild validation failures should keep the existing disposal barrier behavior
Rejected: Treat a tracked non-live child before exit as foreign ownership | the child is module-owned and still within the managed crash contract
Confidence: high
Scope-risk: moderate
Directive: If stop clears esbuildServiceLost, it must spend the same restart budget recovery would spend
Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src
Tested: deno fmt --check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: deno lint extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: deno check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: git diff --check
Tested: perl -CSD -ne 'print if /\x{2014}|\x{2013}/' extensions/ext-bundler-esbuild/src/esbuild-bundler.ts
Not-tested: full pre-push hook pending at commit time
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4974e3803
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The in-flight context setup crash case can leave the esbuild request pending under Deno even though the live review blockers are covered by dispose-only context cleanup and stop-budget accounting. Removing that unstable extra case keeps the PR focused on deterministic recovery behavior. Constraint: Current live review threads cover abandoned context disposal and stop-cleared loss accounting Rejected: Keep the in-flight context setup test with a timeout | would test harness behavior more than adapter contract Confidence: high Scope-risk: narrow Directive: Cover crash recovery through deterministic service ownership and disposal paths Tested: PATH=/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno fmt extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.ts Tested: PATH=/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno test --preload=src/testing/preload.ts --no-check --allow-all --filter 'service crash recovery' extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts Tested: git diff --check Not-tested: Full suite pending CI
Context recovery can abandon one plugin disposal barrier and later create another. The barrier now reports only its own cleanup error while still publishing async failures for shutdown, and context refresh creates fresh mapped plugin options for each retry so a failed refresh cannot leave later generations activated.
Constraint: esbuild plugin onDispose callbacks are tied to a single service/context generation
Rejected: Clear the module-wide disposal error before forced cleanup | this would hide async cleanup failures that shutdown still needs to report
Rejected: Reuse one mapped plugin barrier across refresh retries | activation is one-shot and cannot safely represent later generations
Confidence: high
Scope-risk: narrow
Directive: Context refresh attempts must not reuse an activated plugin disposal barrier across service generations
Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src
Tested: deno fmt --check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: deno lint extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: deno check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Tested: git diff --check
Tested: perl -CSD -ne 'print if /\x{2014}|\x{2013}/' extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts
Not-tested: full pre-push hook pending at commit time
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0efec7681e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| trackedService && !trackedService.expectedClose && !isLiveService(trackedService) && | ||
| remainingServiceRestarts <= 0 | ||
| ) { |
There was a problem hiding this comment.
When stop() runs after the third service's exit event but before its close event, esbuildService still references the dead child. The preceding loss handling decrements remainingServiceRestarts from 1 to 0, and this condition immediately records an ownership error, so the third loss is rejected even though MAX_SERVICE_RESTARTS explicitly permits it. The identical call succeeds if close happens first because that handler clears esbuildService, making shutdown behavior race-dependent; latch exhaustion only when a new loss is observed with no budget remaining. Add coverage for stopping between exit and close.
AGENTS.md reference: AGENTS.md:L11-L13
Useful? React with 👍 / 👎.
1 participant
Fixes VERYFRONT-SERVER-V (160 events, staging, escalating).
Root cause
The immediate staging trigger (esbuild's module evaluating inside a project env scope and losing
ESBUILD_BINARY_PATH) was fixed on main by #3698/#3697. This PR fixes the amplifier those left in place: when the managed esbuild service child dies unexpectedly (OOM-kill, SIGKILL, or the #3698-class spawn failure), the adapter latched a permanent process-wide ownership error. Every subsequent transform in the pod failed until restart, so one child death poisoned the whole process for its lifetime.Fix
The bundler adapter now recovers with a fresh esbuild service after an unexpected managed child death, bounded by a restart budget (
MAX_SERVICE_RESTARTS = 3) to preserve the original guard's protection against genuinely foreign service ownership. Operations in flight during a crash surface esbuild's raw transient error once; the next operation recovers.The recovery path also preserves the module-wide ownership guardrails:
stop()cannot complete and then leave that operation to spawn a new service afterward.BuildContextwrapper.exitmarks the service loss recoverable immediately, whilecloseremains the completion signal, so operations launched in the exit-to-close window do not latch a permanent ownership error.onDisposecallbacks run before recreating a crashed context, preserving exactly one active plugin generation during context recovery.stop()observes the lost service after the dead child has already closed and been cleared from module state.stop()spends restart budget when it clears a managed crash, so crash plus stop loops cannot bypass the restart limit.Testing (red-green TDD)
Regression coverage was added for managed child crash recovery, foreign raw-service replacement, repeated foreign shutdown, shutdown during pre-admission recovery, captured
BuildContextreuse after recovery, the managed child exit-to-close window, stale plugin disposal before context recreation, abandoned crashed context disposal, stale disposal errors from unrelated plugin barriers, context refresh failure followed by a successful retry, and restart-budget exhaustion when losses are cleared bystop().The new regressions fail on the pre-fix code: shutdown settles too early, stale context rebuild throws
The service is no longer running, exit-to-close work latches the permanent module-wide ownership error, stale plugin disposal remains at 0 while a second setup runs, abandoned crashed contexts leave plugin disposal at 0, unrelated stale plugin disposal errors escape later crashed-context cleanup, failed refresh reuses an activated disposal barrier so later cleanup is not awaited, and crash plus stop loops letstop()resolve after the restart budget is exhausted.Local verification at the pushed head (
0efec7681e42b635a0fb897357941bd89f7c3d92):deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts-> 8 passed, 38 steps.deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-bundler-esbuild/src-> 12 passed, 46 steps.deno fmt --check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts-> passed.deno lint extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts-> passed.deno check extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts-> passed.perl -CSD -ne 'print if /\\x{2014}|\\x{2013}/' extensions/ext-bundler-esbuild/src/esbuild-bundler.ts extensions/ext-bundler-esbuild/src/esbuild-bundler.test.ts-> no matches.git diff --check HEAD~1..HEAD-> passed.0efec7681e42b635a0fb897357941bd89f7c3d92-> passed: repo format, lint, typecheck, unit suite (3814 passed, 28499 steps, 0 failed, 1 ignored), cwd tests (10 passed, 193 steps), and cwd-exclusion tests (2 passed, 2 steps).Reviewer notes