Report exceptions from DeferredWorkTimer tasks as uncaught

[robobun]

When a FinalizationRegistry cleanup callback (or any other DeferredWorkTimer task) throws, the exception was leaking out of Bun__runDeferredWork. In debug/ASAN builds this trips the caller's assertNoExceptionExceptTermination() in JSCDeferredWorkTask::run:

ASSERTION FAILED: Unexpected exception observed
!exception()
ExceptionScope.h(62) : void JSC::ExceptionScope::releaseAssertNoException()

Repro

const fr = new FinalizationRegistry(ArrayBuffer);
(() => { fr.register({}, "held"); })();
Bun.gc(true);
setImmediate(() => {});

ArrayBuffer is callable so it passes the FinalizationRegistry constructor check, but throws when invoked without new. Any throwing cleanup callback reproduces.

Fix

Match JSC's own DeferredWorkTimer::doWork: wrap the task invocation in a TopExceptionScope and, if a non-termination exception is pending afterward, clear it and report it via reportUncaughtExceptionAtEventLoop. This routes the error through process.on("uncaughtException") as Node does.

Found by Fuzzilli (fingerprint bfc05fc36c3ce2cf).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dc6110b8-4f5b-46d5-8ddf-1b0799918076

📥 Commits

Reviewing files that changed from the base of the PR and between 93bd662 and 5d967d6.

📒 Files selected for processing (1)

• src/codegen/bake-codegen.ts

💤 Files with no reviewable changes (1)

• src/codegen/bake-codegen.ts

---

Walkthrough

This PR adds exception handling for deferred JavaScript tasks in the JSC binding layer, wraps those changes with integration tests for FinalizationRegistry cleanup callbacks, and updates overlay CSS embedding in the build codegen to use JSON.stringify encoding.

Changes

Exception handling and build updates

Layer / File(s)	Summary
JSC deferred work exception scope and reporting src/jsc/bindings/JSCTaskScheduler.cpp	Adds ZigGlobalObject.h include and wraps deferred job task execution in a top-level exception scope, capturing exceptions and reporting non-termination exceptions to the event loop via the job's realm/global object.
FinalizationRegistry cleanup callback integration tests test/js/bun/util/finalization-registry-throwing-callback.test.ts	Adds test module wiring with helper imports and two subprocess integration tests that trigger FinalizationRegistry cleanup callbacks via GC, validating that thrown errors are reported via stderr or caught by uncaughtException handlers with correct exit codes.
Overlay CSS string encoding in build output src/codegen/bake-codegen.ts	Wraps the OVERLAY_CSS build constant with JSON.stringify to properly encode the CSS string value in generated runtime bundles.

Suggested Reviewers

• RiskyMH
• dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately and specifically summarizes the main change: reporting exceptions from DeferredWorkTimer tasks as uncaught exceptions.
Description check	✅ Passed	The PR description provides comprehensive detail including problem statement, reproducer code, root cause analysis, and solution approach matching the template's structure.
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.

_{✏️ 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 7:27 AM PT - May 22nd, 2026}

❌ @robobun, your commit 5d967d6 has some failures in Build #56857 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 30835

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

bun-30835 --bun

[github-actions]

This PR may be a duplicate of:

1. jsc: handle exceptions thrown from deferred work tasks #30822 - Same fix in JSCTaskScheduler.cpp wrapping DeferredWorkTimer task execution in TopExceptionScope and routing FinalizationRegistry exceptions through reportUncaughtExceptionAtEventLoop
2. Report exceptions thrown from DeferredWorkTimer tasks as uncaught #30824 - Same fix for the same DeferredWorkTimer/FinalizationRegistry exception handling issue, with identical core logic

🤖 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`:
- Line 26: Remove the panic-style negative stderr assertion by deleting the
expect(err).not.toContain("ASSERTION"); check in the test file
finalization-registry-throwing-callback.test.ts and instead rely on existing
positive behavior assertions (or add explicit positive assertions about the
expected output/side-effects) so the test no longer asserts absence of
"ASSERTION" and only verifies correct, stable outcomes.

🪄 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: 5381e193-ca8c-4f29-8d64-3d75a3b2a3f5

📥 Commits

Reviewing files that changed from the base of the PR and between 314d044 and ec12da8.

📒 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]

🟡 nit: per test/CLAUDE.md, tests in the same file that spawn independent subprocesses should use test.concurrent (or wrap in describe.concurrent). Both tests here spawn standalone Bun processes with no shared state, so they can run in parallel — applies to this test and the one at line 31.

Extended reasoning...

What

test/CLAUDE.md (line 22) documents a repo convention:

Prefer concurrent tests over sequential tests: When multiple tests in the same file spawn processes or write files, make them concurrent with test.concurrent or describe.concurrent unless it's very difficult to make them concurrent.

This new file declares two tests with plain test(...):

• "FinalizationRegistry callback that throws does not crash the process" (line 4)
• "FinalizationRegistry callback that throws is catchable via uncaughtException" (line 31)

Both spawn a fresh Bun subprocess via Bun.spawn, read its stdio, and assert on the result.

Why this applies here

Walking through the two tests concretely:

1. Test 1 spawns bun -e <script> with stderr: "pipe", awaits proc.stderr.text() and proc.exited, and asserts on those values.
2. Test 2 spawns a different bun -e <script> with its own stdout/stderr pipes and asserts on those.

There is no shared mutable state between them — no shared temp dir, no shared port, no module-level fixture, no ordering dependency. Each subprocess is hermetic. Running them sequentially is purely wasted wall-clock time; running them concurrently cannot change their outcome.

Why nothing prevents it

The tests are correct as written; this is purely a project-convention / CI-throughput concern. Nothing in the harness enforces test.concurrent, so it has to be caught in review.

Impact

Minor. The two subprocesses are tiny and exit quickly, so the wall-clock saving is small. This is flagged only because it's an explicit, documented convention for new test files in this repo.

Fix

Either change both declarations to test.concurrent(...), or wrap them in describe.concurrent("FinalizationRegistry throwing callback", () => { ... }) and keep the inner test(...) calls.

[claude]

🟡 Wrapping OVERLAY_CSS in JSON.stringify() means bake-codegen.ts no longer passes raw minified CSS verbatim as a define value, so the explanatory comments at src/parsers/json_lexer.rs:1302-1306 and test/bundler/bun-build-api.test.ts:73-76 — which both cite "bake-codegen.ts's OVERLAY_CSS" as the motivating example for auto-quoting define values that start with *{...} — are now factually stale. The lexer special-case and test remain valid for user-supplied raw-CSS define values; only the bake-codegen citation needs updating (or just dropping the example). Also worth noting: this drive-by build fix is unrelated to the PR's stated DeferredWorkTimer purpose.

Extended reasoning...

What the issue is

Commit 345cdc9 changes src/codegen/bake-codegen.ts:56 from passing css(...) directly as the OVERLAY_CSS define value to passing JSON.stringify(css(...)). After this change, the define value is a JSON string literal (e.g. "\"*{box-sizing:...}\""), not raw CSS starting with *{. Two existing in-tree comments document the old behavior as their motivating example:

1. src/parsers/json_lexer.rs:1302-1306 — explains why * / ? / ( / ) are tokenized in JSON mode without erroring, so the auto-quote fallback can rescue "a Bun.build define: whose value is a raw minified CSS string starting with *{...} (bake-codegen.ts's OVERLAY_CSS)".
2. test/bundler/bun-build-api.test.ts:73-76 — "a raw minified CSS string starts with *{...}, which src/codegen/bake-codegen.ts passes verbatim as OVERLAY_CSS".

Both are present-tense factual claims about what bake-codegen.ts does, and after this PR both are false.

The code path

Before this PR, bake-codegen.ts ran Bun.build({ define: { OVERLAY_CSS: css(...) } }) where css() returns minified CSS like *{box-sizing:border-box}.... The bundler's define-value parser (parse_env_json → JSONLikeParser) sees a value starting with *, which is not valid JSON. The json_lexer special-case at lines 1297-1309 tokenizes */?/(/) without erroring so that parse_expr's auto-quote fallback can wrap the whole thing as a string literal. The two comments cite OVERLAY_CSS as the concrete in-tree consumer that motivated this special-case.

After this PR, bake-codegen.ts passes JSON.stringify(css(...)) — a value starting with ", which the JSON lexer parses as a string literal directly. The auto-quote fallback is never reached for OVERLAY_CSS.

Why nothing prevents it

This is purely documentation drift. Nothing in the build or test suite checks that explanatory comments stay synchronized with the code they reference. The json_lexer special-case itself remains load-bearing (the test at bun-build-api.test.ts:77-81 covers *{box-sizing:border-box}..., ?foo, (parenthesized), )close — all user-reachable via Bun.build({ define: { X: "*{...}" } })), so removing the code would be wrong; only the comment's "this is why bake-codegen needs it" justification is stale.

Step-by-step proof

1. css("../runtime/bake/client/overlay.css", ...) returns a string like *{box-sizing:border-box}.root{all:initial}... (minified CSS, starts with *).
2. Before: define: { OVERLAY_CSS: "*{box-sizing:...}" } → bundler parses define value → first char * → json_lexer hits the TAsterisk arm at ~line 1299 without erroring → parse_expr falls back to auto-quote → emits string literal. The comment at line 1304 correctly says "bake-codegen.ts's OVERLAY_CSS" is the motivating example.
3. After (this PR): define: { OVERLAY_CSS: JSON.stringify("*{box-sizing:...}") } = define: { OVERLAY_CSS: "\"*{box-sizing:...}\"" } → first char " → json_lexer parses a string literal directly → auto-quote fallback never invoked.
4. A future maintainer reads json_lexer.rs:1304, follows the cross-reference to bake-codegen.ts:56, finds JSON.stringify(...), and concludes the comment is wrong / the special-case might be dead code. It isn't dead (users can still pass raw CSS), but the citation no longer demonstrates it.

Impact

Zero functional impact — this is a documentation-accuracy nit. The risk is purely maintainability: the json_lexer comment is the documented justification for a non-obvious lexer behavior, and a stale justification invites a future "this looks dead, let's remove it" mistake. Per the repo's CLAUDE.md, code changes that invalidate explanatory comments should update them in the same PR.

Also worth flagging: this bake-codegen.ts change is a drive-by fix for an unrelated musl build failure (per the robobun CI comment on commit 93bd662) and has nothing to do with the PR's stated DeferredWorkTimer / FinalizationRegistry purpose. That's fine, but it makes it easy to miss that two comments elsewhere in the tree reference the changed line.

How to fix

Drop or generalize the bake-codegen citation in both places. For example, in json_lexer.rs:1302-1304:

// ... unquoted value that starts with one — e.g. a `Bun.build`
// `define:` whose value is a raw minified CSS string starting
// with `*{...}`. Erroring here aborts ...

and in bun-build-api.test.ts:75-76:

// first character — e.g. a raw minified CSS string starts with `*{...}`.

Alternatively keep the example but make it past-tense / historical ("bake-codegen.ts used to pass OVERLAY_CSS this way").

[robobun]

Fair — those comments now describe a historical rather than live example, but the json_lexer auto-quote behavior they document is still correct and useful for any other raw define value. Leaving them as-is to keep this PR scoped to the FinalizationRegistry fix; they can be tightened when that file is next touched.

[robobun]

Current CI failures are infrastructure-only and unrelated to this change:

• cargo clippy: transient failure cloning cloudflare/lol-html (vendor/lolhtml/c-api/Cargo.toml missing when clippy ran) — this PR touches no Rust or workflow files
• darwin-x64-build-rust: agent terminated mid-compile (exit status -1)
• darwin-14-aarch64-test-bun: job expired waiting for an agent (same on the last several builds)

The new regression test passes on every platform that executed it, including linux-x64-asan where the original assertion reproduces. Re-running the failed jobs should clear these.