test_runner: catch reporter errors

PR-URL: nodejs/node#49646
Fixes: nodejs/node#48937
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: sercher

graal-nodejs/lib/internal/test_runner/harness.js

@@ -19,12 +19,15 @@ const { kEmptyObject } = require('internal/util');
 const { kCancelledByParent, Test, Suite } = require('internal/test_runner/test');
 const {
   parseCommandLine,
+  reporterScope,
   setupTestReporters,
 } = require('internal/test_runner/utils');
 const { bigint: hrtime } = process.hrtime;
 
 const testResources = new SafeMap();
 
+testResources.set(reporterScope.asyncId(), reporterScope);
+
 function createTestTree(options = kEmptyObject) {
   return setup(new Test({ __proto__: null, ...options, name: '<root>' }));
 }
@@ -38,9 +41,14 @@ function createProcessEventHandler(eventName, rootTest) {
       throw err;
     }
 
-    // Check if this error is coming from a test. If it is, fail the test.
     const test = testResources.get(executionAsyncId());
 
+    // Check if this error is coming from a reporter. If it is, throw it.
+    if (test === reporterScope) {
+      throw err;
+    }
+
+    // Check if this error is coming from a test. If it is, fail the test.
     if (!test || test.finished) {
       // If the test is already finished or the resource that created the error
       // is not mapped to a Test, report this as a top level diagnostic.

graal-nodejs/lib/internal/test_runner/utils.js

@@ -20,6 +20,7 @@ const {
   StringPrototypeSlice,
 } = primordials;
 
+const { AsyncResource } = require('async_hooks');
 const { basename, relative } = require('path');
 const { createWriteStream } = require('fs');
 const { pathToFileURL } = require('internal/url');
@@ -169,15 +170,15 @@ async function getReportersMap(reporters, destinations, rootTest) {
   });
 }
 
-
-async function setupTestReporters(rootTest) {
+const reporterScope = new AsyncResource('TestReporterScope');
+const setupTestReporters = reporterScope.bind(async (rootTest) => {
   const { reporters, destinations } = parseCommandLine();
   const reportersMap = await getReportersMap(reporters, destinations, rootTest);
   for (let i = 0; i < reportersMap.length; i++) {
     const { reporter, destination } = reportersMap[i];
     compose(rootTest.reporter, reporter).pipe(destination);
   }
-}
+});
 
 let globalTestOptions;
 
@@ -420,6 +421,7 @@ module.exports = {
   isSupportedFileType,
   isTestFailureError,
   parseCommandLine,
+  reporterScope,
   setupTestReporters,
   getCoverageReport,
 };

graal-nodejs/test/fixtures/test-runner/custom_reporters/throwing-async.js

@@ -0,0 +1,8 @@
+'use strict';
+
+module.exports = async function * customReporter() {
+  yield 'Going to throw an error\n';
+  setImmediate(() => {
+    throw new Error('Reporting error');
+  });
+};

graal-nodejs/test/fixtures/test-runner/custom_reporters/throwing.js

@@ -0,0 +1,6 @@
+'use strict';
+
+module.exports = async function * customReporter() {
+  yield 'Going to throw an error\n';
+  throw new Error('Reporting error');
+};

graal-nodejs/test/parallel/test-runner-reporters.js

@@ -135,4 +135,25 @@ describe('node:test reporters', { concurrency: true }, () => {
     assert.strictEqual(child.stdout.toString(), '');
     assert.match(child.stderr.toString(), /ERR_INVALID_ARG_TYPE/);
   });
+
+  it('should throw when reporter errors', async () => {
+    const child = spawnSync(process.execPath,
+                            ['--test', '--test-reporter', fixtures.fileURL('test-runner/custom_reporters/throwing.js'),
+                             fixtures.path('test-runner/default-behavior/index.test.js')]);
+    assert.strictEqual(child.status, 7);
+    assert.strictEqual(child.signal, null);
+    assert.strictEqual(child.stdout.toString(), 'Going to throw an error\n');
+    assert.match(child.stderr.toString(), /Error: Reporting error\r?\n\s+at customReporter/);
+  });
+
+  it('should throw when reporter errors asynchronously', async () => {
+    const child = spawnSync(process.execPath,
+                            ['--test', '--test-reporter',
+                             fixtures.fileURL('test-runner/custom_reporters/throwing-async.js'),
+                             fixtures.path('test-runner/default-behavior/index.test.js')]);
+    assert.strictEqual(child.status, 7);
+    assert.strictEqual(child.signal, null);
+    assert.strictEqual(child.stdout.toString(), 'Going to throw an error\n');
+    assert.match(child.stderr.toString(), /Emitted 'error' event on Duplex instance/);
+  });
 });