DOMException is not recognized as an Error by throws() and throwsAsync() in AVA v6

[sholladay]

In the Ky tests, we have a simple assertion that a Node.js DOMException is thrown when a request is aborted. In AVA v5, this assertion works as expected. However, beginning with AVA v6, DOMException seems to no longer be detected as an error, even though error instanceOf Error is true for it.

Minimal reproduction with default config:

import test from 'ava';

test('DOMException should be considered an error', async t => {
	await t.throwsAsync(Promise.reject(new DOMException('an error')));
});

The above test passes on AVA v5 but fails on v6. This applies to both t.throws() and t.throwsAsync().

As an aside, the docs are not very clear on what is considered "an error". I always assumed it meant the object had to be an instanceof Error, but at least in AVA v6, this seems to not be the case. Is it because error.constructor.name doesn't include "Error"? Would be nice to have some clarification.

[Prasanth2706]

`import test from 'ava';

test('DOMException should be considered an error', async t => {
await t.throwsAsync(async () => {
throw new DOMException('an error');
}, {instanceOf: DOMException, message: 'an error'});
});
`

[sholladay]

We don't use the assertion instanceOf option because the error constructor actually depends upon the environment where the code is run. Sometimes it's just a plain Error. So we run a separate t.true() assertion afterwards with an array of constructor names to check that.

[novemberborn]

See https://github.com/avajs/ava/blob/main/docs/08-common-pitfalls.md#error-edge-cases (which isn't linked to from the assertions documentation, so that's not helpful).

The throws() and throwsAsync() assertions use the Node.js built-in isNativeError() to determine whether something is an error. This only recognizes actual instances of Error (and subclasses).

Note that the following is not a native error:

const error = Object.create(Error.prototype);

This can be surprising, since error instanceof Error returns true. You can set any: true in the expectations to handle these values:

const error = Object.create(Error.prototype);
t.throws(() => { throw error }, {any: true});```

How is DOMException created?

[sholladay]

It comes from fetch(), the web API, when you abort the request using an AbortSignal. So how it is created would depend on the platform. But it's a subclass of Error, which seems like it ought to be sufficient for t.throws().

[novemberborn]

var error;fetch('https://novemberborn.net', {signal: AbortSignal.abort() }).catch(e=>{error=e})
// Wait for promise…
util.types.isNativeError(error) // false!

Looking at https://developer.mozilla.org/en-US/docs/Web/API/DOMException I don't think a DOMException is an Error.

However it is included as a global in Node.js, so I think t.throws() and related assertions should treat it as such.

That said, the implementation in Node.js doesn't look to be spec-compatible, e.g. nodejs/node#57372 suggests the errors can't be cloned / sent between workers.

I think an instanceof DOMException check would suffice, wherever we use isNativeError: https://github.com/search?q=repo%3Aavajs%2Fava%20isnativeerror&type=code

[sholladay]

What's the reason for not treating anything where instsnceof Error is true as an error? From the docs, I take it this is intentional.

[novemberborn]

You'll have to look at the commit for all the details, but if I recall we had some heuristics and switched to a built-in method. Error.isError() is being standardized (I think?) to do the same.

Not at my computer currently but I don't think instanceof Error works for DOMException either.

[novemberborn]

OK so instanceof Error yields true, because of this line:

https://github.com/nodejs/node/blob/ad078696026f42605a37a65702a1467968030ae0/lib/internal/per_context/domexception.js#L110

DOMException as originally defined and implemented in Node.js didn't have [[ErrorData]], so it doesn't count as a "native error" (or Error.isError()). That was addressed in the standard here: whatwg/webidl#1421. There's an outstanding issue to add this in Node.js at nodejs/node#56497.

The removal of instanceof Error was intentional, and we'd want to move to Error.isError() as it becomes available. I think the correct workaround at this stage is to add an instanceof DOMException check which can be removed as Node.js catches up to the standard.