[FEATURE] Can't see cause or aggregate details for uncaught errors

[towerofnix]

Is there an existing issue for this?

• I have searched the existing issues

Have you read the CONTRIBUTING guide on posting issues, and CODE_OF_CONDUCT?

• yes I read the things

Description

To aid in debugging without necessitating futzing around with Chrome-style debugger statements, several infrastructural parts of my code wrap uncaught errors with supplemental details by creating errors with cause.

In practice we use a custom (and not-so-appropriately named) showAggregate function to display causes and aggregate errors in a nice, compact tree, for example below:

Node.js already tries to accommodate errors with cause and aggregate errors, too, as below:

Tap doesn't, though! All I get is details about the top-level error.

 FAIL  example.js 1 failed of 1 440ms
 ✖ Example > Oh dear in relation(fakeRelation)
    example.js                                                            
     3 t.test('Example', t => {                                           
     4   t.plan(1);                                                       
     5                                                                    
     6   throw new Error("Oh dear in relation(fakeRelation)", {           
     ━━━━━━━━━━┛                                                           
     7     cause: new Error("Oh my in relation(otherRelation)", {         
     8       cause: new Error("Oh chihuahua in relation(thirdRelation)", {
     9         cause: doSomething(),                                      
    10       }),                                                          
    tapCaught: testFunctionThrow
    Test.<anonymous> (example.js:6:9)

This is problematic for a few reasons, but the biggest is that — provided a simple cause chain, with no aggregate errors — I don't get any information about the bottom error. That's where the code actually failed (since it's an uncaught error). Of course, I hardly get any of the surrounding context I've added through errors with cause; I get the very top layer (since it's the top-level error object that tap considers), but that's it.

Of course, if any layers are aggregate errors, the overall structure really is important — it's not just supplemental context anymore, but describing possibly multiple errors that went wrong. For proper debugging, I need access to not just one of them (or the top layer) but the whole structure.

In practice I've worked around this by temporarily editing the test file, wrapping the line of code that eventually throws the error in debugging boilerplate:

try {
  t.equal(await doSomething(), someResult) // or any other assert
} catch (error) {
  (await import('#sugar')).showAggregate(error);
  throw error;
}

It works, but it's super inconvenient.

I can think of three ways to make this better on tap's part:

• Adapt tap's custom error reporting to handle errors with cause and aggregate errors. This is probably an involved process — I don't know how much tap is built on Node's own error reporting, but if its implementation is highly customized/hand-made, it could require a lot of work to approximate or take inspiration from Node's error handling.
• Have a setting/option which skips out of tap's custom error reporting and instead uses Node's default, unaltered. Since Node is based on V8, this means debugging support should improve at about the same pace that the language itself supports new kinds of errors. It's less nice than customized tap reporting, but would be pretty much bullet-proof.
• Allow and integrate custom uncaught error tracing. This would basically mean you can run a function which indicates, for all tests in the current file/suite/whatever, to use a function you provide to transform an error object into a string to print out. If the test raises an uncaught error, your function will get used, and the result will be displayed in place of tap's usual error tracing.

There's also a big question of what to do with the code context that tap provides, which is genuinely super helpful, but completely fails for errors-with-cause or aggregate errors — providing only code context for the top-level (and generally supplemental) error. But obviously displaying code context for every error in a chain would get very unwieldy. (You could argue that this is why tap only shows the code context for the top line in an error trace!)

On my part there are probably nicer ways to get around this, for example by handling uncaught errors in general (unhandledRejection) or by putting specific error-handling code into a helper function that wraps my tests. (I already have one for snapshot tests, but most of my unit tests just use the top-level t.test() function directly, so those would take some not-so-nice refactoring).

But I'm pretty sure workarounds I'd code on my end wouldn't be able to integrate nicely into tap's own error reporting; I basically only have the option to spit text into stdout/stderr and manually correlate it to the actual test that failed, in tap's summary, once all my tests have evaluated.

Although this gets in the way, it's not like tap is doing anything wrong here — it just doesn't specially report uncaught errors with cause and aggregate errors. So I've filed making any sorts of related improvements as a feature request! And I'd love to find out that actually there are ways to handle this better right now, too, but figured I'd summarize the situation anyway.

Example

Simple reproduction code with only a plain cause chain, no aggregate errors:

const t = require('tap');

t.test('Example', t => {
  t.plan(1);

  throw new Error("Oh dear in relation(fakeRelation)", {
    cause: new Error("Oh my in relation(otherRelation)", {
      cause: new Error("Oh chihuahua in relation(thirdRelation)", {
        cause: doSomething(),
      }),
    }),
  });
});

function doSomething() {
  return new TypeError("Division by zero, ma'am?");
}

Note that the code context for the bottom error is around return new TypeError("Division by zero, ma'am?");, but the context tap provides is around throw new Error("Oh dear in relation(fakeRelation)", {. tap also doesn't give me any way to figure out the trace or even message for that bottom-level error, nor any of the errors in-between.

Edit: Yay, issue #1000! 🥳

[towerofnix]

I'm noting that this is probably related to tapjs/stack-utils#61, which has a whole ton of discussion about cause formatting in general. It's probably the place to go to discuss specifics on that mark! (See also jestjs/jest#11935.) But I'm keeping this issue around as tracking for other ways to integrate custom or Node.js-built-in error tracing for tap itself.

[towerofnix]

I've done some digging in the source to try to get more familiar with basically how tap works and where changes would be needed, and the give-away (after digging in the reporter code) is that the reporter is most certainly parsing the tap results, with no extra "magic" after the fact to generate new information, so the core test runner is responsible (which generates tap output) is responsible for stack tracing.

Specifically, the function we've gotta look at is TestBase.threw. Stack tracing is handled here, essentially joining (one for each line) the results of parseStack. Internal details are super interesting (@tapjs/stack is cool!) but not relevant for this issue, since they're lower level than processing a thrown error object itself.

I'm not sure what the ideal solution would be for embedding information that isn't a plain stack trace into the results here. I guess the good thing is that YAML Diagnostic blocks aren't standardized? So we can choose to add completely new information here, without altering the existing expected contents of stack.

Personally, seeing as it's a structured YAML object anyway, I think we should basically allow recursive details here, exactly the same as the current flat shape is reported, just without tapCaught (since I'm assuming that's only relevant to the top-level error). That means including stack, at, and source for every nested error, as well as cause (single item recursive) and errors (array recursive) when applicable. (Oh, and message for nested errors, too!) It'd be up to the reporter to decide what to do with that data — including not displaying it at all, for example.

Of course, all this means that as-is, cause and errors information is just completely discarded — it's not part of the TAP output (i.e. YAML diagnostic block) and so unavailable for processing by reporters.

tap's error reporting for result details is handled by ResultDetails, and expressly parses the diagnostic object to provide source and stack information (respectively Source and Stack). If we add a new key on the diagnostic results - rather than messing with the existing stack - then we'd need to add a new element for handling recursion, displaying message and at information for sure, and maybe a compressed/context-sensitive version of stack (ala Node.js) and maaaaaaybe source if an option is set, too.

Actually, if stack is context-sensitive, we need the top-level stack to be similarly context-sensitive. We could accomplish that in two ways:

1. By providing the entire diagnostic structure to Stack and a pointer to the current entry in that structure, allowing Stack to infer which lines are duplicates and should be hidden for conciseness
2. By doing that processing ourselves and just providing a concise stack string to Stack from the get-go, and making the recursive component responsible not just for nested stacks, but the top-level one too (since it'll contain the conciseness logic)

I'm definitely inclined more towards the second approach, because it keeps Stack logic simple, isolates recursive or recursion-sensitive logic into a dedicated component, and avoids "special" behavior for trace reporting on the top-level error, since that's just a case where the root node is also the (only) leaf node.

I might give implementing all of this a shot, since implementation-wise it doesn't sound dreadfully complicated (recursion in TypeScript isn't that hard, right!?), and would be a fun project to get familiar with tap's internals — even if any of this is out of scope for tap or some totally different approach is selected. But no guarantees I'll get anywhere with my code! 👾

[towerofnix]

We did it!

We still need to do test cases, and probably some extra-detail adjustments (it doesn't display the error type, nor any otherDiags-style details; stack beneath heading should possibly also be indented to match Node.js...) — but the essential implementation is all here. See our detailed commit messages if so desired. (We'll try to make a PR happen sometime soon.)

Also some checking against Node.js to make sure everything is, in fact, working precisely as intended (that's where we're pulling the essential appearance, "42 lines matching cause trace" messaging, and a chunk of implementation code from).