Better handle not failing over on a read timeout

The existing Velocity logic was pretty optimistic, hoping the player connection would stay alive long enough to accept a connection to another server. Now, if we notice a read timeout on the server end, we'll treat the disconnect as "unsafe" and disconnect the player immediately.

I've added a configuration option to solve the issue in the way BungeeCord does it. This could cause issues with servers that extensively modify the server pipeline and could degrade the user experience, though. Let's try this more conservative and see if it helps,

api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java

@@ -209,7 +209,7 @@ public final class KickedFromServerEvent implements
       return LegacyText3ComponentSerializer.get().serialize(message);
     }
 
-    public net.kyori.adventure.text.Component getMessageComponent() {
+    public net.kyori.adventure.text.@Nullable Component getMessageComponent() {
       return message;
     }
 

proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java

@@ -342,6 +342,10 @@ public class VelocityConfiguration implements ProxyConfig {
     return advanced.isShowPingRequests();
   }
 
+  public boolean isFailoverOnUnexpectedServerDisconnect() {
+    return advanced.isFailoverOnUnexpectedServerDisconnect();
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)
@@ -577,6 +581,7 @@ public class VelocityConfiguration implements ProxyConfig {
     private boolean tcpFastOpen = false;
     private boolean bungeePluginMessageChannel = true;
     private boolean showPingRequests = false;
+    private boolean failoverOnUnexpectedServerDisconnect = false;
 
     private Advanced() {
     }
@@ -592,6 +597,8 @@ public class VelocityConfiguration implements ProxyConfig {
         this.tcpFastOpen = config.getOrElse("tcp-fast-open", false);
         this.bungeePluginMessageChannel = config.getOrElse("bungee-plugin-message-channel", true);
         this.showPingRequests = config.getOrElse("show-ping-requests", false);
+        this.failoverOnUnexpectedServerDisconnect = config
+            .getOrElse("failover-on-unexpected-server-disconnect", true);
       }
     }
 
@@ -631,6 +638,10 @@ public class VelocityConfiguration implements ProxyConfig {
       return showPingRequests;
     }
 
+    public boolean isFailoverOnUnexpectedServerDisconnect() {
+      return failoverOnUnexpectedServerDisconnect;
+    }
+
     @Override
     public String toString() {
       return "Advanced{"
@@ -643,6 +654,7 @@ public class VelocityConfiguration implements ProxyConfig {
           + ", tcpFastOpen=" + tcpFastOpen
           + ", bungeePluginMessageChannel=" + bungeePluginMessageChannel
           + ", showPingRequests=" + showPingRequests
+          + ", failoverOnUnexpectedServerDisconnect=" + failoverOnUnexpectedServerDisconnect
           + '}';
     }
   }

proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java

@@ -29,6 +29,7 @@ import com.velocitypowered.proxy.protocol.util.PluginMessageUtil;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
+import io.netty.handler.timeout.ReadTimeoutException;
 
 public class BackendPlaySessionHandler implements MinecraftSessionHandler {
 
@@ -205,7 +206,8 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler {
   @Override
   public void exception(Throwable throwable) {
     exceptionTriggered = true;
-    serverConn.getPlayer().handleConnectionException(serverConn.getServer(), throwable, true);
+    serverConn.getPlayer().handleConnectionException(serverConn.getServer(), throwable,
+        !(throwable instanceof ReadTimeoutException));
   }
 
   public VelocityServer getServer() {
@@ -216,9 +218,13 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler {
   public void disconnected() {
     serverConn.getServer().removePlayer(serverConn.getPlayer());
     if (!serverConn.isGracefulDisconnect() && !exceptionTriggered) {
+      if (server.getConfiguration().isFailoverOnUnexpectedServerDisconnect()) {
         serverConn.getPlayer().handleConnectionException(serverConn.getServer(),
-          Disconnect.create(ConnectionMessages.UNEXPECTED_DISCONNECT,
+            Disconnect.create(ConnectionMessages.INTERNAL_SERVER_CONNECTION_ERROR,
                 ProtocolVersion.MINECRAFT_1_16), true);
+      } else {
+        serverConn.getPlayer().disconnect(ConnectionMessages.INTERNAL_SERVER_CONNECTION_ERROR);
+      }
     }
   }
 }

proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java

@@ -31,6 +31,7 @@ import com.velocitypowered.proxy.protocol.packet.TabCompleteResponse;
 import com.velocitypowered.proxy.protocol.packet.TabCompleteResponse.Offer;
 import com.velocitypowered.proxy.protocol.packet.TitlePacket;
 import com.velocitypowered.proxy.protocol.util.PluginMessageUtil;
+import com.velocitypowered.proxy.util.VelocityMessages;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
@@ -271,8 +272,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
 
   @Override
   public void exception(Throwable throwable) {
-    player.disconnect(TextComponent.of("Your connection has encountered an error. Try again later.",
-        NamedTextColor.RED));
+    player.disconnect(VelocityMessages.GENERIC_CONNECTION_ERROR);
   }
 
   @Override

proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java

@@ -566,6 +566,11 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
           // There can't be any connection in flight now.
           connectionInFlight = null;
 
+          if (!isActive()) {
+            // If the connection is no longer active, it makes no sense to try and recover it.
+            return;
+          }
+
           if (event.getResult() instanceof DisconnectPlayer) {
             DisconnectPlayer res = (DisconnectPlayer) event.getResult();
             disconnect(res.getReasonComponent());

proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionMessages.java

@@ -10,9 +10,7 @@ public class ConnectionMessages {
   public static final TextComponent IN_PROGRESS = TextComponent
       .of("You are already connecting to a server!", NamedTextColor.RED);
   public static final TextComponent INTERNAL_SERVER_CONNECTION_ERROR = TextComponent
-      .of("Internal server connection error");
-  public static final TextComponent UNEXPECTED_DISCONNECT = TextComponent
-      .of("Unexpectedly disconnected from server - crash?");
+      .of("An internal server connection error occurred.", NamedTextColor.RED);
 
   private ConnectionMessages() {
     throw new AssertionError();

proxy/src/main/java/com/velocitypowered/proxy/util/VelocityMessages.java

@@ -15,11 +15,13 @@ public final class VelocityMessages {
       )
       .build();
   public static final Component NO_AVAILABLE_SERVERS = TextComponent
-      .of("No available servers", NamedTextColor.RED);
+      .of("There are no available servers.", NamedTextColor.RED);
   public static final Component ALREADY_CONNECTED = TextComponent
       .of("You are already connected to this proxy!", NamedTextColor.RED);
   public static final Component MOVED_TO_NEW_SERVER = TextComponent
       .of("The server you were on kicked you: ", NamedTextColor.RED);
+  public static final Component GENERIC_CONNECTION_ERROR = TextComponent
+      .of("An internal error occurred in your connection.", NamedTextColor.RED);
 
   private VelocityMessages() {
     throw new AssertionError();

proxy/src/main/resources/default-velocity.toml

@@ -117,6 +117,12 @@ bungee-plugin-message-channel = true
 # Shows ping requests to the proxy from clients.
 show-ping-requests = false
 
+# By default, Velocity will attempt to gracefully handle situations where the user unexpectedly
+# loses connection to the server without an explicit disconnect message by attempting to fall the
+# user back, except in the case of read timeouts. BungeeCord will disconnect the user instead. You
+# can disable this setting to use the BungeeCord behavior.
+failover-on-unexpected-server-disconnect = true
+
 [query]
 # Whether to enable responding to GameSpy 4 query responses or not.
 enabled = false