Report uncaught exceptions from DeferredWorkTimer tasks

[robobun]

What does this PR do?

Fuzzilli fingerprint: 58b78b97e2e39882

JSC schedules FinalizationRegistry cleanup (and Atomics.waitAsync, wasm streaming compilation) through DeferredWorkTimer. Upstream's DeferredWorkTimer::doWork wraps each task in an exception scope and routes any thrown exception to reportUncaughtExceptionAtEventLoop. Bun replaces that with its own JSCTaskScheduler::runPendingWork but skipped the exception handling, so an exception thrown from e.g. a FinalizationRegistry cleanup callback was left pending and tripped releaseAssertNoException() in the caller (SIGABRT in debug/ASAN builds).

This PR wraps the deferred task in a TopExceptionScope and reports any non-termination exception via reportUncaughtExceptionAtEventLoop, matching upstream and Node.js behavior (the error surfaces as an uncaught exception and can be observed via process.on('uncaughtException')).

Repro:

const registry = new FinalizationRegistry(ArrayBuffer);
(function () {
  for (let i = 0; i < 128; i++) registry.register({}, i);
})();
for (let i = 0; i < 20; i++) Bun.gc(true);
setTimeout(() => { for (let i = 0; i < 20; i++) Bun.gc(true); }, 10);

Before:

ASSERTION FAILED: Unexpected exception observed on thread ...
Error Exception: calling ArrayBuffer constructor without new is invalid
!exception()
JavaScriptCore/ExceptionScope.h(62) : void JSC::ExceptionScope::releaseAssertNoException()

How did you verify your code works?

• Reproduced the SIGABRT with the fuzzer test case on a debug+ASAN build before the change; verified it no longer crashes after the change (exception is reported as uncaught instead).
• Added test/js/bun/util/finalization-registry-throwing-callback.test.ts with two regression tests: they fail (child SIGABRT) on a debug build without the fix and pass with it; both run via bun bd test.
• Verified behavior matches Node.js (uncaught exception, reaches process.on('uncaughtException')).

[coderabbitai]

Walkthrough

This PR adds exception capture and reporting for deferred JavaScriptCore tasks when exceptions occur during garbage collection, validates the behavior with FinalizationRegistry cleanup callback tests, and updates overlay CSS string formatting during the build process.

Changes

Exception Handling and Task Scheduling Updates

Layer / File(s)	Summary
JSC deferred task exception handling src/jsc/bindings/JSCTaskScheduler.cpp	Adds Exception.h and TopExceptionScope.h headers and wraps job->task(...) execution in DECLARE_TOP_EXCEPTION_SCOPE(vm). Captured exceptions are conditionally cleared (preserving termination) and forwarded to globalObject->globalObjectMethodTable()->reportUncaughtExceptionAtEventLoop(...).
FinalizationRegistry cleanup callback exception tests test/js/bun/util/finalization-registry-throwing-callback.test.ts	Adds two concurrent child-process tests that repeatedly force garbage collection: first validates a cleanup callback error is reported on stderr and process exits without signal; second validates a thrown cleanup callback exception reaches a process.on("uncaughtException") handler and allows normal exit.
Overlay CSS build-time stringification src/codegen/bake-codegen.ts	Changes OVERLAY_CSS build define from raw css(...) to JSON.stringify(css(...)) to alter how the generated overlay CSS is encoded in the compiled bundle.

Suggested reviewers

• Jarred-Sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding exception reporting to DeferredWorkTimer tasks in the scheduler.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description thoroughly explains what the change does, why it was needed, how it was verified with concrete test cases and reproduction steps, and includes comparative behavior notes.

_{✏️ 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.}

[robobun]

^{Updated 3:57 PM PT - May 23rd, 2026}

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

• test/regression/issue/8254.test.ts - SIGKILL on 🐧 25.04 x64
• test/regression/issue/8254.test.ts - SIGKILL on 🐧 25.04 x64-baseline
• test/regression/issue/8254.test.ts - SIGKILL on 🐧 13 x64-baseline
• test/regression/issue/8254.test.ts - SIGKILL on 🐧 13 x64

---

🧪   To try this PR locally:

bunx bun-pr 30857

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

bun-30857 --bun

[github-actions]

Found 1 issue this PR may fix:

1. z3 optimizer crashes #25454 - z3 optimizer crashes with "Uncaught exception while handling uncaught exception" panic, which matches the unhandled exception from DeferredWorkTimer tasks that this PR fixes

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #25454

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. Report exceptions from DeferredWorkTimer tasks as uncaught #30835 - Same fix for reporting uncaught exceptions from DeferredWorkTimer tasks in JSCTaskScheduler.cpp
2. Report exceptions thrown from DeferredWorkTimer tasks #30844 - Same fix for exception handling in DeferredWorkTimer tasks with FinalizationRegistry test
3. Report exceptions thrown by DeferredWorkTimer tasks as uncaughtException #30846 - Same fix for reporting DeferredWorkTimer task exceptions as uncaughtException

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/util/finalization-registry-throwing-callback.test.ts`:
- Around line 32-35: Reorder the assertions in both subprocess tests so that
output validations run before exit-code checks: move the expects referencing
proc.signalCode and exitCode to after the assertions that validate captured
output (e.g., err and any stdout expectations). Specifically, in the test that
currently asserts expect(proc.signalCode).toBeNull(); expect([0,
1]).toContain(exitCode); expect(err).toContain(...), place the
proc.signalCode/exitCode assertions at the end, and do the same for the other
test around lines showing exitCode usage; keep the same assertion logic but
assert the exit status last.

🪄 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: a49be4c8-8872-47a0-bd5c-9f26e001cd6d

📥 Commits

Reviewing files that changed from the base of the PR and between 4d443e5 and 2a0f75e.

📒 Files selected for processing (12)

• src/crash_handler/lib.rs
• src/errno/lib.rs
• src/jsc/bindings/JSCTaskScheduler.cpp
• src/perf/tracy.rs
• src/runtime/cli/Arguments.rs
• src/runtime/cli/run_command.rs
• src/runtime/cli/upgrade_command.rs
• src/runtime/jsc_hooks.rs
• src/runtime/webview/ChromeProcess.rs
• src/spawn/process.rs
• src/spawn_sys/spawn_process.rs
• test/js/bun/util/finalization-registry-throwing-callback.test.ts

[claude]

Thanks for addressing the test nits. The JSC fix looks correct (mirrors upstream DeferredWorkTimer::doWork), but since it changes exception handling in the runtime task scheduler and bundles an unrelated bake-codegen.ts change, I'd like a human to sign off.

Extended reasoning...

Overview

The substantive change is ~10 lines in src/jsc/bindings/JSCTaskScheduler.cpp: it wraps deferred-work task execution in a DECLARE_TOP_EXCEPTION_SCOPE and routes any thrown exception to reportUncaughtExceptionAtEventLoop, fixing a releaseAssertNoException() crash when a FinalizationRegistry cleanup callback throws. A new test file exercises both the unhandled-error path and the process.on('uncaughtException') path. The remaining .rs diffs are pure autofix.ci attribute reformatting (no behavior change), and src/codegen/bake-codegen.ts adds a JSON.stringify around the OVERLAY_CSS define value.

Security risks

None identified. No auth, crypto, permissions, or untrusted-input parsing is touched. The change narrows behavior from "crash on assertion" to "report uncaught exception", which is strictly safer.

Level of scrutiny

Moderate-to-high. JSCTaskScheduler::runPendingWork is on the hot path for FinalizationRegistry, Atomics.waitAsync, and wasm streaming compilation — getting the exception-scope / globalObject lookup wrong here could affect every Bun process. The fix faithfully mirrors upstream WebKit's DeferredWorkTimer::doWork pattern (top exception scope → clearExceptionExceptTermination → reportUncaughtExceptionAtEventLoop), and the bug-hunting pass found no issues, but I'd prefer a human familiar with the JSC bindings to confirm the ticket->target()->globalObject() derivation and termination-exception handling.

Other factors

• My two prior inline nits (use test.concurrent; assert output before exit code) were addressed in 5ac2f5a and are now resolved.
• The bake-codegen.ts change is unrelated to the PR title/description — it makes the OVERLAY_CSS define an explicit JSON string instead of relying on the bundler's auto-quote fallback. It looks correct, but a reviewer should confirm it's intentional to ship in this PR.
• github-actions flagged three potential duplicate PRs (#30835, #30844, #30846) for the same fix; a human should decide which one lands.
• No CODEOWNERS apply to the touched files.

[claude]

🟡 🟡 nit: the PR description still has the paragraph about wrapping OVERLAY_CSS in bake-codegen.ts with JSON.stringify, but src/codegen/bake-codegen.ts is no longer modified by this PR — the diff now only touches JSCTaskScheduler.cpp and the new test file. Drop that paragraph so the description matches the actual diff (and the earlier unresolved review comment about stale OVERLAY_CSS references in json_lexer.rs / bun-build-api.test.ts is now moot and can be resolved).

Extended reasoning...

What

The PR description still contains this paragraph:

It also makes the OVERLAY_CSS define in bake-codegen.ts an explicit JSON.stringify string literal so codegen does not depend on the bundler's define auto-quote recovery (needed when the bootstrap bun predates #30679).

But src/codegen/bake-codegen.ts is not in the PR diff anymore. The diff now contains exactly two files: src/jsc/bindings/JSCTaskScheduler.cpp and test/js/bun/util/finalization-registry-throwing-callback.test.ts. On this branch, bake-codegen.ts:56 still reads OVERLAY_CSS: css("../runtime/bake/client/overlay.css", !!debug), with no JSON.stringify wrapper.

Step-by-step

1. An earlier revision of this PR included a drive-by edit to src/codegen/bake-codegen.ts:56 wrapping OVERLAY_CSS in JSON.stringify(...). The CodeRabbit walkthrough comment from that revision still lists "Overlay CSS build-time stringification" as one of the three changes.
2. A subsequent push (likely a rebase onto main, or splitting the change into a separate PR) dropped the bake-codegen.ts hunk. git diff f161e031..ce2fd99b --name-only shows only the two remaining files.
3. The PR description was not updated to remove the paragraph describing the dropped change.
4. Inline review comment 3293301843 (anchored on src/codegen/bake-codegen.ts) discusses follow-on staleness in json_lexer.rs / bun-build-api.test.ts caused by the JSON.stringify wrap. Since the wrap no longer exists in this PR, that comment's premise is gone and it should be resolved/dismissed rather than acted on.

Why nothing in the PR prevents it

This is purely PR-metadata drift — there is no mechanism that keeps the free-text description in sync with the diff after a force-push.

Impact

Zero runtime/code impact; this is a description-accuracy issue only. The risk is reviewer/future-reader confusion: someone reading the merged PR later will look for a bake-codegen.ts change that isn't there, and the unresolved inline comment on a file that's no longer in the diff adds to that confusion.

Fix

• Delete the OVERLAY_CSS paragraph from the PR description.
• Resolve/dismiss inline comment 3293301843 as moot (the change it critiques was dropped).

(Anchored on JSCTaskScheduler.cpp because the file the description references is not in the diff.)