Skip to content

Commit edc71ef

Browse files
committed
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 <[email protected]>
1 parent bcde849 commit edc71ef

File tree

5 files changed

+190
-2
lines changed

5 files changed

+190
-2
lines changed

doc/api/quic.md

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1385,10 +1385,60 @@ The `'ready'` event will not be emitted multiple times.
13851385
added: REPLACEME
13861386
-->
13871387

1388-
Emitted when a new `QuicServerSession` has been created.
1388+
Emitted when a new `QuicServerSession` has been created. The callback is
1389+
invoked with a single argument providing the newly created `QuicServerSession`
1390+
object.
1391+
1392+
```js
1393+
const { createQuicSocket } = require('net');
1394+
1395+
const options = getOptionsSomehow();
1396+
const server = createQuicSocket({ server: options });
1397+
server.listen();
1398+
1399+
server.on('session', (session) => {
1400+
// Attach session event listeners.
1401+
});
1402+
```
13891403

13901404
The `'session'` event will be emitted multiple times.
13911405

1406+
The `'session'` event handler *may* be an async function.
1407+
1408+
If the `'session'` event handler throws an error, or if it returns a `Promise`
1409+
that is rejected, the error will be handled by destroying the `QuicServerSession`
1410+
automatically and emitting a `'sessionError'` event on the `QuicSocket`.
1411+
1412+
#### Event: `'sessionError'`
1413+
<!--YAML
1414+
added: REPLACEME
1415+
-->
1416+
1417+
Emitted when an error occurs processing an event related to a specific
1418+
`QuicSession` instance. The callback is invoked with two arguments:
1419+
1420+
* `error` {Error} The error that was either thrown or rejected.
1421+
* `session` {QuicSession} The `QuicSession` instance that was destroyed.
1422+
1423+
The `QuicSession` instance will have been destroyed by the time the
1424+
`'sessionError'` event is emitted.
1425+
1426+
```js
1427+
const { createQuicSocket } = require('net');
1428+
1429+
const options = getOptionsSomehow();
1430+
const server = createQuicSocket({ server: options });
1431+
server.listen();
1432+
1433+
server.on('session', (session) => {
1434+
throw new Error('boom');
1435+
});
1436+
1437+
server.on('sessionError', (error, session) => {
1438+
console.log('error:', error.message);
1439+
});
1440+
```
1441+
13921442
#### quicsocket.addEndpoint(options)
13931443
<!-- YAML
13941444
added: REPLACEME

lib/internal/quic/core.js

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,8 @@ const kUsePreferredAddress = Symbol('kUsePreferredAddress');
246246
const kVersionNegotiation = Symbol('kVersionNegotiation');
247247
const kWriteGeneric = Symbol('kWriteGeneric');
248248

249+
const kRejections = Symbol.for('nodejs.rejection');
250+
249251
const kSocketUnbound = 0;
250252
const kSocketPending = 1;
251253
const kSocketBound = 2;
@@ -278,7 +280,11 @@ function onSessionReady(handle) {
278280
socket,
279281
handle,
280282
socket[kGetStreamOptions]());
281-
process.nextTick(emit.bind(socket, 'session', session));
283+
try {
284+
socket.emit('session', session);
285+
} catch (error) {
286+
socket[kRejections](error, 'session', session);
287+
}
282288
}
283289

284290
// Called when the C++ QuicSession::Close() method has been called.
@@ -937,6 +943,21 @@ class QuicSocket extends EventEmitter {
937943
});
938944
}
939945

946+
[kRejections](err, eventname, ...args) {
947+
switch (eventname) {
948+
case 'session':
949+
const session = args[0];
950+
session.destroy(err);
951+
process.nextTick(() => {
952+
this.emit('sessionError', err, session);
953+
});
954+
return;
955+
default:
956+
// Fall through
957+
}
958+
this.destroy(err);
959+
}
960+
940961
// Returns the default QuicStream options for peer-initiated
941962
// streams. These are passed on to new client and server
942963
// QuicSession instances when they are created.

test/parallel/test-quic-client-server.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ server.listen({
5858
rejectUnauthorized: false,
5959
alpn: kALPN,
6060
});
61+
6162
server.on('session', common.mustCall((session) => {
6263
debug('QuicServerSession Created');
6364

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Flags: --expose-internals --no-warnings
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasQuic)
6+
common.skip('missing quic');
7+
8+
const { internalBinding } = require('internal/test/binding');
9+
const {
10+
constants: {
11+
NGTCP2_CONNECTION_REFUSED
12+
}
13+
} = internalBinding('quic');
14+
15+
const assert = require('assert');
16+
const {
17+
key,
18+
cert,
19+
ca,
20+
} = require('../common/quic');
21+
22+
const { createQuicSocket } = require('net');
23+
24+
const options = { key, cert, ca, alpn: 'zzz' };
25+
26+
const server = createQuicSocket({ server: options });
27+
28+
server.on('session', common.mustCall(async (session) => {
29+
session.on('close', common.mustCall());
30+
session.on('error', common.mustCall((err) => {
31+
assert.strictEqual(err.message, 'boom');
32+
}));
33+
// Throwing inside the session event handler should cause
34+
// the session to be destroyed immediately. This should
35+
// cause the client side to be closed also.
36+
throw new Error('boom');
37+
}));
38+
39+
server.on('sessionError', common.mustCall((err, session) => {
40+
assert.strictEqual(err.message, 'boom');
41+
assert(session.destroyed);
42+
}));
43+
44+
server.listen();
45+
46+
server.once('listening', common.mustCall(() => {
47+
const client = createQuicSocket({ client: options });
48+
const req = client.connect({
49+
address: 'localhost',
50+
port: server.endpoints[0].address.port
51+
});
52+
req.on('close', common.mustCall(() => {
53+
assert.strictEqual(req.closeCode.code, NGTCP2_CONNECTION_REFUSED);
54+
assert.strictEqual(req.closeCode.silent, true);
55+
server.close();
56+
client.close();
57+
}));
58+
}));
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Flags: --expose-internals --no-warnings
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasQuic)
6+
common.skip('missing quic');
7+
8+
const { internalBinding } = require('internal/test/binding');
9+
const {
10+
constants: {
11+
NGTCP2_CONNECTION_REFUSED
12+
}
13+
} = internalBinding('quic');
14+
15+
const assert = require('assert');
16+
const {
17+
key,
18+
cert,
19+
ca,
20+
} = require('../common/quic');
21+
22+
const { createQuicSocket } = require('net');
23+
24+
const options = { key, cert, ca, alpn: 'zzz' };
25+
26+
const server = createQuicSocket({ server: options });
27+
28+
server.on('session', common.mustCall((session) => {
29+
session.on('close', common.mustCall());
30+
session.on('error', common.mustCall((err) => {
31+
assert.strictEqual(err.message, 'boom');
32+
}));
33+
// Throwing inside the session event handler should cause
34+
// the session to be destroyed immediately. This should
35+
// cause the client side to be closed also.
36+
throw new Error('boom');
37+
}));
38+
39+
server.on('sessionError', common.mustCall((err, session) => {
40+
assert.strictEqual(err.message, 'boom');
41+
assert(session.destroyed);
42+
}));
43+
44+
server.listen();
45+
46+
server.once('listening', common.mustCall(() => {
47+
const client = createQuicSocket({ client: options });
48+
const req = client.connect({
49+
address: 'localhost',
50+
port: server.endpoints[0].address.port
51+
});
52+
req.on('close', common.mustCall(() => {
53+
assert.strictEqual(req.closeCode.code, NGTCP2_CONNECTION_REFUSED);
54+
assert.strictEqual(req.closeCode.silent, true);
55+
server.close();
56+
client.close();
57+
}));
58+
}));

0 commit comments

Comments
 (0)