util: inspect: do not crash on an Error stack pointing to itself
See https://github.com/nodejs/node/issues/58195
This avoids a maximum call stack size exceeded crash when the error stack is pointing to the error itself error.stack = error. This bug was introduced by https://github.com/nodejs/node/pull/56573.
Open to ideas on how to show the circularity. I think it's a good idea.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.14%. Comparing base (
e272637) to head (111b9ca). Report is 327 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #58196 +/- ##
==========================================
+ Coverage 90.13% 90.14% +0.01%
==========================================
Files 630 637 +7
Lines 186780 188028 +1248
Branches 36653 36890 +237
==========================================
+ Hits 168347 169504 +1157
- Misses 11207 11279 +72
- Partials 7226 7245 +19
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/internal/util/inspect.js | 99.96% <100.00%> (+<0.01%) |
:arrow_up: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@SamVerschueren instead of only returning the ErrorPrototypeToString call result, we could add additional information that the stack is circular similar to the other circular outputs.
So I checked how it's done with objects where it looks like this
<ref *1> { bar: [Circular *1] }
The problem here though is that it's a method to get the stack string itself, so I don't see a way how to represent it in the same way more or less.
So I just tried some things out
in the case of the following code
const foo = new Error('foo');
foo.stack = foo;
console.log(foo.stack);
[[Circular] Error: foo][<Circular> Error: foo][[Circular stack] Error: foo][<Circular stack> Error: foo][[Circular *stack] Error: foo][<Circular *stack> Error: foo]
Let me know what any one of you would like to see. Or something completely different, although might be a bit more work.
<ref *1> Error: foo { stack: [Circular *1] }
But this deviates way too much from what it was I feel like.
@SamVerschueren could you take another look? :)
We could also land this as intermediate improvement and have another PR on top, I just thought it makes sense to combine the points that I looked at.
Yes I'll definitely look into this. I was out this week so I'll probably do it over the weekend.
Ping @SamVerschueren
Ugh Sorry. I totally forgot about this. I'll do it tomorrow!
CI: https://ci.nodejs.org/job/node-test-pull-request/67429/
CI: https://ci.nodejs.org/job/node-test-pull-request/67431/
CI: https://ci.nodejs.org/job/node-test-pull-request/67433/
CI: https://ci.nodejs.org/job/node-test-pull-request/67451/
Landed in ea5d37ecbebabd754201b2a0b4fe47d1e2ed07e1