http2: don't throw when destroying socket proxy · nodejs/node@b075b33 · GitHub
Skip to content

Commit b075b33

Browse files
mcollinaaduh95
mcollina
authored and
aduh95
committed
http2: don't throw when destroying socket proxy
Calling destroy() on the http2session.socket proxy now destroys the session instead of throwing ERR_HTTP2_NO_SOCKET_MANIPULATION. Refs: nodejs/undici#5525 Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #64427 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 329a288 commit b075b33

4 files changed

Lines changed: 69 additions & 6 deletions

File tree

‎doc/api/http2.md‎

Lines changed: 8 additions & 2 deletions

‎lib/internal/http2/core.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -931,8 +931,8 @@ const proxySocketHandler = {
931931
case 'setTimeout':
932932
case 'ref':
933933
case 'unref':
934-
return session[prop].bind(session);
935934
case 'destroy':
935+
return session[prop].bind(session);
936936
case 'emit':
937937
case 'end':
938938
case 'pause':
@@ -965,9 +965,9 @@ const proxySocketHandler = {
965965
case 'setTimeout':
966966
case 'ref':
967967
case 'unref':
968+
case 'destroy':
968969
session[prop] = value;
969970
return true;
970-
case 'destroy':
971971
case 'emit':
972972
case 'end':
973973
case 'pause':

‎test/parallel/test-http2-socket-proxy-destroy.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const h2 = require('http2');
8+
9+
// Calling .destroy() on the socket proxy must not throw
10+
// (previously threw ERR_HTTP2_NO_SOCKET_MANIPULATION) and must
11+
// destroy the Http2Session and the underlying socket.
12+
13+
const server = h2.createServer();
14+
15+
server.on('stream', common.mustCall(function(stream) {
16+
stream.respond();
17+
stream.end('ok');
18+
}, 2));
19+
20+
server.listen(0, common.mustCall(() => {
21+
const port = server.address().port;
22+
23+
// Test destroy() without an error.
24+
{
25+
const client = h2.connect(`http://localhost:${port}`);
26+
const request = client.request();
27+
request.resume();
28+
request.on('close', common.mustCall(() => {
29+
const socket = client.socket;
30+
socket.destroy();
31+
assert.strictEqual(client.destroyed, true);
32+
client.on('close', common.mustCall(() => {
33+
assert.strictEqual(client.socket, undefined);
34+
// Destroying again through a stale proxy reference must not throw.
35+
socket.destroy();
36+
}));
37+
}));
38+
client.on('error', common.mustNotCall());
39+
}
40+
41+
// Test destroy() with an error.
42+
{
43+
const client = h2.connect(`http://localhost:${port}`);
44+
const request = client.request();
45+
request.resume();
46+
request.on('close', common.mustCall(() => {
47+
client.socket.destroy(new Error('boom'));
48+
assert.strictEqual(client.destroyed, true);
49+
}));
50+
client.on('error', common.mustCall((err) => {
51+
assert.strictEqual(err.message, 'boom');
52+
}));
53+
client.on('close', common.mustCall(() => {
54+
server.close();
55+
}));
56+
}
57+
}));

‎test/parallel/test-http2-socket-proxy.js‎

Lines changed: 2 additions & 2 deletions

0 commit comments

Comments
 (0)