quic: handle errors thrown / rejections in the session event

Errors thrown within the session event handler will be handled by destroying the session (allowing a proper connection close to be sent to the client peer). They will not crash the parent QuicSocket by default. Instead, a `'sessionError'` event will be emitted, allowing the error to be logged or handled. PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs · Jul 9, 2020 · edc71ef008cb8ac68b86da3a3df8d826aae6a249 · edc71ef
1 parent bcde849
commit edc71ef008cb8ac68b86da3a3df8d826aae6a249
Showing with 190 additions and 2 deletions.

+51 −1 doc/api/quic.md

+22 −1 lib/internal/quic/core.js

+1 −0 test/parallel/test-quic-client-server.js

+58 −0 test/parallel/test-quic-server-session-event-error-async.js

+58 −0 test/parallel/test-quic-server-session-event-error.js
diff --git a/doc/api/quic.md b/doc/api/quic.md
@@ -1385,10 +1385,60 @@ The `'ready'` event will not be emitted multiple times.
 added: REPLACEME
 -->
 
-Emitted when a new `QuicServerSession` has been created.
+Emitted when a new `QuicServerSession` has been created. The callback is
+invoked with a single argument providing the newly created `QuicServerSession`
+object.
+
+```js
+const { createQuicSocket } = require('net');
+
+const options = getOptionsSomehow();
+const server = createQuicSocket({ server: options });
+server.listen();
+
+server.on('session', (session) => {
+  // Attach session event listeners.
+});
+```
 
 The `'session'` event will be emitted multiple times.
 
+The `'session'` event handler *may* be an async function.
+
+If the `'session'` event handler throws an error, or if it returns a `Promise`
+that is rejected, the error will be handled by destroying the `QuicServerSession`
+automatically and emitting a `'sessionError'` event on the `QuicSocket`.
+
+#### Event: `'sessionError'`
+<!--YAML
+added: REPLACEME
+-->
+
+Emitted when an error occurs processing an event related to a specific
+`QuicSession` instance. The callback is invoked with two arguments:
+
+* `error` {Error} The error that was either thrown or rejected.
+* `session` {QuicSession} The `QuicSession` instance that was destroyed.
+
+The `QuicSession` instance will have been destroyed by the time the
+`'sessionError'` event is emitted.
+
+```js
+const { createQuicSocket } = require('net');
+
+const options = getOptionsSomehow();
+const server = createQuicSocket({ server: options });
+server.listen();
+
+server.on('session', (session) => {
+  throw new Error('boom');
+});
+
+server.on('sessionError', (error, session) => {
+  console.log('error:', error.message);
+});
+```
+
 #### quicsocket.addEndpoint(options)
 <!-- YAML
 added: REPLACEME

diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js
@@ -246,6 +246,8 @@ const kUsePreferredAddress = Symbol('kUsePreferredAddress');
 const kVersionNegotiation = Symbol('kVersionNegotiation');
 const kWriteGeneric = Symbol('kWriteGeneric');
 
+const kRejections = Symbol.for('nodejs.rejection');
+
 const kSocketUnbound = 0;
 const kSocketPending = 1;
 const kSocketBound = 2;
@@ -278,7 +280,11 @@ function onSessionReady(handle) {
       socket,
       handle,
       socket[kGetStreamOptions]());
-  process.nextTick(emit.bind(socket, 'session', session));
+  try {
+    socket.emit('session', session);
+  } catch (error) {
+    socket[kRejections](error, 'session', session);
+  }
 }
 
 // Called when the C++ QuicSession::Close() method has been called.
@@ -937,6 +943,21 @@ class QuicSocket extends EventEmitter {
     });
   }
 
+  [kRejections](err, eventname, ...args) {
+    switch (eventname) {
+      case 'session':
+        const session = args[0];
+        session.destroy(err);
+        process.nextTick(() => {
+          this.emit('sessionError', err, session);
+        });
+        return;
+      default:
+        // Fall through
+    }
+    this.destroy(err);
+  }
+
   // Returns the default QuicStream options for peer-initiated
   // streams. These are passed on to new client and server
   // QuicSession instances when they are created.

diff --git a/test/parallel/test-quic-client-server.js b/test/parallel/test-quic-client-server.js
@@ -58,6 +58,7 @@ server.listen({
   rejectUnauthorized: false,
   alpn: kALPN,
 });
+
 server.on('session', common.mustCall((session) => {
   debug('QuicServerSession Created');
 

diff --git a/test/parallel/test-quic-server-session-event-error-async.js b/test/parallel/test-quic-server-session-event-error-async.js
@@ -0,0 +1,58 @@
+// Flags: --expose-internals --no-warnings
+'use strict';
+
+const common = require('../common');
+if (!common.hasQuic)
+  common.skip('missing quic');
+
+const { internalBinding } = require('internal/test/binding');
+const {
+  constants: {
+    NGTCP2_CONNECTION_REFUSED
+  }
+} = internalBinding('quic');
+
+const assert = require('assert');
+const {
+  key,
+  cert,
+  ca,
+} = require('../common/quic');
+
+const { createQuicSocket } = require('net');
+
+const options = { key, cert, ca, alpn: 'zzz' };
+
+const server = createQuicSocket({ server: options });
+
+server.on('session', common.mustCall(async (session) => {
+  session.on('close', common.mustCall());
+  session.on('error', common.mustCall((err) => {
+    assert.strictEqual(err.message, 'boom');
+  }));
+  // Throwing inside the session event handler should cause
+  // the session to be destroyed immediately. This should
+  // cause the client side to be closed also.
+  throw new Error('boom');
+}));
+
+server.on('sessionError', common.mustCall((err, session) => {
+  assert.strictEqual(err.message, 'boom');
+  assert(session.destroyed);
+}));
+
+server.listen();
+
+server.once('listening', common.mustCall(() => {
+  const client = createQuicSocket({ client: options });
+  const req = client.connect({
+    address: 'localhost',
+    port: server.endpoints[0].address.port
+  });
+  req.on('close', common.mustCall(() => {
+    assert.strictEqual(req.closeCode.code, NGTCP2_CONNECTION_REFUSED);
+    assert.strictEqual(req.closeCode.silent, true);
+    server.close();
+    client.close();
+  }));
+}));
diff --git a/test/parallel/test-quic-server-session-event-error.js b/test/parallel/test-quic-server-session-event-error.js
@@ -0,0 +1,58 @@
+// Flags: --expose-internals --no-warnings
+'use strict';
+
+const common = require('../common');
+if (!common.hasQuic)
+  common.skip('missing quic');
+
+const { internalBinding } = require('internal/test/binding');
+const {
+  constants: {
+    NGTCP2_CONNECTION_REFUSED
+  }
+} = internalBinding('quic');
+
+const assert = require('assert');
+const {
+  key,
+  cert,
+  ca,
+} = require('../common/quic');
+
+const { createQuicSocket } = require('net');
+
+const options = { key, cert, ca, alpn: 'zzz' };
+
+const server = createQuicSocket({ server: options });
+
+server.on('session', common.mustCall((session) => {
+  session.on('close', common.mustCall());
+  session.on('error', common.mustCall((err) => {
+    assert.strictEqual(err.message, 'boom');
+  }));
+  // Throwing inside the session event handler should cause
+  // the session to be destroyed immediately. This should
+  // cause the client side to be closed also.
+  throw new Error('boom');
+}));
+
+server.on('sessionError', common.mustCall((err, session) => {
+  assert.strictEqual(err.message, 'boom');
+  assert(session.destroyed);
+}));
+
+server.listen();
+
+server.once('listening', common.mustCall(() => {
+  const client = createQuicSocket({ client: options });
+  const req = client.connect({
+    address: 'localhost',
+    port: server.endpoints[0].address.port
+  });
+  req.on('close', common.mustCall(() => {
+    assert.strictEqual(req.closeCode.code, NGTCP2_CONNECTION_REFUSED);
+    assert.strictEqual(req.closeCode.silent, true);
+    server.close();
+    client.close();
+  }));
+}));