node icon indicating copy to clipboard operation
node copied to clipboard

util: inspect: do not crash on an Error stack pointing to itself

Open SamVerschueren opened this issue 8 months ago • 12 comments

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.

SamVerschueren avatar May 06 '25 15:05 SamVerschueren

Open to ideas on how to show the circularity. I think it's a good idea.

SamVerschueren avatar May 06 '25 16:05 SamVerschueren

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:

... and 185 files with indirect coverage changes

: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.

codecov[bot] avatar May 06 '25 16:05 codecov[bot]

@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.

BridgeAR avatar May 06 '25 17:05 BridgeAR

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);
  1. [[Circular] Error: foo]
  2. [<Circular> Error: foo]
  3. [[Circular stack] Error: foo]
  4. [<Circular stack> Error: foo]
  5. [[Circular *stack] Error: foo]
  6. [<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 avatar May 07 '25 08:05 SamVerschueren

@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.

BridgeAR avatar May 16 '25 13:05 BridgeAR

Yes I'll definitely look into this. I was out this week so I'll probably do it over the weekend.

SamVerschueren avatar May 16 '25 14:05 SamVerschueren

Ping @SamVerschueren

BridgeAR avatar Jun 10 '25 14:06 BridgeAR

Ugh Sorry. I totally forgot about this. I'll do it tomorrow!

SamVerschueren avatar Jun 12 '25 19:06 SamVerschueren

CI: https://ci.nodejs.org/job/node-test-pull-request/67429/

nodejs-github-bot avatar Jun 13 '25 13:06 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/67431/

nodejs-github-bot avatar Jun 13 '25 17:06 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/67433/

nodejs-github-bot avatar Jun 14 '25 00:06 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/67451/

nodejs-github-bot avatar Jun 15 '25 00:06 nodejs-github-bot

Landed in ea5d37ecbebabd754201b2a0b4fe47d1e2ed07e1

nodejs-github-bot avatar Jun 16 '25 23:06 nodejs-github-bot