How should stack traces of SuppressedError look like?

[debadree25]

Opening this issue for discussion and getting feedback, I apologise if this may not be the right place/topic to ask this please feel free to close this if thats the case.

While implementing SuppressedError for SpiderMonkey we came across this question of what should the stack trace of SuppressedError look like when there are multiple errors suppressed. The present idea is to make the trace point to the end of the scope that is being disposed and additionally print out the trace of the error that is suppressing the other error, another idea could be just giving the st of the latest error and nothing more. Would like to know what would be best for DX and if anyone else has any other ideas on this!

Thank you!

[ljharb]

What do you do for AggregateError?

[debadree25]

For aggregate error the trace is at the point where the AggregateError is being generated for example for this code

function fn1() {
  return Promise.reject("1");
}

function fn2() {
  return Promise.reject("2");
}

await Promise.any([fn1(), fn2()]);

we would have the trace

AggregateError: No Promise in Promise.any was resolved
Stack:
  Promise.any*@/Users/debadreechatterjee/Documents/personal/mozilla-unified/test.js:9:15

so basically will be wherever the Promise.any threw

[ljharb]

Right, but i mean, if you have a new AggregateError (no need for Promise.any), that aggregates multiple errors with stack traces, how do you display those?

[debadree25]

Ah in that case from what i can tell we just show the st of wherever the AggregateError is being created we are not including the traces of the errors being aggregated

[mhofman]

Technically the stack of a suppressed or aggregated error can be entirely disjunct from the outer error's. The captured stack should probably be complete like for any normal error created. Of course error sinks like the console and reporting libraries might want to truncate the common part of these related errors stack, if any, when displaying to the user.

[debadree25]

The captured stack should probably be complete like for any normal error created.

I guess my question can be reduced to: where exactly would you expect the SuppressedError created? at the end of the scope where the disposables are being disposed or at the point where @@dispose is being called with an error pending

[ljharb]

It should be created in DisposeResources in step 3.e.iii.1.c

[jridgewell]

SuppressedError should definitely have its own distinct stack trace. It should not reuse the stack traces of its .error/.suppressed errors.

If a suppressed error is thrown, I want to know where the block enclosing the using is located in my codebase. It might be a little weird to point to the closing }, but that's the closest we can get to the internal-only disposal routine.

[ljharb]

maybe pointing to the using that queued it up would make more sense?

[rbuckton]

A SuppressedError represents an exception that suppresses another exception. The only reason we even have a SuppressedError is that we can't reliably tack suppression information onto the actual .error since errors in JS can be anything. My intuition is that the stack trace of a SuppressedError should mirror that of the .error's stack, or something close to it. Barring that, we'd probably have to use the } as the position of the error and just depend on users looking at .error for the actual stack.

maybe pointing to the using that queued it up would make more sense?

I'm not sure that makes sense. If I were a debugger with "Break on uncaught exceptions" enabled, I would not expect my cursor to be on the using. If I were a tool like jest which uses the stack trace to pretty-print the surrounding context of the exception that failed a test, I don't think I would expect the context to be the using either, as I might expect that context to indicate the exception was in the evaluation of the statement, not the evaluation of cleanup.

Instead, for something like jest I'd expect the context to be around the location where the .error was thrown, since that's more likely to be actionable to the user.

[mhofman]

It might be a little weird to point to the closing }, but that's the closest we can get to the internal-only disposal routine.

I think this is what makes the most sense. That or the statement that caused an exit from the scope (in case of explicit return/break/continue).

I'd expect the context to be around the location where the .error was thrown, since that's more likely to be actionable to the user.

This sounds like a matter for the error reporting tooling to highlight the stack of the .error more prominently than the SuppresedError's own stack.