http2: compat ERR_STREAM_ALREADY_FINISHED

Make http/2 compat end() match Writable and http/1.
nodejs · Aug 22, 2019 · 0605719 · 0605719
1 parent 605d7c4
commit 0605719
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 12 deletions.
diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
@@ -31,7 +31,8 @@ const {
     ERR_HTTP2_STATUS_INVALID,
     ERR_INVALID_ARG_VALUE,
     ERR_INVALID_CALLBACK,
-    ERR_INVALID_HTTP_TOKEN
+    ERR_INVALID_HTTP_TOKEN,
+    ERR_STREAM_ALREADY_FINISHED
   },
   hideStackFrames
 } = require('internal/errors');
@@ -652,11 +653,6 @@ class Http2ServerResponse extends Stream {
     const stream = this[kStream];
     const state = this[kState];
 
-    if ((state.closed || state.ending) &&
-        state.headRequest === stream.headRequest) {
-      return this;
-    }
-
     if (typeof chunk === 'function') {
       cb = chunk;
       chunk = null;
@@ -665,6 +661,22 @@ class Http2ServerResponse extends Stream {
       encoding = 'utf8';
     }
 
+    if (this.writableEnded) {
+      if (typeof cb === 'function') {
+        if (!this.finished) {
+          this.on('finish', cb);
+        } else {
+          cb(new ERR_STREAM_ALREADY_FINISHED('end'));
+        }
+      }
+      return this;
+    }
+
+    if ((state.closed || state.ending) &&
+        state.headRequest === stream.headRequest) {
+      return this;
+    }
+
     if (chunk !== null && chunk !== undefined)
       this.write(chunk, encoding);
 

diff --git a/test/parallel/test-http2-compat-serverresponse-end.js b/test/parallel/test-http2-compat-serverresponse-end.js
@@ -3,6 +3,7 @@
 const {
   mustCall,
   mustNotCall,
+  expectsError,
   hasCrypto,
   platformTimeout,
   skip
@@ -25,16 +26,22 @@ const {
   // but callback will only be called once
   const server = createServer(mustCall((request, response) => {
     response.end('end', 'utf8', mustCall(() => {
-      response.end(mustNotCall());
+      response.end(mustCall());
       process.nextTick(() => {
-        response.end(mustNotCall());
+        response.end(expectsError({
+          code: 'ERR_STREAM_ALREADY_FINISHED'
+        }));
         server.close();
       });
     }));
     response.on('finish', mustCall(() => {
-      response.end(mustNotCall());
+      response.end(expectsError({
+        code: 'ERR_STREAM_ALREADY_FINISHED'
+      }));
+    }));
+    response.end(expectsError({
+      code: 'ERR_STREAM_ALREADY_FINISHED'
     }));
-    response.end(mustNotCall());
   }));
   server.listen(0, mustCall(() => {
     let data = '';
@@ -292,7 +299,10 @@ const {
     }));
     response.end('data', mustCall(() => {
       strictEqual(finished, false);
-      response.end('data', mustNotCall());
+      strictEqual(response.end('data', expectsError({
+        code: 'ERR_STREAM_ALREADY_FINISHED'
+      })), response);
+      strictEqual(response.end(), response);
     }));
   }));
   server.listen(0, mustCall(() => {
@@ -326,7 +336,9 @@ const {
   // Should be able to respond to HEAD with just .end
   const server = createServer(mustCall((request, response) => {
     response.end('data', mustCall());
-    response.end(mustNotCall());
+    strictEqual(response.end('data', expectsError({
+      code: 'ERR_STREAM_ALREADY_FINISHED'
+    })), response);
   }));
   server.listen(0, mustCall(() => {
     const { port } = server.address();