From d86132488dd3445cdd058bb6ed08b45a7a93cec0 Mon Sep 17 00:00:00 2001 From: legendecas Date: Tue, 6 Apr 2021 11:49:38 +0800 Subject: [PATCH] lib: properly process JavaScript exceptions on async_hooks fatal error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JavaScript exceptions could be arbitrary values. PR-URL: https://github.com/nodejs/node/pull/38106 Reviewed-By: Antoine du Hamel Reviewed-By: Gerhard Stöbich Reviewed-By: James M Snell --- lib/internal/async_hooks.js | 12 ++++- test/parallel/test-async-hooks-fatal-error.js | 53 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-async-hooks-fatal-error.js diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index 43ba749cd69946..69ca8d6db987ed 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -100,6 +100,9 @@ const { const { async_id_symbol, trigger_async_id_symbol } = internalBinding('symbols'); +// Lazy load of internal/util/inspect; +let inspect; + // Used in AsyncHook and AsyncResource. const init_symbol = Symbol('init'); const before_symbol = Symbol('before'); @@ -156,12 +159,17 @@ function executionAsyncResource() { return lookupPublicResource(resource); } +function inspectExceptionValue(e) { + inspect ??= require('internal/util/inspect').inspect; + return { message: inspect(e) }; +} + // Used to fatally abort the process if a callback throws. function fatalError(e) { - if (typeof e.stack === 'string') { + if (typeof e?.stack === 'string') { process._rawDebug(e.stack); } else { - const o = { message: e }; + const o = inspectExceptionValue(e); ErrorCaptureStackTrace(o, fatalError); process._rawDebug(o.stack); } diff --git a/test/parallel/test-async-hooks-fatal-error.js b/test/parallel/test-async-hooks-fatal-error.js new file mode 100644 index 00000000000000..40c2edf329df6e --- /dev/null +++ b/test/parallel/test-async-hooks-fatal-error.js @@ -0,0 +1,53 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const childProcess = require('child_process'); +const os = require('os'); + +if (process.argv[2] === 'child') { + child(process.argv[3], process.argv[4]); +} else { + main(); +} + +function child(type, valueType) { + const { createHook } = require('async_hooks'); + const fs = require('fs'); + + createHook({ + [type]() { + if (valueType === 'symbol') { + throw Symbol('foo'); + } + // eslint-disable-next-line no-throw-literal + throw null; + } + }).enable(); + + // Trigger `promiseResolve`. + new Promise((resolve) => resolve()) + // Trigger `after`/`destroy`. + .then(() => fs.promises.readFile(__filename, 'utf8')) + // Make process exit with code 0 if no error caught. + .then(() => process.exit(0)); +} + +function main() { + const types = [ 'init', 'before', 'after', 'destroy', 'promiseResolve' ]; + const valueTypes = [ + [ 'null', 'Error: null' ], + [ 'symbol', 'Error: Symbol(foo)' ], + ]; + for (const type of types) { + for (const [valueType, expect] of valueTypes) { + const cp = childProcess.spawnSync( + process.execPath, + [ __filename, 'child', type, valueType ], + { + encoding: 'utf8', + }); + assert.strictEqual(cp.status, 1, type); + assert.strictEqual(cp.stderr.trim().split(os.EOL)[0], expect, type); + } + } +}