Skip to content

Commit ae67ce3

Browse files
authored
Merge pull request #655 from zhenlineo/4.0-error-messages
Improved error messages on driver.
2 parents 10b7d43 + ad03711 commit ae67ce3

File tree

5 files changed

+90
-36
lines changed

5 files changed

+90
-36
lines changed
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright (c) 2002-2019 "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.exceptions;
20+
21+
/**
22+
* An error has happened while getting routing table with a remote server.
23+
* While this error is not fatal and we might be able to recover if we continue trying on another server.
24+
* If we fail to get a valid routing table from all routing servers known to this driver,
25+
* then we will end up with a fatal error {@link ServiceUnavailableException}.
26+
*
27+
* If you see this error in your logs, it is safe to ignore if your cluster is temporarily changing structure during that time.
28+
*/
29+
public class DiscoveryException extends Neo4jException
30+
{
31+
public DiscoveryException( String message, Throwable cause )
32+
{
33+
super( message, cause );
34+
}
35+
}

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

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import org.neo4j.driver.Bookmark;
3434
import org.neo4j.driver.Logger;
35+
import org.neo4j.driver.exceptions.DiscoveryException;
3536
import org.neo4j.driver.exceptions.FatalDiscoveryException;
3637
import org.neo4j.driver.exceptions.SecurityException;
3738
import org.neo4j.driver.exceptions.ServiceUnavailableException;
@@ -55,6 +56,7 @@
5556
public class RediscoveryImpl implements Rediscovery
5657
{
5758
private static final String NO_ROUTERS_AVAILABLE = "Could not perform discovery for database '%s'. No routing server available.";
59+
private static final String RECOVERABLE_ROUTING_ERROR = "Failed to update routing table with server '%s'.";
5860

5961
private final BoltServerAddress initialRouter;
6062
private final RoutingSettings settings;
@@ -86,14 +88,16 @@ public RediscoveryImpl( BoltServerAddress initialRouter, RoutingSettings setting
8688
public CompletionStage<ClusterComposition> lookupClusterComposition( RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark )
8789
{
8890
CompletableFuture<ClusterComposition> result = new CompletableFuture<>();
89-
lookupClusterComposition( routingTable, connectionPool, 0, 0, result, bookmark );
91+
// if we failed discovery, we will chain all errors into this one.
92+
ServiceUnavailableException baseError = new ServiceUnavailableException( String.format( NO_ROUTERS_AVAILABLE, routingTable.database().description() ) );
93+
lookupClusterComposition( routingTable, connectionPool, 0, 0, result, bookmark, baseError );
9094
return result;
9195
}
9296

9397
private void lookupClusterComposition( RoutingTable routingTable, ConnectionPool pool,
94-
int failures, long previousDelay, CompletableFuture<ClusterComposition> result, Bookmark bookmark )
98+
int failures, long previousDelay, CompletableFuture<ClusterComposition> result, Bookmark bookmark, Throwable baseError )
9599
{
96-
lookup( routingTable, pool, bookmark ).whenComplete( ( composition, completionError ) ->
100+
lookup( routingTable, pool, bookmark, baseError ).whenComplete( ( composition, completionError ) ->
97101
{
98102
Throwable error = Futures.completionExceptionCause( completionError );
99103
if ( error != null )
@@ -109,67 +113,68 @@ else if ( composition != null )
109113
int newFailures = failures + 1;
110114
if ( newFailures >= settings.maxRoutingFailures() )
111115
{
112-
result.completeExceptionally( new ServiceUnavailableException( String.format( NO_ROUTERS_AVAILABLE, routingTable.database().description() ) ) );
116+
// now we throw our saved error out
117+
result.completeExceptionally( baseError );
113118
}
114119
else
115120
{
116121
long nextDelay = Math.max( settings.retryTimeoutDelay(), previousDelay * 2 );
117122
logger.info( "Unable to fetch new routing table, will try again in " + nextDelay + "ms" );
118123
eventExecutorGroup.next().schedule(
119-
() -> lookupClusterComposition( routingTable, pool, newFailures, nextDelay, result, bookmark ),
124+
() -> lookupClusterComposition( routingTable, pool, newFailures, nextDelay, result, bookmark, baseError ),
120125
nextDelay, TimeUnit.MILLISECONDS
121126
);
122127
}
123128
}
124129
} );
125130
}
126131

127-
private CompletionStage<ClusterComposition> lookup( RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark )
132+
private CompletionStage<ClusterComposition> lookup( RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark, Throwable baseError )
128133
{
129134
CompletionStage<ClusterComposition> compositionStage;
130135

131136
if ( routingTable.preferInitialRouter() )
132137
{
133-
compositionStage = lookupOnInitialRouterThenOnKnownRouters( routingTable, connectionPool, bookmark );
138+
compositionStage = lookupOnInitialRouterThenOnKnownRouters( routingTable, connectionPool, bookmark, baseError );
134139
}
135140
else
136141
{
137-
compositionStage = lookupOnKnownRoutersThenOnInitialRouter( routingTable, connectionPool, bookmark );
142+
compositionStage = lookupOnKnownRoutersThenOnInitialRouter( routingTable, connectionPool, bookmark, baseError );
138143
}
139144

140145
return compositionStage;
141146
}
142147

143-
private CompletionStage<ClusterComposition> lookupOnKnownRoutersThenOnInitialRouter( RoutingTable routingTable,
144-
ConnectionPool connectionPool, Bookmark bookmark )
148+
private CompletionStage<ClusterComposition> lookupOnKnownRoutersThenOnInitialRouter( RoutingTable routingTable, ConnectionPool connectionPool,
149+
Bookmark bookmark, Throwable baseError )
145150
{
146151
Set<BoltServerAddress> seenServers = new HashSet<>();
147-
return lookupOnKnownRouters( routingTable, connectionPool, seenServers, bookmark ).thenCompose( composition ->
152+
return lookupOnKnownRouters( routingTable, connectionPool, seenServers, bookmark, baseError ).thenCompose( composition ->
148153
{
149154
if ( composition != null )
150155
{
151156
return completedFuture( composition );
152157
}
153-
return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark );
158+
return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark, baseError );
154159
} );
155160
}
156161

157162
private CompletionStage<ClusterComposition> lookupOnInitialRouterThenOnKnownRouters( RoutingTable routingTable,
158-
ConnectionPool connectionPool, Bookmark bookmark )
163+
ConnectionPool connectionPool, Bookmark bookmark, Throwable baseError )
159164
{
160165
Set<BoltServerAddress> seenServers = emptySet();
161-
return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark ).thenCompose( composition ->
166+
return lookupOnInitialRouter( routingTable, connectionPool, seenServers, bookmark, baseError ).thenCompose( composition ->
162167
{
163168
if ( composition != null )
164169
{
165170
return completedFuture( composition );
166171
}
167-
return lookupOnKnownRouters( routingTable, connectionPool, new HashSet<>(), bookmark );
172+
return lookupOnKnownRouters( routingTable, connectionPool, new HashSet<>(), bookmark, baseError );
168173
} );
169174
}
170175

171-
private CompletionStage<ClusterComposition> lookupOnKnownRouters( RoutingTable routingTable,
172-
ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark )
176+
private CompletionStage<ClusterComposition> lookupOnKnownRouters( RoutingTable routingTable, ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark,
177+
Throwable baseError )
173178
{
174179
BoltServerAddress[] addresses = routingTable.routers().toArray();
175180

@@ -184,16 +189,16 @@ private CompletionStage<ClusterComposition> lookupOnKnownRouters( RoutingTable r
184189
}
185190
else
186191
{
187-
return lookupOnRouter( address, routingTable, connectionPool, bookmark )
192+
return lookupOnRouter( address, routingTable, connectionPool, bookmark, baseError )
188193
.whenComplete( ( ignore, error ) -> seenServers.add( address ) );
189194
}
190195
} );
191196
}
192197
return result;
193198
}
194199

195-
private CompletionStage<ClusterComposition> lookupOnInitialRouter( RoutingTable routingTable,
196-
ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark )
200+
private CompletionStage<ClusterComposition> lookupOnInitialRouter( RoutingTable routingTable, ConnectionPool connectionPool, Set<BoltServerAddress> seenServers, Bookmark bookmark,
201+
Throwable baseError )
197202
{
198203
List<BoltServerAddress> addresses;
199204
try
@@ -215,14 +220,14 @@ private CompletionStage<ClusterComposition> lookupOnInitialRouter( RoutingTable
215220
{
216221
return completedFuture( composition );
217222
}
218-
return lookupOnRouter( address, routingTable, connectionPool, bookmark );
223+
return lookupOnRouter( address, routingTable, connectionPool, bookmark, baseError );
219224
} );
220225
}
221226
return result;
222227
}
223228

224229
private CompletionStage<ClusterComposition> lookupOnRouter( BoltServerAddress routerAddress,
225-
RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark )
230+
RoutingTable routingTable, ConnectionPool connectionPool, Bookmark bookmark, Throwable baseError )
226231
{
227232
CompletionStage<Connection> connectionStage = connectionPool.acquire( routerAddress );
228233

@@ -232,7 +237,7 @@ private CompletionStage<ClusterComposition> lookupOnRouter( BoltServerAddress ro
232237
Throwable cause = Futures.completionExceptionCause( error );
233238
if ( cause != null )
234239
{
235-
return handleRoutingProcedureError( cause, routingTable, routerAddress );
240+
return handleRoutingProcedureError( cause, routingTable, routerAddress, baseError );
236241
}
237242
else
238243
{
@@ -242,7 +247,7 @@ private CompletionStage<ClusterComposition> lookupOnRouter( BoltServerAddress ro
242247
}
243248

244249
private ClusterComposition handleRoutingProcedureError( Throwable error, RoutingTable routingTable,
245-
BoltServerAddress routerAddress )
250+
BoltServerAddress routerAddress, Throwable baseError )
246251
{
247252
if ( error instanceof SecurityException || error instanceof FatalDiscoveryException )
248253
{
@@ -251,7 +256,10 @@ private ClusterComposition handleRoutingProcedureError( Throwable error, Routing
251256
}
252257

253258
// Retriable error happened during discovery.
254-
logger.warn( format( "Failed to update routing table with server '%s'.", routerAddress ), error );
259+
DiscoveryException discoveryError = new DiscoveryException( format( RECOVERABLE_ROUTING_ERROR, routerAddress ), error );
260+
Futures.combineErrors( baseError, discoveryError ); // we record each failure here
261+
logger.warn( format( "Received a recoverable discovery error with server '%s', will continue discovery with other routing servers if available.",
262+
routerAddress ), discoveryError );
255263
routingTable.forget( routerAddress );
256264
return null;
257265
}

driver/src/main/java/org/neo4j/driver/internal/util/ErrorUtil.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ public static ServiceUnavailableException newConnectionTerminatedError( String r
5050
public static ServiceUnavailableException newConnectionTerminatedError()
5151
{
5252
return new ServiceUnavailableException( "Connection to the database terminated. " +
53-
"This can happen due to network instabilities, " +
54-
"or due to restarts of the database" );
53+
"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. " +
54+
"Note that the default encryption setting has changed in Neo4j 4.0." );
5555
}
5656

5757
public static ResultConsumedException newResultConsumedError()

driver/src/test/java/org/neo4j/driver/integration/EncryptionIT.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
import org.neo4j.driver.Driver;
2626
import org.neo4j.driver.GraphDatabase;
2727
import org.neo4j.driver.Record;
28-
import org.neo4j.driver.Session;
2928
import org.neo4j.driver.Result;
29+
import org.neo4j.driver.Session;
3030
import org.neo4j.driver.exceptions.ServiceUnavailableException;
3131
import org.neo4j.driver.util.DatabaseExtension;
3232
import org.neo4j.driver.util.Neo4jSettings;
@@ -35,7 +35,6 @@
3535

3636
import static org.hamcrest.CoreMatchers.equalTo;
3737
import static org.hamcrest.CoreMatchers.startsWith;
38-
import static org.hamcrest.Matchers.instanceOf;
3938
import static org.hamcrest.junit.MatcherAssert.assertThat;
4039
import static org.junit.jupiter.api.Assertions.assertThrows;
4140
import static org.neo4j.driver.Config.TrustStrategy.trustAllCertificates;
@@ -107,10 +106,9 @@ private void testMismatchingEncryption( BoltTlsLevel tlsLevel, boolean driverEnc
107106
neo4j.restartDb( Neo4jSettings.TEST_SETTINGS.updateWith( Neo4jSettings.BOLT_TLS_LEVEL, tlsLevel.toString() ) );
108107
Config config = newConfig( driverEncrypted );
109108

110-
RuntimeException e = assertThrows( RuntimeException.class,
109+
ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class,
111110
() -> GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), config ).verifyConnectivity() );
112111

113-
assertThat( e, instanceOf( ServiceUnavailableException.class ) );
114112
assertThat( e.getMessage(), startsWith( "Connection to the database terminated" ) );
115113
}
116114

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@
2020

2121
import io.netty.util.concurrent.GlobalEventExecutor;
2222
import org.junit.jupiter.api.Test;
23+
import org.mockito.ArgumentCaptor;
2324

2425
import java.io.IOException;
2526
import java.util.HashMap;
2627
import java.util.Map;
2728

2829
import org.neo4j.driver.Logger;
2930
import org.neo4j.driver.exceptions.AuthenticationException;
31+
import org.neo4j.driver.exceptions.DiscoveryException;
3032
import org.neo4j.driver.exceptions.ProtocolException;
3133
import org.neo4j.driver.exceptions.ServiceUnavailableException;
3234
import org.neo4j.driver.exceptions.SessionExpiredException;
@@ -44,11 +46,13 @@
4446
import static java.util.Collections.singletonMap;
4547
import static java.util.concurrent.CompletableFuture.completedFuture;
4648
import static org.hamcrest.CoreMatchers.containsString;
49+
import static org.hamcrest.CoreMatchers.equalTo;
4750
import static org.junit.Assert.assertThat;
4851
import static org.junit.jupiter.api.Assertions.assertEquals;
4952
import static org.junit.jupiter.api.Assertions.assertNotNull;
5053
import static org.junit.jupiter.api.Assertions.assertThrows;
5154
import static org.mockito.ArgumentMatchers.any;
55+
import static org.mockito.ArgumentMatchers.anyString;
5256
import static org.mockito.Mockito.mock;
5357
import static org.mockito.Mockito.never;
5458
import static org.mockito.Mockito.startsWith;
@@ -178,7 +182,9 @@ void shouldFailImmediatelyWhenClusterCompositionProviderReturnsFailure()
178182
ClusterComposition composition = await( rediscovery.lookupClusterComposition( table, pool, empty() ) );
179183
assertEquals( validComposition, composition );
180184

181-
verify( logger ).warn( String.format( "Failed to update routing table with server '%s'.", B ), protocolError );
185+
ArgumentCaptor<DiscoveryException> argument = ArgumentCaptor.forClass( DiscoveryException.class );
186+
verify( logger ).warn( anyString(), argument.capture() );
187+
assertThat( argument.getValue().getCause(), equalTo( protocolError ) );
182188
}
183189

184190
@Test
@@ -260,19 +266,26 @@ void shouldPropagateFailureWhenResolverFails()
260266
}
261267

262268
@Test
263-
void shouldFailWhenNoRoutersRespond()
269+
void shouldRecordAllErrorsWhenNoRouterRespond()
264270
{
265271
Map<BoltServerAddress,Object> responsesByAddress = new HashMap<>();
266-
responsesByAddress.put( A, new ServiceUnavailableException( "Hi!" ) ); // first -> non-fatal failure
267-
responsesByAddress.put( B, new SessionExpiredException( "Hi!" ) ); // second -> non-fatal failure
268-
responsesByAddress.put( C, new IOException( "Hi!" ) ); // third -> non-fatal failure
272+
ServiceUnavailableException first = new ServiceUnavailableException( "Hi!" );
273+
responsesByAddress.put( A, first ); // first -> non-fatal failure
274+
SessionExpiredException second = new SessionExpiredException( "Hi!" );
275+
responsesByAddress.put( B, second ); // second -> non-fatal failure
276+
IOException third = new IOException( "Hi!" );
277+
responsesByAddress.put( C, third ); // third -> non-fatal failure
269278

270279
ClusterCompositionProvider compositionProvider = compositionProviderMock( responsesByAddress );
271280
Rediscovery rediscovery = newRediscovery( A, compositionProvider, mock( ServerAddressResolver.class ) );
272281
RoutingTable table = routingTableMock( A, B, C );
273282

274283
ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class, () -> await( rediscovery.lookupClusterComposition( table, pool, empty() ) ) );
275284
assertThat( e.getMessage(), containsString( "Could not perform discovery" ) );
285+
assertThat( e.getSuppressed().length, equalTo( 3 ) );
286+
assertThat( e.getSuppressed()[0].getCause(), equalTo( first ) );
287+
assertThat( e.getSuppressed()[1].getCause(), equalTo( second ) );
288+
assertThat( e.getSuppressed()[2].getCause(), equalTo( third ) );
276289
}
277290

278291
@Test

0 commit comments

Comments
 (0)