Only treat native errors as errors

* Track worker errors. They're not native due to nodejs/node#48716, but we want to treat them as such anyway. * Only treat native errors as errors * Remove is-error dependency * Document edge case where `error instanceof Error` can be true, yet AVA does not recognize `error` as an error See also #2911 for an earlier attempt.

Author: novemberborn

docs/08-common-pitfalls.md

@@ -4,6 +4,18 @@ Translations: [Français](https://github.com/avajs/ava-docs/blob/main/fr_FR/docs
 
 If you use [ESLint](https://eslint.org), you can install [eslint-plugin-ava](https://github.com/avajs/eslint-plugin-ava). It will help you use AVA correctly and avoid some common pitfalls.
 
+## Error edge cases
+
+The `throws()` and `throwsAsync()` assertions use the Node.js built-in [`isNativeError()`](https://nodejs.org/api/util.html#utiltypesisnativeerrorvalue) 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:
+
+```js
+const error = Object.create(Error.prototype);
+```
+
+This can be surprising, since `error instanceof Error` returns `true`.
+
 ## AVA in Docker
 
 If you run AVA in Docker as part of your CI, you need to fix the appropriate environment variables. Specifically, adding `-e CI=true` in the `docker exec` command. See [#751](https://github.com/avajs/ava/issues/751).

lib/assert.js

@@ -1,5 +1,6 @@
+import {isNativeError} from 'node:util/types';
+
 import concordance from 'concordance';
-import isError from 'is-error';
 import isPromise from 'is-promise';
 
 import concordanceOptions from './concordance-options.js';
@@ -163,7 +164,7 @@ function validateExpectations(assertion, expectations, numberArgs) { // eslint-d
 // Note: this function *must* throw exceptions, since it can be used
 // as part of a pending assertion for promises.
 function assertExpectations({assertion, actual, expectations, message, prefix, savedError}) {
-if (!isError(actual)) {
+if (!isNativeError(actual)) {
 throw new AssertionError({
 assertion,
 message,

lib/fork.js

@@ -6,7 +6,7 @@ import {Worker} from 'node:worker_threads';
 import Emittery from 'emittery';
 
 import {controlFlow} from './ipc-flow-control.cjs';
-import serializeError from './serialize-error.js';
+import serializeError, {tagWorkerError} from './serialize-error.js';
 
 let workerPath = new URL('worker/base.js', import.meta.url);
 export function _testOnlyReplaceWorkerPath(replacement) {
@@ -134,7 +134,7 @@ export default function loadFork(file, options, execArgv = process.execArgv) {
 });
 
 worker.on('error', error => {
-emitStateChange({type: 'worker-failed', err: serializeError('Worker error', false, error, file)});
+emitStateChange({type: 'worker-failed', err: serializeError('Worker error', false, tagWorkerError(error), file)});
 finish();
 });
 

lib/plugin-support/shared-workers.js

@@ -2,7 +2,7 @@ import events from 'node:events';
 import {pathToFileURL} from 'node:url';
 import {Worker} from 'node:worker_threads';
 
-import serializeError from '../serialize-error.js';
+import serializeError, {tagWorkerError} from '../serialize-error.js';
 
 const LOADER = new URL('shared-worker-loader.js', import.meta.url);
 
@@ -34,7 +34,7 @@ function launchWorker(filename, initialData) {
 const launched = {
 statePromises: {
 available: waitForAvailable(worker),
-error: events.once(worker, 'error').then(([error]) => error),
+error: events.once(worker, 'error').then(([error]) => tagWorkerError(error)),
 },
 exited: false,
 worker,

lib/serialize-error.js

@@ -1,10 +1,10 @@
 import path from 'node:path';
 import process from 'node:process';
 import {fileURLToPath, pathToFileURL} from 'node:url';
+import {isNativeError} from 'node:util/types';
 
 import cleanYamlObject from 'clean-yaml-object';
 import concordance from 'concordance';
-import isError from 'is-error';
 import StackUtils from 'stack-utils';
 
 import {AssertionError} from './assert.js';
@@ -145,8 +145,21 @@ function trySerializeError(error, shouldBeautifyStack, testFile) {
 return retval;
 }
 
+const workerErrors = new WeakSet();
+export function tagWorkerError(error) {
+// Track worker errors, which aren't native due to https://github.com/nodejs/node/issues/48716.
+// Still include the check for isNativeError() in case the issue is fixed in the future.
+if (isNativeError(error) || error instanceof Error) {
+workerErrors.add(error);
+}
+
+return error;
+}
+
+const isWorkerError = error => workerErrors.has(error);
+
 export default function serializeError(origin, shouldBeautifyStack, error, testFile) {
-if (!isError(error)) {
+if (!isNativeError(error) && !isWorkerError(error)) {
 return {
 avaAssertionError: false,
 nonErrorObject: true,

package-lock.json

@@ -104,7 +104,6 @@
 "globby": "^13.2.1",
 "ignore-by-default": "^2.1.0",
 "indent-string": "^5.0.0",
-"is-error": "^2.2.2",
 "is-plain-object": "^5.0.0",
 "is-promise": "^4.0.0",
 "matcher": "^5.0.0",

package.json

@@ -0,0 +1,13 @@
+import test from 'ava';
+
+test('throws native error', async t => {
+await t.throwsAsync(async () => {
+throw new Error('foo');
+});
+});
+
+test('throws object that extends the error prototype', async t => {
+await t.throwsAsync(async () => {
+throw Object.create(Error.prototype);
+});
+});

test/assertions/fixtures/throws-async.js

@@ -0,0 +1,13 @@
+import test from 'ava';
+
+test('throws native error', t => {
+t.throws(() => {
+throw new Error('foo');
+});
+});
+
+test('throws object that extends the error prototype', t => {
+t.throws(() => {
+throw Object.create(Error.prototype);
+});
+});

test/assertions/fixtures/throws.js

@@ -28,3 +28,31 @@ Generated by [AVA](https://avajs.dev).
   't.true(true) passes',
   't.truthy(1) passes',
  ]
+
+## throws requires native errors
+
+> passed tests
+
+ [
+  'throws native error',
+ ]
+
+> failed tests
+
+ [
+  'throws object that extends the error prototype',
+ ]
+
+## throwsAsync requires native errors
+
+> passed tests
+
+ [
+  'throws native error',
+ ]
+
+> failed tests
+
+ [
+  'throws object that extends the error prototype',
+ ]