From eb854e2dd05e41ca6a87cd0de51e7881d63472dd Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Mon, 3 Oct 2016 13:53:19 +0200 Subject: [PATCH] Always log the message received from the server before handling the message so that even a error is thrown in handling message, the message itself still get logged first --- .../socket/LoggingResponseHandler.java | 20 +++++----- .../socket/LoggingResponseHandlerTest.java | 37 +++++++++++++++++-- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/driver/src/main/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandler.java b/driver/src/main/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandler.java index 74e4fc4a52..c1f6130d71 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandler.java +++ b/driver/src/main/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandler.java @@ -43,70 +43,70 @@ public LoggingResponseHandler( Logger logger ) @Override public void handleInitMessage( String clientNameAndVersion, Map authToken ) { - super.handleInitMessage( clientNameAndVersion, authToken ); logger.debug( "S: [INIT \"%s\"]", clientNameAndVersion ); + super.handleInitMessage( clientNameAndVersion, authToken ); } @Override public void handleRunMessage( String statement, Map parameters ) { - super.handleRunMessage( statement, parameters ); logger.debug( "S: [RUN \"%s\" %s]", statement, parameters ); + super.handleRunMessage( statement, parameters ); } @Override public void handlePullAllMessage() { - super.handlePullAllMessage(); logger.debug( DEFAULT_DEBUG_LOGGING_FORMAT, PULL_ALL ); + super.handlePullAllMessage(); } @Override public void handleDiscardAllMessage() { - super.handleDiscardAllMessage(); logger.debug( DEFAULT_DEBUG_LOGGING_FORMAT, DISCARD_ALL ); + super.handleDiscardAllMessage(); } @Override public void handleResetMessage() { - super.handleResetMessage(); logger.debug( DEFAULT_DEBUG_LOGGING_FORMAT, RESET ); + super.handleResetMessage(); } @Override public void handleAckFailureMessage() { - super.handleAckFailureMessage(); logger.debug( DEFAULT_DEBUG_LOGGING_FORMAT, ACK_FAILURE ); + super.handleAckFailureMessage(); } @Override public void handleSuccessMessage( Map meta ) { - super.handleSuccessMessage( meta ); logger.debug( "S: [SUCCESS %s]", meta ); + super.handleSuccessMessage( meta ); } @Override public void handleRecordMessage( Value[] fields ) { - super.handleRecordMessage( fields ); logger.debug( "S: [RECORD %s]", Arrays.asList( fields ) ); + super.handleRecordMessage( fields ); } @Override public void handleFailureMessage( String code, String message ) { - super.handleFailureMessage( code, message ); logger.debug("S: [FAILURE %s \"%s\"]", code, message ); + super.handleFailureMessage( code, message ); } @Override public void handleIgnoredMessage() { - super.handleIgnoredMessage(); logger.debug( DEFAULT_DEBUG_LOGGING_FORMAT, IGNORED ); + super.handleIgnoredMessage(); } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandlerTest.java b/driver/src/test/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandlerTest.java index e63039c297..4582663cff 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandlerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/connector/socket/LoggingResponseHandlerTest.java @@ -18,10 +18,12 @@ */ package org.neo4j.driver.internal.connector.socket; +import org.junit.Rule; import org.junit.Test; import java.util.HashMap; +import org.junit.rules.ExpectedException; import org.neo4j.driver.internal.logging.DevNullLogger; import org.neo4j.driver.internal.messaging.DiscardAllMessage; import org.neo4j.driver.internal.messaging.FailureMessage; @@ -34,9 +36,13 @@ import org.neo4j.driver.internal.messaging.RunMessage; import org.neo4j.driver.internal.messaging.SuccessMessage; import org.neo4j.driver.internal.spi.StreamCollector; +import org.neo4j.driver.v1.Logger; import org.neo4j.driver.v1.Value; import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.neo4j.driver.v1.Values.parameters; import static org.neo4j.driver.v1.Values.ofValue; @@ -44,15 +50,19 @@ public class LoggingResponseHandlerTest { private String log; - - private LoggingResponseHandler handler = new LoggingResponseHandler( new DevNullLogger() + private Logger debugLogger = new DevNullLogger() { @Override public void debug( String message, Object... params ) { log = String.format( message, params ); } - } ); + }; + + @Rule + public ExpectedException exception = ExpectedException.none(); + + private LoggingResponseHandler handler = new LoggingResponseHandler( debugLogger ); @Test public void shouldLogInitMessage() throws Throwable @@ -159,6 +169,27 @@ public void shouldLogIgnoredMessage() throws Throwable assertEquals( format( new IgnoredMessage() ), log ); } + @Test + public void shouldLogMessageWhenHandleMessageThrowsError() throws Throwable + { + // Given + SocketResponseHandler handler = new LoggingResponseHandler( debugLogger ) + { + @Override + public void handleIgnoredMessage() { + throw new RuntimeException( "This will not stop logging" ); + } + }; + + // When + exception.expect( RuntimeException.class ); + exception.expectMessage( "This will not stop logging" ); + handler.handleIgnoredMessage(); + + // Then + assertEquals( "S: [IGNORED]", log ); + } + private String format( Message message ) {