fix: use JSONParseWithException for proper error handling #25881

robobun · 2026-01-08T00:29:37Z

Summary

SQLClient.cpp: Fix bug where RETURN_IF_EXCEPTION after JSONParse would never trigger on JSON parse failure since JSONParse doesn't throw
BunString.cpp: Simplify by using JSONParseWithException instead of manually checking for empty result and throwing

Details

JSC::JSONParse returns an empty JSValue on failure without throwing an exception. This means that RETURN_IF_EXCEPTION(scope, {}) will never catch JSON parsing errors when used after JSONParse.

Before this fix in SQLClient.cpp:

JSC::JSValue json = JSC::JSONParse(globalObject, str);
RETURN_IF_EXCEPTION(scope, {});  // This never triggers on parse failure!
return json;  // Returns empty JSValue

This could cause issues when parsing invalid JSON data from SQL databases (e.g., PostgreSQL's JSON/JSONB columns).

JSONParseWithException properly throws a SyntaxError exception that RETURN_IF_EXCEPTION can catch.

Test plan

Build succeeds with bun bd
The changes follow the same pattern used in ModuleLoader.cpp and BunObject.cpp

🤖 Generated with Claude Code

robobun · 2026-01-08T00:29:48Z

^{Updated 5:37 PM PT - Jan 7th, 2026}

❌ Your commit 6414524d has 2 failures in Build #34203 (All Failures):

test/bake/dev/stress.test.ts - 1 failing on 🐧 13 x64
test/regression/issue/11297/11297.test.ts - 1 failing on 🪟 2019 x64-baseline

🧪 To try this PR locally:

bunx bun-pr 25881

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

bun-25881 --bun

coderabbitai · 2026-01-08T00:32:16Z

Walkthrough

The SQLClient.cpp file was updated to use JSONParseWithException instead of JSONParse within the DataCellTag::Json code path, improving exception handling for JSON parsing operations without modifying public API signatures.

Changes

Cohort / File(s)	Change Summary
SQL Client Bindings `src/bun.js/bindings/SQLClient.cpp`	Updated JSON parsing to use JSONParseWithException instead of JSONParse in DataCellTag::Json path; exception handling flow preserved

Suggested reviewers

cirospaciari
Jarred-Sumner

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: replacing JSONParse with JSONParseWithException for proper error handling in SQL client code.
Description check	✅ Passed	The description exceeds template requirements, providing detailed context, before/after code examples, rationale, and test verification.

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

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e7ce2be and 6414524.

📒 Files selected for processing (1)

src/bun.js/bindings/SQLClient.cpp

🧰 Additional context used

📓 Path-based instructions (1)

src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings

Files:

src/bun.js/bindings/SQLClient.cpp

🧠 Learnings (6)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

📚 Learning: 2025-09-25T22:07:13.851Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Define properties using HashTableValue arrays in C++ JavaScript class bindings

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-11-24T18:37:47.899Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods

Applied to files:

src/bun.js/bindings/SQLClient.cpp

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

Fix bug where RETURN_IF_EXCEPTION after JSONParse would never trigger on JSON parse failure since JSONParse doesn't throw. JSONParse returns an empty JSValue on failure without throwing, so RETURN_IF_EXCEPTION will not catch parsing errors. JSONParseWithException properly throws a SyntaxError which RETURN_IF_EXCEPTION can catch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/bun.js/bindings/SQLClient.cpp (1)
188-196: The SQLClient.cpp fix is correct, but verification found an additional location with the same unfixed pattern.

The change from JSONParse to JSONParseWithException in SQLClient.cpp (line 191) correctly addresses the bug where RETURN_IF_EXCEPTION would never trigger on JSON parse failures—since JSONParse returns an empty JSValue without throwing, while JSONParseWithException properly throws a SyntaxError that the exception handler can catch.

However, verification found the same unfixed pattern in src/bun.js/modules/BunJSCModule.h at lines 671–674:
JSValue stackTraces = JSONParse(globalObject, samplingProfiler.stackTracesAsJSON()->toJSONString());
// ... code ...
RETURN_IF_EXCEPTION(throwScope, {});
This location was modified in the same commit but still contains the original bug. Apply the same fix: replace JSONParse with JSONParseWithException to ensure parse errors are properly thrown and caught.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e7ce2be and 6414524.

📒 Files selected for processing (1)

src/bun.js/bindings/SQLClient.cpp

🧰 Additional context used

📓 Path-based instructions (1)

src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings

Files:

src/bun.js/bindings/SQLClient.cpp

🧠 Learnings (6)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

📚 Learning: 2025-09-25T22:07:13.851Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Define properties using HashTableValue arrays in C++ JavaScript class bindings

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-11-24T18:37:47.899Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>

Applied to files:

src/bun.js/bindings/SQLClient.cpp

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods

Applied to files:

src/bun.js/bindings/SQLClient.cpp

github-actions bot added the claude label Jan 8, 2026

robobun force-pushed the claude/fix-jsonparse-exception-handling branch from e7ce2be to 6414524 Compare January 8, 2026 00:34

coderabbitai bot reviewed Jan 8, 2026

View reviewed changes

Jarred-Sumner merged commit b20a70d into main Jan 8, 2026
54 of 55 checks passed

Jarred-Sumner deleted the claude/fix-jsonparse-exception-handling branch January 8, 2026 04:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: use JSONParseWithException for proper error handling #25881

fix: use JSONParseWithException for proper error handling #25881

robobun commented Jan 8, 2026

Uh oh!

robobun commented Jan 8, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Jan 8, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: use JSONParseWithException for proper error handling #25881

fix: use JSONParseWithException for proper error handling #25881

Conversation

robobun commented Jan 8, 2026

Summary

Details

Test plan

Uh oh!

robobun commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robobun commented Jan 8, 2026 •

edited

Loading

coderabbitai bot commented Jan 8, 2026 •

edited

Loading