src: handle TryCatch with empty message

bnoordhuis · MylesBorins · commit 5eb0765fc9bb · 2018-05-22T14:09:48.000-04:00
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>
diff --git a/src/node.cc b/src/node.cc
@@ -1413,9 +1413,11 @@ static void ReportException(Environment* env,
                             Local<Value> er,
                             Local<Message> message) {
   CHECK(!er.IsEmpty());
-  CHECK(!message.IsEmpty());
   HandleScope scope(env->isolate());
 
+  if (message.IsEmpty())
+    message = Exception::CreateMessage(env->isolate(), er);
+
   AppendExceptionLine(env, er, message, FATAL_ERROR);
 
   Local<Value> trace_value;
diff --git a/test/parallel/test-tls-handshake-exception.js b/test/parallel/test-tls-handshake-exception.js
@@ -0,0 +1,57 @@
+'use strict';
+
+// Verify that exceptions from a callback don't result in
+// failed CHECKs when trying to print the exception message.
+
+// This test is convoluted because it needs to trigger a callback
+// into JS land at just the right time when an exception is pending,
+// and does so by exploiting a weakness in the streams infrastructure.
+// I won't shed any tears if this test ever becomes invalidated.
+
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+if (process.argv[2] === 'child') {
+  const fixtures = require('../common/fixtures');
+  const https = require('https');
+  const net = require('net');
+  const tls = require('tls');
+  const { Duplex } = require('stream');
+  const { mustCall } = common;
+
+  const cert = fixtures.readSync('test_cert.pem');
+  const key = fixtures.readSync('test_key.pem');
+
+  net.createServer(mustCall(onplaintext)).listen(0, mustCall(onlisten));
+
+  function onlisten() {
+    const { port } = this.address();
+    https.get({ port, rejectUnauthorized: false });
+  }
+
+  function onplaintext(c) {
+    const d = new class extends Duplex {
+      _read(n) {
+        const data = c.read(n);
+        if (data) d.push(data);
+      }
+      _write(...xs) {
+        c.write(...xs);
+      }
+    }();
+    c.on('data', d.push.bind(d));
+
+    const options = { key, cert };
+    const fail = () => { throw new Error('eyecatcher'); };
+    tls.createServer(options, mustCall(fail)).emit('connection', d);
+  }
+} else {
+  const assert = require('assert');
+  const { spawnSync } = require('child_process');
+  const result = spawnSync(process.execPath, [__filename, 'child']);
+  const stderr = result.stderr.toString();
+  const ok = stderr.includes('Error: eyecatcher');
+  assert(ok, stderr);
+}
