-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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: #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>
- Loading branch information
Showing
3 changed files
with
46 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
} |