Skip to content

Commit c66c834

Browse files
committed
Wrap IOException in ServiceUnavailableException
When establishing connection it was possible for `IOException` to get propagated directly to the user without being wrapped in a `ServiceUnavailableException`. This happened because `HandshakeHandler` only wrapped security-related exceptions with driver exceptions. It was problematic because rest of the codebase expects to deal with `ServiceUnavailableException`s. For example rediscovery and retries code can deal nicely with `ServiceUnavailableException`. This commit fixes the problem by making `HandshakeHandler` wrap all non-security related exceptions in `ServiceUnavailableException`.
1 parent 6860578 commit c66c834

File tree

3 files changed

+48
-8
lines changed

3 files changed

+48
-8
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public void exceptionCaught( ChannelHandlerContext ctx, Throwable error )
103103
}
104104
else
105105
{
106-
fail( ctx, cause );
106+
fail( ctx, new ServiceUnavailableException( "Failed to establish connection with the server", cause ) );
107107
}
108108
}
109109
}

driver/src/test/java/org/neo4j/driver/internal/async/ChannelConnectorImplIT.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
import org.junit.Test;
3030

3131
import java.io.IOException;
32+
import java.io.UncheckedIOException;
3233
import java.net.ServerSocket;
34+
import java.net.Socket;
3335
import java.util.concurrent.ExecutionException;
3436
import java.util.concurrent.TimeUnit;
3537

@@ -44,6 +46,7 @@
4446
import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
4547
import org.neo4j.driver.v1.util.TestNeo4j;
4648

49+
import static java.util.concurrent.CompletableFuture.runAsync;
4750
import static org.hamcrest.Matchers.instanceOf;
4851
import static org.hamcrest.Matchers.startsWith;
4952
import static org.junit.Assert.assertEquals;
@@ -186,6 +189,43 @@ public void shouldFailWhenTLSHandshakeTakesTooLong() throws Exception
186189
testReadTimeoutOnConnect( SecurityPlan.forAllCertificates() );
187190
}
188191

192+
@Test
193+
public void shouldThrowServiceUnavailableExceptionOnFailureDuringConnect() throws Exception
194+
{
195+
ServerSocket server = new ServerSocket( 0 );
196+
BoltServerAddress address = new BoltServerAddress( "localhost", server.getLocalPort() );
197+
198+
runAsync( () ->
199+
{
200+
try
201+
{
202+
// wait for a connection
203+
Socket socket = server.accept();
204+
// and terminate it immediately so that client gets a "reset by peer" IOException
205+
socket.close();
206+
server.close();
207+
}
208+
catch ( IOException e )
209+
{
210+
throw new UncheckedIOException( e );
211+
}
212+
} );
213+
214+
ChannelConnector connector = newConnector( neo4j.authToken() );
215+
ChannelFuture channelFuture = connector.connect( address, bootstrap );
216+
217+
try
218+
{
219+
await( channelFuture );
220+
fail( "Exception expected" );
221+
}
222+
catch ( ServiceUnavailableException e )
223+
{
224+
assertEquals( "Failed to establish connection with the server", e.getMessage() );
225+
assertThat( e.getCause(), instanceOf( IOException.class ) );
226+
}
227+
}
228+
189229
private void testReadTimeoutOnConnect( SecurityPlan securityPlan ) throws IOException
190230
{
191231
try ( ServerSocket server = new ServerSocket( 0 ) ) // server that accepts connections but does not reply

driver/src/test/java/org/neo4j/driver/internal/async/HandshakeHandlerTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@
4646
import static org.junit.Assert.assertNull;
4747
import static org.junit.Assert.assertThat;
4848
import static org.junit.Assert.fail;
49-
import static org.neo4j.driver.internal.async.ChannelAttributes.setMessageDispatcher;
5049
import static org.neo4j.driver.internal.async.BoltProtocolV1Util.HTTP;
5150
import static org.neo4j.driver.internal.async.BoltProtocolV1Util.NO_PROTOCOL_VERSION;
5251
import static org.neo4j.driver.internal.async.BoltProtocolV1Util.PROTOCOL_VERSION_1;
52+
import static org.neo4j.driver.internal.async.ChannelAttributes.setMessageDispatcher;
5353
import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING;
5454
import static org.neo4j.driver.v1.util.TestUtil.await;
5555

@@ -85,9 +85,9 @@ public void shouldFailGivenPromiseWhenExceptionCaught()
8585
await( handshakeCompletedPromise );
8686
fail( "Exception expected" );
8787
}
88-
catch ( Exception e )
88+
catch ( ServiceUnavailableException e )
8989
{
90-
assertEquals( cause, e );
90+
assertEquals( cause, e.getCause() );
9191
}
9292

9393
// channel should be closed
@@ -112,9 +112,9 @@ public void shouldFailGivenPromiseWhenMultipleExceptionsCaught()
112112
await( handshakeCompletedPromise );
113113
fail( "Exception expected" );
114114
}
115-
catch ( RuntimeException e )
115+
catch ( ServiceUnavailableException e )
116116
{
117-
assertEquals( error1, e );
117+
assertEquals( error1, e.getCause() );
118118
}
119119

120120
// channel should be closed
@@ -147,9 +147,9 @@ public void shouldUnwrapDecoderException()
147147
await( handshakeCompletedPromise );
148148
fail( "Exception expected" );
149149
}
150-
catch ( Exception e )
150+
catch ( ServiceUnavailableException e )
151151
{
152-
assertEquals( cause, e );
152+
assertEquals( cause, e.getCause() );
153153
}
154154

155155
// channel should be closed

0 commit comments

Comments
 (0)