From 44d5a14e79fce5956973bc468c64455a3acf11bb Mon Sep 17 00:00:00 2001
From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>
Date: Sat, 31 Jul 2021 19:09:14 +0200
Subject: [PATCH] stream: do not emit `end` on readable error

---
 lib/internal/streams/readable.js                          | 2 +-
 test/parallel/test-http2-client-destroy.js                | 2 +-
 test/parallel/test-http2-client-onconnect-errors.js       | 2 +-
 test/parallel/test-http2-compat-serverresponse-destroy.js | 4 ++--
 test/parallel/test-http2-empty-frame-without-eof.js       | 4 +++-
 test/parallel/test-http2-max-concurrent-streams.js        | 2 +-
 test/parallel/test-http2-multi-content-length.js          | 2 +-
 test/parallel/test-http2-respond-file-fd-invalid.js       | 3 ++-
 test/parallel/test-http2-respond-nghttperrors.js          | 3 ++-
 test/parallel/test-http2-respond-with-fd-errors.js        | 3 ++-
 .../parallel/test-http2-server-shutdown-before-respond.js | 3 ++-
 test/parallel/test-http2-server-socket-destroy.js         | 8 ++++++--
 12 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js
index e7dd60a6d78c13..c2204443e4951f 100644
--- a/lib/internal/streams/readable.js
+++ b/lib/internal/streams/readable.js
@@ -1333,7 +1333,7 @@ function endReadableNT(state, stream) {
   debug('endReadableNT', state.endEmitted, state.length);
 
   // Check that we didn't get one last unshift.
-  if (!state.errorEmitted && !state.closeEmitted &&
+  if (!state.errored && !state.closeEmitted &&
       !state.endEmitted && state.length === 0) {
     state.endEmitted = true;
     stream.emit('end');
diff --git a/test/parallel/test-http2-client-destroy.js b/test/parallel/test-http2-client-destroy.js
index c407ef1f508466..3dfd46abdd55a4 100644
--- a/test/parallel/test-http2-client-destroy.js
+++ b/test/parallel/test-http2-client-destroy.js
@@ -163,7 +163,7 @@ const { getEventListeners } = require('events');
 
     client.close();
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
     req.on('close', common.mustCall(() => server.close()));
   }));
 }
diff --git a/test/parallel/test-http2-client-onconnect-errors.js b/test/parallel/test-http2-client-onconnect-errors.js
index 04cbfe5befaeec..382f442d2a0cfc 100644
--- a/test/parallel/test-http2-client-onconnect-errors.js
+++ b/test/parallel/test-http2-client-onconnect-errors.js
@@ -102,7 +102,7 @@ function runTest(test) {
     });
   }
 
-  req.on('end', common.mustCall());
+  req.on('end', common.mustNotCall());
   req.on('close', common.mustCall(() => {
     client.destroy();
 
diff --git a/test/parallel/test-http2-compat-serverresponse-destroy.js b/test/parallel/test-http2-compat-serverresponse-destroy.js
index 94d67330e86061..1154b69df41548 100644
--- a/test/parallel/test-http2-compat-serverresponse-destroy.js
+++ b/test/parallel/test-http2-compat-serverresponse-destroy.js
@@ -62,7 +62,7 @@ server.listen(0, common.mustCall(() => {
     req.on('close', common.mustCall(() => countdown.dec()));
 
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
   }
 
   {
@@ -77,6 +77,6 @@ server.listen(0, common.mustCall(() => {
     req.on('close', common.mustCall(() => countdown.dec()));
 
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
   }
 }));
diff --git a/test/parallel/test-http2-empty-frame-without-eof.js b/test/parallel/test-http2-empty-frame-without-eof.js
index fe65f26bb31d49..2cd7d799769882 100644
--- a/test/parallel/test-http2-empty-frame-without-eof.js
+++ b/test/parallel/test-http2-empty-frame-without-eof.js
@@ -34,7 +34,9 @@ async function main() {
       client.on('error', common.expectsError(expected));
     }
     stream.resume();
-    await once(stream, 'end');
+    await new Promise((resolve) => {
+      stream.once('close', resolve);
+    });
     client.close();
   }
   server.close();
diff --git a/test/parallel/test-http2-max-concurrent-streams.js b/test/parallel/test-http2-max-concurrent-streams.js
index a280d04a34f980..73c1285d02e5a6 100644
--- a/test/parallel/test-http2-max-concurrent-streams.js
+++ b/test/parallel/test-http2-max-concurrent-streams.js
@@ -45,7 +45,7 @@ server.listen(0, common.mustCall(() => {
     req.on('aborted', common.mustCall());
     req.on('response', common.mustNotCall());
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
     req.on('close', common.mustCall(() => countdown.dec()));
     req.on('error', common.expectsError({
       code: 'ERR_HTTP2_STREAM_ERROR',
diff --git a/test/parallel/test-http2-multi-content-length.js b/test/parallel/test-http2-multi-content-length.js
index 013dc8935c2a3a..78d58cdcb5cc1c 100644
--- a/test/parallel/test-http2-multi-content-length.js
+++ b/test/parallel/test-http2-multi-content-length.js
@@ -54,7 +54,7 @@ server.listen(0, common.mustCall(() => {
     // header to be set for non-payload bearing requests...
     const req = client.request({ 'content-length': 1 });
     req.resume();
-    req.on('end', common.mustCall());
+    req.on('end', common.mustNotCall());
     req.on('close', common.mustCall(() => countdown.dec()));
     req.on('error', common.expectsError({
       code: 'ERR_HTTP2_STREAM_ERROR',
diff --git a/test/parallel/test-http2-respond-file-fd-invalid.js b/test/parallel/test-http2-respond-file-fd-invalid.js
index 448258ef97b976..0a4fbcf7a696f2 100644
--- a/test/parallel/test-http2-respond-file-fd-invalid.js
+++ b/test/parallel/test-http2-respond-file-fd-invalid.js
@@ -40,7 +40,8 @@ server.listen(0, () => {
   req.on('response', common.mustCall());
   req.on('error', errorCheck);
   req.on('data', common.mustNotCall());
-  req.on('end', common.mustCall(() => {
+  req.on('end', common.mustNotCall());
+  req.on('close', common.mustCall(() => {
     assert.strictEqual(req.rstCode, NGHTTP2_INTERNAL_ERROR);
     client.close();
     server.close();
diff --git a/test/parallel/test-http2-respond-nghttperrors.js b/test/parallel/test-http2-respond-nghttperrors.js
index cce01ada928b9c..36d1ad00bdcff7 100644
--- a/test/parallel/test-http2-respond-nghttperrors.js
+++ b/test/parallel/test-http2-respond-nghttperrors.js
@@ -83,12 +83,13 @@ function runTest(test) {
     name: 'Error',
     message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'
   }));
+  req.on('end', common.mustNotCall());
 
   currentError = test;
   req.resume();
   req.end();
 
-  req.on('end', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     client.close();
 
     if (!tests.length) {
diff --git a/test/parallel/test-http2-respond-with-fd-errors.js b/test/parallel/test-http2-respond-with-fd-errors.js
index 03cda5d7e7d0a4..e7f8d03804fb78 100644
--- a/test/parallel/test-http2-respond-with-fd-errors.js
+++ b/test/parallel/test-http2-respond-with-fd-errors.js
@@ -91,12 +91,13 @@ function runTest(test) {
     name: 'Error',
     message: 'Stream closed with error code NGHTTP2_INTERNAL_ERROR'
   }));
+  req.on('end', common.mustNotCall());
 
   currentError = test;
   req.resume();
   req.end();
 
-  req.on('end', common.mustCall(() => {
+  req.on('close', common.mustCall(() => {
     client.close();
 
     if (!tests.length) {
diff --git a/test/parallel/test-http2-server-shutdown-before-respond.js b/test/parallel/test-http2-server-shutdown-before-respond.js
index 65dd853f8392c2..b168716de42717 100644
--- a/test/parallel/test-http2-server-shutdown-before-respond.js
+++ b/test/parallel/test-http2-server-shutdown-before-respond.js
@@ -32,5 +32,6 @@ server.on('listening', common.mustCall(() => {
   }));
   req.resume();
   req.on('data', common.mustNotCall());
-  req.on('end', common.mustCall(() => server.close()));
+  req.on('end', common.mustNotCall());
+  req.on('close', common.mustCall(() => server.close()));
 }));
diff --git a/test/parallel/test-http2-server-socket-destroy.js b/test/parallel/test-http2-server-socket-destroy.js
index e7fbc2ceb7d1b3..ffb7ea48dc3343 100644
--- a/test/parallel/test-http2-server-socket-destroy.js
+++ b/test/parallel/test-http2-server-socket-destroy.js
@@ -8,6 +8,7 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const h2 = require('http2');
 const { kSocket } = require('internal/http2/util');
+const { once } = require('events');
 
 const server = h2.createServer();
 
@@ -36,7 +37,7 @@ function onStream(stream) {
 
 server.listen(0);
 
-server.on('listening', common.mustCall(() => {
+server.on('listening', common.mustCall(async () => {
   const client = h2.connect(`http://localhost:${server.address().port}`);
   // The client may have an ECONNRESET error here depending on the operating
   // system, due mainly to differences in the timing of socket closing. Do
@@ -58,5 +59,8 @@ server.on('listening', common.mustCall(() => {
 
   req.on('aborted', common.mustCall());
   req.resume();
-  req.on('end', common.mustCall());
+
+  try {
+    await once(req, 'end');
+  } catch {}
 }));