fix(ext/console): degrade gracefully when getKeys throws (#24980)#34464
Merged
Conversation
`getKeys` previously asserted that any error from `Object.keys` was a ReferenceError on an unevaluated module namespace object, falling back to `Object.getOwnPropertyNames`. Any other failure surfaced as the opaque "AssertionError: Assertion failed". While the proxy unwrap added in #34373 stops the inspect path from calling `Object.keys` on a Proxy in the common case, this assertion was still load-bearing for resilience and easy to trip from unrelated regressions. Drop it and fall through to `Object.getOwnPropertyNames` (then to an empty key list) on any throw, so unexpected exotic objects degrade to a less-detailed but valid inspection instead of a crash. Also wrap `Object.getOwnPropertySymbols` and the enumerability filter in try/catch for the same defense-in-depth reason. Adds a regression test covering the exact reproduction from #24980 (a Proxy whose `getOwnPropertyDescriptor` trap throws). Closes #24980. Closes denoland/orchid#277 Co-Authored-By: Divy Srivastava <[email protected]>
littledivy
approved these changes
May 29, 2026
littledivy
added a commit
to crowlKats/deno
that referenced
this pull request
Jun 10, 2026
) (denoland#34464) ## Summary Fixes the `Uncaught AssertionError: Assertion failed` raised by `console.log` / `Deno.inspect` when handed a Proxy whose `getOwnPropertyDescriptor` trap throws, e.g. (from denoland#24980): ```js console.log( new Proxy({ x: 10 }, { getOwnPropertyDescriptor: () => { throw new Error("oops"); }, }), ); ``` Before: ``` Uncaught AssertionError: Assertion failed at assert (ext:deno_web/01_console.js:…) at getKeys (ext:deno_web/01_console.js:…) … ``` After: ``` { x: 10 } ``` ## Background `getKeys` previously asserted that any error from `Object.keys(value)` was a `ReferenceError` raised by accessing an unevaluated module-namespace object, and otherwise re-threw via `assert(false)` — i.e. surfaced as `AssertionError: Assertion failed`. denoland#34373 added proxy unwrapping in `formatValue` (when `showProxy` is `false`, Deno now inspects the proxy target instead of the proxy itself, matching Node.js). That removes the common path that brought a throwing-trap Proxy to `getKeys`, but the assertion was still load-bearing for resilience: anything else that ever throws from `Object.keys` (or, before it, `Object.getOwnPropertySymbols`) would crash inspect with a useless message. This PR softens `getKeys` so unexpected failures degrade to a less-detailed but valid inspection instead of a crash: - `Object.keys` → on throw, fall back to `Object.getOwnPropertyNames`, then to an empty key list. - `Object.getOwnPropertySymbols` → on throw, treat as no symbols. - The enumerability filter (`propertyIsEnumerable`) is guarded too. The now-unused `assert` / `AssertionError` internal helpers are removed. ## Tests `tests/unit/console_test.ts` — extends the existing `inspectProxy` test with the exact repro from denoland#24980 (Proxy with a throwing `getOwnPropertyDescriptor`). Closes denoland#24980. Closes denoland/orchid#277 Co-authored-by: divybot <[email protected]> Co-authored-by: Divy Srivastava <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the
Uncaught AssertionError: Assertion failedraised byconsole.log/Deno.inspectwhen handed a Proxy whosegetOwnPropertyDescriptortrap throws,e.g. (from #24980):
Before:
After:
Background
getKeyspreviously asserted that any error fromObject.keys(value)was aReferenceErrorraised by accessing an unevaluated module-namespace object,and otherwise re-threw via
assert(false)— i.e. surfaced asAssertionError: Assertion failed.#34373 added proxy unwrapping in
formatValue(whenshowProxyisfalse,Deno now inspects the proxy target instead of the proxy itself, matching
Node.js). That removes the common path that brought a throwing-trap Proxy to
getKeys, but the assertion was still load-bearing for resilience: anythingelse that ever throws from
Object.keys(or, before it,Object.getOwnPropertySymbols)would crash inspect with a useless message.
This PR softens
getKeysso unexpected failures degrade to a less-detailedbut valid inspection instead of a crash:
Object.keys→ on throw, fall back toObject.getOwnPropertyNames, thento an empty key list.
Object.getOwnPropertySymbols→ on throw, treat as no symbols.propertyIsEnumerable) is guarded too.The now-unused
assert/AssertionErrorinternal helpers are removed.Tests
tests/unit/console_test.ts— extends the existinginspectProxytest withthe exact repro from #24980 (Proxy with a throwing
getOwnPropertyDescriptor).Closes #24980.
Closes denoland/orchid#277