tapjs/tcompare

No failures for nested errors in aggregate errors

Opened this issue · 3 comments

For my unit test (with tap) I'm looking to catch and describe the details of a thrown AggregateError. I've been running into an issue where it seems like the errors on the aggregate are being totally ignored — it compares the message but not the contents of the error. So tests which match the name but fail to match content errors end up passing!

This is reflected in tcompare's same() behavior, which has this for errors:

  : !this.isError() && b instanceof Error
  ? false
  : this.isError() &&
    ((b.message && b.message !== a.message) ||
      (b.name && b.name !== a.name))
  ? false

And in match():

  : this.isError() &&
    ((pattern.message &&
      !new Match(obj.message, {
        expect: pattern.message,
      }).test()) ||
      (pattern.name &&
        !new Match(obj.name, {
          expect: pattern.name,
        }).test()))
  ? false

I thought this would be handled by deeper matching (i.e. foreign property errors on A doesn't match errors on B), but I'm not sure where that's written in the code.

This might speak to a larger issue on custom error objects in general? E.g, attaching other kinds of data to a custom Error-extending object — it would be important to both catch that the thrown error has the right constructor/instanceof and carries the right exposed data. I'm not sure if any of that, nor aggregate errors, are covered by tcompare right now.


Just in case I'm making a dumb mistake, here's my unit test code:

  t.throws(
    () => Template.validateDescription({
      annotation: `my cool template`,
      content: 'this is not a function',
    }),
    new AggregateError([
      new TypeError(`Expected description.content to be a function`),
    ], `Errors validating template "my cool template" description`));

Making a change to the message of the AggregateError it's comparing against causes this to fail, as it should. But changing the message of the TypeError should also make it fail (since the message doesn't match), and it doesn't — the error still passes. Even removing the TypeError or adding totally unrelated errors still doesn't cause a fail.

I'm most likely able to get around this in my own project by just try-catching the error and matching each of its properties manually, but this seems like something tcompare should be able to handle. Even if it doesn't respect other custom types of extended errors, AggregateError is a native ECMAScript class, so it feels to me like it would be in scope of tcompare.

Here's my workaround for handling AggregateErrors cleanly in my tests:

export function strictlyThrows(t, fn, pattern) {
  const error = catchErrorOrNull(fn);

  t.currentAssert = strictlyThrows;

  if (error === null) {
    t.fail(`expected to throw`);
    return;
  }

  const nameAndMessage = `${pattern.constructor.name} ${pattern.message}`;
  t.match(
    prepareErrorForMatch(error),
    prepareErrorForMatch(pattern),
    (pattern instanceof AggregateError
      ? `expected to throw: ${nameAndMessage} (${pattern.errors.length} error(s))`
      : `expected to throw: ${nameAndMessage}`));
}

function prepareErrorForMatch(error) {
  if (!(error instanceof Error)) {
    return error;
  }

  const matchable = {
    name: error.constructor.name,
    message: error.message,
  };

  if (error instanceof AggregateError) {
    matchable.errors = error.errors.map(prepareErrorForMatch);
  }

  return matchable;
}

function catchErrorOrNull(fn) {
  try {
    fn();
    return null;
  } catch (error) {
    return error;
  }
}
​ FAIL ​ test/unit/util/html.js
 ✖ expected to throw: AggregateError Errors validating template description (1 error(s))

  --- expected                                       
  +++ actual                                         
  @@ -4,7 +4,7 @@                                    
     "errors": Array [                               
       Object {                                      
         "name": "TypeError",                        
  -      "message": "some error message", 
  +      "message": "Expected description.content",  
       },                                            
     ],                                              
   }                                                 

  test: test/unit/util/html.js Template - description errors
  at:
    line: 568
    column: 3
    file: /path/to/test/unit/util/html.js
    type: Test

tcompare doesn't seem to handle error.cause either, which can be added similarly. The gist is that matching needs to be done on errors recursively, which I've done by preparing / "serializing" errors recursively - cause is another error reference so it can be prepared the same way.