fix(infra): avoid req.destroy(err) in request body limiters

This commit is contained in:
Peter Steinberger
2026-02-15 03:19:16 +01:00
parent 4a44da7d91
commit 444a910d9e
2 changed files with 41 additions and 6 deletions

View File

@@ -13,11 +13,26 @@ function createMockRequest(params: {
headers?: Record<string, string>;
emitEnd?: boolean;
}): IncomingMessage {
const req = new EventEmitter() as IncomingMessage & { destroyed?: boolean; destroy: () => void };
const req = new EventEmitter() as IncomingMessage & {
destroyed?: boolean;
destroy: (error?: Error) => void;
__unhandledDestroyError?: unknown;
};
req.destroyed = false;
req.headers = params.headers ?? {};
req.destroy = () => {
req.destroy = (error?: Error) => {
req.destroyed = true;
if (error) {
// Simulate Node's async 'error' emission on destroy(err). If no listener is
// present at that time, EventEmitter throws; capture that as "unhandled".
queueMicrotask(() => {
try {
req.emit("error", error);
} catch (err) {
req.__unhandledDestroyError = err;
}
});
}
};
if (params.chunks) {
@@ -66,6 +81,7 @@ describe("http body limits", () => {
await expect(readRequestBodyWithLimit(req, { maxBytes: 64 })).rejects.toMatchObject({
message: "PayloadTooLarge",
});
expect(req.__unhandledDestroyError).toBeUndefined();
});
it("returns json parse error when body is invalid", async () => {
@@ -104,6 +120,7 @@ describe("http body limits", () => {
expect(guard.code()).toBe("PAYLOAD_TOO_LARGE");
expect(res.statusCode).toBe(413);
expect(res.body).toBe("Payload too large");
expect(req.__unhandledDestroyError).toBeUndefined();
});
it("timeout surfaces typed error", async () => {
@@ -112,5 +129,19 @@ describe("http body limits", () => {
await expect(promise).rejects.toSatisfy((error: unknown) =>
isRequestBodyLimitError(error, "REQUEST_BODY_TIMEOUT"),
);
expect(req.__unhandledDestroyError).toBeUndefined();
});
it("declared oversized content-length does not emit unhandled error", async () => {
const req = createMockRequest({
headers: { "content-length": "9999" },
emitEnd: false,
});
await expect(readRequestBodyWithLimit(req, { maxBytes: 128 })).rejects.toMatchObject({
message: "PayloadTooLarge",
});
// Wait a tick for any async destroy(err) emission.
await new Promise((resolve) => setTimeout(resolve, 0));
expect(req.__unhandledDestroyError).toBeUndefined();
});
});

View File

@@ -96,7 +96,9 @@ export async function readRequestBodyWithLimit(
if (declaredLength !== null && declaredLength > maxBytes) {
const error = new RequestBodyLimitError({ code: "PAYLOAD_TOO_LARGE" });
if (!req.destroyed) {
req.destroy(error);
// Limit violations are expected user input; destroying with an Error causes
// an async 'error' event which can crash the process if no listener remains.
req.destroy();
}
throw error;
}
@@ -131,7 +133,7 @@ export async function readRequestBodyWithLimit(
const timer = setTimeout(() => {
const error = new RequestBodyLimitError({ code: "REQUEST_BODY_TIMEOUT" });
if (!req.destroyed) {
req.destroy(error);
req.destroy();
}
fail(error);
}, timeoutMs);
@@ -145,7 +147,7 @@ export async function readRequestBodyWithLimit(
if (totalBytes > maxBytes) {
const error = new RequestBodyLimitError({ code: "PAYLOAD_TOO_LARGE" });
if (!req.destroyed) {
req.destroy(error);
req.destroy();
}
fail(error);
return;
@@ -294,7 +296,9 @@ export function installRequestBodyLimitGuard(
finish();
respond(error);
if (!req.destroyed) {
req.destroy(error);
// Limit violations are expected user input; destroying with an Error causes
// an async 'error' event which can crash the process if no listener remains.
req.destroy();
}
};