Defend against crashing getters in util.inspect: fix error when logging DOMException.prototype (#60107)#60210
Closed
kshiteej-mali wants to merge 4 commits intonodejs:mainfrom
Closed
Defend against crashing getters in util.inspect: fix error when logging DOMException.prototype (#60107)#60210kshiteej-mali wants to merge 4 commits intonodejs:mainfrom
kshiteej-mali wants to merge 4 commits intonodejs:mainfrom
Conversation
Collaborator
|
Review requested:
|
Author
|
@nodejs/loaders @nodejs/vm This is a small and safe fix if possible, fast review/merge would be awesome. Thank you 
!
|
Contributor
|
There are no tests and a bunch of unrelated changes. There's already a PR to fix the linked issue (#60139). |
Author
|
hold on, my build has failed upon running after doing this PR, i'm in the process of reviewing it. |
Author
should i close this? if that other PR works |
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
3 participants
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.
This PR fixes nodejs/node#60107, where running console.log(DOMException.prototype) (or inspecting other objects with dangerous getters) would throw a TypeError and crash Node.js or the REPL.
Context to this fix -
The failure occurred because DOMException's property getters (like name) throw when accessed on the prototype, violating normal inspection expectations.
Browsers handle this gracefully; Node.js would crash due to unguarded property inspection in util.inspect.
This PR wraps all calls to property getters in the formatProperty function with a try/catch method , so any thrown error results in a readable <Inspection threw (...)> comment instead of crashing.
Output for normal properties, arrays, objects, symbols, and keys is unchanged.
What i have changed
Updated formatProperty (lib/internal/util/inspect.js) to handle getter exceptions robustly.
Now, console.log(DOMException.prototype) prints a readable string that describes the thrown error.
Includes code comments for maintainability.
How to test this thing:
Run console.log(DOMException.prototype) in Node.js; confirm it outputs a non-crashing result.
All util.inspect and console inspection behavior should remain unchanged for valid objects/properties.
Issue link
This PR resolves Issue #60107.
Credits
Patch by Kshiteej Mali (@kshiteej-mali)
Thanks to samualtnorman, ljharb, Fayti1703 for community context and reproducing steps.