Fix unchecked exception when Bun.plugin target coercion throws

[robobun]

Fuzzer found a debug assertion failure in Bun.plugin().

Repro

Bun.plugin({
  target: { toString() { return {}; } },
  setup() {},
});

ASSERTION FAILED: Unexpected exception observed
Error Exception: No default value
!exception()
ExceptionScope.h(61) : void JSC::ExceptionScope::assertNoException()

Cause

setupBunPlugin() calls targetValue.toStringOrNull(globalObject) to coerce the target option to a string. When coercion throws (e.g. toString() returns an object, producing a "No default value" TypeError), toStringOrNull returns nullptr and the if body is skipped — but the pending exception is never checked before continuing to constructEmptyObject(), which asserts.

Fix

Add RETURN_IF_EXCEPTION after the toStringOrNull path (and after value(globalObject) for rope resolution).

Also adds a missing <cassert> include in wtf-bindings.cpp that was relying on transitive includes from other files in its unified-build source group.

Fingerprint: 0462f40eaa1971c9

[robobun]

^{Updated 9:25 PM PT - May 18th, 2026}

❌ @robobun, your commit 661b12d has 1 failures in Build #55946 (All Failures):

• test/js/node/async_hooks/AsyncLocalStorage-tracking.test.ts - pid 21595 segmentation fault at address 0xc on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 31017

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

bun-31017 --bun

[claude]

LGTM — straightforward exception-check fix following the standard JSC pattern, with a regression test.

Extended reasoning...

Overview

This PR fixes a debug-build assertion failure in Bun.plugin() when coercing the target option to a string throws (e.g. toString() returns an object). It adds two RETURN_IF_EXCEPTION(throwScope, {}) checks in setupBunPlugin() (BunPlugin.cpp) — one after targetJSString->value(globalObject) for rope resolution, and one after the toStringOrNull block to catch the nullptr-with-pending-exception case. It also adds a missing <cassert> include in wtf-bindings.cpp (which uses assert() in uv__tty_make_raw) and a regression test in plugins.test.ts.

Security risks

None. The change only adds early-return exception checks on an error path; it does not alter parsing, permissions, or any trust boundary. The added include is a build-hygiene fix.

Level of scrutiny

Low. This is a tiny, mechanical fix that follows the exact RETURN_IF_EXCEPTION idiom already used dozens of times in the same file and throughout the JSC bindings. The placement is correct: toStringOrNull returns nullptr on exception, so the outer check after the if block catches it before constructEmptyObject() asserts. The inner check after value() is the standard guard for rope resolution.

Other factors

The fix is fuzzer-driven with a clear repro, and the new test (propagates exception thrown while coercing 'target' to string) directly exercises the previously-crashing path and asserts a TypeError is thrown. No bugs were flagged by the bug-hunting system, and there are no outstanding reviewer comments.

[github-actions]

This PR may be a duplicate of:

1. Handle exceptions from toStringOrNull in Bun.plugin target parsing #28369 - Adds the same two RETURN_IF_EXCEPTION guards in setupBunPlugin() after target string coercion, fixing the same unchecked exception bug
2. wtf-bindings: use WTF ASSERT() so debug builds compile #30992 - Fixes the same wtf-bindings.cpp debug build break (uses ASSERT() instead of adding #include <cassert>)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@robobun has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 34 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5586df9e-90f4-4a38-a34c-2fb2ddfdf71b

📥 Commits

Reviewing files that changed from the base of the PR and between 4c8a21b and 661b12d.

📒 Files selected for processing (3)

• src/jsc/bindings/BunPlugin.cpp
• src/jsc/bindings/wtf-bindings.cpp
• test/js/bun/plugin/plugins.test.ts

---

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

[claude]

LGTM — straightforward exception-check fix following the existing RETURN_IF_EXCEPTION pattern, with a regression test.

Extended reasoning...

Overview

This PR touches three files: BunPlugin.cpp adds two RETURN_IF_EXCEPTION(throwScope, {}) guards in setupBunPlugin() after toStringOrNull / value(globalObject) so that an exception thrown during target string coercion propagates instead of tripping a debug assertion in constructEmptyObject(). wtf-bindings.cpp adds #include <cassert> for the existing assert(tio != NULL) in uv__tty_make_raw, which was previously relying on a transitive include from its unified-build group. The test file adds a regression test exercising the fuzzer repro (toString() returning an object).

Security risks

None. This only adds early-return exception checks on an error path and a standard-library include. No new inputs are accepted, no auth/crypto/permissions code is touched, and behavior for valid inputs is unchanged.

Level of scrutiny

Low. The RETURN_IF_EXCEPTION macro is the canonical JSC idiom and is already used dozens of times in this same function and file; the two new call sites are placed exactly where JSC convention requires (immediately after the potentially-throwing calls). The <cassert> include is a trivial build-hygiene fix with no runtime effect. The added test directly covers the fixed path.

Other factors

No prior human review comments are outstanding. The duplicate-PR bot flagged overlap with #28369 and #30992 — that's a triage/process matter for maintainers but doesn't affect the correctness of this change. CI reported failures on an earlier commit and a retrigger commit was pushed; the diff itself is unlikely to be the cause given its narrow scope, and CI status is already visible to the author.

[robobun]

CI status (build #55946): 71 checks passing. test/js/bun/plugin/plugins.test.ts (the file this PR touches) passed on every platform including darwin-14-x64 (30 pass, 0 fail).

Remaining failures are unrelated to this change:

• darwin-14-aarch64-test-bun / darwin-26-aarch64-test-bun: Expired in queue (never ran, 0 bytes log) — macOS aarch64 agent unavailability, same on builds #55923 and #55946.
• darwin-14-x64-test-bun: the only real test failure is AsyncLocalStorage-tracking.test.ts > fs-watch — segfault at 0xC in async-context-fs-watch.js. This is a known pre-existing macOS fs.watch/FSEvents crash being addressed in fs.watch(macOS): make FSEventsLoop Sync and retain the CFRunLoop across shutdown #30758 / fs_events(darwin): fix broken double-checked locking on fsevents_default_loop #30111 / fs.watch: fully initialize ConcurrentTask in FSWatchTask.enqueue #29950. node-http-backpressure-max.test.ts timed out 3× then passed on retry Support import assertions #4.

This PR only adds RETURN_IF_EXCEPTION guards in BunPlugin.cpp and a <cassert> include — no overlap with fs.watch, async_hooks, or HTTP. Verified by debian-13-x64-asan-test-bun which passed.

[robobun]

Superseded by #31286, which has the same exception-check fix as a minimal diff (no unrelated wtf-bindings.cpp change) on top of current main, plus a regression test. Closing to keep a single PR for this fix.