does not matter if TCP FIN or TCP RST

nodejs · Oct 10, 2024 · 7c152da · 7c152da
1 parent 9f96023
commit 7c152da
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 28 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -3020,11 +3020,12 @@ ObjectDefineProperty(Http2Session.prototype, 'setTimeout', setTimeoutValue);
 function socketOnError(error) {
  const session = this[kBoundSession];
  if (session !== undefined) {
- // We can ignore ECONNRESET after GOAWAY was received as there's nothing
- // we can do and the other side is fully within its rights to do so.
- if (error.code === 'ECONNRESET' && session[kState].goawayCode !== null)
- return session.destroy();
- debugSessionObj(this, 'socket error [%s]', error.message);
+ if (error.code === 'ECONNRESET') {
+ // Handled by socketOnClose, it does not matter if the endpoint
+ // sends TCP FIN prematurely or TCP RST.
+ return;
+ }
+ debugSessionObj(session, 'socket error [%s]', error.message);
  session.destroy(error);
  }
 }
@@ -3305,15 +3306,28 @@ function socketOnClose() {
  const err = session.connecting ? new ERR_SOCKET_CLOSED() : null;
  const state = session[kState];
 
+ // Code for premature close
  let code = state.goawayCode ?? state.destroyCode;
- if (code === NGHTTP2_NO_ERROR || code === null) {
- code = session[kType] === NGHTTP2_SESSION_SERVER ?
+ if (code === NGHTTP2_NO_ERROR) {
+ // Got NO_ERROR but still closed prematurely
+ code = null;
+ }
+
+ const defaultCode = session[kType] === NGHTTP2_SESSION_SERVER ?
  NGHTTP2_CANCEL :
  NGHTTP2_INTERNAL_ERROR;
- }
 
- state.streams.forEach((stream) => stream.close(code));
- state.pendingStreams.forEach((stream) => stream.close(code));
+ const closeStream = (stream) => {
+ if (code === null) {
+ const connect = stream[kSentHeaders]?.[HTTP2_HEADER_METHOD] === HTTP2_METHOD_CONNECT;
+ stream.close(connect ? NGHTTP2_CONNECT_ERROR : defaultCode);
+ } else {
+ stream.close(code ?? defaultCode);
+ }
+ };
+
+ state.streams.forEach(closeStream);
+ state.pendingStreams.forEach(closeStream);
  session.close();
  session[kMaybeDestroy](err);
  }

diff --git a/test/parallel/test-http2-client-connection-tunnel-reset.js b/test/parallel/test-http2-client-connection-tunnel-reset.js
@@ -20,18 +20,14 @@ h2Server.on('connect', (req, res) => {
 
 h2Server.listen(0, common.mustCall(() => {
  const proxyClient = h2.connect(`http://localhost:${h2Server.address().port}`);
- proxyClient.once('error', common.expectsError({
- name: 'Error',
- code: 'ECONNRESET',
- message: 'read ECONNRESET'
- }));
+ proxyClient.once('error', common.mustNotCall());
  const proxyReq = proxyClient.request({
  ':method': 'CONNECT',
  ':authority': 'example.com:443'
  });
  proxyReq.once('error', common.expectsError({
  name: 'Error',
- code: 'ECONNRESET',
- message: 'read ECONNRESET'
+ code: 'ERR_HTTP2_STREAM_ERROR',
+ message: 'Stream closed with error code NGHTTP2_CONNECT_ERROR'
  }));
 }));
diff --git a/test/parallel/test-http2-respond-with-file-connection-abort.js b/test/parallel/test-http2-respond-with-file-connection-abort.js
@@ -13,7 +13,11 @@ const {
 
 const server = http2.createServer();
 server.on('stream', common.mustCall((stream) => {
- stream.on('error', (err) => assert.strictEqual(err.code, 'ECONNRESET'));
+ stream.on('error', common.expectsError({
+ code: 'ERR_HTTP2_STREAM_ERROR',
+ name: 'Error',
+ message: 'Stream closed with error code NGHTTP2_CANCEL'
+ }));
  stream.respondWithFile(process.execPath, {
  [HTTP2_HEADER_CONTENT_TYPE]: 'application/octet-stream'
  });

diff --git a/test/parallel/test-http2-server-socket-destroy.js b/test/parallel/test-http2-server-socket-destroy.js
@@ -39,9 +39,6 @@ function onStream(stream) {
  message: 'Stream closed with error code NGHTTP2_CANCEL'
  }));
 
- // Do not use destroy() as it sends FIN on Linux.
- // On macOS, it sends RST.
-
  // Always send RST.
  socket.resetAndDestroy();
 }
@@ -50,18 +47,14 @@ server.listen(0);
 
 server.on('listening', common.mustCall(async () => {
  const client = h2.connect(`http://localhost:${server.address().port}`);
- client.on('error', common.expectsError({
- name: 'Error',
- code: 'ECONNRESET',
- message: 'read ECONNRESET'
- }));
+ client.on('error', common.mustNotCall());
  client.on('close', common.mustCall());
 
  const req = client.request({ ':method': 'POST' });
  req.on('error', common.expectsError({
  name: 'Error',
- code: 'ECONNRESET',
- message: 'read ECONNRESET'
+ code: 'ERR_HTTP2_STREAM_ERROR',
+ message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'
  }));
 
  req.on('aborted', common.mustCall());