avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Add Error .cause property to output #3186

Open Cobertos opened 1 year ago

Cobertos commented 1 year ago

Ava does not currently output an information related the Error.cause property.

Error.cause is somewhat new with support in NodeJS 16+, Deno 1.13+, and major browsers since ~September 2021. The .cause property can contain another Error which is useful when tracing re-throws. But, it can be of any type. It's currently supported .

When you console.log such an Error with .cause of another Error in the options, you get something like the following

image

Transcription of above image ``` Welcome to Node.js v18.12.0. Type ".help" for more information. > function test_a(){ throw new Error('test', { cause: new Error('test cause') }); } undefined > test_a() Uncaught Error: test at test_a (REPL1:1:26) { [cause]: Error: test cause at test_a (REPL1:1:53) at REPL2:1:1 at Script.runInThisContext (node:vm:129:12) at REPLServer.defaultEval (node:repl:572:29) at bound (node:domain:433:15) at REPLServer.runBound [as eval] (node:domain:444:12) at REPLServer.onLine (node:repl:902:10) at REPLServer.emit (node:events:525:35) at REPLServer.emit (node:domain:489:12) at [_onLine] [as _onLine] (node:internal/readline/interface:425:12) } > ```

When .cause is just structured data, a string representation of that data is output after [cause]:

For comparison, in Ava, when you throw an error with a cause, the cause is omitted

image

Transcription of above image ``` FnCaster › this is a test of error cause test/FnCaster.ts:51 50: function test_a(){ 51: throw new Error('test', { cause: 'test_cause' }); 52: } Error thrown in test: Error { message: 'test', } › test_a (file://test/FnCaster.ts:51:11) › file://test/FnCaster.ts:53:3 ─ ```

It would be nice for Ava to show the .cause in some form.

Cobertos commented 1 year ago

If my understanding of the codebase is right? It looks like you'd need to add .cause to serializeError so it carries over from a worker to the main process. Once there, reporters would need to be updated to use the new .cause part of the Error object in the emitted error events.

novemberborn commented 1 year ago

Alternatively the line above: https://github.com/avajs/ava/blob/678f9caf22343ba05efd54cbfebb37962f590cab/lib/serialize-error.js#L109

That function is pretty old, there must be a better alternative nowadays.

If the cause has a cause, how do you suggest we format that?

Cobertos commented 1 year ago

If the cause has a cause, how do you suggest we format that?

The most simple would be to tack it on after the original error with some sort of caused by: heading. This is what Node.js does in the above screenshot, but I've seen other places do the same.

I think getting any more fancy than that would be annoying to parse out with the eyes. Especially with the limits of CLI.

So the same formatting as the current error, but just duplicated below for the cause under a caused by: