ava icon indicating copy to clipboard operation
ava copied to clipboard

Add Error .cause property to output

Open Cobertos opened this issue 2 years ago • 3 comments

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 avatar Mar 21 '23 09:03 Cobertos

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.

Cobertos avatar Mar 21 '23 09:03 Cobertos

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?

novemberborn avatar Apr 10 '23 15:04 novemberborn

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:

Cobertos avatar Apr 30 '23 11:04 Cobertos