[GH-ISSUE #5817] bug(browser): web analyzer cancellation replaces an observed failing exit #3517

Closed
opened 2026-10-31 00:41:24 +00:00 by bsakaguchi · 3 comments
Owner

Originally created by @XiaoBinGan on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5817

Before submitting

  • I searched open and closed issues and related PRs. #5393 was the same precedence defect in the JavaScript analysis worker and is fixed; #5388 covers the source-map owner; #5292 covers the shared OwnedCommand owner. This report is the web-bundle worker, which uses its own supervisor path.
  • This report contains no credentials, private target samples, or sensitive values.

Area

Bugs

Observed behavior

WebBundleAnalysisWorker.#next selects an interruption before it selects an exit the owned supervisor already observed.

  1. Loop head (src/browser/WebBundleAnalysisWorker.ts, #next): deadline.interruption === "timeout" and deadline.interruption === "cancelled" throw before #observedFailure is consulted; only non-exit kinds are checked afterwards.
  2. After the exit branch is entered (snapshot.exitCode/snapshot.signal present), an interrupted drain (#waitForFrameOrOutputClose → "interrupted") throws AnalysisTimeoutError/AnalysisCancelledError even though the exit is already part of #observedFailure.

By inspection, a web analyzer that exits non-zero and is aborted afterwards is therefore reported as an interruption rather than the physical exit, exactly the shape reported for the JavaScript worker in #5393 (fixed there by letting #primaryFailure outrank #interruptionFailure).

Expected behavior

An exit or signal the owned supervisor already holds stays primary when the interruption arrives afterwards; an interruption observed before any exit remains an interruption; memory-limit classification, captured output and cleanup ownership keep their behaviour.

Verification notes

The sibling fix in #5393 and its regression test (tests/boundary/javascript/javascriptWorkerFailurePriority.test.ts) show the ordering that reproduces this shape: the child exits non-zero, the supervisor's synchronous exit listener records it, then a setImmediate abort follows. The same peer shape applies here through the existing launcher seam.

Impact and scope

An operator who aborts a slow analysis loses the analyzer's exit status and stderr, so a real analyzer crash becomes indistinguishable from a caller cancellation. The projection sites are #next and #waitForFrameOrOutputClose in src/browser/WebBundleAnalysisWorker.ts.

Originally created by @XiaoBinGan on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5817 ### Before submitting - [x] I searched open and closed issues and related PRs. #5393 was the same precedence defect in the JavaScript analysis worker and is fixed; #5388 covers the source-map owner; #5292 covers the shared `OwnedCommand` owner. This report is the web-bundle worker, which uses its own supervisor path. - [x] This report contains no credentials, private target samples, or sensitive values. ### Area Bugs ### Observed behavior `WebBundleAnalysisWorker.#next` selects an interruption before it selects an exit the owned supervisor already observed. 1. Loop head (`src/browser/WebBundleAnalysisWorker.ts`, `#next`): `deadline.interruption === "timeout"` and `deadline.interruption === "cancelled"` throw before `#observedFailure` is consulted; only non-exit kinds are checked afterwards. 2. After the exit branch is entered (`snapshot.exitCode`/`snapshot.signal` present), an interrupted drain (`#waitForFrameOrOutputClose` → `"interrupted"`) throws `AnalysisTimeoutError`/`AnalysisCancelledError` even though the exit is already part of `#observedFailure`. By inspection, a web analyzer that exits non-zero and is aborted afterwards is therefore reported as an interruption rather than the physical exit, exactly the shape reported for the JavaScript worker in #5393 (fixed there by letting `#primaryFailure` outrank `#interruptionFailure`). ### Expected behavior An exit or signal the owned supervisor already holds stays primary when the interruption arrives afterwards; an interruption observed before any exit remains an interruption; memory-limit classification, captured output and cleanup ownership keep their behaviour. ### Verification notes The sibling fix in #5393 and its regression test (`tests/boundary/javascript/javascriptWorkerFailurePriority.test.ts`) show the ordering that reproduces this shape: the child exits non-zero, the supervisor's synchronous exit listener records it, then a `setImmediate` abort follows. The same peer shape applies here through the existing launcher seam. ### Impact and scope An operator who aborts a slow analysis loses the analyzer's exit status and stderr, so a real analyzer crash becomes indistinguishable from a caller cancellation. The projection sites are `#next` and `#waitForFrameOrOutputClose` in `src/browser/WebBundleAnalysisWorker.ts`.
Author
Owner

@XinwangTao commented on GitHub (Oct 21, 2026):

This is the same precedence defect as #5393, just in the web-bundle worker's own supervisor path — the interruption branches in #next are evaluated before #observedFailure, so a post-exit abort masks the real analyzer exit and stderr. The #5393 fix (letting #primaryFailure outrank #interruptionFailure) should transfer cleanly here since the shape is identical: child exits non-zero, synchronous exit listener records it, then a setImmediate abort follows. One thing to watch in the regression test: #waitForFrameOrOutputClose also throws on an interrupted drain, so the test should assert that the interrupted-drain error does not replace the already-recorded exit — same trap as the JavaScript worker's drain path.

<!-- gh-comment-id:6311255943 --> @XinwangTao commented on GitHub (Oct 21, 2026): This is the same precedence defect as #5393, just in the web-bundle worker's own supervisor path — the interruption branches in #next are evaluated before #observedFailure, so a post-exit abort masks the real analyzer exit and stderr. The #5393 fix (letting #primaryFailure outrank #interruptionFailure) should transfer cleanly here since the shape is identical: child exits non-zero, synchronous exit listener records it, then a setImmediate abort follows. One thing to watch in the regression test: #waitForFrameOrOutputClose also throws on an interrupted drain, so the test should assert that the interrupted-drain error does not replace the already-recorded exit — same trap as the JavaScript worker's drain path.
Author
Owner

@XiaoBinGan commented on GitHub (Oct 21, 2026):

Withdrawing this report: the behaviour is already covered on main. WebBundleAnalysisWorker.#next keeps the exit branch and projects #observedFailure through #projectObservedFailure, and the existing test retains exit-7 output when abort follows the observed exit (src/browser/WebBundleAnalysisWorker.test.ts) exercises exactly this ordering — a worker that exits 7 and is aborted afterwards keeps capturedOutput.exit_code: 7 and records the cancellation as a secondary failure. I verified it passes on a5f982de8, so my reading of the loop-head ordering was wrong. Closing.

<!-- gh-comment-id:6312029673 --> @XiaoBinGan commented on GitHub (Oct 21, 2026): Withdrawing this report: the behaviour is already covered on `main`. `WebBundleAnalysisWorker.#next` keeps the exit branch and projects `#observedFailure` through `#projectObservedFailure`, and the existing test `retains exit-7 output when abort follows the observed exit` (`src/browser/WebBundleAnalysisWorker.test.ts`) exercises exactly this ordering — a worker that exits 7 and is aborted afterwards keeps `capturedOutput.exit_code: 7` and records the cancellation as a secondary failure. I verified it passes on `a5f982de8`, so my reading of the loop-head ordering was wrong. Closing.
Author
Owner

@XinwangTao commented on GitHub (Oct 22, 2026):

Thanks for verifying on a5f982de8 — the existing exit-7-then-abort test in WebBundleAnalysisWorker.test.ts covering exactly the ordering this report claimed was broken settles it. That the interrupted drain is projected through #projectObservedFailure rather than thrown also addresses the trap I flagged; the drain error stays secondary and exit_code 7 remains primary. Closing as not planned is correct.

<!-- gh-comment-id:6330329044 --> @XinwangTao commented on GitHub (Oct 22, 2026): Thanks for verifying on a5f982de8 — the existing exit-7-then-abort test in WebBundleAnalysisWorker.test.ts covering exactly the ordering this report claimed was broken settles it. That the interrupted drain is projected through #projectObservedFailure rather than thrown also addresses the trap I flagged; the drain error stays secondary and exit_code 7 remains primary. Closing as not planned is correct.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#3517
No description provided.