[robobun]

Fixes #29043

Repro

import { Worker } from 'node:worker_threads';
const worker = new Worker(`
  globalThis.addEventListener('error', (e) => {
    console.log('handled:' + e.error?.message);
    e.preventDefault();
  });
  queueMicrotask(() => { throw new Error('hmm'); });
  setTimeout(() => console.log('still alive'), 50);
`, { eval: true });
worker.on('error', err => console.log('PARENT:', err.message));
worker.on('exit', code => console.log('exit:', code));

Before:

handled:hmm
PARENT: hmm          ← should not propagate
exit: 1              ← should be 0, worker should keep running

After (matches Deno / HTML spec):

handled:hmm
still alive
exit: 0

Cause

Two independent bugs in WebWorker__dispatchError combined to ignore preventDefault():

1. In src/bun.js/bindings/webcore/Worker.cpp, the ErrorEvent was created with cancelable = false, so preventDefault() on it was a no-op.
2. The return value of dispatchEvent was discarded, and the caller always dispatched the error to the parent Worker and terminated the worker.

Per the HTML spec's "runtime script errors" algorithm for WorkerGlobalScope: the ErrorEvent must be cancelable, and if the event is cancelled (preventDefault() called or onerror returned true) the error must not be reported to the parent Worker.

Fix

• Worker.cpp: create the ErrorEvent with cancelable = true, check event.defaultPrevented() after dispatching to the worker's global scope, and return a bool to the Zig side so the caller can react.
• web_worker.zig onUnhandledRejection: when the event was prevented, roll back the caller's pre-dispatch bookkeeping (the unhandled_error_counter increment and the exit_code = 1 set by uncaughtException), restore the handler, clear any pending exception, and return instead of terminating.
• Propagate the same signal through spin's synchronous-rejected entry-point path via an error_event_prevented flag on WebWorker.
• Update flushLogs to forward the same flag.

Verification

test/regression/issue/29043.test.ts:

• bun bd test ⇒ pass
• git stash && bun bd test ⇒ fails on "Expected to contain: 'alive'" (worker terminated before the follow-up microtask ran) → confirming the test actually exercises the fix.

Existing worker error tests (worker.test.ts > error event > is fired with a string of the error, worker_threads.test.ts > error event > *) still pass unchanged.

Rebase note

Rebased onto main after #29957 (worker: rewrite lifetime/lifecycle). Conflicts
in src/bun.js/web_worker.zig were due to the upstream rewrite moving all
worker-thread functions (create, threadMain, startVM, spin,
shutdown, flushLogs, onUnhandledRejection) into a new file layout and
switching terminate paths from exitAndDeinit → shutdown() (noreturn).

Reapplied semantically on top of the new structure:

• error_event_prevented: bool field on WebWorker (under "Worker-thread only" block, next to exit_called).
• flushLogs(this, vm) — now returns bool; resets vm.log in both success and catch branches.
• onUnhandledRejection — snapshot/restore unhandled_error_counter + exit_handler.exit_code; on prevent, restore state, set worker.error_event_prevented = true, and return (caller keeps the worker alive) instead of the main-path setRequestedTerminate → shutdown.
• spin() — wraps entry-point evaluation in an evaluate_entry_point: labeled block so the load-failure catch and the rejected-promise branch can break out on prevent and fall through to the event loop (instead of shutdown()ing).

VirtualMachine.zig and Worker.cpp auto-merged cleanly. All 5 tests
(test/regression/issue/29043.test.ts) still pass on the rebased HEAD, and
test/js/web/workers/worker-terminate-lifetime.test.ts (3 cases from #29957)
remains green.

[robobun]

^{Updated 11:46 AM PT - May 1st, 2026}

❌ @robobun, your commit 9412725 has 1 failures in Build #49748 (All Failures):

• test/cli/run/no-orphans.test.ts - timeout on 🍎 26 aarch64
Details
test/cli/run/no-orphans.test.ts - timeout on 🍎 14 x64
Details
test/cli/run/no-orphans.test.ts - timeout on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 29060

That installs a local version of the PR into your bun-29060 executable, so you can run:

bun-29060 --bun

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Worker error dispatch now returns whether the dispatched error event was cancelled. The runtime records this in a new WebWorker.error_event_prevented flag and suppresses worker termination/exit-code changes when preventDefault() is used. A regression test was added for Node worker_threads async and top-level throws.

Changes

Cohort / File(s)	Summary
Worker C++ binding src/bun.js/bindings/webcore/Worker.cpp	Changed WebWorker__dispatchError signature from void to bool; make ErrorEvent cancelable, dispatch via globalEventScope->dispatchEvent(...), return whether the event was defaultPrevented() and ensure all switch paths return a bool.
WebWorker Zig runtime src/bun.js/web_worker.zig	Added error_event_prevented: bool. Updated extern WebWorker__dispatchError to return bool and propagate the result: flushLogs now returns bool and short-circuits when no VM/logs; onUnhandledRejection snapshots/restores handler, clones message, and avoids termination when prevented; spin/entrypoint load paths observe the new boolean to skip exitAndDeinit and preserve prior exitCode when appropriate. Several calls updated to handle the non-void return.
VM exit handling src/bun.js/VirtualMachine.zig	In uncaughtException, save prior exit_handler.exit_code before forcing 1; after onUnhandledRejection, restore the saved exit code when running in a worker and error_event_prevented is set, but only if Bun left it as 1.
Regression tests test/regression/issue/29043.test.ts	Added test validating that a worker error listener calling preventDefault() prevents parent-side error emission and allows the worker to remain alive and exit with code 0 for both microtask (async) and top-level throw scenarios.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: honoring preventDefault() on Worker error events, which is the core objective of this PR.
Linked Issues check	✅ Passed	The PR comprehensively addresses issue #29043 by fixing both root causes (cancelable=false and ignored return value) and propagating prevention through the worker lifecycle, matching HTML spec behavior as required.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the preventDefault() handling in Worker error events and propagating that state through the worker lifecycle. No unrelated modifications detected.
Description check	✅ Passed	The PR description comprehensively covers both required sections: 'What does this PR do?' (detailed explanation of the fix with root causes and changes) and 'How did you verify your code works?' (test verification with before/after outputs).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/29043.test.ts`:
- Around line 10-70: Add a second test case that mirrors "Worker error event
preventDefault() stops propagation and keeps worker running" but triggers the
error during initial worker evaluation (top-level throw) instead of inside
queueMicrotask; inside the new workerCode install the
globalThis.addEventListener('error', (e) => { e.preventDefault(); ... })
listener, schedule a macrotask (e.g., setImmediate or setTimeout) to keep the
worker alive and then throw at top-level so the spin / error_event_prevented
path is exercised, spawn the worker the same way (Worker with eval: true),
collect stdout/stderr and exit code, and assert that the worker's handler saw
the error, the worker printed "alive", the parent observed exit:0, and no parent
'error' events were emitted (same expectations as the original test).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 409c7cc4-dfaa-408a-a97f-f17a7370e7ad

📥 Commits

Reviewing files that changed from the base of the PR and between d63aa71 and 077497be152155d7c02cfb25879938de28dff768.

📒 Files selected for processing (3)

• src/bun.js/bindings/webcore/Worker.cpp
• src/bun.js/web_worker.zig
• test/regression/issue/29043.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/bun.js/web_worker.zig`:
- Around line 462-473: The rollback currently force-sets
vm.exit_handler.exit_code = 0 which wipes any prior exit status; instead restore
the pre-dispatch exit code. Capture the previous exit code into a local (e.g.,
prev_exit_code) at the same place you increment unhandled_error_counter / set
exit_code = 1, then here set vm.exit_handler.exit_code = prev_exit_code (or, if
prev_exit_code is absent, avoid overwriting vm.exit_handler.exit_code). Update
references around worker.error_event_prevented, vm.unhandled_error_counter,
vm.exit_handler.exit_code, vm.onUnhandledRejection and ensure prev_handler and
the new prev_exit_code are threaded through the same scope so the original exit
code is preserved when rolling back.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0b3cd9ff-ae86-4413-9912-97a1d911d11f

📥 Commits

Reviewing files that changed from the base of the PR and between 077497be152155d7c02cfb25879938de28dff768 and f507f3fafbc0432d004c2ec97197811cab6e8601.

📒 Files selected for processing (2)

• src/bun.js/web_worker.zig
• test/regression/issue/29043.test.ts

[coderabbitai]

✅ No merge conflicts detected when merging into main.

Your branch is good to go!

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/bun.js/web_worker.zig`:
- Around line 438-445: The flushLogs() path currently sets
this.error_event_prevented = true when WebWorker__dispatchError returns
prevented, but callers (e.g., code that calls flushLogs(); exitAndDeinit();)
don’t observe that and still tear down the worker; change flushLogs() to return
the prevented boolean (or a tri-state indicating handled) instead of only
mutating this.error_event_prevented, propagate that return value to callers and
update callers that currently call flushLogs(); exitAndDeinit(); to check the
returned prevented flag and skip exitAndDeinit() when true; keep setting
this.error_event_prevented for backward compatibility but ensure the
immediate-returning callers honor the returned prevented value so cancelled
error events prevent shutdown.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0a07f9dc-1e0f-40fb-bd36-19567fc99573

📥 Commits

Reviewing files that changed from the base of the PR and between f507f3fafbc0432d004c2ec97197811cab6e8601 and f4a6b5f466c7d8581dd846f2eaafd86dbc5b051b.

📒 Files selected for processing (3)

• src/bun.js/VirtualMachine.zig
• src/bun.js/bindings/webcore/Worker.cpp
• src/bun.js/web_worker.zig

[coderabbitai]

♻️ Duplicate comments (2)

src/bun.js/web_worker.zig (2)

641-642: ⚠️ Potential issue | 🟠 Major

Honor canceled flushLogs() errors before final teardown.

This tail path still calls exitAndDeinit() unconditionally. If the dispatched worker error event is canceled here and its handler schedules more work, the worker is terminated anyway, which defeats the new flushLogs(): bool contract.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/bun.js/web_worker.zig` around lines 641 - 642, The tail path currently
ignores the flushLogs() outcome and always calls exitAndDeinit(); change it to
honor flushLogs()'s boolean contract by capturing its return value (e.g. let ok
= this.flushLogs()) and only call this.exitAndDeinit() when flushLogs() returns
true; if flushLogs() indicates cancellation/false, skip/return early so the
worker isn't torn down while a canceled error handler intends to schedule more
work.

---

570-581: ⚠️ Potential issue | 🟠 Major

Restore the pre-error exit code here, not 0.

Lines 579-581 still overwrite any earlier process.exitCode = N set by preloads. If the load error is canceled, this path should restore the prior exit code rather than hard-resetting to success.

Suggested fix

-        var promise = vm.loadEntryPointForWebWorker(path) catch {
-            // If we called process.exit(), don't override the exit code
-            if (!this.exit_called) vm.exit_handler.exit_code = 1;
+        var promise = vm.loadEntryPointForWebWorker(path) catch {
+            const prev_exit_code = vm.exit_handler.exit_code;
+            // If we called process.exit(), don't override the exit code
+            if (!this.exit_called) vm.exit_handler.exit_code = 1;
             // A preload may have run and registered an `error` event
             // listener before the main module failed to load. If that
             // listener cancels the event, skip termination and let the
             // worker's event loop decide when to exit. There is no module
             // to finish running, but a preload may have scheduled work
             // (timers, microtasks) that should keep the worker alive.
             if (this.flushLogs()) {
-                vm.exit_handler.exit_code = 0;
+                vm.exit_handler.exit_code = prev_exit_code;
                 break :evaluate_entry_point;
             }
             this.exitAndDeinit();
             return;
         };

Based on learnings, when a prevented worker error fires, the implementation should preserve any pre-error process.exitCode = N rather than forcing 0.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/bun.js/web_worker.zig` around lines 570 - 581, The error-handling branch
after vm.loadEntryPointForWebWorker currently forces vm.exit_handler.exit_code =
0 when this.flushLogs() returns true, which overwrites any prior exit code set
by preloads; change this to restore the pre-error exit code instead of
hard-resetting to 0 by using the previously saved exit code (e.g., a
savedExitCode/this.saved_exit_code captured before the load attempt) or by
avoiding assignment if a prior exit code exists; update the block around
vm.loadEntryPointForWebWorker, this.exit_called, this.flushLogs(), and
vm.exit_handler.exit_code in the evaluate_entry_point flow to restore that saved
value.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/bun.js/web_worker.zig`:
- Around line 641-642: The tail path currently ignores the flushLogs() outcome
and always calls exitAndDeinit(); change it to honor flushLogs()'s boolean
contract by capturing its return value (e.g. let ok = this.flushLogs()) and only
call this.exitAndDeinit() when flushLogs() returns true; if flushLogs()
indicates cancellation/false, skip/return early so the worker isn't torn down
while a canceled error handler intends to schedule more work.
- Around line 570-581: The error-handling branch after
vm.loadEntryPointForWebWorker currently forces vm.exit_handler.exit_code = 0
when this.flushLogs() returns true, which overwrites any prior exit code set by
preloads; change this to restore the pre-error exit code instead of
hard-resetting to 0 by using the previously saved exit code (e.g., a
savedExitCode/this.saved_exit_code captured before the load attempt) or by
avoiding assignment if a prior exit code exists; update the block around
vm.loadEntryPointForWebWorker, this.exit_called, this.flushLogs(), and
vm.exit_handler.exit_code in the evaluate_entry_point flow to restore that saved
value.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 86ebc53c-eb9a-4c5d-bbff-46d85053b5b5

📥 Commits

Reviewing files that changed from the base of the PR and between f4a6b5f466c7d8581dd846f2eaafd86dbc5b051b and 24df2a5ca9dd1a3928e9ace55593812a357a31a1.

📒 Files selected for processing (4)

• src/bun.js/VirtualMachine.zig
• src/bun.js/bindings/webcore/Worker.cpp
• src/bun.js/web_worker.zig
• test/regression/issue/29043.test.ts

[claude]

---

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 src/bun.js/VirtualMachine.zig:685-706 — When --unhandled-rejections=strict or =throw is used in a worker (via execArgv), a preventDefault()-cancelled error event still triggers a spurious UnhandledRejectionWarning (strict mode) or a second dispatch of the error event (throw mode), because uncaughtException returns handled=false even when worker.error_event_prevented is set. The fix is to return true from uncaughtException when worker.error_event_prevented is set, and to guard the emitUnhandledRejectionWarning call in the .strict branch with a worker.error_event_prevented check.

  Extended reasoning...

  Root cause: In VirtualMachine.zig, uncaughtException() (lines 685–706) correctly saves/restores exit_handler.exit_code when worker.error_event_prevented is set, but still returns handled = Bun__handleUncaughtException(...) > 0 — which is false when the C++ handler returns 0 (the normal case). The flag error_event_prevented does not influence the return handled at line 705.

  How it manifests — .strict mode (line 621):
  The call is _ = this.uncaughtException(globalObject, wrapped_reason, true) — the return value is discarded. Inside uncaughtException, onUnhandledRejection is called, the worker error listener detects preventDefault(), sets worker.error_event_prevented = true, restores the handler, and returns early (the new path this PR introduces). uncaughtException restores prev_exit_code and returns false. Back in .strict, execution continues unconditionally to Bun__handleUnhandledRejection (returns 0, no JS rejection handler) and then to Bun__promises__emitUnhandledRejectionWarning at line 623 — emitting a spurious UnhandledRejectionWarning even though the error was properly cancelled.

  How it manifests — .throw mode (lines 636–649):
  The call is if (this.uncaughtException(globalObject, wrapped_reason, true)) — when the event is prevented, uncaughtException returns false, so the early-return guard at lines 637–641 is NOT taken. After drainMicrotasks(), execution falls through to line 648 (unhandled_error_counter += 1) and line 649 (this.onUnhandledRejection(this, globalObject, reason)) — a second dispatch of the error event with the original unwrapped reason. The onUnhandledRejection function resets error_event_prevented = false at its entry, so a stateful one-shot listener that set a handled-flag on the first dispatch will NOT call preventDefault() the second time, causing the worker to terminate despite the first prevention succeeding.

  Why these paths were unreachable before this PR: Before this PR, web_worker.onUnhandledRejection was effectively noreturn — it always ended with exitAndDeinit(). This PR's new if (prevented) { ... return; } path is what makes lines 623 and 648–649 reachable for the first time from these code paths.

  Step-by-step proof for .throw mode:

  1. Worker spawned with execArgv: ['--unhandled-rejections=throw']. Listener: let once = false; globalThis.addEventListener('error', e => { if (\!once) { once = true; e.preventDefault(); } }).
  2. Unhandled rejection fires. unhandledRejection() enters .throw case; Bun__handleUnhandledRejection returns 0.
  3. uncaughtException(wrapped_reason) is called — first dispatch via onUnhandledRejection. Listener calls preventDefault(), sets worker.error_event_prevented = true, returns early.
  4. uncaughtException restores exit code and returns handled = false.
  5. .throw branch: uncaughtException returned false, early return NOT taken. Falls through to line 649: second call to onUnhandledRejection with unwrapped reason.
  6. onUnhandledRejection resets error_event_prevented = false. Listener has once = true, does NOT call preventDefault(). Worker calls setRequestedTerminate() and exitAndDeinit() — terminates despite first dispatch being properly prevented.

  Fix: Return true from uncaughtException when worker.error_event_prevented is set (fixes .throw double-dispatch). Additionally, guard the emitUnhandledRejectionWarning call in the .strict case with a worker.error_event_prevented check (since the return value is discarded there, the fix to uncaughtException alone does not help for strict mode).

[claude]

---

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 src/bun.js/VirtualMachine.zig:679-700 — In --unhandled-rejections=strict mode, the .strict case of unhandledRejection() calls _ = this.uncaughtException(...) and discards the return value; before this PR uncaughtException() in a worker was effectively noreturn, so the subsequent Bun__handleUnhandledRejection and emitUnhandledRejectionWarning calls were unreachable from a worker context. After this PR, when the worker's error listener calls preventDefault(), uncaughtException() returns instead of terminating, but because the .strict case discards the return value it continues executing and emits a spurious UnhandledPromiseRejectionWarning even though the error was properly cancelled. Add a guard after the _ = this.uncaughtException(...) call in the .strict case that checks worker.error_event_prevented and returns early if set, mirroring the fix already applied to the .throw case.

  Extended reasoning...

  What the bug is and how it manifests

  In VirtualMachine.zig, the unhandledRejection() function has a .strict case (for --unhandled-rejections=strict mode) that calls uncaughtException() and explicitly discards the return value with _ = this.uncaughtException(globalObject, wrapped_reason, true). It then unconditionally proceeds to call Bun__handleUnhandledRejection and (if that returns 0) emitUnhandledRejectionWarning. Before this PR, uncaughtException() in a worker context always ended in exitAndDeinit() (noreturn) because onUnhandledRejection always called exitAndDeinit. This made the post-call lines unreachable from a worker in .strict mode. After this PR, when the worker's error listener calls preventDefault(), onUnhandledRejection returns early instead of calling exitAndDeinit(), which means uncaughtException() now returns — but the .strict case cannot observe this because it discards the return value.

  The specific code path that triggers it

  Worker spawned with execArgv: ['--unhandled-rejections=strict'] registers an error listener calling e.preventDefault(). An unhandled rejection fires. The .strict case: (1) calls _ = this.uncaughtException(globalObject, wrapped_reason, true) — inside this, onUnhandledRejection dispatches the error event, the listener calls preventDefault(), sets worker.error_event_prevented = true, and returns early. uncaughtException restores the exit code and returns false. (2) Execution continues in .strict — there is no guard, so it reaches Bun__handleUnhandledRejection (line 616), which returns 0 since there is no JS-level process.on('unhandledRejection') handler. (3) emitUnhandledRejectionWarning fires — printing a spurious UnhandledPromiseRejectionWarning to stderr even though the error was properly cancelled.

  Why existing code does not prevent it

  The .throw case (lines 622–636) correctly checks the return value of uncaughtException(): if (this.uncaughtException(...)) and returns early if true. The .strict case uses _ = to discard the same return value, so it has no way to know the error was prevented. The new early-return path introduced by this PR (when error_event_prevented is true) makes lines 616–617 reachable for the first time in a worker context, turning previously dead code into a live bug path.

  What the impact would be

  The worker stays alive correctly and exits with code 0 — the primary fix from this PR works. However, a spurious UnhandledPromiseRejectionWarning is printed to the worker's stderr. This is incorrect per HTML spec intent (a prevented error should be fully suppressed) and confuses users who see a warning for an error they explicitly handled. The impact is limited to --unhandled-rejections=strict, which is a non-default, deprecated Node.js mode.

  How to fix it

  After the _ = this.uncaughtException(...) call in the .strict case, add a guard that checks worker.error_event_prevented and returns early if set — mirroring the behavior of the .throw case:

  // In the .strict case, after _ = this.uncaughtException(...)
  if (this.worker) |worker| {
      if (worker.error_event_prevented) {
          worker.error_event_prevented = false;
          return;
      }
  }

  Step-by-step proof

  1. Worker is spawned with execArgv: ['--unhandled-rejections=strict'] and an error listener: globalThis.addEventListener('error', e => e.preventDefault()).
  2. An unhandled rejection fires; unhandledRejection() enters the .strict case.
  3. Bun__handleUnhandledRejection returns 0 (no JS-registered unhandledRejection handler).
  4. wrapped_reason is created; _ = this.uncaughtException(globalObject, wrapped_reason, true) is called.
  5. Inside uncaughtException, onUnhandledRejection dispatches the cancelable ErrorEvent; the listener calls e.preventDefault(). worker.error_event_prevented is set to true; onUnhandledRejection returns early. uncaughtException restores the exit code and returns false.
  6. Back in .strict: the return value (false) is discarded. No guard checks error_event_prevented.
  7. Bun__handleUnhandledRejection is called again (line 616) and returns 0.
  8. emitUnhandledRejectionWarning fires — a spurious warning is printed to stderr even though preventDefault() was called.

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 test/regression/issue/29043.test.ts:115-121 — The comment in Test 3 incorrectly claims to exercise the stale-flag reset at onUnhandledRejection entry (web_worker.zig:574), but the test only throws one error — making that reset a no-op (false→false). The test still provides real value as a guard against double-dispatch of a single error, but removing the reset at line 574 entirely would not cause this test to fail. To actually exercise the stale-flag scenario described in the comment, the test would need two consecutive errors: the first prevented (leaving error_event_prevented=true), then the second dispatching normally to verify the reset cleared the stale flag before dispatch.

  Extended reasoning...

  What the comment claims vs. what the test actually does

  The comment on Test 3 (lines 115-121) reads: "This exercises onUnhandledRejection's vm.worker.error_event_prevented = false reset at entry (which guards against a stale prevented-flag from an earlier cancelled error being observed by a new dispatch)." However, the test worker only throws a single error via queueMicrotask(() => { throw new Error("first"); }). With a single error, error_event_prevented begins at its default value of false, so the reset at web_worker.zig line 574 executes false→false — a complete no-op. There is no "earlier cancelled error" to leave a stale true in place.

  The specific code path and why the reset is never meaningful here

  When onUnhandledRejection is entered, it unconditionally runs if (vm.worker) |w| w.error_event_prevented = false; before dispatching the event. For the stale-flag reset to matter, a prior invocation must have set error_event_prevented = true (i.e., a previous error was cancelled via preventDefault()). With only one error, that condition never occurs. The flag goes false→false on entry, then false→true inside the prevented branch when the listener calls preventDefault(). No stale state is ever present.

  What the test actually guards against

  Despite the inaccurate comment, Test 3 provides genuine regression value. The dispatch:1 / not dispatch:2 / total:1 assertions would catch any bug that caused a single error to be dispatched to the worker's error listener twice — for example, a regression in the vm.log.reset() fix in flushLogs() added in this PR. That double-dispatch protection is real and useful, but it is a different mechanism from the stale-flag reset described in the comment.

  Why removing the reset would not break this test

  If the reset at line 574 were deleted entirely, this test would still pass with identical output. The only way to detect a regression in that reset would be to throw two consecutive errors: first one with preventDefault() called (leaving error_event_prevented = true), then a second error dispatched normally — and assert that the second dispatch is not skipped due to a stale true. Without the second error, the reset is simply never tested against a non-default state.

  Addressing the refutation

  The refuter correctly notes this is a comment-accuracy issue, not a runtime bug. The test behavior is correct and the test name accurately describes what it verifies. This is filed as a nit because the comment misleads future maintainers into believing the stale-flag reset is covered by regression tests when it is not, creating a false sense of coverage for that specific code path. The fix is either to update the comment to accurately describe the double-dispatch guard, or to add a second error throw to genuinely cover the stale-flag scenario.

• 🟡 src/bun.js/VirtualMachine.zig:638-640 — The .strict unhandled-rejection guard at VirtualMachine.zig:639-641 returns early without resetting worker.error_event_prevented to false, leaving a stale true that would cause a subsequent error's Bun__handleUnhandledRejection dispatch to be incorrectly skipped — even if no listener called preventDefault() for that second error. The fix is a one-liner: add worker.error_event_prevented = false; before the return. This path is currently unreachable since .strict mode is not yet wired for worker threads via execArgv (TODO at web_worker.zig:427), but the logic flaw lives in newly-added code and will activate when that TODO is resolved.

  Extended reasoning...

  What the bug is and how it manifests

  In VirtualMachine.zig's .strict case of unhandledRejection(), the PR added a guard (lines 639-641):

  if (this.worker) |worker| {
      if (worker.error_event_prevented) return;
  }

  This guard correctly prevents Bun__handleUnhandledRejection from firing when a worker error listener called preventDefault(). However, it returns without first resetting worker.error_event_prevented = false. The flag stays true after the early return.

  The specific code path that triggers it

  The only place error_event_prevented is reset to false is at the entry of onUnhandledRejection (web_worker.zig, inside the if (vm.worker) |w| w.error_event_prevented = false; line). That reset only fires when onUnhandledRejection is invoked. Inside uncaughtException, onUnhandledRejection is only called when Bun__handleUncaughtException returns 0 (the if (\!handled) block). If Bun__handleUncaughtException returns > 0 for a subsequent error — meaning a process.on('uncaughtException') handler handled it — the if (\!handled) block is skipped entirely, onUnhandledRejection is never called, and the reset never fires.

  Concrete trigger sequence

  1. Worker runs in .strict mode (via the unreachable-until-TODO-resolved execArgv path) with both globalThis.addEventListener('error', e => e.preventDefault()) AND process.on('uncaughtException', handler).
  2. Error 1: Bun__handleUncaughtException returns 0 → onUnhandledRejection called → listener prevents → error_event_prevented = true. The .strict guard sees true and returns — without resetting the flag.
  3. Error 2: Bun__handleUncaughtException returns > 0 (the process.on('uncaughtException') handler fires) → onUnhandledRejection is NOT called → the entry-level reset never fires → error_event_prevented remains true (stale).
  4. The .strict guard sees the stale true and returns early, skipping Bun__handleUnhandledRejection for Error 2 — even though no listener actually called preventDefault() for it.

  Why existing code doesn't prevent it

  The .throw mode counterpart avoids this because it checks the return value of uncaughtException() directly (if (this.uncaughtException(...)) { ... return; }), and uncaughtException itself sets handled = true when error_event_prevented is observed (line 723). The .strict mode discards the return value with _ = and relies solely on the field-based guard — which is the guard that fails to clear the flag.

  Addressing the refutation

  The refuter correctly notes this path is currently unreachable: web_worker.zig:427 has a TODO confirming that --unhandled-rejections=strict/throw from execArgv is not yet routed to the worker's transform options. This is accurate and means no user can currently trigger the bug. However, the guard was added precisely as defense-in-depth for when that TODO is addressed, and the missing reset is a correctness flaw in the newly-introduced logic. A one-line fix now avoids a silent regression later.

  How to fix it

  Add worker.error_event_prevented = false; before the return at line 640:

  if (this.worker) |worker| {
      if (worker.error_event_prevented) {
          worker.error_event_prevented = false;
          return;
      }
  }

[claude]

LGTM — all prior review concerns addressed in the final commit.

Extended reasoning...

Overview

This PR fixes two independent bugs that caused on a Worker's event to be silently ignored: (1) was created with , making a no-op; (2) the return value of was discarded, so the worker always propagated the error to the parent and terminated. The fix spans (make the event cancelable, return a ), (consume the bool, roll back / / handler, add bool return, reset log after dispatch), and (save/restore around , guard the case before ).

Security risks

No security-sensitive code paths touched. The change is entirely within worker error-event dispatch semantics.

Level of scrutiny

This PR went through extensive multi-round review. Multiple real bugs were identified and fixed iteratively: BunString leak in the prevented path, exit-code clobbering, log double-dispatch after the new fallthrough path, mode firing despite the event being cancelled, and a misleading test that claimed to exercise but didn't. All were resolved in the final commit.

Other factors

• Three regression tests covering async-throw, top-level-throw, and single-dispatch guard paths.
• The mode guard is now correctly placed before .
• after a successful dispatch prevents double-dispatch on subsequent calls.
• No new bugs found by the automated bug hunting system.

[claude]

LGTM — all prior issues have been addressed. The mode guard, double-dispatch fix, memory leak, log re-dispatch, and exit-code preservation are all correct in the final state.

Extended reasoning...

Overview

This PR fixes two root-cause bugs in WebWorker__dispatchError: cancelable = false on the ErrorEvent prevented preventDefault() from having any effect, and the dispatch return value was never checked, so the worker always terminated. The fix spans Worker.cpp, web_worker.zig, and VirtualMachine.zig.

Review of issues raised in prior runs

All issues I raised across multiple prior reviews have been addressed in subsequent commits:

• Memory leak of bun.String in onUnhandledRejection (prevented path) → fixed with defer message_str.deref()
• flushLogs() re-dispatching the same error (Log.toJS does not clear msgs) → fixed with vm.log.reset() after dispatch
• flushLogs() return value not consumed by early-startup callers in spin() → fixed by returning bool and checking in evaluate_entry_point
• Exit-code clobbering when listener sets process.exitCode = N during dispatch → fixed with prev_exit_code snapshot + conditional restore
• .strict mode calling Bun__handleUnhandledRejection before the error_event_prevented guard → fixed (guard now precedes the call)
• Test 3 using queueMicrotask instead of a Promise rejection to exercise the .throw branch → fixed (test removed; author noted execArgv flags are not yet routed to worker transform options, so the branch is unreachable in tests)

Security risks

No security-sensitive code paths are involved. The change is scoped to worker lifecycle management within the same origin/process.

Level of scrutiny

The logic spans non-trivial async state (error flags across function-call boundaries, exit-code bookkeeping, C++/Zig FFI return values). Multiple rounds of review were necessary to get it right. The current state is correct: the takeWorkerErrorEventPrevented() read-and-clear helper in the .strict path is a good safety measure against stale flags.

Other factors

The regression test covers three cases (async throw, top-level throw, double-dispatch guard). The --unhandled-rejections=throw/strict worker paths are currently unreachable (TODO in web_worker.zig:427) and are handled as defense-in-depth. Existing worker error tests pass.

[claude]

No issues found on the current revision — all prior feedback has been addressed. That said, this threads a new keep-alive path through worker error handling, exit-code restoration, and the .bun/.strict/.throw rejection modes with several snapshot/restore interactions, so it's worth a maintainer pass before merge.

Extended reasoning...

Overview

This PR makes preventDefault() on a worker's global error event actually suppress propagation to the parent and keep the worker alive, per the HTML spec. It touches:

• src/bun.js/bindings/webcore/Worker.cpp — WebWorker__dispatchError now creates a cancelable ErrorEvent, checks defaultPrevented(), and returns bool.
• src/bun.js/web_worker.zig — adds error_event_prevented flag; flushLogs() returns bool and resets vm.log; onUnhandledRejection snapshots/restores unhandled_error_counter + exit_handler.exit_code and early-returns on prevent; spin() wraps entry-point evaluation in a labeled block so a cancelled load-failure error falls through to the event loop.
• src/bun.js/VirtualMachine.zig — uncaughtException clears the flag at entry, snapshots/restores exit_code, marks handled = true on prevent; .strict mode in unhandledRejection consumes the flag via takeWorkerErrorEventPrevented() before Bun__handleUnhandledRejection; two new helper functions.
• New regression test with 5 cases covering async throw, top-level throw, no-state-leak across dispatches, listener-that-throws, and inherited --unhandled-rejections=strict.

Security risks

None identified. This is error-propagation/lifecycle logic with no auth, crypto, network, or untrusted-input parsing implications. The FFI signature change (void → bool) is matched on both sides.

Level of scrutiny

High. This is core runtime error-handling on a hot path (worker uncaught exceptions / unhandled rejections), with state spread across three files and at least three distinct entry routes (uncaughtException direct, .bun-mode unhandledRejection fallthrough, spin() entry-point rejection) plus the flushLogs load-failure path. The PR went through ~12 rounds of bot review uncovering subtle bugs — memory leak, exit-code clobbering, double-dispatch, stale-flag leak, snapshot placement — all of which were fixed, but the density of edge cases is itself a signal that the design has many interacting invariants.

Other factors

• All inline comments (mine and CodeRabbit's) are resolved; the current revision is clean per the bug hunter.
• The author explicitly scoped out two preexisting issues (listener-throws → re-entrancy panic via is_handling_uncaught_exception; release-build worker-teardown race) as follow-ups — reasonable, but a maintainer should confirm that scoping.
• The tests deliberately don't assert on stderr or the outer process exit code due to unrelated flakiness, and stay sequential because test.concurrent tickles a teardown race; both are documented in comments.
• No human reviewer has engaged yet — only bots.

Given the complexity and the number of subtle interactions surfaced during review, this should get a maintainer's eyes rather than a bot approval.

[claude]

No new issues found and all prior feedback is addressed, but this threads subtle state (error_event_prevented, exit-code/counter snapshot-restore) through uncaughtException/unhandledRejection/spin() across the Zig↔C++ boundary and was just rebased onto the #29957 worker-lifecycle rewrite — worth a human pass before merge.

Extended reasoning...

Overview

The PR makes the Worker global error event cancelable and honors preventDefault(): Worker.cpp now creates the ErrorEvent with cancelable = true and returns defaultPrevented() to Zig; web_worker.zig adds an error_event_prevented flag plus snapshot/restore of unhandled_error_counter and exit_handler.exit_code in onUnhandledRejection/flushLogs, and reworks spin()'s entry-point evaluation into a labeled block so a cancelled error falls through to the event loop instead of shutdown(); VirtualMachine.zig gains entry-reset of the flag in uncaughtException, a .strict-mode guard, and workerErrorEventPrevented/takeWorkerErrorEventPrevented helpers. A 5-case regression test is added.

Security risks

None apparent — this is error-propagation/lifecycle control flow inside the worker runtime; no auth, parsing, or untrusted-input surface is touched.

Level of scrutiny

High. This is core runtime error handling that went through ~12 iterative bug-fix rounds in review (memory leak, double-dispatch, stale-flag leakage across .strict mode, exit-code clobbering, counter over/under-count on listener re-entry), and was just semantically reapplied on top of the #29957 worker lifetime/lifecycle rewrite. The state machine is subtle: the error_event_prevented flag is set in one place, read/consumed in four, and reset in three, with ordering constraints documented in comments. The rebase note is thorough and tests (including the #29957 lifetime tests) reportedly pass, but the interaction surface is large enough that a maintainer familiar with the new shutdown()-noreturn structure should confirm the reapplied semantics.

Other factors

All inline comments from prior review rounds are resolved; the bug-hunting system found no new issues on the rebased HEAD. Two items were explicitly scoped out as preexisting/follow-ups (the is_handling_uncaught_exception re-entry panic when a listener itself throws on the direct-uncaughtException path, and the release-build worker-teardown race). Those are reasonable deferrals but worth a maintainer's awareness.