From cff397a46139411528314d5a9399ae12e1b48660 Mon Sep 17 00:00:00 2001 From: MURAKAMI Masahiko Date: Mon, 7 Nov 2022 21:02:56 +0900 Subject: [PATCH] fix: avoid swallowing of asynchronously thrown errors Fixes: https://github.com/nodejs/node/issues/44612 PR-URL: https://github.com/nodejs/node/pull/45264 Reviewed-By: Moshe Atlow Reviewed-By: Benjamin Gruenbaum (cherry picked from commit 06603c44a5b0e92b1a3591ace467ce9770bf9658) --- README.md | 4 +-- lib/internal/test_runner/harness.js | 3 +- .../extraneous_set_immediate_async.mjs | 6 ++++ .../extraneous_set_timeout_async.mjs | 6 ++++ .../test-runner-extraneous-async-activity.js | 32 +++++++++++++++++++ 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/test-runner/extraneous_set_immediate_async.mjs create mode 100644 test/fixtures/test-runner/extraneous_set_timeout_async.mjs create mode 100644 test/parallel/test-runner-extraneous-async-activity.js diff --git a/README.md b/README.md index 0407239..563e158 100644 --- a/README.md +++ b/README.md @@ -279,8 +279,8 @@ top level of the file's TAP output. The second `setImmediate()` creates an `uncaughtException` event. `uncaughtException` and `unhandledRejection` events originating from a completed -test are handled by the `test` module and reported as diagnostic warnings in -the top level of the file's TAP output. +test are marked as failed by the `test` module and reported as diagnostic +warnings in the top level of the file's TAP output. ```js test('a test that creates asynchronous activity', t => { diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index bddfedc..26d35ba 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/harness.js +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/lib/internal/test_runner/harness.js 'use strict' const { ArrayPrototypeForEach, @@ -47,6 +47,7 @@ function createProcessEventHandler (eventName, rootTest) { `triggered an ${eventName} event.` rootTest.diagnostic(msg) + process.exitCode = 1 return } diff --git a/test/fixtures/test-runner/extraneous_set_immediate_async.mjs b/test/fixtures/test-runner/extraneous_set_immediate_async.mjs new file mode 100644 index 0000000..241e262 --- /dev/null +++ b/test/fixtures/test-runner/extraneous_set_immediate_async.mjs @@ -0,0 +1,6 @@ +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/test/fixtures/test-runner/extraneous_set_immediate_async.mjs +import test from '#node:test' + +test('extraneous async activity test', () => { + setImmediate(() => { throw new Error() }) +}) diff --git a/test/fixtures/test-runner/extraneous_set_timeout_async.mjs b/test/fixtures/test-runner/extraneous_set_timeout_async.mjs new file mode 100644 index 0000000..2b5ee72 --- /dev/null +++ b/test/fixtures/test-runner/extraneous_set_timeout_async.mjs @@ -0,0 +1,6 @@ +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/test/fixtures/test-runner/extraneous_set_timeout_async.mjs +import test from '#node:test' + +test('extraneous async activity test', () => { + setTimeout(() => { throw new Error() }, 100) +}) diff --git a/test/parallel/test-runner-extraneous-async-activity.js b/test/parallel/test-runner-extraneous-async-activity.js new file mode 100644 index 0000000..da826c4 --- /dev/null +++ b/test/parallel/test-runner-extraneous-async-activity.js @@ -0,0 +1,32 @@ +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/test/parallel/test-runner-extraneous-async-activity.js +'use strict' +require('../common') +const fixtures = require('../common/fixtures') +const assert = require('assert') +const { spawnSync } = require('child_process') + +{ + const child = spawnSync(process.execPath, [ + '--test', + fixtures.path('test-runner', 'extraneous_set_immediate_async.mjs') + ]) + const stdout = child.stdout.toString() + assert.match(stdout, /^# pass 0$/m) + assert.match(stdout, /^# fail 1$/m) + assert.match(stdout, /^# cancelled 0$/m) + assert.strictEqual(child.status, 1) + assert.strictEqual(child.signal, null) +} + +{ + const child = spawnSync(process.execPath, [ + '--test', + fixtures.path('test-runner', 'extraneous_set_timeout_async.mjs') + ]) + const stdout = child.stdout.toString() + assert.match(stdout, /^# pass 0$/m) + assert.match(stdout, /^# fail 1$/m) + assert.match(stdout, /^# cancelled 0$/m) + assert.strictEqual(child.status, 1) + assert.strictEqual(child.signal, null) +}