From 3680dc354f5119d103bfcbdcd6f71279cf412155 Mon Sep 17 00:00:00 2001 From: Antony Rizzitelli Date: Tue, 10 Nov 2026 21:16:36 +0000 Subject: [PATCH] perf(http-result): send buffered responses with Content-Length --- .../verify-interface/scenarios-prelude.ts | 2 +- .../verify-interface/scenarios-second.ts | 2 +- docs/3.http-handling.md | 2 + src/http-result.ts | 34 ++++-- src/tests/http-result.test.ts | 102 +++++++++++++++++- 5 files changed, 133 insertions(+), 9 deletions(-) diff --git a/.agents/skills/verify-interface/scenarios-prelude.ts b/.agents/skills/verify-interface/scenarios-prelude.ts index 95bbe4d..11d16dc 100644 --- a/.agents/skills/verify-interface/scenarios-prelude.ts +++ b/.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"; diff --git a/.agents/skills/verify-interface/scenarios-second.ts b/.agents/skills/verify-interface/scenarios-second.ts index 18fc6ef..2feb639 100644 --- a/.agents/skills/verify-interface/scenarios-second.ts +++ b/.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", diff --git a/docs/3.http-handling.md b/docs/3.http-handling.md index 2f7e5f8..92edcc5 100644 --- a/docs/3.http-handling.md +++ b/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"; diff --git a/src/http-result.ts b/src/http-result.ts index dfb1750..002b7d5 100644 --- a/src/http-result.ts +++ b/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 = 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())); } } diff --git a/src/tests/http-result.test.ts b/src/tests/http-result.test.ts index 420e3b4..0b53e31 100644 --- a/src/tests/http-result.test.ts +++ b/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 { + const server = createServer((_request, response) => { + prepare(response); + result.sendResponse(response); + }); + await new Promise((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"); + }); +});