Fix disconnect when recording during a server switch - #65
Open
lucasfrederico wants to merge 1 commit into
Open
Conversation
Entity.getId() throws IllegalStateException if the entity has no id assigned yet. The null check on the local player isn't enough to make the call safe: on a proxied network the client builds the LocalPlayer for the new backend while packets are still coming in, so there's a window where the player is non-null but has no id. The exception escapes channelRead0 and the client disconnects with a Packet handling error. Catching it and treating an unassigned id as not-the-local- player keeps the packet and the connection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We run a Minecraft network (LoverFella) behind Velocity. Staff who record with Flashback get disconnected when the proxy moves them between backend servers. Two reports so far, both with the same stack:
Flashback 0.42.1, Minecraft 26.2, Fabric.
writePacketAsyncnull-checks the local player before callinggetId(), butgetId()throws when the entity has not been assigned an id yet, so non-null is not enough. On a proxy the client has already built the LocalPlayer for the server it is being sent to while packets for it are still arriving, which lands inside that window. The exception goes up throughchannelRead0and the client drops the connection with a packet handling error.Both of our reports happened inside our scheduled restart window, which is exactly when players get moved between backends.
The patch catches the exception and falls back to -1. No real entity id matches that, so the two filters below simply do not match and the packet is recorded like any other. Recording two packets that would otherwise be skipped seemed better than losing the connection.