neo4j · zhenlineo · Dec 2, 2019 · Nov 29, 2019
diff --git a/driver/src/main/java/org/neo4j/driver/exceptions/DiscoveryException.java b/driver/src/main/java/org/neo4j/driver/exceptions/DiscoveryException.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.neo4j.driver.exceptions;
+
+/**
+ * An error has happened while getting routing table with a remote server.
+ * While this error is not fatal and we might be able to recover if we continue trying on another server.
+ * If we fail to get a valid routing table from all routing servers known to this driver,
+ * then we will end up with a fatal error {@link ServiceUnavailableException}.
+ *
+ * If you see this error in your logs, it is safe to ignore if your cluster is temporarily changing structure during that time.
+ */
+public class DiscoveryException extends Neo4jException
+{
+    public DiscoveryException( String message, Throwable cause )
+    {
+        super( message, cause );
+    }
+}
diff --git a/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java b/driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java
@@ -32,6 +32,7 @@
 
 import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.Logger;
+import org.neo4j.driver.exceptions.DiscoveryException;
 import org.neo4j.driver.exceptions.FatalDiscoveryException;
 import org.neo4j.driver.exceptions.SecurityException;
 import org.neo4j.driver.exceptions.ServiceUnavailableException;
@@ -55,6 +56,7 @@
 public class RediscoveryImpl implements Rediscovery
 {
     private static final String NO_ROUTERS_AVAILABLE = "Could not perform discovery for database '%s'. No routing server available.";
+    private static final String RECOVERABLE_ROUTING_ERROR = "Failed to update routing table with server '%s'.";
 
     private final BoltServerAddress initialRouter;
     private final RoutingSettings settings;
@@ -86,14 +88,16 @@ public RediscoveryImpl( BoltServerAddress initialRouter, RoutingSettings setting
     public CompletionStage<ClusterComposition> lookupClusterComposition( RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark )
     {
         CompletableFuture<ClusterComposition> result = new CompletableFuture<>();
-        lookupClusterComposition( routingTable, connectionPool, 0, 0, result, bookmark );
+        // if we failed discovery, we will chain all errors into this one.
+        ServiceUnavailableException baseError = new ServiceUnavailableException( String.format( NO_ROUTERS_AVAILABLE, routingTable.database().description() ) );
+        lookupClusterComposition( routingTable, connectionPool, 0, 0, result, bookmark, baseError );
         return result;
     }
 
     private void lookupClusterComposition( RoutingTable routingTable, ConnectionPool pool,
-            int failures, long previousDelay, CompletableFuture<ClusterComposition> result, Bookmark bookmark )
+            int failures, long previousDelay, CompletableFuture<ClusterComposition> result, Bookmark bookmark, Throwable baseError )
     {
-        lookup( routingTable, pool, bookmark ).whenComplete( ( composition, completionError ) ->
+        lookup( routingTable, pool, bookmark, baseError ).whenComplete( ( composition, completionError ) ->
         {
             Throwable error = Futures.completionExceptionCause( completionError );
             if ( error != null )
@@ -109,67 +113,68 @@ else if ( composition != null )
                 int newFailures = failures + 1;
                 if ( newFailures >= settings.maxRoutingFailures() )
                 {
-                    result.completeExceptionally( new ServiceUnavailableException( String.format( NO_ROUTERS_AVAILABLE, routingTable.database().description() ) ) );
+                    // now we throw our saved error out
+                    result.completeExceptionally( baseError );
                 }
                 else
                 {
                     long nextDelay = Math.max( settings.retryTimeoutDelay(), previousDelay * 2 );
                     logger.info( "Unable to fetch new routing table, will try again in " + nextDelay + "ms" );
                     eventExecutorGroup.next().schedule(
-                            () -> lookupClusterComposition( routingTable, pool, newFailures, nextDelay, result, bookmark ),
+                            () -> lookupClusterComposition( routingTable, pool, newFailures, nextDelay, result, bookmark, baseError ),
                             nextDelay, TimeUnit.MILLISECONDS
                     );
                 }
             }
         } );
     }
 
-    private CompletionStage<ClusterComposition> lookup( RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark )
+    private CompletionStage<ClusterComposition> lookup( RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark, Throwable baseError )
     {
         CompletionStage<ClusterComposition> compositionStage;
 
         if ( routingTable.preferInitialRouter() )
         {
-            compositionStage = lookupOnInitialRouterThenOnKnownRouters( routingTable, connectionPool, bookmark );
+            compositionStage = lookupOnInitialRouterThenOnKnownRouters( routingTable, connectionPool, bookmark, baseError );
         }
         else
         {
-            compositionStage = lookupOnKnownRoutersThenOnInitialRouter( routingTable, connectionPool, bookmark );
+            compositionStage = lookupOnKnownRoutersThenOnInitialRouter( routingTable, connectionPool, bookmark, baseError );
         }
 
         return compositionStage;
     }
 
-    private CompletionStage<ClusterComposition> lookupOnKnownRoutersThenOnInitialRouter( RoutingTable routingTable,
-            ConnectionPool connectionPool, Bookmark bookmark )
+    private CompletionStage<ClusterComposition> lookupOnKnownRoutersThenOnInitialRouter( RoutingTable routingTable, ConnectionPool connectionPool,
+            Bookmark bookmark, Throwable baseError )
     {
         Set<BoltServerAddress> seenServers = new HashSet<>();
-        return lookupOnKnownRouters( routingTable, connectionPool, seenServers, bookmark ).thenCompose( composition ->
+        return lookupOnKnownRouters( routingTable, connectionPool, seenServers, bookmark, baseError ).thenCompose( composition ->
         {
             if ( composition != null )
             {
                 return completedFuture( composition );
             }
-            return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark );
+            return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark, baseError );
         } );
     }
 
     private CompletionStage<ClusterComposition> lookupOnInitialRouterThenOnKnownRouters( RoutingTable routingTable,
-            ConnectionPool connectionPool, Bookmark bookmark )
+            ConnectionPool connectionPool, Bookmark bookmark, Throwable baseError )
     {
         Set<BoltServerAddress> seenServers = emptySet();
-        return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark ).thenCompose( composition ->
+        return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark, baseError ).thenCompose( composition ->
         {
             if ( composition != null )
             {
                 return completedFuture( composition );
             }
-            return lookupOnKnownRouters( routingTable, connectionPool, new HashSet<>(), bookmark );
+            return lookupOnKnownRouters( routingTable, connectionPool, new HashSet<>(), bookmark, baseError );
         } );
     }
 
-    private CompletionStage<ClusterComposition> lookupOnKnownRouters( RoutingTable routingTable,
-            ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark )
+    private CompletionStage<ClusterComposition> lookupOnKnownRouters( RoutingTable routingTable, ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark,
+            Throwable baseError )
     {
         BoltServerAddress[] addresses = routingTable.routers().toArray();
 
@@ -184,16 +189,16 @@ private CompletionStage<ClusterComposition> lookupOnKnownRouters( RoutingTable r
                 }
                 else
                 {
-                    return lookupOnRouter( address, routingTable, connectionPool, bookmark )
+                    return lookupOnRouter( address, routingTable, connectionPool, bookmark, baseError )
                             .whenComplete( ( ignore, error ) -> seenServers.add( address ) );
                 }
             } );
         }
         return result;
     }
 
-    private CompletionStage<ClusterComposition> lookupOnInitialRouter( RoutingTable routingTable,
-            ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark )
+    private CompletionStage<ClusterComposition> lookupOnInitialRouter( RoutingTable routingTable, ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark,
+            Throwable baseError )
     {
         List<BoltServerAddress> addresses;
         try
@@ -215,14 +220,14 @@ private CompletionStage<ClusterComposition> lookupOnInitialRouter( RoutingTable
                 {
                     return completedFuture( composition );
                 }
-                return lookupOnRouter( address, routingTable, connectionPool, bookmark );
+                return lookupOnRouter( address, routingTable, connectionPool, bookmark, baseError );
             } );
         }
         return result;
     }
 
     private CompletionStage<ClusterComposition> lookupOnRouter( BoltServerAddress routerAddress,
-            RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark )
+            RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark, Throwable baseError )
     {
         CompletionStage<Connection> connectionStage = connectionPool.acquire( routerAddress );
 
@@ -232,7 +237,7 @@ private CompletionStage<ClusterComposition> lookupOnRouter( BoltServerAddress ro
                     Throwable cause = Futures.completionExceptionCause( error );
                     if ( cause != null )
                     {
-                        return handleRoutingProcedureError( cause, routingTable, routerAddress );
+                        return handleRoutingProcedureError( cause, routingTable, routerAddress, baseError );
                     }
                     else
                     {
@@ -242,7 +247,7 @@ private CompletionStage<ClusterComposition> lookupOnRouter( BoltServerAddress ro
     }
 
     private ClusterComposition handleRoutingProcedureError( Throwable error, RoutingTable routingTable,
-            BoltServerAddress routerAddress )
+            BoltServerAddress routerAddress, Throwable baseError )
     {
         if ( error instanceof SecurityException || error instanceof FatalDiscoveryException )
         {
@@ -251,7 +256,10 @@ private ClusterComposition handleRoutingProcedureError( Throwable error, Routing
         }
 
         // Retriable error happened during discovery.
-        logger.warn( format( "Failed to update routing table with server '%s'.", routerAddress ), error );
+        DiscoveryException discoveryError = new DiscoveryException( format( RECOVERABLE_ROUTING_ERROR, routerAddress ), error );
+        Futures.combineErrors( baseError, discoveryError ); // we record each failure here
+        logger.warn( format( "Received a recoverable discovery error with server '%s', will continue discovery with other routing servers if available.",
+                routerAddress ), discoveryError );
         routingTable.forget( routerAddress );
         return null;
     }

diff --git a/driver/src/main/java/org/neo4j/driver/internal/util/ErrorUtil.java b/driver/src/main/java/org/neo4j/driver/internal/util/ErrorUtil.java
@@ -50,8 +50,8 @@ public static ServiceUnavailableException newConnectionTerminatedError( String r
     public static ServiceUnavailableException newConnectionTerminatedError()
     {
         return new ServiceUnavailableException( "Connection to the database terminated. " +
-                                                "This can happen due to network instabilities, " +
-                                                "or due to restarts of the database" );
+                "Please ensure that your database is listening on the correct host and port and that you have compatible encryption settings both on Neo4j server and driver. " +
+                "Note that the default encryption setting has changed in Neo4j 4.0." );
     }
 
     public static ResultConsumedException newResultConsumedError()

diff --git a/driver/src/test/java/org/neo4j/driver/integration/EncryptionIT.java b/driver/src/test/java/org/neo4j/driver/integration/EncryptionIT.java
@@ -25,8 +25,8 @@
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.GraphDatabase;
 import org.neo4j.driver.Record;
-import org.neo4j.driver.Session;
 import org.neo4j.driver.Result;
+import org.neo4j.driver.Session;
 import org.neo4j.driver.exceptions.ServiceUnavailableException;
 import org.neo4j.driver.util.DatabaseExtension;
 import org.neo4j.driver.util.Neo4jSettings;
@@ -35,7 +35,6 @@
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.startsWith;
-import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.Config.TrustStrategy.trustAllCertificates;
@@ -107,10 +106,9 @@ private void testMismatchingEncryption( BoltTlsLevel tlsLevel, boolean driverEnc
         neo4j.restartDb( Neo4jSettings.TEST_SETTINGS.updateWith( Neo4jSettings.BOLT_TLS_LEVEL, tlsLevel.toString() ) );
         Config config = newConfig( driverEncrypted );
 
-        RuntimeException e = assertThrows( RuntimeException.class,
+        ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class,
                 () -> GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), config ).verifyConnectivity() );
 
-        assertThat( e, instanceOf( ServiceUnavailableException.class ) );
         assertThat( e.getMessage(), startsWith( "Connection to the database terminated" ) );
     }
 

diff --git a/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java b/driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java
@@ -20,13 +20,15 @@
 
 import io.netty.util.concurrent.GlobalEventExecutor;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
 import org.neo4j.driver.Logger;
 import org.neo4j.driver.exceptions.AuthenticationException;
+import org.neo4j.driver.exceptions.DiscoveryException;
 import org.neo4j.driver.exceptions.ProtocolException;
 import org.neo4j.driver.exceptions.ServiceUnavailableException;
 import org.neo4j.driver.exceptions.SessionExpiredException;
@@ -44,11 +46,13 @@
 import static java.util.Collections.singletonMap;
 import static java.util.concurrent.CompletableFuture.completedFuture;
 import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.startsWith;
@@ -178,7 +182,9 @@ void shouldFailImmediatelyWhenClusterCompositionProviderReturnsFailure()
         ClusterComposition composition = await( rediscovery.lookupClusterComposition( table, pool, empty() ) );
         assertEquals( validComposition, composition );
 
-        verify( logger ).warn( String.format( "Failed to update routing table with server '%s'.", B ), protocolError );
+        ArgumentCaptor<DiscoveryException> argument = ArgumentCaptor.forClass( DiscoveryException.class );
+        verify( logger ).warn( anyString(), argument.capture() );
+        assertThat( argument.getValue().getCause(), equalTo( protocolError ) );
     }
 
     @Test
@@ -260,19 +266,26 @@ void shouldPropagateFailureWhenResolverFails()
     }
 
     @Test
-    void shouldFailWhenNoRoutersRespond()
+    void shouldRecordAllErrorsWhenNoRouterRespond()
     {
         Map<BoltServerAddress,Object> responsesByAddress = new HashMap<>();
-        responsesByAddress.put( A, new ServiceUnavailableException( "Hi!" ) ); // first -> non-fatal failure
-        responsesByAddress.put( B, new SessionExpiredException( "Hi!" ) ); // second -> non-fatal failure
-        responsesByAddress.put( C, new IOException( "Hi!" ) ); // third -> non-fatal failure
+        ServiceUnavailableException first = new ServiceUnavailableException( "Hi!" );
+        responsesByAddress.put( A, first ); // first -> non-fatal failure
+        SessionExpiredException second = new SessionExpiredException( "Hi!" );
+        responsesByAddress.put( B, second ); // second -> non-fatal failure
+        IOException third = new IOException( "Hi!" );
+        responsesByAddress.put( C, third ); // third -> non-fatal failure
 
         ClusterCompositionProvider compositionProvider = compositionProviderMock( responsesByAddress );
         Rediscovery rediscovery = newRediscovery( A, compositionProvider, mock( ServerAddressResolver.class ) );
         RoutingTable table = routingTableMock( A, B, C );
 
         ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class, () -> await( rediscovery.lookupClusterComposition( table, pool, empty() ) ) );
         assertThat( e.getMessage(), containsString( "Could not perform discovery" ) );
+        assertThat( e.getSuppressed().length, equalTo( 3 ) );
+        assertThat( e.getSuppressed()[0].getCause(), equalTo( first ) );
+        assertThat( e.getSuppressed()[1].getCause(), equalTo( second ) );
+        assertThat( e.getSuppressed()[2].getCause(), equalTo( third ) );
     }
 
     @Test