colyseus / uWebSockets-express

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

HEAD requests not stripping body #23

Open hunkydoryrepair opened 1 year ago

hunkydoryrepair commented 1 year ago

Normally, express will route HEAD requests to the get handler if there is not an explicit head handler. This library seems to do that appropriately as well.

However, what express does that this does not do, is strip the BODY response. uWebSockets-express seems to be sending the body back on a HEAD request.

This is particularly bad when used with the @colyseus/proxy, since the presence of the BODY creates an error event on the proxy, and the proxy, as a result, unregisters the server, and then subsequently tries every other proxy server, killing them 1 by 1. So, a single call to curl takes down anything running @colyseus/proxy and uWebSockets w/ express simulation pretty much totally.

curl -I http://your-colyseus-proxy/express-endpoint

hunkydoryrepair commented 1 year ago

actually...disregard. I think this is only an issue with the new health check in the proxy.

hunkydoryrepair commented 1 year ago

Well, I'm reopening because the proxy still errors and deregisters server it when using the HEAD method to any "express" endpoint in the server. I'm not sure it is the body included or exactly what, because I don't see body content returned. Maybe the proxy errors out after sending the headers, so I only get what appears correct on the client end, but it actually does cause an error in the proxy.

endel commented 1 year ago

Hi @hunkydoryrepair, if you can afford to migrate to version 0.15@preview there's a deployment configuration that eliminates the need for the proxy. Version 0.15 should be released this month, and the proxy configuration with the proxy is going to be considered "legacy".

I'm in direct contact with another user that is having this problem of Colyseus processes de-registering themselves, but we haven't found the root cause yet (it may be a different one since this user is not using WebSockets)

If you need assistance please reach me out on endel@gamestd.io

hunkydoryrepair commented 1 year ago

There is another one I have seen, even before we switched to uWebSockets. I had already addressed it. Sometimes the regular socket error is something like "The socket connection was closed by the other party"

I just added a check for !errorMessage.includes("by the other party") &&

in the error handling

hunkydoryrepair commented 1 year ago

We will look at 0.15. I suspect it has other improvements as well. I also had to kind of hack together a schema update for player state, so each player could get schema updates to their own player state (which is big, so not sent to all users, with 200+ users in one room). It isn't great, though, because the schema update create an array, which then goes to JSON and sent as a message. Still smaller than sending full player state or sending to all players, though.

endel commented 1 year ago

That is a good catch! Gonna add this check to the proxy, thanks for sharing.

I'm curious which approach you took for this "hack" as it may break if upgrading to the latest schema version. Ideally I'd love for everybody to have a smooth migration experience 🙏

hunkydoryrepair commented 1 year ago

emails sent.

gamedevsam commented 1 year ago

Is this issue still relevant? If not close it.