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

Informative warnings for (de)serialization errors #146

Open
HimbeerserverDE opened this issue Jun 2, 2024 · 1 comment
Open

Informative warnings for (de)serialization errors #146

HimbeerserverDE opened this issue Jun 2, 2024 · 1 comment
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@HimbeerserverDE
Copy link
Owner

Packet (de)serialization errors (particularly trailing data errors) are a sign of version mismatches and only result in log messages on the proxy. This feature should have 3 modes of operation:

  • Disabled: Keep the current behavior of logging the error and skipping the packet.
  • Warn: Log the error and send a chat message to the client. Must not cause spam on subsequent errors. To be discussed: Should the technical details be included? Likely yes.
  • Kick: Log the error and kick the client with an error message.

The proxy-side log messages should also be improved to include hints at a potential version mismatch.

The mode switch can either be a string or a pair of booleans (the latter is more robust to parse).

The message could be similar to "Proxy (de)serialization error in your connection. Potential version mismatch. Please ensure that your client is on Minetest . [Technical details: ]" for client-side errors and "Proxy (de)serialization error in upstream connection. Please contact the server administrator [and give them the following technical information: mt error]" for server-side errors. This should be favored over a simple "unexpected data" or "serialization version mismatch" error.

This feature could help with detecting incompatible development builds of Minetest which currently can't be detected properly due to a bad upstream versioning policy.

@HimbeerserverDE HimbeerserverDE added enhancement New feature or request good first issue Good for newcomers labels Jun 2, 2024
@HimbeerserverDE
Copy link
Owner Author

This is mostly fixed by 1481566. Its implementation is not configurable and kicks clients whose maximum protocol version exceeds the proxy protocol version. Minetest would normally downgrade such connections, but the updated packets would still be sent by the client in some cases, thus still breaking compatibility with the proxy. The log message also describes a version mismatch though it doesn't explain which version ranges are considered valid. A log FAQ may be a better solution though since other messages cause confusion too (such as (de)serialization errors).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant