Null-safe error code checks in connection layer

Author: lutovich

driver/src/main/java/org/neo4j/driver/internal/cluster/RoutingPooledConnection.java

@@ -19,6 +19,7 @@
 package org.neo4j.driver.internal.cluster;
 
 import java.util.Map;
+import java.util.Objects;
 
 import org.neo4j.driver.internal.RoutingErrorHandler;
 import org.neo4j.driver.internal.net.BoltServerAddress;
@@ -283,7 +284,8 @@ private RuntimeException handledClientException( ClientException e )
 
     private static boolean isFailureToWrite( ClientException e )
     {
-        return e.code().equals( "Neo.ClientError.Cluster.NotALeader" ) ||
-               e.code().equals( "Neo.ClientError.General.ForbiddenOnReadOnlyDatabase" );
+        String errorCode = e.code();
+        return Objects.equals( errorCode, "Neo.ClientError.Cluster.NotALeader" ) ||
+               Objects.equals( errorCode, "Neo.ClientError.General.ForbiddenOnReadOnlyDatabase" );
     }
 }

driver/src/main/java/org/neo4j/driver/internal/net/pooling/PooledSocketConnection.java

@@ -291,18 +291,31 @@ public long lastUsedTimestamp()
         return lastUsedTimestamp;
     }
 
-    private boolean isProtocolViolationError(RuntimeException e )
+    private boolean isProtocolViolationError( RuntimeException e )
     {
-        return e instanceof Neo4jException
-               && ((Neo4jException) e).code().startsWith( "Neo.ClientError.Request" );
+        if ( e instanceof Neo4jException )
+        {
+            String errorCode = ((Neo4jException) e).code();
+            if ( errorCode != null )
+            {
+                return errorCode.startsWith( "Neo.ClientError.Request" );
+            }
+        }
+        return false;
     }
 
     private boolean isClientOrTransientError( RuntimeException e )
     {
         // Eg: DatabaseErrors and unknown (no status code or not neo4j exception) cause session to be discarded
-        return e instanceof Neo4jException
-               && (((Neo4jException) e).code().contains( "ClientError" )
-                   || ((Neo4jException) e).code().contains( "TransientError" ));
+        if ( e instanceof Neo4jException )
+        {
+            String errorCode = ((Neo4jException) e).code();
+            if ( errorCode != null )
+            {
+                return errorCode.contains( "ClientError" ) || errorCode.contains( "TransientError" );
+            }
+        }
+        return false;
     }
 
     private void updateLastUsedTimestamp()

driver/src/test/java/org/neo4j/driver/internal/cluster/RoutingPooledConnectionErrorHandlingTest.java

@@ -127,20 +127,13 @@ public void shouldHandleFailureToWrite()
     @Test
     public void shouldPropagateThrowable()
     {
-        RuntimeException error = new RuntimeException( "Random error" );
-        Connector connector = newConnectorWithThrowingConnections( error );
-        RoutingTable routingTable = newRoutingTable( ADDRESS1, ADDRESS2, ADDRESS3 );
-        ConnectionPool connectionPool = newConnectionPool( connector, ADDRESS1, ADDRESS2, ADDRESS3 );
-        LoadBalancer loadBalancer = newLoadBalancer( routingTable, connectionPool );
-
-        Connection readConnection = loadBalancer.acquireReadConnection();
-        verifyThrowablePropagation( readConnection, method, routingTable, connectionPool );
-
-        Connection writeConnection = loadBalancer.acquireWriteConnection();
-        verifyThrowablePropagation( writeConnection, method, routingTable, connectionPool );
+        testThrowablePropagation( new RuntimeException( "Random error" ) );
+    }
 
-        assertThat( routingTable, containsRouter( ADDRESS3 ) );
-        assertTrue( connectionPool.hasAddress( ADDRESS3 ) );
+    @Test
+    public void shouldPropagateClientExceptionWithoutErrorCode()
+    {
+        testThrowablePropagation( new ClientException( null, "Message" ) );
     }
 
     private void testHandleFailureToWriteWithWriteConnection( ClientException error )
@@ -199,6 +192,23 @@ private void testHandleFailureToWrite( ClientException error )
         assertTrue( connectionPool.hasAddress( ADDRESS3 ) );
     }
 
+    private void testThrowablePropagation( Throwable error )
+    {
+        Connector connector = newConnectorWithThrowingConnections( error );
+        RoutingTable routingTable = newRoutingTable( ADDRESS1, ADDRESS2, ADDRESS3 );
+        ConnectionPool connectionPool = newConnectionPool( connector, ADDRESS1, ADDRESS2, ADDRESS3 );
+        LoadBalancer loadBalancer = newLoadBalancer( routingTable, connectionPool );
+
+        Connection readConnection = loadBalancer.acquireReadConnection();
+        verifyThrowablePropagation( readConnection, routingTable, connectionPool, error.getClass() );
+
+        Connection writeConnection = loadBalancer.acquireWriteConnection();
+        verifyThrowablePropagation( writeConnection, routingTable, connectionPool, error.getClass() );
+
+        assertThat( routingTable, containsRouter( ADDRESS3 ) );
+        assertTrue( connectionPool.hasAddress( ADDRESS3 ) );
+    }
+
     private void verifyServiceUnavailableHandling( Connection connection, RoutingTable routingTable,
             ConnectionPool connectionPool )
     {
@@ -220,8 +230,8 @@ private void verifyServiceUnavailableHandling( Connection connection, RoutingTab
         }
     }
 
-    private void verifyThrowablePropagation( Connection connection, ConnectionMethod method, RoutingTable routingTable,
-            ConnectionPool connectionPool )
+    private <T extends Throwable> void verifyThrowablePropagation( Connection connection, RoutingTable routingTable,
+            ConnectionPool connectionPool, Class<T> expectedClass )
     {
         try
         {
@@ -230,7 +240,7 @@ private void verifyThrowablePropagation( Connection connection, ConnectionMethod
         }
         catch ( Exception e )
         {
-            assertThat( e, instanceOf( RuntimeException.class ) );
+            assertThat( e, instanceOf( expectedClass ) );
 
             BoltServerAddress address = connection.boltServerAddress();
             assertThat( routingTable, containsRouter( address ) );