From a675370d8eb39a4317206837b022cc672c4b7304 Mon Sep 17 00:00:00 2001 From: Vincent Quatrevieux Date: Wed, 17 Jul 2024 19:23:12 +0200 Subject: [PATCH] chore: Move connection reset error ignore to SessionLogger --- .../session/extension/SessionLogger.java | 9 ++++++ .../quatrevieux/araknemu/game/GameModule.java | 2 +- .../game/GameExceptionConfigurator.java | 14 --------- .../realm/RealmSessionConfigurator.java | 14 --------- .../araknemu/realm/RealmModule.java | 2 +- .../session/extension/SessionLoggerTest.java | 10 +++++++ .../game/GameExceptionConfiguratorTest.java | 20 ------------- .../realm/RealmSessionConfiguratorTest.java | 30 ------------------- 8 files changed, 21 insertions(+), 80 deletions(-) diff --git a/src/main/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLogger.java b/src/main/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLogger.java index 6891bad1b..334405ba0 100644 --- a/src/main/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLogger.java +++ b/src/main/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLogger.java @@ -30,6 +30,7 @@ import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.MarkerManager; +import java.io.IOException; import java.util.function.Consumer; /** @@ -66,6 +67,14 @@ public boolean handleException(Throwable cause) { return false; } + // Ignore connection reset errors + if ( + cause instanceof IOException + && ("Connection reset by peer".equals(cause.getMessage()) || "Connexion ré-initialisée par le correspondant".equals(cause.getMessage())) + ) { + return false; + } + logger.error(NETWORK_ERROR_MARKER, "[{}] Uncaught exception", session, cause); if (cause.getCause() != null) { diff --git a/src/main/java/fr/quatrevieux/araknemu/game/GameModule.java b/src/main/java/fr/quatrevieux/araknemu/game/GameModule.java index 6981de3cb..b4f93110c 100644 --- a/src/main/java/fr/quatrevieux/araknemu/game/GameModule.java +++ b/src/main/java/fr/quatrevieux/araknemu/game/GameModule.java @@ -363,8 +363,8 @@ public void configure(ContainerConfigurator configurator) { container -> new SessionConfigurator<>(GameSession::new) .add(new BanIpCheck<>(container.get(BanIpService.class))) .add(new RateLimiter.Configurator<>(container.get(GameConfiguration.class).packetRateLimit())) - .add(new GameExceptionConfigurator(container.get(Logger.class))) .add(new SessionLogger.Configurator<>(container.get(Logger.class))) + .add(new GameExceptionConfigurator(container.get(Logger.class))) .add(new GamePacketConfigurator( container.get(Dispatcher.class), container.get(PacketParser.class) diff --git a/src/main/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfigurator.java b/src/main/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfigurator.java index b288f085a..2b103d8ac 100644 --- a/src/main/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfigurator.java +++ b/src/main/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfigurator.java @@ -29,8 +29,6 @@ import org.apache.logging.log4j.MarkerManager; import org.checkerframework.checker.nullness.qual.Nullable; -import java.io.IOException; - /** * Configure base exception for a game session */ @@ -91,18 +89,6 @@ public void configure(ConfigurableSession inner, GameSession session) { return true; }); - - inner.addExceptionHandler(IOException.class, cause -> { - // Ignore connection reset errors - if ( - !"Connection reset by peer".equals(cause.getMessage()) - && !"Connexion ré-initialisée par le correspondant".equals(cause.getMessage()) - ) { - logger.error("[{}] IOException : {}", session, cause.getMessage() == null ? cause.toString() : cause.getMessage()); - } - - return false; - }); } /** diff --git a/src/main/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfigurator.java b/src/main/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfigurator.java index 9f01cb032..e9b1a99cd 100644 --- a/src/main/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfigurator.java +++ b/src/main/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfigurator.java @@ -28,8 +28,6 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.MarkerManager; -import java.io.IOException; - /** * Configure session for realm */ @@ -65,18 +63,6 @@ public void configure(ConfigurableSession inner, RealmSession session) { return true; }); - inner.addExceptionHandler(IOException.class, cause -> { - // Ignore connection reset errors - if ( - !"Connection reset by peer".equals(cause.getMessage()) - && !"Connexion ré-initialisée par le correspondant".equals(cause.getMessage()) - ) { - logger.error("[{}] IOException : {}", session, cause.getMessage() == null ? cause.toString() : cause.getMessage()); - } - - return false; - }); - inner.addReceiveMiddleware(new RealmPacketParserMiddleware(loginPackets, baseParser)); inner.addReceiveMiddleware((packet, next) -> dispatcher.dispatch(session, (Packet) packet)); } diff --git a/src/main/java/fr/quatrevieux/araknemu/realm/RealmModule.java b/src/main/java/fr/quatrevieux/araknemu/realm/RealmModule.java index 1af61a173..a12f968b5 100644 --- a/src/main/java/fr/quatrevieux/araknemu/realm/RealmModule.java +++ b/src/main/java/fr/quatrevieux/araknemu/realm/RealmModule.java @@ -128,13 +128,13 @@ public void configure(ContainerConfigurator configurator) { container -> new SessionConfigurator<>(RealmSession::new) .add(new BanIpCheck<>(container.get(BanIpService.class))) .add(new RateLimiter.Configurator<>(container.get(RealmConfiguration.class).packetRateLimit())) + .add(new SessionLogger.Configurator<>(container.get(Logger.class))) .add(new RealmSessionConfigurator( container.get(Dispatcher.class), new PacketParser[] {DofusVersion.parser(), Credentials.parser()}, container.get(PacketParser.class), container.get(Logger.class) )) - .add(new SessionLogger.Configurator<>(container.get(Logger.class))) ); configurator.factory( diff --git a/src/test/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLoggerTest.java b/src/test/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLoggerTest.java index fd2a69036..d37588450 100644 --- a/src/test/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLoggerTest.java +++ b/src/test/java/fr/quatrevieux/araknemu/core/network/session/extension/SessionLoggerTest.java @@ -33,6 +33,8 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import java.io.IOException; + class SessionLoggerTest { private Session session; private Logger logger; @@ -106,6 +108,14 @@ void exceptionHandlerNotFound() { Mockito.verify(logger).warn(MarkerManager.getMarker("NETWORK_ERROR"), "Cannot found handler for packet Ping"); } + @Test + void exceptionConnectionResetShouldBeIgnored() { + session.exception(new IOException("Connection reset by peer")); + session.exception(new IOException("Connexion ré-initialisée par le correspondant")); + + Mockito.verifyNoInteractions(logger); + } + @Test void exceptionWithPacket() { Exception e = new Exception("my error"); diff --git a/src/test/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfiguratorTest.java b/src/test/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfiguratorTest.java index 58d50921f..19e16937b 100644 --- a/src/test/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfiguratorTest.java +++ b/src/test/java/fr/quatrevieux/araknemu/network/game/GameExceptionConfiguratorTest.java @@ -32,10 +32,7 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.IOException; - import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; class GameExceptionConfiguratorTest extends GameBaseCase { private GameExceptionConfigurator configurator; @@ -116,21 +113,4 @@ void exceptionCaughtRateLimit() { assertFalse(session.isAlive()); Mockito.verify(logger).warn(MarkerManager.getMarker("RATE_LIMIT"), "[{}] RateLimit : close session", gameSession); } - - @Test - void exceptionShouldIgnoreConnectionReset() { - gameSession.exception(new IOException("Connection reset by peer")); - gameSession.exception(new IOException("Connexion ré-initialisée par le correspondant")); - - assertTrue(session.isAlive()); - Mockito.verifyNoInteractions(logger); - } - - @Test - void exceptionShouldLogIOException() { - gameSession.exception(new IOException("My error")); - - assertTrue(session.isAlive()); - Mockito.verify(logger).error("[{}] IOException : {}", gameSession, "My error"); - } } diff --git a/src/test/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfiguratorTest.java b/src/test/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfiguratorTest.java index 89a9e4d3d..7e556a06f 100644 --- a/src/test/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfiguratorTest.java +++ b/src/test/java/fr/quatrevieux/araknemu/network/realm/RealmSessionConfiguratorTest.java @@ -40,10 +40,7 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.IOException; - import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; class RealmSessionConfiguratorTest extends RealmBaseCase { private RealmSessionConfigurator configurator; @@ -112,31 +109,4 @@ void exceptionCaughtRateLimit() { assertFalse(session.isAlive()); Mockito.verify(logger).error(MarkerManager.getMarker("RATE_LIMIT"), "[{}] RateLimit : close session", realmSession); } - - @Test - void exceptionShouldIgnoreConnectionReset() { - ConfigurableSession session = new ConfigurableSession(channel); - RealmSession realmSession = new RealmSession(session); - - configurator.configure(session, realmSession); - - realmSession.exception(new IOException("Connection reset by peer")); - realmSession.exception(new IOException("Connexion ré-initialisée par le correspondant")); - - assertFalse(session.isAlive()); - Mockito.verifyNoInteractions(logger); - } - - @Test - void exceptionShouldLogIOException() { - ConfigurableSession session = new ConfigurableSession(channel); - RealmSession realmSession = new RealmSession(session); - - configurator.configure(session, realmSession); - - realmSession.exception(new IOException("My error")); - - assertFalse(session.isAlive()); - Mockito.verify(logger).error("[{}] IOException : {}", realmSession, "My error"); - } }