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

HEAD requests not stripping body #23

Open
hunkydoryrepair opened this issue Apr 13, 2023 · 8 comments
Open

HEAD requests not stripping body #23

hunkydoryrepair opened this issue Apr 13, 2023 · 8 comments

Comments

@hunkydoryrepair
Copy link
Contributor

hunkydoryrepair commented Apr 13, 2023

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
Copy link
Contributor Author

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

@hunkydoryrepair
Copy link
Contributor Author

hunkydoryrepair commented Apr 13, 2023

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
Copy link
Member

endel commented Apr 13, 2023

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 [email protected]

@hunkydoryrepair
Copy link
Contributor Author

hunkydoryrepair commented Apr 13, 2023

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
Copy link
Contributor Author

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
Copy link
Member

endel commented Apr 13, 2023

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
Copy link
Contributor Author

emails sent.

@gamedevsam
Copy link

Is this issue still relevant? If not close it.

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