Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 5eb0765

Browse filesBrowse files
bnoordhuisMylesBorins
authored andcommitted
src: handle TryCatch with empty message
This bug needs a test case with a high goldilocks factor to trigger but the synopsis is that `v8::TryCatch::Message()` returns an empty handle when the TryCatch is declared at a time when an exception is already pending. We now recompute the message inside `node::ReportException()` and all is well again. PR-URL: #20708 Fixes: #8854 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
1 parent a4cbe30 commit 5eb0765
Copy full SHA for 5eb0765

File tree

Expand file treeCollapse file tree

2 files changed

+60
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+60
-1
lines changed
Open diff view settings
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1413,9 +1413,11 @@ static void ReportException(Environment* env,
14131413
Local<Value> er,
14141414
Local<Message> message) {
14151415
CHECK(!er.IsEmpty());
1416-
CHECK(!message.IsEmpty());
14171416
HandleScope scope(env->isolate());
14181417

1418+
if (message.IsEmpty())
1419+
message = Exception::CreateMessage(env->isolate(), er);
1420+
14191421
AppendExceptionLine(env, er, message, FATAL_ERROR);
14201422

14211423
Local<Value> trace_value;
Collapse file
+57Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use strict';
2+
3+
// Verify that exceptions from a callback don't result in
4+
// failed CHECKs when trying to print the exception message.
5+
6+
// This test is convoluted because it needs to trigger a callback
7+
// into JS land at just the right time when an exception is pending,
8+
// and does so by exploiting a weakness in the streams infrastructure.
9+
// I won't shed any tears if this test ever becomes invalidated.
10+
11+
const common = require('../common');
12+
13+
if (!common.hasCrypto)
14+
common.skip('missing crypto');
15+
16+
if (process.argv[2] === 'child') {
17+
const fixtures = require('../common/fixtures');
18+
const https = require('https');
19+
const net = require('net');
20+
const tls = require('tls');
21+
const { Duplex } = require('stream');
22+
const { mustCall } = common;
23+
24+
const cert = fixtures.readSync('test_cert.pem');
25+
const key = fixtures.readSync('test_key.pem');
26+
27+
net.createServer(mustCall(onplaintext)).listen(0, mustCall(onlisten));
28+
29+
function onlisten() {
30+
const { port } = this.address();
31+
https.get({ port, rejectUnauthorized: false });
32+
}
33+
34+
function onplaintext(c) {
35+
const d = new class extends Duplex {
36+
_read(n) {
37+
const data = c.read(n);
38+
if (data) d.push(data);
39+
}
40+
_write(...xs) {
41+
c.write(...xs);
42+
}
43+
}();
44+
c.on('data', d.push.bind(d));
45+
46+
const options = { key, cert };
47+
const fail = () => { throw new Error('eyecatcher'); };
48+
tls.createServer(options, mustCall(fail)).emit('connection', d);
49+
}
50+
} else {
51+
const assert = require('assert');
52+
const { spawnSync } = require('child_process');
53+
const result = spawnSync(process.execPath, [__filename, 'child']);
54+
const stderr = result.stderr.toString();
55+
const ok = stderr.includes('Error: eyecatcher');
56+
assert(ok, stderr);
57+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.