node-server
node-server copied to clipboard
Consuming and cancelling the IncomingMessage stream crashes the server.
I have an endpoint for accepting huge files, so instead of loading them at once into memory, I choose to consume incoming
as stream and process it with busboy
. The happy path works properly, but the server crashes when I start to do stream error handling:
import { pipeline } from "stream/promises";
import { Busboy } from "@fastify/busboy";
import { HttpBindings } from "@hono/node-server";
import { Hono } from "hono";
const app = new Hono<{ Bindings: HttpBindings }>();
app.post("/", async (c) => {
const { incoming, outgoing } = c.env;
const busboy = Busboy({
headers: incoming.headers,
});
busboy.on("file", async (fieldname, file, filename, encoding, mimetype) => {
busboy.emit("error", new Error("Something bad happened"));
}
try {
await pipeline(incoming, busboy);
return c.html("never reached, as stream/promises/pipeline will throw on error event");
} catch (err: Error) {
outgoing.writeHead(500, { Connection: "close" });
outgoing.end(err.message);
return RESPONSE_ALREADY_SENT;
}
)
The whole server crashes with the above code after responding to the client. The error is caused by https://github.com/nodejs/undici/blob/main/lib/web/fetch/body.js#L191, which basically happen when a disturbed stream is being used to initial a request body (cancelled is considered as disturbed in this case). And since those node apis are out of our control, I wonder if we should
- wrap
newRequestFromIncoming
with some try catch logic? - don't initialize the request when
RESPONSE_ALREADY_SENT
is returned? or at least makereq.body
empty? - most importantly, we shouldn't let the whole app crash in any case,
app.onError
should have caught the error.
Thank you!