Skip to content

Commit

Permalink
quic: handle errors thrown / rejections in the session event
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jasnell committed Jul 9, 2020
1 parent bcde849 commit edc71ef
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 2 deletions.
52 changes: 51 additions & 1 deletion doc/api/quic.md
Expand Up @@ -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
Expand Down
23 changes: 22 additions & 1 deletion lib/internal/quic/core.js
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-quic-client-server.js
Expand Up @@ -58,6 +58,7 @@ server.listen({
rejectUnauthorized: false,
alpn: kALPN,
});

server.on('session', common.mustCall((session) => {
debug('QuicServerSession Created');

Expand Down
58 changes: 58 additions & 0 deletions 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();
}));
}));
58 changes: 58 additions & 0 deletions 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();
}));
}));

0 comments on commit edc71ef

Please sign in to comment.