fix(node/http): do not buffer first chunk (denoland#2989)

bartlomieju · Dec 8, 2022 · eca4dbf · eca4dbf
1 parent 40fdfde
commit eca4dbf
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 22 deletions.
diff --git a/node/http.ts b/node/http.ts
@@ -285,7 +285,6 @@ export class ServerResponse extends NodeWritable {
   // used by `npm:on-finished`
   finished = false;
   headersSent = false;
-  #firstChunk: Chunk | null = null;
   // Used if --unstable flag IS NOT present
   #reqEvent?: Deno.RequestEvent;
   // Used if --unstable flag IS present
@@ -307,23 +306,14 @@ export class ServerResponse extends NodeWritable {
       defaultEncoding: "utf-8",
       emitClose: true,
       write: (chunk, _encoding, cb) => {
+        controller.enqueue(chunkToU8(chunk));
         if (!this.headersSent) {
-          if (this.#firstChunk === null) {
-            this.#firstChunk = chunk;
-            return cb();
-          } else {
-            controller.enqueue(chunkToU8(this.#firstChunk));
-            this.#firstChunk = null;
-            this.respond(false);
-          }
+          this.respond(false);
         }
-        controller.enqueue(chunkToU8(chunk));
         return cb();
       },
       final: (cb) => {
-        if (this.#firstChunk) {
-          this.respond(true, this.#firstChunk);
-        } else if (!this.headersSent) {
+        if (!this.headersSent) {
           this.respond(true);
         }
         controller.close();
@@ -368,23 +358,16 @@ export class ServerResponse extends NodeWritable {
     return this;
   }
 
-  #ensureHeaders(singleChunk?: Chunk) {
+  #ensureHeaders() {
     if (this.statusCode === undefined) {
       this.statusCode = 200;
       this.statusMessage = "OK";
     }
-    // Only taken if --unstable IS NOT present
-    if (
-      !this.#isFlashRequest && typeof singleChunk === "string" &&
-      !this.hasHeader("content-type")
-    ) {
-      this.setHeader("content-type", "text/plain;charset=UTF-8");
-    }
   }
 
   respond(final: boolean, singleChunk?: Chunk) {
     this.headersSent = true;
-    this.#ensureHeaders(singleChunk);
+    this.#ensureHeaders();
     const body = singleChunk ?? (final ? null : this.#readable);
     if (this.#isFlashRequest) {
       this.#resolve!(

diff --git a/node/http_test.ts b/node/http_test.ts
@@ -5,6 +5,7 @@ import http, { type RequestOptions } from "./http.ts";
 import { ERR_SERVER_NOT_RUNNING } from "./internal/errors.ts";
 import { assert, assertEquals } from "../testing/asserts.ts";
 import { deferred } from "../async/deferred.ts";
+import { deadline } from "../async/deadline.ts";
 import { gzip } from "./zlib.ts";
 import { Buffer } from "./buffer.ts";
 import { serve } from "../http/server.ts";
@@ -210,6 +211,34 @@ Deno.test("[node/http] non-string buffer response", async () => {
   await promise;
 });
 
+Deno.test("[node/http] server response - first chunk is not buffered", async () => {
+  const promise = deferred<void>();
+  const server = http.createServer((_, res) => {
+    res.write("A");
+  });
+  server.listen(async () => {
+    try {
+      const res = await deadline(
+        fetch(`http://localhost:${server.address().port}`),
+        500,
+      );
+      const reader = res.body?.getReader();
+      const dataA = await deadline(reader?.read()!, 500);
+      // Can read the first chunk even if the response not finished
+      assertEquals(new TextDecoder().decode(dataA!.value), "A");
+      reader?.cancel();
+    } catch (e) {
+      server.emit("error", e);
+    } finally {
+      server.close();
+    }
+  });
+  server.on("close", () => {
+    promise.resolve();
+  });
+  await promise;
+});
+
 Deno.test("[node/http] http.IncomingMessage can be created without url", () => {
   const message = new http.IncomingMessage(
     // adapted from https://github.com/dougmoscrop/serverless-http/blob/80bfb3e940057d694874a8b0bc12ad96d2abe7ab/lib/request.js#L7