colyseus / uWebSockets-express

Express API compatibility layer for uWebSockets.js
https://npmjs.com/package/uwebsockets-express
MIT License
56 stars 13 forks source link

500ms Content-Length timeout is not enough #30

Closed hunkydoryrepair closed 10 months ago

hunkydoryrepair commented 11 months ago

We've run into occasional issues getting "request size did not match content length".

It is very sporadic, but ultimately determine that the timeout, which requires the full content is received within 500ms is causing problems. I'm not sure if this is 500ms for the full content body, or if timeout resets with each packet received. Our payloads are around 1500 bytes, which is often 2 packets.

Scenarios where we end up getting this error:

Would it be possible to make that timeout value configurable? Obviously, we don't want the client to hang forever with open socket because they sent 1 bad request, but in our case, it needs to be a little more patient.

endel commented 11 months ago

Hi @hunkydoryrepair, thanks for the detailed report!

I forgot we had this 500ms limitation. I think we can increase the default limit + have some detection for aborted requests to avoid waiting forever.

For customization I think we can provide expressify(app, { readBodyTimeout: 3000 }) + process.env.UWS_READ_BODY_TIMEOUT for customization, what do you think?


It's great to see Pixels getting popular! I'd love to see it supporting Colyseus financially if possible 💖 Cheers!

hunkydoryrepair commented 11 months ago

You are suggesting two different solutions, if I understand?

Either one should work, I think, but currently I don't think uWebSockets-express reads anything from environment, so that would be adding a new dependency, so the one to pass parameters to expressify is lighter weight in that sense (although a little more work, possibly).

hunkydoryrepair commented 11 months ago

I don't know how you would handle aborted requests. I think it does handle a socket disconnection, but I don't think we experience that. A socket timeout is probably closer to 30-60 seconds vs a 1/2 second timeout. I'm not sure if you have a way to detect if the socket has been upgraded to websocket or not. We actually have this issue primarily before the socket is upgraded, when it seems unnecessary to even have the timeout feature.

endel commented 10 months ago

Addressed by @theweiweiway on https://github.com/colyseus/uWebSockets-express/pull/32 💙