Skip to content

Commit 28ed7d0

Browse files
mmomtchevdanielleadams
authored andcommitted
http2: centralise socket event binding in Http2Session
Move the socket event binding to the HTTP2Session constructor so that an error event could be delivered should the constructor fail Ref: #35772 PR-URL: #35772 Fixes: #35695 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ricky Zhou <[email protected]>
1 parent 429113e commit 28ed7d0

File tree

2 files changed

+5
-12
lines changed

2 files changed

+5
-12
lines changed

lib/internal/http2/core.js

+2-9
Original file line numberDiff line numberDiff line change
@@ -1129,6 +1129,8 @@ class Http2Session extends EventEmitter {
11291129
if (!socket._handle || !socket._handle.isStreamBase) {
11301130
socket = new JSStreamSocket(socket);
11311131
}
1132+
socket.on('error', socketOnError);
1133+
socket.on('close', socketOnClose);
11321134

11331135
// No validation is performed on the input parameters because this
11341136
// constructor is not exported directly for users.
@@ -2909,9 +2911,6 @@ function connectionListener(socket) {
29092911
return;
29102912
}
29112913

2912-
socket.on('error', socketOnError);
2913-
socket.on('close', socketOnClose);
2914-
29152914
// Set up the Session
29162915
const session = new ServerHttp2Session(options, socket, this);
29172916

@@ -3134,12 +3133,6 @@ function connect(authority, options, listener) {
31343133

31353134
const session = new ClientHttp2Session(options, socket);
31363135

3137-
// ClientHttp2Session may create a new socket object
3138-
// when socket is a JSSocket (socket != kSocket)
3139-
// https://github.com/nodejs/node/issues/35695
3140-
session[kSocket].on('error', socketOnError);
3141-
session[kSocket].on('close', socketOnClose);
3142-
31433136
session[kAuthority] = `${options.servername || host}:${port}`;
31443137
session[kProtocol] = protocol;
31453138

test/parallel/test-http2-client-jsstream-destroy.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ server.listen(0, common.mustCall(function() {
4646
});
4747
const req = client.request();
4848

49-
setTimeout(() => socket.destroy(), 200);
50-
setTimeout(() => client.close(), 400);
51-
setTimeout(() => server.close(), 600);
49+
setTimeout(() => socket.destroy(), common.platformTimeout(100));
50+
setTimeout(() => client.close(), common.platformTimeout(200));
51+
setTimeout(() => server.close(), common.platformTimeout(300));
5252

5353
req.on('close', common.mustCall(() => { }));
5454
});

0 commit comments

Comments
 (0)