Conversation
CDP sends null as a {type:"object", subtype:"null"} RemoteObject and
NaN, Infinity, -Infinity and -0 as {type:"number", unserializableValue}
with no value field. remoteToJS handed the console callback the raw
null wrapper and turned all four numbers into 0. Map both to the
primitive they stand for, matching the documented behavior of the
console option.
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Reproduced on the released build (1.4.0) with Chrome 151 on Linux: a page-side With this branch the callback receives Tests: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — targeted fix in remoteToJS mapping CDP's unserializableValue numbers and subtype:"null" to their actual JS values.
Checked: the four unserializableValue strings match the CDP RemoteObject spec, with NaN as the safe fallback; the subtype check is placed before the generic object JSONParse so non-null objects (arrays, dates, plain objects) still fall through unchanged; getString on a missing key returns empty so the == "null" compare is safe. The new test asserts the full arg list including Object.is(-0), and the tightened globalThis.console test now asserts exact stdout, covering the ConsoleClient path. The separate Method::RuntimeEvaluate result decoder is intentionally untouched per the PR description.
Extended reasoning...
Overview
This PR fixes value unwrapping in the Chrome WebView backend's Runtime.consoleAPICalled handler (remoteToJS in src/runtime/webview/ChromeBackend.cpp). Two cases were mishandled: (1) type:"number" RemoteObjects with no value field (NaN, ±Infinity, -0) fell through getDouble("value").value_or(0) and became 0; (2) type:"object", subtype:"null" fell into the generic object branch and reached the callback as a JSONParsed descriptor instead of null. The fix adds an unserializableValue string switch for the number branch and a subtype == "null" early return before the object fallthrough. <limits> is added for std::numeric_limits<double>::infinity(). Tests add a new case asserting the full callback arg list for these values and tighten the existing globalThis.console subprocess test from toContain to an exact toBe on stdout.
Security risks
None. This is pure value-mapping of already-parsed WTF::JSON fields into JS primitives on the console-forwarding path. No user-controlled paths, no allocation sizing from untrusted lengths, no auth/crypto.
Level of scrutiny
Low-to-medium. The change is ~13 lines confined to one lambda in the console event handler; the surrounding ThrowScope/RETURN_IF_EXCEPTION structure is unchanged. jsNumber, jsNaN, and jsNull don't throw, and getString/getDouble on WTF::JSON::Object are non-throwing C++ accessors, so no new exception-check obligations. The generic object/function branch and all other primitive branches are byte-identical to before.
Other factors
No CODEOWNERS cover these files. No prior human review comments to address. Tests follow the file's existing conventions (same chrome backend const, await using, subprocess with bunEnv/bunExe, drained stdout/stderr/exited concurrently). The PR description explicitly scopes out evaluate()'s separate Method::RuntimeEvaluate decoder, which is a distinct code path with a different documented contract, so the fix is correctly not applied there. robobun independently reproduced the before/after behavior on a released build.
|
Updated 12:05 PM PT - Aug 15th, 2026
❌ @robobun, your commit 29d339e has some failures in 🧪 To try this PR locally: bunx bun-pr 39064That installs a local version of the PR into your bun-39064 --bun |
This branch has not been deployed
1 participant
Problem
new Bun.WebView({ backend: "chrome", console: (type, ...args) => ... }): a page-sideconsole.log(null)hands the callback{ type: "object", subtype: "null", value: null }instead ofnull.console.log(NaN, Infinity, -Infinity, -0)in the page arrives as0, 0, 0, 0.console: globalThis.consoleis affected the same way, since it shares the conversion: the parent prints the null wrapper as an object and the four numbers as0.consoleoption is documented (docs/runtime/webview.mdx,ConsoleCaptureinpackages/bun-types/bun.d.ts) as unwrapping primitive arguments to their raw values; only objects are supposed to arrive as the CDP descriptor.remoteToJSin theRuntime.consoleAPICalledhandler (src/runtime/webview/ChromeBackend.cpp,Transport::handleEvent) only looks atRemoteObject.typeandRemoteObject.value. CDP encodesnullastype: "object"withsubtype: "null", so it fell into the object branch; and for numbers JSON cannot represent V8 omitsvalueand sendsunserializableValueinstead, sogetDouble("value").value_or(0)produced0.Fix
type: "number"without avaluefield now mapsunserializableValue"-0"/"Infinity"/"-Infinity"to those numbers and anything else ("NaN") to NaN.type: "object"withsubtype: "null"now returnsnullbefore the generic object branch.null(its arguments travel throughJSON.stringify, sonullround-trips asnull). Regular numbers, strings, booleans,undefined, the bigint/symbol description strings, and object descriptors are unchanged.evaluate()decodesRuntime.evaluateresults in a separate code path (Method::RuntimeEvaluate) and has a different documented contract (a JSON round trip), so it is intentionally not touched here; its handling of these numbers is tracked separately.test/js/bun/webview/webview-chrome.test.ts:chrome: console callback unwraps args whose RemoteObject has no value fieldasserts the whole callback argument list fornull, NaN, Infinity, -Infinity, -0(plusundefined, bigint and symbol, which share the "no value field" shape) and checks-0withObject.is.chrome: console: globalThis.console forwards to parent's stdoutadditionally logs the same values and now asserts the exact stdout (null NaN Infinity -Infinity -0), covering theConsoleClientdispatch path.nullarrives as the wrapper object, the numbers as0); with the fix the whole file passes (52 tests). The container runs as root, so the runs usedBUN_CHROME_PATHpointing at a wrapper that adds--no-sandbox; both the runtime and the test file'sfindChrome()honor that variable.BUN_JSC_validateExceptionChecks=1, clean.Background
Runtime.consoleAPICalledis the event Chrome emits for each page-sideconsole.*call; itsargsareRemoteObjects, not the values themselves.RemoteObjectdescribes a value:type(string,number,boolean,undefined,bigint,symbol,object,function), an optionalsubtypefor objects (null,array,date, ...), and for primitives eithervalue(when JSON can carry it) orunserializableValue(a string naming a value JSON cannot carry:"NaN","Infinity","-Infinity","-0", or a bigint literal such as"1n"). JavaScriptnullis the one primitive CDP reports withtype: "object".remoteToJSis the Chrome backend's conversion from aRemoteObjectto the JS value passed to the user's callback (or to Bun's console when the option isglobalThis.console).