Skip to content

Commit

Permalink
chore: Move connection reset error ignore to SessionLogger
Browse files Browse the repository at this point in the history
  • Loading branch information
vincent4vx committed Jul 17, 2024
1 parent 0c00041 commit a675370
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/fr/quatrevieux/araknemu/game/GameModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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;
});
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.MarkerManager;

import java.io.IOException;

/**
* Configure session for realm
*/
Expand Down Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
}

0 comments on commit a675370

Please sign in to comment.