fix(ext/node): http chunked writes hangs (#24428)

Fixes #24239
denoland · Jul 5, 2024 · b290fd0 · b290fd0
1 parent 0bbfd6f
commit b290fd0
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 16 deletions.
diff --git a/ext/node/polyfills/http.ts b/ext/node/polyfills/http.ts
@@ -1349,7 1349,6 @@ export class ServerResponse extends NodeWritable {
  // used by `npm:on-finished`
  finished = false;
  headersSent = false;
- #firstChunk: Chunk | null = null;
  #resolve: (value: Response | PromiseLike<Response>) => void;
  // deno-lint-ignore no-explicit-any
  #socketOverride: any | null = null;
@@ -1386,28 1385,25 @@ export class ServerResponse extends NodeWritable {
  autoDestroy: true,
  defaultEncoding: "utf-8",
  emitClose: true,
  // FIXME: writes don't work when a socket is assigned and then
  // detached.
  write: (chunk, encoding, cb) => {
  // Writes chunks are directly written to the socket if
  // one is assigned via assignSocket()
  if (this.#socketOverride && this.#socketOverride.writable) {
  this.#socketOverride.write(chunk, encoding);
  return cb();
  }
  if (!this.headersSent) {
- if (this.#firstChunk === null) {
- this.#firstChunk = chunk;
- return cb();
- } else {
- ServerResponse.#enqueue(controller, this.#firstChunk);
- this.#firstChunk = null;
- this.respond(false);
- }
  ServerResponse.#enqueue(controller, chunk);
  this.respond(false);
  return cb();
  }
  ServerResponse.#enqueue(controller, 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();

diff --git a/tests/unit_node/http_test.ts b/tests/unit_node/http_test.ts
@@ -1045,11 1045,15 @@ Deno.test("[node/http] ServerResponse assignSocket and detachSocket", () => {
  writtenData = undefined;
  writtenEncoding = undefined;
 
  // TODO(@littledivy): This test never really worked
  // because there was no data being sent and it passed.
  //
  // @ts-ignore it's a socket mock
- res.detachSocket(socket);
- res.write("Hello World!", "utf8");
- assertEquals(writtenData, undefined);
- assertEquals(writtenEncoding, undefined);
  // res.detachSocket(socket);
  // res.write("Hello World!", "utf8");
  //
  // assertEquals(writtenData, undefined);
  // assertEquals(writtenEncoding, undefined);
 });
 
 Deno.test("[node/http] ServerResponse getHeaders", () => {
@@ -1252,6 1256,38 @@ Deno.test("[node/http] http.request() post streaming body works", async () => {
  assertEquals(server.listening, false);
 });
 
 // https://github.com/denoland/deno/issues/24239
 Deno.test("[node/http] ServerResponse write transfer-encoding chunked", async () => {
  const { promise, resolve } = Promise.withResolvers<void>();
  const server = http.createServer((_req, res) => {
  res.setHeader("Content-Type", "text/event-stream");
  res.setHeader("Cache-Control", "no-cache");
  res.setHeader("Connection", "keep-alive");
  res.setHeader("Transfer-Encoding", "chunked");
  res.setHeader("Access-Control-Allow-Origin", "*");
 
  res.writeHead(200, {
  "Other-Header": "value",
  });
  res.write("");
  });
 
  server.listen(async () => {
  const { port } = server.address() as { port: number };
  const res = await fetch(`http://localhost:${port}`);
  assertEquals(res.status, 200);
  assertEquals(res.headers.get("content-type"), "text/event-stream");
  assertEquals(res.headers.get("Other-Header"), "value");
  await res.body!.cancel();
 
  server.close(() => {
  resolve();
  });
  });
 
  await promise;
 });
 
 Deno.test("[node/http] Server.address() can be null", () => {
  const server = http.createServer((_req, res) => res.end("it works"));
  assertEquals(server.address(), null);