Skip to content

Commit fa51e5c

Browse files
injectivesinjectives
authored
Add channel error logger for channel errors (#1039)
The new logger provides a dedicated facility for channel error logging in debug and trace modes.
1 parent a82ecbc commit fa51e5c

File tree

5 files changed

+87
-15
lines changed

5 files changed

+87
-15
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/async/connection/HandshakeHandler.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,16 @@
2727
import java.util.List;
2828
import javax.net.ssl.SSLHandshakeException;
2929

30+
import org.neo4j.driver.Logging;
31+
import org.neo4j.driver.exceptions.ClientException;
32+
import org.neo4j.driver.exceptions.SecurityException;
33+
import org.neo4j.driver.exceptions.ServiceUnavailableException;
3034
import org.neo4j.driver.internal.logging.ChannelActivityLogger;
35+
import org.neo4j.driver.internal.logging.ChannelErrorLogger;
3136
import org.neo4j.driver.internal.messaging.BoltProtocol;
3237
import org.neo4j.driver.internal.messaging.BoltProtocolVersion;
3338
import org.neo4j.driver.internal.messaging.MessageFormat;
3439
import org.neo4j.driver.internal.util.ErrorUtil;
35-
import org.neo4j.driver.Logger;
36-
import org.neo4j.driver.Logging;
37-
import org.neo4j.driver.exceptions.ClientException;
38-
import org.neo4j.driver.exceptions.SecurityException;
39-
import org.neo4j.driver.exceptions.ServiceUnavailableException;
4040

4141
import static org.neo4j.driver.internal.async.connection.BoltProtocolUtil.NO_PROTOCOL_VERSION;
4242
import static org.neo4j.driver.internal.messaging.BoltProtocolVersion.isHttp;
@@ -48,10 +48,11 @@ public class HandshakeHandler extends ReplayingDecoder<Void>
4848
private final Logging logging;
4949

5050
private boolean failed;
51-
private Logger log;
51+
private ChannelActivityLogger log;
52+
private ChannelErrorLogger errorLog;
5253

5354
public HandshakeHandler( ChannelPipelineBuilder pipelineBuilder, ChannelPromise handshakeCompletedPromise,
54-
Logging logging )
55+
Logging logging )
5556
{
5657
this.pipelineBuilder = pipelineBuilder;
5758
this.handshakeCompletedPromise = handshakeCompletedPromise;
@@ -62,6 +63,7 @@ public HandshakeHandler( ChannelPipelineBuilder pipelineBuilder, ChannelPromise
6263
public void handlerAdded( ChannelHandlerContext ctx )
6364
{
6465
log = new ChannelActivityLogger( ctx.channel(), logging, getClass() );
66+
errorLog = new ChannelErrorLogger( ctx.channel(), logging );
6567
}
6668

6769
@Override
@@ -89,7 +91,7 @@ public void exceptionCaught( ChannelHandlerContext ctx, Throwable error )
8991
{
9092
if ( failed )
9193
{
92-
log.warn( "Another fatal error occurred in the pipeline", error );
94+
errorLog.traceOrDebug( "Another fatal error occurred in the pipeline", error );
9395
}
9496
else
9597
{

‎driver/src/main/java/org/neo4j/driver/internal/async/inbound/ChannelErrorHandler.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@
2424

2525
import java.io.IOException;
2626

27-
import org.neo4j.driver.Logger;
2827
import org.neo4j.driver.Logging;
2928
import org.neo4j.driver.exceptions.ConnectionReadTimeoutException;
3029
import org.neo4j.driver.exceptions.ServiceUnavailableException;
3130
import org.neo4j.driver.internal.logging.ChannelActivityLogger;
31+
import org.neo4j.driver.internal.logging.ChannelErrorLogger;
3232
import org.neo4j.driver.internal.util.ErrorUtil;
3333

3434
import static java.util.Objects.requireNonNull;
@@ -40,7 +40,8 @@ public class ChannelErrorHandler extends ChannelInboundHandlerAdapter
4040
private final Logging logging;
4141

4242
private InboundMessageDispatcher messageDispatcher;
43-
private Logger log;
43+
private ChannelActivityLogger log;
44+
private ChannelErrorLogger errorLog;
4445
private boolean failed;
4546

4647
public ChannelErrorHandler( Logging logging )
@@ -53,6 +54,7 @@ public void handlerAdded( ChannelHandlerContext ctx )
5354
{
5455
messageDispatcher = requireNonNull( messageDispatcher( ctx.channel() ) );
5556
log = new ChannelActivityLogger( ctx.channel(), logging, getClass() );
57+
errorLog = new ChannelErrorLogger( ctx.channel(), logging );
5658
}
5759

5860
@Override
@@ -90,7 +92,7 @@ public void exceptionCaught( ChannelHandlerContext ctx, Throwable error )
9092
{
9193
if ( failed )
9294
{
93-
log.warn( "Another fatal error occurred in the pipeline", error );
95+
errorLog.traceOrDebug( "Another fatal error occurred in the pipeline", error );
9496
}
9597
else
9698
{
@@ -104,7 +106,7 @@ private void logUnexpectedErrorWarning( Throwable error )
104106
{
105107
if ( !(error instanceof ConnectionReadTimeoutException) )
106108
{
107-
log.warn( "Fatal error occurred in the pipeline", error );
109+
errorLog.traceOrDebug( "Fatal error occurred in the pipeline", error );
108110
}
109111
}
110112

‎driver/src/main/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcher.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.neo4j.driver.exceptions.ClientException;
3333
import org.neo4j.driver.internal.handlers.ResetResponseHandler;
3434
import org.neo4j.driver.internal.logging.ChannelActivityLogger;
35+
import org.neo4j.driver.internal.logging.ChannelErrorLogger;
3536
import org.neo4j.driver.internal.messaging.ResponseMessageHandler;
3637
import org.neo4j.driver.internal.spi.ResponseHandler;
3738
import org.neo4j.driver.internal.util.ErrorUtil;
@@ -46,6 +47,7 @@ public class InboundMessageDispatcher implements ResponseMessageHandler
4647
private final Channel channel;
4748
private final Queue<ResponseHandler> handlers = new LinkedList<>();
4849
private final Logger log;
50+
private final ChannelErrorLogger errorLog;
4951

5052
private volatile boolean gracefullyClosed;
5153
private Throwable currentError;
@@ -58,6 +60,7 @@ public InboundMessageDispatcher( Channel channel, Logging logging )
5860
{
5961
this.channel = requireNonNull( channel );
6062
this.log = new ChannelActivityLogger( channel, logging, getClass() );
63+
this.errorLog = new ChannelErrorLogger( channel, logging );
6164
}
6265

6366
public void enqueue( ResponseHandler handler )
@@ -198,7 +201,7 @@ public void handleChannelError( Throwable error )
198201
handler.onFailure( currentError );
199202
}
200203

201-
log.debug( "Closing channel because of a failure", error );
204+
errorLog.traceOrDebug( "Closing channel because of a failure", error );
202205
channel.close();
203206
}
204207

@@ -287,4 +290,10 @@ Logger getLog()
287290
{
288291
return log;
289292
}
293+
294+
// For testing only
295+
Logger getErrorLog()
296+
{
297+
return errorLog;
298+
}
290299
}

‎driver/src/main/java/org/neo4j/driver/internal/logging/ChannelErrorLogger.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright (c) "Neo4j"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.internal.logging;
20+
21+
import io.netty.channel.Channel;
22+
23+
import org.neo4j.driver.Logging;
24+
25+
/**
26+
* Dedicated logger for channel error logging.
27+
* <p>
28+
* It keeps messages shorter in debug mode and provides more details in trace mode.
29+
*/
30+
public class ChannelErrorLogger extends ChannelActivityLogger
31+
{
32+
private static final String DEBUG_MESSAGE_FORMAT = "%s (%s)";
33+
34+
public ChannelErrorLogger( Channel channel, Logging logging )
35+
{
36+
super( channel, logging, ChannelErrorLogger.class );
37+
}
38+
39+
public void traceOrDebug( String message, Throwable error )
40+
{
41+
if ( isTraceEnabled() )
42+
{
43+
trace( message, error );
44+
}
45+
else
46+
{
47+
debug( String.format( DEBUG_MESSAGE_FORMAT, message, error.getClass() ) );
48+
}
49+
}
50+
}

‎driver/src/test/java/org/neo4j/driver/internal/async/inbound/InboundMessageDispatcherTest.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.neo4j.driver.exceptions.ClientException;
3939
import org.neo4j.driver.exceptions.Neo4jException;
4040
import org.neo4j.driver.internal.logging.ChannelActivityLogger;
41+
import org.neo4j.driver.internal.logging.ChannelErrorLogger;
4142
import org.neo4j.driver.internal.messaging.Message;
4243
import org.neo4j.driver.internal.messaging.response.FailureMessage;
4344
import org.neo4j.driver.internal.messaging.response.IgnoredMessage;
@@ -56,6 +57,7 @@
5657
import static org.mockito.ArgumentMatchers.anyBoolean;
5758
import static org.mockito.ArgumentMatchers.anyString;
5859
import static org.mockito.ArgumentMatchers.argThat;
60+
import static org.mockito.ArgumentMatchers.contains;
5961
import static org.mockito.ArgumentMatchers.eq;
6062
import static org.mockito.Mockito.inOrder;
6163
import static org.mockito.Mockito.mock;
@@ -378,6 +380,8 @@ void shouldCreateChannelActivityLoggerAndLogDebugMessageOnMessageHandling( Class
378380
Logger logger = mock( Logger.class );
379381
when( logger.isDebugEnabled() ).thenReturn( true );
380382
when( logging.getLog( InboundMessageDispatcher.class ) ).thenReturn( logger );
383+
ChannelErrorLogger errorLogger = mock( ChannelErrorLogger.class );
384+
when( logging.getLog( ChannelErrorLogger.class ) ).thenReturn( errorLogger );
381385
InboundMessageDispatcher dispatcher = new InboundMessageDispatcher( channel, logging );
382386
ResponseHandler handler = mock( ResponseHandler.class );
383387
dispatcher.enqueue( handler );
@@ -406,18 +410,22 @@ else if ( IgnoredMessage.class.isAssignableFrom( message ) )
406410

407411
// THEN
408412
assertTrue( dispatcher.getLog() instanceof ChannelActivityLogger );
413+
assertTrue( dispatcher.getErrorLog() instanceof ChannelErrorLogger );
409414
verify( logger ).debug( anyString(), any( Object.class ) );
410415
}
411416

412417
@Test
413-
void shouldCreateChannelActivityLoggerAndLogDebugMessageOnChannelError()
418+
void shouldCreateChannelErrorLoggerAndLogDebugMessageOnChannelError()
414419
{
415420
// GIVEN
416421
Channel channel = newChannelMock();
417422
Logging logging = mock( Logging.class );
418423
Logger logger = mock( Logger.class );
419424
when( logger.isDebugEnabled() ).thenReturn( true );
420425
when( logging.getLog( InboundMessageDispatcher.class ) ).thenReturn( logger );
426+
ChannelErrorLogger errorLogger = mock( ChannelErrorLogger.class );
427+
when( errorLogger.isDebugEnabled() ).thenReturn( true );
428+
when( logging.getLog( ChannelErrorLogger.class ) ).thenReturn( errorLogger );
421429
InboundMessageDispatcher dispatcher = new InboundMessageDispatcher( channel, logging );
422430
ResponseHandler handler = mock( ResponseHandler.class );
423431
dispatcher.enqueue( handler );
@@ -428,7 +436,8 @@ void shouldCreateChannelActivityLoggerAndLogDebugMessageOnChannelError()
428436

429437
// THEN
430438
assertTrue( dispatcher.getLog() instanceof ChannelActivityLogger );
431-
verify( logger ).debug( anyString(), eq( throwable ) );
439+
assertTrue( dispatcher.getErrorLog() instanceof ChannelErrorLogger );
440+
verify( errorLogger ).debug( contains( throwable.getClass().toString() ) );
432441
}
433442

434443
private static void verifyFailure( ResponseHandler handler )

0 commit comments

Comments
 (0)