Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 28ed7d0

Browse filesBrowse 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 <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
1 parent 429113e commit 28ed7d0
Copy full SHA for 28ed7d0

File tree

Expand file treeCollapse file tree

2 files changed

+5
-12
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+5
-12
lines changed
Open diff view settings
Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+2-9Lines changed: 2 additions & 9 deletions
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

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http2-client-jsstream-destroy.js
+3-3Lines changed: 3 additions & 3 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.