Ensure test run failures crash worker (#1265)

* Ensure Test#run() returns exit promise

This allows errors from the exit logic to propagate to the runner.

* Treat runner errors as uncaught exceptions

Errors that occur inside the runner are treated as uncaught exceptions.
This prevents them from being swallowed in the promise chain, causing
the test to hang.

* fixup! Treat runner errors as uncaught exceptions

* fixup! Treat runner errors as uncaught exceptions

Author: novemberborn

lib/main.js

@@ -70,7 +70,11 @@ globals.setImmediate(() => {
 	runner.on('test', test);
 
 	process.on('ava-run', options => {
-		runner.run(options).then(exit);
+		runner.run(options)
+			.then(exit)
+			.catch(err => {
+				process.emit('uncaughtException', err);
+			});
 	});
 
 	process.on('ava-init-exit', () => {

lib/test-worker.js

@@ -36,7 +36,20 @@ process.on('unhandledRejection', throwsHelper);
 
 process.on('uncaughtException', exception => {
 	throwsHelper(exception);
-	send('uncaughtException', {exception: serializeError(exception)});
+
+	let serialized;
+	try {
+		serialized = serializeError(exception);
+	} catch (ignore) { // eslint-disable-line unicorn/catch-error-name
+		// Avoid using serializeError
+		const err = new Error('Failed to serialize uncaught exception');
+		serialized = {
+			name: err.name,
+			message: err.message,
+			stack: err.stack
+		};
+	}
+	send('uncaughtException', {exception: serialized});
 });
 
 // If AVA was not required, show an error

lib/test.js

@@ -240,10 +240,9 @@ class Test {
 				this._setAssertError(new Error(`Do not return ${asyncType} from tests declared via \`test.cb(...)\`, if you want to return a promise simply declare the test via \`test(...)\``));
 			}
 
-			ret.then(
-				() => {
-					this.exit();
-				},
+			// Convert to a Bluebird promise
+			return Promise.resolve(ret).then(
+				() => this.exit(),
 				err => {
 					if (!(err instanceof Error)) {
 						err = new assert.AssertionError({
@@ -254,10 +253,9 @@ class Test {
 					}
 
 					this._setAssertError(err);
-					this.exit();
-				});
-
-			return this.promise().promise;
+					return this.exit();
+				}
+			);
 		}
 
 		if (this.metadata.callback && !this.threwSync) {

test/cli.js

@@ -412,3 +412,17 @@ test('workers ensure test files load the same version of ava', t => {
 		t.end();
 	});
 });
+
+test('worker errors are treated as uncaught exceptions', t => {
+	execCli(['--no-color', '--verbose', 'test.js'], {dirname: 'fixture/trigger-worker-exception'}, (_, __, stderr) => {
+		t.match(stderr, /Forced error/);
+		t.end();
+	});
+});
+
+test('uncaught exceptions are raised for worker errors even if the error cannot be serialized', t => {
+	execCli(['--no-color', '--verbose', 'test-fallback.js'], {dirname: 'fixture/trigger-worker-exception'}, (_, __, stderr) => {
+		t.match(stderr, /Failed to serialize uncaught exception/);
+		t.end();
+	});
+});

test/fixture/trigger-worker-exception/hack.js

@@ -0,0 +1,23 @@
+'use strict';
+
+require('../../../lib/serialize-error');
+
+const serializeModule = require.cache[require.resolve('../../../lib/serialize-error')];
+
+const original = serializeModule.exports;
+let restored = false;
+let restoreAfterFirstCall = false;
+serializeModule.exports = error => {
+	if (restored) {
+		return original(error);
+	}
+	if (restoreAfterFirstCall) {
+		restored = true;
+	}
+
+	throw new Error('Forced error');
+};
+
+exports.restoreAfterFirstCall = () => {
+	restoreAfterFirstCall = true;
+};

test/fixture/trigger-worker-exception/package.json

@@ -0,0 +1,5 @@
+{
+  "ava": {
+    "require": "./hack.js"
+  }
+}

test/fixture/trigger-worker-exception/test-fallback.js

@@ -0,0 +1,5 @@
+import test from '../../../';
+
+test(async () => {
+	throw new Error('Hi :)');
+});

test/fixture/trigger-worker-exception/test.js

@@ -0,0 +1,8 @@
+import test from '../../../';
+
+import { restoreAfterFirstCall } from './hack';
+restoreAfterFirstCall();
+
+test(async () => {
+	throw new Error('Hi :)');
+});