fix(sql): don't crash in debug builds when bun:sql module load throws

[robobun]

Fuzzilli found a crash (fingerprint 281fcb3ecf3202d4) where accessing Bun.SQL or Bun.sql after globalThis.Symbol was replaced with a non-function would crash the process in debug/ASAN builds instead of throwing.

What

The Bun.sql / Bun.SQL lazy property callbacks in BunObject.cpp call requireId() to load the bun:sql internal module. In debug builds, if that evaluation threw, a #if BUN_DEBUG block called reportUncaughtExceptionAtEventLoop() before the RETURN_IF_EXCEPTION check.

reportUncaughtExceptionAtEventLoop routes through Bun__reportUnhandledError -> uncaught_exception -> Bun__handleUncaughtException, which can clear the pending VM exception (via tryClearException on the _fatalException lookup). With the exception cleared, RETURN_IF_EXCEPTION doesn't fire and sqlValue.getObject()->get(...) dereferences a null pointer.

Release builds were unaffected since the block is #if BUN_DEBUG only.

How

Remove the debug-only reportUncaughtExceptionAtEventLoop calls. The exception now propagates to the caller the same way in debug and release builds.

Repro

globalThis.Symbol = "i";
try { Bun.SQL; } catch (e) { console.log(e.message); }

Before: member call on null pointer of type 'JSC::JSCell' (UBSAN) or assertNoException (exception scope check)
After: throws TypeError: Symbol is not a function which is catchable

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ec5ead70-7c9e-404e-960c-898c3e31a059

📥 Commits

Reviewing files that changed from the base of the PR and between 9f4682d and b9f302c.

📒 Files selected for processing (2)

• src/jsc/bindings/BunObject.cpp
• test/js/sql/sql-lazy-load-throw.test.ts

---

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

[robobun]

^{Updated 6:33 AM PT - Jul 20th, 2026}

@autofix-ci[bot], your commit b9f302c is building: #76290

[github-actions]

This PR may be a duplicate of:

1. Fix segfault when a lazy Bun.* getter throws during reification #33211 - Fixes the same segfault in lazy Bun.sql/Bun.SQL getters when they throw during reification, using a comprehensive lazyPropertyResult() guard that is a superset of this PR's fix
2. Fix Bun.inspect null deref with Proxy prototypes; stop reporting from inside the sql lazy-property builders #30245 - Also fixes null JSCell dereferences in Bun object lazy init (including Bun.sql/Bun.SQL), though it carries an additional Bun.inspect fix

🤖 Generated with Claude Code

[robobun]

Closing as a duplicate of #33211.

That PR has the complete fix. Mine only removed the debug-only reportUncaughtExceptionAtEventLoop call, which stops the first read from crashing in debug builds but leaves the underlying release-build problem: reifyStaticProperty stores the callback's empty return via putDirect with no exception check, so a second Bun.SQL read after the first one throws still segfaults at address 0x5:

globalThis.Symbol = "i";
try { Bun.SQL; } catch {}   // throws, reifies empty JSValue into the slot
typeof Bun.SQL;             // segfault on release and debug

#33211 handles both by clearing the exception and reifying undefined, and already covers this exact Symbol-tampering trigger in its test. The fuzzer fingerprint 281fcb3ecf3202d4 is the same mechanism it documents.

[claude]

🟡 Per REVIEW.md, avoid asserting stderr is exactly empty — ASAN/debug builds can emit benign diagnostics that make this flake in CI. The stdout === "caught=2" and exitCode === 0 assertions already prove the process didn't crash and both exceptions were caught; consider dropping this line or asserting a combined { stdout, exitCode } snapshot instead.

Extended reasoning...

What the issue is

Line 29 asserts expect(stderr).toBe(""). REVIEW.md's Tests reviewers reject section explicitly calls this out:

Never assert stderr is exactly empty (ASAN/debug builds emit benign warnings); assert a combined { stdout, stderr, exitCode } object.

Why it can flake

The test spawns bunExe() with bunEnv. bunEnv sets BUN_DEBUG_QUIET_LOGS=1, which suppresses Bun's own scoped debug loggers, but it does not suppress ASAN/UBSAN runtime diagnostics or other libc/loader warnings that can appear on stderr in the debug+ASAN CI lanes. When such a benign line appears, stderr becomes non-empty and this assertion fails even though the behavior under test — the catchable TypeError propagating out of the Bun.sql/Bun.SQL lazy getters — is working correctly.

Step-by-step

1. CI runs the debug+ASAN lane.
2. The spawned child prints caught=2\n to stdout and exits 0 — the fix is working.
3. ASAN (or another runtime component) writes a benign one-line warning to stderr.
4. expect(stdout.trim()).toBe("caught=2") passes.
5. expect(stderr).toBe("") fails → spurious red CI.

Why the existing assertions already suffice

The purpose of this test is to prove the process doesn't crash and that the module-load exception propagates as a catchable JS error. Both facts are fully established by:

• stdout.trim() === "caught=2" → both try { Bun.sql } and try { Bun.SQL } threw and were caught.
• exitCode === 0 → no crash, no abort, no uncaught exception.

The stderr === "" assertion adds no additional signal — it only introduces flake risk.

Suggested fix

Either drop line 29 entirely, or use the repo-conventional combined-object snapshot:

expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "caught=2", exitCode: 0 });

(If you want stderr visible in failure output for debugging, include it in the object but don't assert it's exactly empty.)

Severity

Marking as nit: this is a test-hygiene / potential-flake concern, not a correctness bug in the fix itself. The pattern also appears widely elsewhere in the test suite, so it's not uniquely blocking here — just worth aligning with the stated guideline while the file is new.