ERR_HTTP_HEADERS_SENT caused by asynchronous hooks #4908
Description
Prerequisites
- I have written a descriptive issue title
- I have searched existing issues to ensure the bug has not already been reported
Fastify version
4.19.2
Plugin version
No response
Node.js version
20.3.0
Operating system
macOS
Operating system version (i.e. 20.04, 11.3, 10)
13.4.1
Description
We frequently get this error:
Unhandled promise rejection: Error [ERR_HTTP_HEADERS_SENT]: Cannot write headers after they are sent to the client
at new NodeError (node:internal/errors:405:5)
at ServerResponse.writeHead (node:_http_server:342:11)
at onSendEnd (/snipped-path/node_modules/fastify/lib/reply.js:572:9)
at wrapOnSendEnd (/snipped-path/node_modules/fastify/lib/reply.js:538:5)
at next (/snipped-path/node_modules/fastify/lib/hooks.js:224:7)
at handleResolve (/snipped-path/node_modules/fastify/lib/hooks.js:241:5)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
The problem is triggered when one of my plugins has an asynchronous onSendHook, this is the code:
fastify.addHook("onSend", (request, reply, payload) => {
console.log("in onSend");
let promise = new Promise(resolve => {
setTimeout(() => {
console.log("onSend: payload=" + payload);
resolve(payload);
}, 1);
});
return promise;
});
My code handles the request with this code:
app.get(
"*",
async (req, reply) => await this._defaultUrlHandler(req, reply)
);
and then:
async _defaultUrlHandler(req, reply, url) {
reply.send("hello");
return;
}
My _defaultUrlHandler is called async, and it does call reply.send() - but reply.sent is never set to true.
Moments later, in fastify/lib/wrapThenable in the wrapThenable function, the code says:
function wrapThenable (thenable, reply) {
thenable.then(function (payload) {
if (reply[kReplyHijacked] === true) {
return
}
// this is for async functions that are using reply.send directly
//
// since wrap-thenable will be called when using reply.send directly
// without actual return. the response can be sent already or
// the request may be terminated during the reply. in this situation,
// it require an extra checking of request.aborted to see whether
// the request is killed by client.
if (payload !== undefined || (reply.sent === false && reply.raw.headersSent === false && reply.request.raw.aborted === false)) {
// we use a try-catch internally to avoid adding a catch to another
// promise, increase promise perf by 10%
try {
reply.send(payload)
The payload is undefined because that was what what returned by my _defaultUrlHandler and reply.sent is false. This triggers the second call to reply.send which is the cause of the error.
The call stack looks like this:
If I set a breakpoint in reply.send, when it is called for the second time, this is in the console:
in onSend
Note that the in onSend message has appeared, but the async function in onSend has not resolved, otherwise we would see additional messages in the console.
The problem is that in lib/handleRequest.js the preHandlerCallback calls my URL handler and receives a promise, so uses wrapThenable; my async URL handler calls reply.send but the async onSendHook delays the writing of the reply. My URL handler resolves, wrapThenable decides that nothing has been sent (because reply.sent === false), and then wrapThenable calls reply.send for a second time.
This is the complete output of the console:
Webserver started on http://localhost:8088
in onSend
in onSend
onSend: payload=hello
onSend: payload=undefined
Unhandled promise rejection: Error [ERR_HTTP_HEADERS_SENT]: Cannot write headers after they are sent to the client
at new NodeError (node:internal/errors:405:5)
at ServerResponse.writeHead (node:_http_server:342:11)
at onSendEnd (/snipped-path/node_modules/fastify/lib/reply.js:572:9)
at wrapOnSendEnd (/snipped-path/node_modules/fastify/lib/reply.js:538:5)
at next (/snipped-path/node_modules/fastify/lib/hooks.js:224:7)
at handleResolve (/snipped-path/node_modules/fastify/lib/hooks.js:241:5)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
Note how the onSendHook is called twice, and so is the resolved promise - once with the correct payload and once with undefined (thanks to the wrapThenable).
Solution (well, workaround / hack)
The problem seems to be that wrapThenable relies on reply.sent to know if data has been passed to reply.send, but that property is defined as:
sent: {
enumerable: true,
get () {
// We are checking whether reply was hijacked or the response has ended.
return (this[kReplyHijacked] || this.raw.writableEnded) === true
},
and so reply.sent actually means that "the reply has finished sending to the client" and not "there is data which will is being sent"
A quick fix is to call .hijack:
Reply.prototype.send = function (payload) {
if (this[kReplyIsRunningOnErrorHook] === true) {
throw new FST_ERR_SEND_INSIDE_ONERR()
}
if (this.sent) {
this.log.warn({ err: new FST_ERR_REP_ALREADY_SENT() }, 'Reply already sent')
return this
}
this.hijack(); // workaround for asynchronous `onSendHook`
Steps to Reproduce
see above
Expected Behavior
reply.send should not be called for a second time