Skip to content

Commit

Permalink
fix: avoid swallowing of asynchronously thrown errors
Browse files Browse the repository at this point in the history
Fixes: nodejs/node#44612
PR-URL: nodejs/node#45264
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
(cherry picked from commit 06603c44a5b0e92b1a3591ace467ce9770bf9658)
  • Loading branch information
fossamagna authored and MoLow committed Feb 2, 2023
1 parent f2815af commit cff397a
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 3 deletions.
4 changes: 2 additions & 2 deletions README.md
Expand Up @@ -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 => {
Expand Down
3 changes: 2 additions & 1 deletion 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,
Expand Down Expand Up @@ -47,6 +47,7 @@ function createProcessEventHandler (eventName, rootTest) {
`triggered an ${eventName} event.`

rootTest.diagnostic(msg)
process.exitCode = 1
return
}

Expand Down
6 changes: 6 additions & 0 deletions 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() })
})
6 changes: 6 additions & 0 deletions 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)
})
32 changes: 32 additions & 0 deletions 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)
}

0 comments on commit cff397a

Please sign in to comment.