Merge pull request #236 from zhenlineo/1.0-coredge

Better error message when running statements against incompatible server

Author: Zhen Li

driver/src/main/java/org/neo4j/driver/internal/pool/PooledConnection.java

@@ -25,7 +25,11 @@
 import org.neo4j.driver.internal.util.Clock;
 import org.neo4j.driver.internal.util.Consumer;
 import org.neo4j.driver.v1.Value;
+import org.neo4j.driver.v1.exceptions.ClientException;
 import org.neo4j.driver.v1.exceptions.Neo4jException;
+
+import static java.lang.String.format;
+
 /**
  * The state of a pooledConnection from a pool point of view could be one of the following:
  * Created,
@@ -91,7 +95,7 @@ public void run( String statement, Map<String,Value> parameters,
         {
             delegate.run( statement, parameters, collector );
         }
-        catch(RuntimeException e)
+        catch( RuntimeException e )
         {
             onDelegateException( e );
         }
@@ -104,7 +108,7 @@ public void discardAll()
         {
             delegate.discardAll();
         }
-        catch(RuntimeException e)
+        catch( RuntimeException e )
         {
             onDelegateException( e );
         }
@@ -117,7 +121,7 @@ public void pullAll( StreamCollector collector )
         {
             delegate.pullAll( collector );
         }
-        catch(RuntimeException e)
+        catch( RuntimeException e )
         {
             onDelegateException( e );
         }
@@ -224,9 +228,18 @@ public void dispose()
      */
     private void onDelegateException( RuntimeException e )
     {
-        if ( !isClientOrTransientError( e ) || isProtocolViolationError( e ) )
+        if ( isUnrecoverableErrorsOccurred( e ) )
         {
             unrecoverableErrorsOccurred = true;
+            if( isClusterError( e ) )
+            {
+                e = new ClientException( format(
+                        "Failed to run a statement on a 3.1+ Neo4j core-edge cluster server. %n" +
+                        "Please retry the statement if you have defined your own routing strategy to route driver messages to the cluster. " +
+                        "Note: 1.0 java driver does not support routing statements to a 3.1+ Neo4j core edge cluster. " +
+                        "Please upgrade to 1.1 driver and use `bolt+routing` scheme to route and run statements " +
+                        "directly to a 3.1+ core edge cluster." ), e );
+            }
         }
         else
         {
@@ -245,18 +258,27 @@ public void onError( Runnable runnable )
         this.onError = runnable;
     }
 
-    private boolean isProtocolViolationError(RuntimeException e )
+    private boolean isUnrecoverableErrorsOccurred( RuntimeException e )
     {
-        return e instanceof Neo4jException
-               && ((Neo4jException) e).neo4jErrorCode().startsWith( "Neo.ClientError.Request" );
+        return !isClientOrTransientError( e ) || isProtocolViolationError( e ) || isClusterError( e );
+    }
+
+    private boolean isProtocolViolationError( RuntimeException e )
+    {
+        return ( e instanceof Neo4jException ) &&
+                ( ( Neo4jException ) e ).neo4jErrorCode().startsWith( "Neo.ClientError.Request" );
     }
 
     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).neo4jErrorCode().contains( "ClientError" )
-                   || ((Neo4jException) e).neo4jErrorCode().contains( "TransientError" ));
+        return ( e instanceof Neo4jException ) && ( ( Neo4jException ) e ).neo4jErrorCode().contains( "ClientError" )
+                || ( e instanceof Neo4jException ) && ( ( Neo4jException ) e ).neo4jErrorCode().contains( "TransientError" );
+    }
+
+    private boolean isClusterError( RuntimeException e )
+    {
+        return ( e instanceof Neo4jException ) && ( ( Neo4jException ) e ).neo4jErrorCode().contains( "Cluster" );
     }
 
     public long idleTime()

driver/src/test/java/org/neo4j/driver/internal/pool/ConnectionInvalidationTest.java

@@ -36,6 +36,7 @@
 import org.neo4j.driver.v1.exceptions.Neo4jException;
 import org.neo4j.driver.v1.exceptions.TransientException;
 
+import static java.lang.String.format;
 import static junit.framework.TestCase.assertFalse;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -124,6 +125,29 @@ public void shouldInvalidateOnUnrecoverableProblems() throws Throwable
         assertUnrecoverable( new ClientException( "Hello, world!" ) );
     }
 
+    @Test
+    public void shouldInvalidateOnUnrecoverableClusterErrors() throws Throwable
+    {
+        // Given
+        String clientErrorMessage = format( "Failed to run a statement on a 3.1+ Neo4j core-edge cluster server. %n" +
+                        "Please retry the statement if you have defined your own routing strategy to route driver messages to the cluster. " +
+                        "Note: 1.0 java driver does not support routing statements to a 3.1+ Neo4j core edge cluster. " +
+                        "Please upgrade to 1.1 driver and use `bolt+routing` scheme to route and run statements " +
+                        "directly to a 3.1+ core edge cluster." );
+        ClientException serverException = new ClientException( "Neo.ClientError.Cluster.NotALeader", "Hello, world!" );
+        ClientException clientException = new ClientException( clientErrorMessage, serverException );
+
+        // When/Then
+        assertUnrecoverable( serverException, clientException );
+
+        // Given
+        serverException = new ClientException( "Neo.TransientError.Cluster.NoLeaderAvailable", "Hello, world" );
+        clientException = new ClientException( clientErrorMessage, serverException );
+
+        // When/Then
+        assertUnrecoverable( serverException, clientException );
+    }
+
     @Test
     public void shouldNotInvalidateOnKnownRecoverableExceptions() throws Throwable
     {
@@ -138,8 +162,13 @@ public void shouldInvalidateOnProtocolViolationExceptions() throws Throwable
         assertUnrecoverable( new ClientException( "Neo.ClientError.Request.Invalid", "Hello, world!" ) );
     }
 
-    @SuppressWarnings( "unchecked" )
     private void assertUnrecoverable( Neo4jException exception )
+    {
+        assertUnrecoverable( exception, exception );
+    }
+
+    @SuppressWarnings( "unchecked" )
+    private void assertUnrecoverable( Neo4jException exception, Neo4jException expectingException )
     {
         doThrow( exception ).when( delegate )
                 .run( eq("assert unrecoverable"), anyMap(), any( StreamCollector.class ) );
@@ -152,7 +181,8 @@ private void assertUnrecoverable( Neo4jException exception )
         }
         catch ( Neo4jException e )
         {
-            assertThat( e, equalTo( exception ) );
+            assertThat( e.getMessage(), equalTo( expectingException.getMessage() ) );
+            assertThat( e.getCause(), equalTo( expectingException.getCause() ) );
         }
 
         // Then