Repository navigation
Conversation
Node keys a null-proto object by each properties entry and reads the column set back via ObjectKeys, so a Symbol entry is silently dropped. Bun's native TablePrinter called ToString on every entry instead, and ToString on a Symbol throws, so console.table(data, [Symbol(), 'a']) and Bun.inspect.table(data, [Symbol(), 'a']) both threw a TypeError instead of printing the table. Skip Symbol entries when pre-populating columns from the properties array. Non-Symbol primitives still ToString into a column header (matching Node).
|
Updated 4:44 PM PT - Jul 27th, 2026
❌ @robobun, your commit 43ef67d has 1 failures in
🧪 To try this PR locally: bunx bun-pr 36150That installs a local version of the PR into your bun-36150 --bun |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
WalkthroughChangesThe table printer now ignores Symbol entries in the Console table Symbol filtering
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM. The boxed-Symbol edge case I raised earlier was resolved with a solid rationale (no JSType::SymbolObject in JSC, no existing Rust binding for the check) and is not worth a new C++ export here.
What was reviewed:
- The
is_symbol()guard sits directly before the onlyto_bun_stringcoercion in the properties-filter path — no sibling sites inTablePrinterneed the same treatment. - Tests cover both entry points (
Bun.inspect.tablein-process and spawnedconsole.table), include a non-Symbol-primitive control, and follow harness conventions (concurrent pipe drain, combined-object assert).
Extended reasoning...
Overview
Three-line guard in src/jsc/ConsoleObject.rs (TablePrinter::print_table) that skips Symbol entries when iterating the properties filter array, plus a 26-line test block in test/js/bun/console/console-table.test.ts. Without the guard, to_bun_string (spec ToString) throws on a Symbol; Node silently drops Symbol columns because it keys a map via ToPropertyKey and reads columns back via ObjectKeys.
Security risks
None. This is display-only formatting logic in console.table. The change narrows the set of inputs that reach ToString; it does not add any new coercion, allocation, or side-effect path.
Level of scrutiny
Low. A tiny, self-contained Node-compat fix in a non-critical output-formatting path. The guard is placed immediately before the single throwing call site; grep confirms this is the only to_bun_string(global_object) in the properties-iteration branch. Error propagation via ? is unchanged for every other entry type.
Other factors
- The comment-cop bot's paragraph-comment complaint was addressed in 9db7a09 (trimmed to one line); thread resolved.
- My prior nit about
Object(Symbol())was answered with concrete evidence: JSC assignsSymbolObjectplainObjectType(no dedicatedJSTypeto test), notoPropertyKeyRust binding exists, and Node itself throws on the adjacent@@toPrimitive → Symbolcase. Covering the wrapper would require a new C++ export for a case nobody writes; I explicitly marked it non-blocking and the thread is resolved. - Tests follow REVIEW.md conventions:
await usingfor the spawned process,Promise.allon stdout/stderr/exited, combined-object assertion,bunEnv/bunExe, exact-output.toBe(no snapshot drift), and a control case proving non-Symbol primitives still coerce. PR description confirmsUSE_SYSTEM_BUN=1fails andbun bdpasses. - No bugs found by the bug-hunting system this run.
|
CI on build 83642:
This diff is 4 lines in |
|
Closing this PR because it has been inactive for more than 90 days. |
|
Replaced by #46986. It also covers |
What
console.table(data, properties)andBun.inspect.table(data, properties)throwTypeError: Cannot convert a symbol to a stringwhen thepropertiesarray contains a Symbol. Node prints the table and drops the Symbol column.Bun's own
new console.Console(stream).table(...)(the JS port) already has Node parity here; only the nativeTablePrinterpath throws.Cause
TablePrinter::print_tablepre-populates columns by iterating thepropertiesarray and callingto_bun_string(ToString) on every entry. ToString on a Symbol throws per spec.Node keys a null-proto
mapobject by eachpropertiesentry and later reads the column set back viaObjectKeys(map), which never enumerates symbol keys, so a Symbol entry is silently dropped. It never ToStrings the entry.Fix
Skip
is_symbol()entries in thepropertiesiterator before coercing. Non-Symbol primitives (numbers, plain objects) still ToString into a column header, which matches Node.Verification
New
console.table with a Symbol in the properties filterblock intest/js/bun/console/console-table.test.tscovering bothBun.inspect.tableand a spawnedconsole.table, with a non-Symbol primitive control. Both new tests throw onUSE_SYSTEM_BUN=1; all 35 existing tests in the file (plus the 35 inbun-inspect-table.test.ts) continue to pass underbun bd.Related: #34241 addresses Symbol keys on the data objects but still ToStrings
propertiesentries, so it does not fix this throw.[stamp-90s] gate passed · iteration 1 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file