Fix RELEASE_ASSERT in LazyProperty when node:util fails to load during Bun.inspect #29235
Claude / Claude Code Review
completed
Apr 16, 2026 in 20m 19s
Code review found 1 important issue
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/bun.js/bindings/webcore/JSBroadcastChannel.cpp:194-195 |
optionsValue.toObject() can throw TypeError when utilInspect succeeds but stylize-color fails |
| 🟡 Nit | test/js/bun/util/inspect-custom-lazy-init-exception.test.ts:4-6 |
Tests should use test.concurrent for independent subprocess tests |
Annotations
Check failure on line 195 in src/bun.js/bindings/webcore/JSBroadcastChannel.cpp
claude / Claude Code Review
optionsValue.toObject() can throw TypeError when utilInspect succeeds but stylize-color fails
When `m_utilInspectFunction` succeeds but `m_utilInspectStylizeColorFunction` independently fails (e.g. OOM in `JSFunction::create` for the `getStylize` builtin), `createInspectOptionsObject` returns `nullptr` with no exception pending, so `callCustomInspectFunction` passes `jsUndefined()` as `argument(1)`. In `jsBroadcastChannelPrototype_inspectCustom`, the new `\!utilInspect` guard at line 191 does not fire (since `m_utilInspectFunction` is non-null), and `optionsValue.toObject()` on `jsUndefi
Check warning on line 6 in test/js/bun/util/inspect-custom-lazy-init-exception.test.ts
claude / Claude Code Review
Tests should use test.concurrent for independent subprocess tests
Both test cases in inspect-custom-lazy-init-exception.test.ts use test() instead of test.concurrent(), violating the test/CLAUDE.md convention (line 22) which requires concurrent tests when multiple tests spawn processes. The fix is a one-word change: replace test() with test.concurrent() for both test cases.
Loading