From 278930a0083707de2b64080f2f9e252844de339a Mon Sep 17 00:00:00 2001 From: A248 Date: Mon, 26 Oct 2020 13:52:04 -0400 Subject: [PATCH] Handle exceptions relating to CompletableFuture operations Solves #374 --- .../velocitypowered/proxy/VelocityServer.java | 5 ++- .../backend/BackendPlaySessionHandler.java | 12 +++++-- .../client/ClientPlaySessionHandler.java | 36 ++++++++++++++++--- .../connection/client/ConnectedPlayer.java | 8 ++++- .../client/LoginSessionHandler.java | 22 ++++++++++-- .../client/StatusSessionHandler.java | 12 +++++-- .../proxy/plugin/VelocityEventManager.java | 8 ++--- .../proxy/protocol/netty/GS4QueryHandler.java | 8 ++++- 8 files changed, 91 insertions(+), 20 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index 54c446a88..863a481c7 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -425,8 +425,11 @@ public class VelocityServer implements ProxyServer, ForwardingAudience { .toArray((IntFunction[]>) CompletableFuture[]::new)); playersTeardownFuture.get(10, TimeUnit.SECONDS); - } catch (TimeoutException | ExecutionException e) { + } catch (TimeoutException e) { timedOut = true; + } catch (ExecutionException e) { + timedOut = true; + logger.error("Exception while tearing down player connections", e); } eventManager.fireShutdownEvent(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index bbba322d9..abd1f7b91 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java @@ -149,7 +149,11 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { Unpooled.wrappedBuffer(copy)); playerConnection.write(copied); } - }, playerConnection.eventLoop()); + }, playerConnection.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while handling plugin message {}", packet, ex); + return null; + }); return true; } @@ -186,7 +190,11 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { server.getEventManager().fire( new PlayerAvailableCommandsEvent(serverConn.getPlayer(), rootNode)) - .thenAcceptAsync(event -> playerConnection.write(commands), playerConnection.eventLoop()); + .thenAcceptAsync(event -> playerConnection.write(commands), playerConnection.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while handling available commands for {}", playerConnection, ex); + return null; + }); return true; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index c2f31eb98..956772000 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java @@ -157,7 +157,11 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { smc.write(packet); } } - }, smc.eventLoop()); + }, smc.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while handling player chat for {}", player, ex); + return null; + }); } return true; } @@ -225,7 +229,12 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { Unpooled.wrappedBuffer(copy)); backendConn.write(message); } - }, backendConn.eventLoop()); + }, backendConn.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while handling plugin message packet for {}", + player, ex); + return null; + }); } } } @@ -423,7 +432,12 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { resp.getOffers().addAll(offers); player.getConnection().write(resp); } - }, player.getConnection().eventLoop()); + }, player.getConnection().eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while handling command tab completion for player {} executing {}", + player, command, ex); + return null; + }); return true; // Sorry, handler; we're just gonna have to lie to you here. } @@ -475,7 +489,13 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { player.getUsername(), command, e); } - }, player.getConnection().eventLoop()); + }, player.getConnection().eventLoop()) + .exceptionally((ex) -> { + logger.error( + "Exception while finishing command tab completion, with request {} and response {}", + request, response, ex); + return null; + }); } private void finishRegularTabComplete(TabCompleteRequest request, TabCompleteResponse response) { @@ -490,7 +510,13 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler { response.getOffers().add(new Offer(s)); } player.getConnection().write(response); - }, player.getConnection().eventLoop()); + }, player.getConnection().eventLoop()) + .exceptionally((ex) -> { + logger.error( + "Exception while finishing regular tab completion, with request {} and response{}", + request, response, ex); + return null; + }); } private CompletableFuture processCommandExecuteResult(String originalCommand, diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 2cad65f05..f635babf5 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java @@ -757,7 +757,13 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { } DisconnectEvent event = new DisconnectEvent(this, status); - server.getEventManager().fire(event).thenRun(() -> this.teardownFuture.complete(null)); + server.getEventManager().fire(event).whenComplete((val, ex) -> { + if (ex == null) { + this.teardownFuture.complete(null); + } else { + this.teardownFuture.completeExceptionally(ex); + } + }); } public CompletableFuture getTeardownFuture() { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java index 826baec66..fbfca874f 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/LoginSessionHandler.java @@ -183,7 +183,11 @@ public class LoginSessionHandler implements MinecraftSessionHandler { } else { initializePlayer(GameProfile.forOfflinePlayer(login.getUsername()), false); } - }, mcConnection.eventLoop()); + }, mcConnection.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception in pre-login stage", ex); + return null; + }); } private EncryptionRequest generateEncryptionRequest() { @@ -202,6 +206,7 @@ public class LoginSessionHandler implements MinecraftSessionHandler { server.getConfiguration().getPlayerInfoForwardingMode()); GameProfileRequestEvent profileRequestEvent = new GameProfileRequestEvent(inbound, profile, onlineMode); + final GameProfile finalProfile = profile; server.getEventManager().fire(profileRequestEvent).thenCompose(profileEvent -> { if (mcConnection.isClosed()) { @@ -229,6 +234,9 @@ public class LoginSessionHandler implements MinecraftSessionHandler { completeLoginProtocolPhaseAndInitialize(player); } }, mcConnection.eventLoop()); + }).exceptionally((ex) -> { + logger.error("Exception during connection of {}", finalProfile, ex); + return null; }); } @@ -274,7 +282,11 @@ public class LoginSessionHandler implements MinecraftSessionHandler { server.getEventManager().fire(new PostLoginEvent(player)) .thenRun(() -> connectToInitialServer(player)); } - }, mcConnection.eventLoop()); + }, mcConnection.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while completing login initialisation phase for {}", player, ex); + return null; + }); } private void connectToInitialServer(ConnectedPlayer player) { @@ -291,7 +303,11 @@ public class LoginSessionHandler implements MinecraftSessionHandler { return; } player.createConnectionRequest(toTry.get()).fireAndForget(); - }, mcConnection.eventLoop()); + }, mcConnection.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while connecting {} to initial server", player, ex); + return null; + }); } @Override diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java index cffc71377..2b44c654b 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/StatusSessionHandler.java @@ -163,7 +163,11 @@ public class StatusSessionHandler implements MinecraftSessionHandler { .thenCompose(ping -> server.getEventManager().fire(new ProxyPingEvent(inbound, ping))) .thenAcceptAsync(event -> connection.closeWith( LegacyDisconnect.fromServerPing(event.getPing(), packet.getVersion())), - connection.eventLoop()); + connection.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while handling legacy ping {}", packet, ex); + return null; + }); return true; } @@ -189,7 +193,11 @@ public class StatusSessionHandler implements MinecraftSessionHandler { .toJson(event.getPing(), json); connection.write(new StatusResponse(json)); }, - connection.eventLoop()); + connection.eventLoop()) + .exceptionally((ex) -> { + logger.error("Exception while handling status request {}", packet, ex); + return null; + }); return true; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java index 8d9b40bbf..5d0d58551 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java @@ -115,12 +115,10 @@ public class VelocityEventManager implements EventManager { return CompletableFuture.completedFuture(event); } - CompletableFuture eventFuture = new CompletableFuture<>(); - service.execute(() -> { + return CompletableFuture.supplyAsync(() -> { fireEvent(event); - eventFuture.complete(event); - }); - return eventFuture; + return event; + }, service); } @Override diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java index 888f6ab29..ffe2a24b4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/GS4QueryHandler.java @@ -29,6 +29,7 @@ import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import net.kyori.adventure.text.serializer.plain.PlainComponentSerializer; +import org.apache.logging.log4j.LogManager; public class GS4QueryHandler extends SimpleChannelInboundHandler { @@ -162,7 +163,12 @@ public class GS4QueryHandler extends SimpleChannelInboundHandler // Send the response DatagramPacket responsePacket = new DatagramPacket(queryResponse, msg.sender()); ctx.writeAndFlush(responsePacket, ctx.voidPromise()); - }, ctx.channel().eventLoop()); + }, ctx.channel().eventLoop()) + .exceptionally((ex) -> { + LogManager.getLogger(getClass()).error( + "Exception while writing GS4 response for query from {}", senderAddress, ex); + return null; + }); break; } default: