From a2ccba8d8b4dd029c6482ed7f25bb78d2a2acd09 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Wed, 15 Feb 2017 15:38:31 +0000 Subject: [PATCH 1/4] Ensure Test#run() returns exit promise This allows errors from the exit logic to propagate to the runner. --- lib/test.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/test.js b/lib/test.js index 377d5bdd6..2e4e869c8 100644 --- a/lib/test.js +++ b/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) { From 539c741ba2d32bf51afcc79880e0b35c4ac5d7d7 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Wed, 15 Feb 2017 15:40:03 +0000 Subject: [PATCH 2/4] 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. --- lib/main.js | 18 ++++++++++++++- test/cli.js | 14 +++++++++++ test/fixture/trigger-worker-exception/hack.js | 23 +++++++++++++++++++ .../trigger-worker-exception/package.json | 5 ++++ .../trigger-worker-exception/test-fallback.js | 5 ++++ test/fixture/trigger-worker-exception/test.js | 8 +++++++ 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 test/fixture/trigger-worker-exception/hack.js create mode 100644 test/fixture/trigger-worker-exception/package.json create mode 100644 test/fixture/trigger-worker-exception/test-fallback.js create mode 100644 test/fixture/trigger-worker-exception/test.js diff --git a/lib/main.js b/lib/main.js index a9205371a..798e6e14a 100644 --- a/lib/main.js +++ b/lib/main.js @@ -70,7 +70,23 @@ globals.setImmediate(() => { runner.on('test', test); process.on('ava-run', options => { - runner.run(options).then(exit); + runner.run(options) + .then(exit) + .catch(err => { + let exception; + try { + exception = serializeError(err); + } catch (_) { + // Avoid using serializeError + const err = new Error('Runner failed with an unserializable exception'); + exception = { + name: 'Error', + message: err.message, + stack: err.stack + }; + } + send('uncaughtException', {exception}); + }); }); process.on('ava-init-exit', () => { diff --git a/test/cli.js b/test/cli.js index 7cd6809ca..8f277f3fb 100644 --- a/test/cli.js +++ b/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, /Runner failed with an unserializable exception/); + t.end(); + }); +}); diff --git a/test/fixture/trigger-worker-exception/hack.js b/test/fixture/trigger-worker-exception/hack.js new file mode 100644 index 000000000..5db3daf4b --- /dev/null +++ b/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; +}; diff --git a/test/fixture/trigger-worker-exception/package.json b/test/fixture/trigger-worker-exception/package.json new file mode 100644 index 000000000..1027c004d --- /dev/null +++ b/test/fixture/trigger-worker-exception/package.json @@ -0,0 +1,5 @@ +{ + "ava": { + "require": "./hack.js" + } +} diff --git a/test/fixture/trigger-worker-exception/test-fallback.js b/test/fixture/trigger-worker-exception/test-fallback.js new file mode 100644 index 000000000..51ae59068 --- /dev/null +++ b/test/fixture/trigger-worker-exception/test-fallback.js @@ -0,0 +1,5 @@ +import test from '../../../'; + +test(() => { + return Promise.reject(new Error('Hi :)')); +}); diff --git a/test/fixture/trigger-worker-exception/test.js b/test/fixture/trigger-worker-exception/test.js new file mode 100644 index 000000000..ae3780b1b --- /dev/null +++ b/test/fixture/trigger-worker-exception/test.js @@ -0,0 +1,8 @@ +import test from '../../../'; + +import { restoreAfterFirstCall } from './hack'; +restoreAfterFirstCall(); + +test(() => { + return Promise.reject(new Error('Hi :)')); +}); From a87d1bbd038137f12a5b4b5d470ea20059c14518 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 17 Feb 2017 11:58:22 +0000 Subject: [PATCH 3/4] fixup! Treat runner errors as uncaught exceptions --- test/fixture/trigger-worker-exception/test-fallback.js | 4 ++-- test/fixture/trigger-worker-exception/test.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/fixture/trigger-worker-exception/test-fallback.js b/test/fixture/trigger-worker-exception/test-fallback.js index 51ae59068..27b164653 100644 --- a/test/fixture/trigger-worker-exception/test-fallback.js +++ b/test/fixture/trigger-worker-exception/test-fallback.js @@ -1,5 +1,5 @@ import test from '../../../'; -test(() => { - return Promise.reject(new Error('Hi :)')); +test(async () => { + throw new Error('Hi :)'); }); diff --git a/test/fixture/trigger-worker-exception/test.js b/test/fixture/trigger-worker-exception/test.js index ae3780b1b..d60069e80 100644 --- a/test/fixture/trigger-worker-exception/test.js +++ b/test/fixture/trigger-worker-exception/test.js @@ -3,6 +3,6 @@ import test from '../../../'; import { restoreAfterFirstCall } from './hack'; restoreAfterFirstCall(); -test(() => { - return Promise.reject(new Error('Hi :)')); +test(async () => { + throw new Error('Hi :)'); }); From dfc156793c237ebff987095d24671ba59cc18bed Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 17 Feb 2017 11:58:50 +0000 Subject: [PATCH 4/4] fixup! Treat runner errors as uncaught exceptions --- lib/main.js | 14 +------------- lib/test-worker.js | 15 ++++++++++++++- test/cli.js | 2 +- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/lib/main.js b/lib/main.js index 798e6e14a..e56b94d8c 100644 --- a/lib/main.js +++ b/lib/main.js @@ -73,19 +73,7 @@ globals.setImmediate(() => { runner.run(options) .then(exit) .catch(err => { - let exception; - try { - exception = serializeError(err); - } catch (_) { - // Avoid using serializeError - const err = new Error('Runner failed with an unserializable exception'); - exception = { - name: 'Error', - message: err.message, - stack: err.stack - }; - } - send('uncaughtException', {exception}); + process.emit('uncaughtException', err); }); }); diff --git a/lib/test-worker.js b/lib/test-worker.js index df568cd96..1209a9ea4 100644 --- a/lib/test-worker.js +++ b/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 diff --git a/test/cli.js b/test/cli.js index 8f277f3fb..abbf6e841 100644 --- a/test/cli.js +++ b/test/cli.js @@ -422,7 +422,7 @@ test('worker errors are treated as uncaught exceptions', t => { 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, /Runner failed with an unserializable exception/); + t.match(stderr, /Failed to serialize uncaught exception/); t.end(); }); });