Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Every connection gets closed by ERR_HTTP_REQUEST_TIMEOUT after a minute #36

Closed
Mwni opened this issue Aug 3, 2022 · 3 comments
Closed

Comments

@Mwni
Copy link

Mwni commented Aug 3, 2022

It appears that an upgraded connection is still considered as an ongoing http request, which then gets killed after the default timeout.

 Error [ERR_HTTP_REQUEST_TIMEOUT]: Request timeout
      at new NodeError (node:internal/errors:377:5)
      at onRequestTimeout (node:_http_server:706:30)
      at Server.checkConnections (node:_http_server:519:7)
      at listOnTimeout (node:internal/timers:564:17)
      at process.processTimers (node:internal/timers:507:7)
@gchallen
Copy link

I was bit by the same bug, which looks like it came in with Node 18. What worked for me is disabling the server requestTimeout and headersTimeout by setting them to 0:

const server = app.listen(PORT)
server.requestTimeout = 0
server.headersTimeout = 0

However, this is probably safest if your app is deployed behind a proxy that can close down partial requests properly and prevent against associated DoS attacks.

@Mwni
Copy link
Author

Mwni commented Jan 12, 2023

Yeah this is a bug in Node's C++ socket implementation. This is another workaround:

koa.use(async (ctx, next) => {
	if(ctx.ws){
		ctx.req.socket.ignoreTimeout = true
		//handle socket
	}else{
		return await next(ctx)
	}
})

koa.listen(80)
	.on('clientError', (error, socket) => {
		if(error.code === 'ERR_HTTP_REQUEST_TIMEOUT' && socket.ignoreTimeout)
			return

		console.warn(`client error:`, error)
		socket.destroy()
	})
	

@Mwni Mwni closed this as completed Jan 12, 2023
AaronFriel added a commit to AaronFriel/next.js that referenced this issue Dec 2, 2023
When the request is upgraded as part of a request listener, one of three things
must happen in order to prevent Node.js from closing the connection:

1. Disable request timeouts.

2. The request handler may mark the `OutgoingMessage` as having the response
   headers sent before the timeout expires. This happens as part of
   `flushHeaders()` calling `Socket#_send()` in
   `node.js/lib/_http_outgoing.js`[^1].

3. Add an event listener on the `http.Server` for `clientError` events to handle
   the HTTP request timeout error. In `node.js/lib/_http_server.js`[^2], the
   presence of an event listener bypasses error handling.

We rule out the first two options as:

1. This would re-introduce Slowloris and other vulnerabilities that are the
   reason these timeouts were added.

2. The most popular Websocket library, `ws`, writes to the raw socket and does
   not use `OutgoingMessage` methods which would mark the headers as sent.

This leaves the third option, and we implement a workaround as seen in
[koa-easy-ws#36](b3nsn0w/koa-easy-ws#36).

[^1]: https://github.com/nodejs/node/blob/v20.10.0/lib/_http_outgoing.js#L378
[^2]: https://github.com/nodejs/node/blob/a2206640f366cc145b17a2ece66d36bfa75720ee/lib/_http_server.js#L883
@AaronFriel
Copy link

Thanks for this pointer @Mwni, I've cited this thread in a PR I made as a contribution to add a feature like this library does for Next.js. Your comment gave me the pointer I needed to write this commit:

vercel/next.js@486b362

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants