Fix undefined descriptor error in property formatting - #60724
Conversation
Sorry, something went wrong.
BridgeAR
left a comment
There was a problem hiding this comment.
The change and test is LGTM.
After giving it some more thought, I just lean on not printing these when the properties are not own properties. That aligns more with other inspected parts.
@ljharb since you often have an opinion about inspect, what is your take on it?
Sorry, something went wrong.
|
@siaeyy do you mind changing the code as follows instead: diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index 83c254c3d6c..813c7eec659 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -1955,14 +1955,14 @@ function formatError(err, constructor, tag, ctx, keys) {
}
name ??= 'Error';
- if ('cause' in err &&
+ if (Object.hasOwn(err, 'cause') &&
(keys.length === 0 || !ArrayPrototypeIncludes(keys, 'cause'))) {
ArrayPrototypePush(keys, 'cause');
}
// Print errors aggregated into AggregateError
try {
- const errors = err.errors;
+ const errors = Object.hasOwn(err, 'errors') ? err.errors : undefined;
if (ArrayIsArray(errors) &&
(keys.length === 0 || !ArrayPrototypeIncludes(keys, 'errors'))) {
ArrayPrototypePush(keys, 'errors');That way only own properties are handled as with other objects. The test should still pass with the new change. |
Sorry, something went wrong.
|
I think that makes sense, since both cause and errors will only ever be own properties. |
Sorry, something went wrong.
Error's cause and errors properties would be visible even if these were not own properties. This is changed to align with all other parts of the inspect handling. Fixes: nodejs#60717 Closes: nodejs#60724
|
There is already another pr that fixes the issue. |
Sorry, something went wrong.
Fixes: #60717