src: print exceptions from PromiseRejectCallback
Previously, leaving the exception lying around would leave the JS engine in an invalid state, as it was not expecting exceptions to be thrown from the C++ `PromiseRejectCallback`, and lead to hard crashes under some conditions (e.g. with coverage enabled). PR-URL: https://github.com/nodejs/node/pull/29513 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
parent
233cdb64a9
commit
e095e645e5
@ -10,6 +10,7 @@
|
|||||||
|
|
||||||
namespace node {
|
namespace node {
|
||||||
|
|
||||||
|
using errors::TryCatchScope;
|
||||||
using v8::Array;
|
using v8::Array;
|
||||||
using v8::Context;
|
using v8::Context;
|
||||||
using v8::Function;
|
using v8::Function;
|
||||||
@ -111,8 +112,17 @@ void PromiseRejectCallback(PromiseRejectMessage message) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
Local<Value> args[] = { type, promise, value };
|
Local<Value> args[] = { type, promise, value };
|
||||||
|
|
||||||
|
// V8 does not expect this callback to have a scheduled exceptions once it
|
||||||
|
// returns, so we print them out in a best effort to do something about it
|
||||||
|
// without failing silently and without crashing the process.
|
||||||
|
TryCatchScope try_catch(env);
|
||||||
USE(callback->Call(
|
USE(callback->Call(
|
||||||
env->context(), Undefined(isolate), arraysize(args), args));
|
env->context(), Undefined(isolate), arraysize(args), args));
|
||||||
|
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
|
||||||
|
fprintf(stderr, "Exception in PromiseRejectCallback:\n");
|
||||||
|
PrintCaughtException(isolate, env->context(), try_catch);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
static void SetPromiseRejectCallback(
|
static void SetPromiseRejectCallback(
|
||||||
|
@ -23,4 +23,4 @@ assert.strictEqual(ret.status, 0,
|
|||||||
`EXIT CODE: ${ret.status}, STDERR:\n${ret.stderr}`);
|
`EXIT CODE: ${ret.status}, STDERR:\n${ret.stderr}`);
|
||||||
const stderr = ret.stderr.toString('utf8', 0, 2048);
|
const stderr = ret.stderr.toString('utf8', 0, 2048);
|
||||||
assert.ok(!/async.*hook/i.test(stderr));
|
assert.ok(!/async.*hook/i.test(stderr));
|
||||||
assert.ok(stderr.includes('UnhandledPromiseRejectionWarning: Error'), stderr);
|
assert.ok(stderr.includes('Maximum call stack size exceeded'), stderr);
|
||||||
|
35
test/parallel/test-promise-reject-callback-exception.js
Normal file
35
test/parallel/test-promise-reject-callback-exception.js
Normal file
@ -0,0 +1,35 @@
|
|||||||
|
'use strict';
|
||||||
|
require('../common');
|
||||||
|
const tmpdir = require('../common/tmpdir');
|
||||||
|
const assert = require('assert');
|
||||||
|
const path = require('path');
|
||||||
|
const child_process = require('child_process');
|
||||||
|
|
||||||
|
tmpdir.refresh();
|
||||||
|
|
||||||
|
// Tests that exceptions from the PromiseRejectCallback are printed to stderr
|
||||||
|
// when they occur as a best-effort way of handling them, and that calling
|
||||||
|
// `console.log()` works after that. Earlier, the latter did not work because
|
||||||
|
// of the exception left lying around by the PromiseRejectCallback when its JS
|
||||||
|
// part exceeded the call stack limit, and when the inspector/built-in coverage
|
||||||
|
// was enabled, it resulted in a hard crash.
|
||||||
|
|
||||||
|
for (const NODE_V8_COVERAGE of ['', tmpdir.path]) {
|
||||||
|
// NODE_V8_COVERAGE does not work without the inspector.
|
||||||
|
// Refs: https://github.com/nodejs/node/issues/29542
|
||||||
|
if (!process.features.inspector && NODE_V8_COVERAGE !== '') continue;
|
||||||
|
|
||||||
|
const { status, signal, stdout, stderr } =
|
||||||
|
child_process.spawnSync(process.execPath,
|
||||||
|
[path.join(__dirname, 'test-ttywrap-stack.js')],
|
||||||
|
{ env: { ...process.env, NODE_V8_COVERAGE } });
|
||||||
|
|
||||||
|
assert(stdout.toString('utf8')
|
||||||
|
.startsWith('RangeError: Maximum call stack size exceeded'),
|
||||||
|
`stdout: <${stdout}>`);
|
||||||
|
assert(stderr.toString('utf8')
|
||||||
|
.startsWith('Exception in PromiseRejectCallback'),
|
||||||
|
`stderr: <${stderr}>`);
|
||||||
|
assert.strictEqual(status, 0);
|
||||||
|
assert.strictEqual(signal, null);
|
||||||
|
}
|
Loading…
x
Reference in New Issue
Block a user