perf(http-result): send buffered responses with Content-Length

.agents/skills/verify-interface/scenarios-prelude.ts

@@ -1,3 +1,3 @@
 /** Generated from the scenario bodies: the probe each journey runs against the packed `dist/index.js`, and the JSON the docs-stated behaviour prints. */
 
-export const PRELUDE = "const a = require(process.argv[2]);\nconst { Readable, PassThrough } = require(\"node:stream\");\nconst mk = (chunks, headers = {}) => { const r = Readable.from(chunks); r.headers = headers; return r; };\nconst ctx = (over = {}) => ({ rawRequest: mk([], { authorization: \"Bearer t\", \"x-multi\": [\"a\", \"b\"], \"x-one\": \"solo\", \"x-csv\": \"a, b\" }), rawResponse: {}, url: new URL(\"http://h/p?page=2&flag&empty=&t=js&t=api&t=&q=first&q=second\"), routeParameters: { id: \"42\" }, response: new a.HTTPResult(), connection: { c: 1 }, ...over });\nconst named = (key) => { const f = function () {}; Object.defineProperty(f, \"name\", { value: key }); return f; };\nconst deco = (d, cls, key) => d(cls.prototype, key, { value: named(key) });\nclass C extends a.Controller(\"/c\") {}\nconst routeOf = (key) => a.getRegisteredRoutes().find((r) => r.callbackName === key);\nlet n = 0;\nconst compute = async (decos, c = ctx()) => { const key = \"h\" + n++; decos.forEach((d) => d(C.prototype, key, 0)); deco(a.Get(key), C, key); return a.computeParameter(c, routeOf(key).parameters[0], new C()); };\nconst settle = (promise, pick) => promise.then((v) => [\"ok\", v], (e) => [\"rejected\", pick(e)]);\nconst status = (e) => [e instanceof a.HTTPResult, e.getStatus(), e.getBody()];\nconst fakeRes = () => ({ head: null, body: null, ended: 0, writeHead(s, h) { this.head = [s, h]; return this; }, end(b) { this.body = b ?? null; this.ended++; return this; }, req: { method: \"GET\", url: \"/x\" } });\n";
+export const PRELUDE = "const a = require(process.argv[2]);\nconst { Readable, PassThrough } = require(\"node:stream\");\nconst mk = (chunks, headers = {}) => { const r = Readable.from(chunks); r.headers = headers; return r; };\nconst ctx = (over = {}) => ({ rawRequest: mk([], { authorization: \"Bearer t\", \"x-multi\": [\"a\", \"b\"], \"x-one\": \"solo\", \"x-csv\": \"a, b\" }), rawResponse: {}, url: new URL(\"http://h/p?page=2&flag&empty=&t=js&t=api&t=&q=first&q=second\"), routeParameters: { id: \"42\" }, response: new a.HTTPResult(), connection: { c: 1 }, ...over });\nconst named = (key) => { const f = function () {}; Object.defineProperty(f, \"name\", { value: key }); return f; };\nconst deco = (d, cls, key) => d(cls.prototype, key, { value: named(key) });\nclass C extends a.Controller(\"/c\") {}\nconst routeOf = (key) => a.getRegisteredRoutes().find((r) => r.callbackName === key);\nlet n = 0;\nconst compute = async (decos, c = ctx()) => { const key = \"h\" + n++; decos.forEach((d) => d(C.prototype, key, 0)); deco(a.Get(key), C, key); return a.computeParameter(c, routeOf(key).parameters[0], new C()); };\nconst settle = (promise, pick) => promise.then((v) => [\"ok\", v], (e) => [\"rejected\", pick(e)]);\nconst status = (e) => [e instanceof a.HTTPResult, e.getStatus(), e.getBody()];\nconst fakeRes = () => ({ head: null, body: null, ended: 0, writeHead(s, h) { this.head = [s, h]; return this; }, end(b) { this.body = b ?? null; this.ended++; return this; }, getHeaderNames() { return []; }, req: { method: \"GET\", url: \"/x\" } });\n";

.agents/skills/verify-interface/scenarios-second.ts

@@ -14,7 +14,7 @@ export const SECOND_SCENARIOS = {
   },
   "result-send": {
     body: "const st = new a.HTTPResult(404, \"body\"); const ps = st.getWriteStream();\nR.stream = [ps.constructor.name, st.isStream(), st.getContentType(), st.getStatus(), st.getBody(), st.getWriteStream() === ps];\nconst st2 = new a.HTTPResult(); st2.getWriteStream(\"text/event-stream\", 206); R.streamArgs = [st2.getContentType(), st2.getStatus()];\nconst r1 = fakeRes(); const sr = new a.HTTPResult(201, { k: 1 }); sr.addHeader(\"X-T\", \"v\"); sr.sendResponse(r1); R.send = [r1.head, r1.body, r1.ended];\nconst r2 = fakeRes(); sr.sendHeadResponse(r2); R.sendHead = [r2.head, r2.body, r2.ended];\nconst sw = new a.HTTPResult(); const pt = sw.getWriteStream(\"text/plain\"); const sink = [];\nconst fake = Object.assign(fakeRes(), { on() { return this; }, once() { return this; }, emit() { return true; }, write(c) { sink.push(String(c)); return true; } });\nsw.sendResponse(fake); pt.end(\"streamed\");\nawait new Promise((resolve) => setTimeout(resolve, 50));\nR.sendStream = [fake.head[0], fake.head[1][\"Content-Type\"], sink.join(\"\"), fake.body];\nconst r4 = fakeRes(); sw.sendResponse(r4, true); R.abort = [r4.body, r4.ended];\n",
-    printed: "{\"stream\":[\"PassThrough\",true,\"text/plain\",200,\"\",true],\"streamArgs\":[\"text/event-stream\",206],\"send\":[[201,{\"X-T\":\"v\",\"Content-Type\":\"application/json\"}],\"{\\\"k\\\":1}\",1],\"sendHead\":[[201,{\"X-T\":\"v\",\"Content-Type\":\"application/json\"}],null,1],\"sendStream\":[200,\"text/plain\",\"streamed\",null],\"abort\":[\"\",1]}",
+    printed: "{\"stream\":[\"PassThrough\",true,\"text/plain\",200,\"\",true],\"streamArgs\":[\"text/event-stream\",206],\"send\":[[201,{\"X-T\":\"v\",\"Content-Type\":\"application/json\",\"Content-Length\":7}],\"{\\\"k\\\":1}\",1],\"sendHead\":[[201,{\"X-T\":\"v\",\"Content-Type\":\"application/json\"}],null,1],\"sendStream\":[200,\"text/plain\",\"streamed\",null],\"abort\":[\"\",1]}",
   },
   "route-errors": {
     body: "class Users extends a.Controller(\"/users\") {}\nclass Plain {}\nconst err = (f) => { try { f(); return \"no error\"; } catch (e) { return [e.name, e.message]; } };\nR.plain = err(() => deco(a.Get(\"z\"), Plain, \"m\"));\nR.novalue = err(() => a.Get(\"z\")(Users.prototype, \"nov\", {}));\nR.symbol = err(() => deco(a.Get(), Users, Symbol(\"s\")));\nR.noController = err(() => a.RegisterRoute({ mode: \"handler\", method: \"get\", location: \"/q\", callback() {}, parameters: [], properties: {} }));\nR.none = a.getRegisteredRoutes().length;\n",

docs/3.http-handling.md

@@ -93,6 +93,8 @@ The object member of `HTTPBody` has a string index signature. A value typed by a
 
 `addHeader(name, value)`, `removeHeader(name)`, `getHeaders()`, and `peekHeaders()` manage the custom response headers.
 
+A response that is not a stream is sent with a `Content-Length` header holding its body's byte length, except for status 204, 304 and 1xx, which carry no body, and when the custom headers, or headers set on `rawResponse`, already set `Content-Length`, `Transfer-Encoding` or `Trailer`. A stream response is sent chunked.
+
 ```typescript
 import { HTTPResult } from "@antelopejs/interface-api";
 

src/http-result.ts

@@ -1,11 +1,17 @@
 import { PassThrough } from "node:stream";
-import type { ServerResponse } from "node:http";
+import type { OutgoingHttpHeaders, ServerResponse } from "node:http";
 import { Logging } from "@antelopejs/interface-core/logging";
 
 import { isString } from "./guards";
 
 const SERVER_ERROR_BODY_LOG_LIMIT = 2048;
 
+const FRAMING_HEADERS: ReadonlySet<string> = new Set([
+  "content-length",
+  "trailer",
+  "transfer-encoding",
+]);
+
 /**
  * A response body: a string, sent as is, or a value `JSON.stringify` writes as JSON.
  *
@@ -227,20 +233,36 @@ export class HTTPResult {
   }
 
   /**
-   * Send response.
+   * Send response. A body that is not a stream is sent with its
+   * `Content-Length`, unless the status carries no body (1xx, 204, 304) or
+   * the headers of this result or of `res` already set `Content-Length`,
+   * `Transfer-Encoding` or `Trailer`.
    *
    * @param res Response object
    */
   public sendResponse(res: ServerResponse, abortStream = false) {
     this.logServerError(res);
-    res.writeHead(this.status, {
+    const headers: OutgoingHttpHeaders = {
       ...this.headers,
       "Content-Type": this.contentType,
-    });
+    };
     if (this.stream && !abortStream) {
+      res.writeHead(this.status, headers);
       this.stream.pipe(res);
-    } else {
-      res.end(this.body);
+      return;
+    }
+    if (this.hasUnframedBody(res)) {
+      headers["Content-Length"] = Buffer.byteLength(this.body);
+    }
+    res.writeHead(this.status, headers).end(this.body);
+  }
+
+  private hasUnframedBody(res: ServerResponse): boolean {
+    const status = this.status | 0;
+    if (status < 200 || status === 204 || status === 304) {
+      return false;
     }
+    const names = [...Object.keys(this.headers ?? {}), ...res.getHeaderNames()];
+    return !names.some((name) => FRAMING_HEADERS.has(name.toLowerCase()));
   }
 }

src/tests/http-result.test.ts

@@ -1,8 +1,9 @@
 import assert from "node:assert";
 import sinon from "sinon";
-import { ServerResponse } from "node:http";
+import { createServer, ServerResponse } from "node:http";
 
 import { HTTPResult as LocalHTTPResult } from "../index";
+import { isString } from "../guards";
 import { createRequest } from "./request-context";
 
 function createCapturedResponse() {
@@ -127,3 +128,102 @@ describe("HTTPResult response contract", () => {
     assert.equal(captured.response.writableEnded, true);
   });
 });
+
+interface Received {
+  status: number;
+  contentLength: string | null;
+  transferEncoding: string | null;
+  body: string;
+}
+
+async function receive(
+  result: LocalHTTPResult,
+  prepare: (response: ServerResponse) => void = () => {},
+): Promise<Received> {
+  const server = createServer((_request, response) => {
+    prepare(response);
+    result.sendResponse(response);
+  });
+  await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
+  try {
+    const address = server.address();
+    assert(address !== null && !isString(address), "Server listens on TCP");
+    const response = await fetch(`http://127.0.0.1:${address.port}/`);
+    return {
+      status: response.status,
+      contentLength: response.headers.get("content-length"),
+      transferEncoding: response.headers.get("transfer-encoding"),
+      body: await response.text(),
+    };
+  } finally {
+    server.closeAllConnections();
+    await new Promise((resolve) => server.close(resolve));
+  }
+}
+
+describe("HTTPResult body framing", () => {
+  it("Frames a buffered body with its byte length", async () => {
+    assert.deepEqual(await receive(new LocalHTTPResult(200, "héllo")), {
+      status: 200,
+      contentLength: "6",
+      transferEncoding: null,
+      body: "héllo",
+    });
+    assert.equal(
+      (await receive(new LocalHTTPResult(201, { ok: true }))).contentLength,
+      "11",
+    );
+    assert.equal((await receive(new LocalHTTPResult())).contentLength, "0");
+  });
+
+  it("Sends no Content-Length with a status that carries no body", async () => {
+    for (const status of [204, 304, 204.5]) {
+      const received = await receive(new LocalHTTPResult(status, null));
+      assert.equal(received.status, Math.trunc(status));
+      assert.equal(received.contentLength, null, String(status));
+    }
+  });
+
+  it("Leaves the framing the result's headers set", async () => {
+    const declared = await receive(
+      LocalHTTPResult.withHeaders("body", { Trailer: "X-Checksum" }),
+    );
+    assert.equal(declared.transferEncoding, "chunked");
+    assert.equal(declared.contentLength, null);
+    assert.equal(declared.body, "body");
+    const chunked = await receive(
+      LocalHTTPResult.withHeaders("body", { "Transfer-Encoding": "chunked" }),
+    );
+    assert.equal(chunked.contentLength, null);
+    const framed = await receive(
+      LocalHTTPResult.withHeaders("body", { "content-length": "4" }),
+    );
+    assert.equal(framed.contentLength, "4");
+  });
+
+  it("Leaves the framing set on the raw response", async () => {
+    for (const [name, value] of [
+      ["Transfer-Encoding", "chunked"],
+      ["Trailer", "X-Checksum"],
+    ]) {
+      const received = await receive(
+        new LocalHTTPResult(200, "body"),
+        (response) => response.setHeader(name, value),
+      );
+      assert.equal(received.transferEncoding, "chunked", name);
+      assert.equal(received.contentLength, null, name);
+      assert.equal(received.body, "body", name);
+    }
+  });
+
+  it("Streams a response without a Content-Length", async () => {
+    const result = new LocalHTTPResult();
+    result.getWriteStream().end("streamed");
+
+    const received = await receive(result);
+
+    assert.equal(received.transferEncoding, "chunked");
+    assert.equal(received.contentLength, null);
+    assert.equal(received.body, "streamed");
+  });
+});