Add RetryableException interface (#1219)

* Add RetryableException interface This interface is meant to indicate what exceptions are retryable. * Remove unused line
neo4j · May 13, 2022 · 7628d69 · 7628d69
1 parent 2b9a8f0
commit 7628d69
Show file tree

Hide file tree

Showing 11 changed files with 89 additions and 53 deletions.
diff --git a/driver/src/main/java/org/neo4j/driver/exceptions/AuthorizationExpiredException.java b/driver/src/main/java/org/neo4j/driver/exceptions/AuthorizationExpiredException.java
@@ -23,7 +23,7 @@
  * <p>
  * Error code: Neo.ClientError.Security.AuthorizationExpired
  */
-public class AuthorizationExpiredException extends SecurityException
+public class AuthorizationExpiredException extends SecurityException implements RetryableException
 {
     public static final String DESCRIPTION = "Authorization information kept on the server has expired, this connection is no longer valid.";
 

diff --git a/driver/src/main/java/org/neo4j/driver/exceptions/RetryableException.java b/driver/src/main/java/org/neo4j/driver/exceptions/RetryableException.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) "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;
+
+/**
+ * A marker interface for retryable exceptions.
+ * <p>
+ * This indicates whether an operation that resulted in retryable exception is worth retrying.
+ */
+public interface RetryableException
+{
+}
diff --git a/driver/src/main/java/org/neo4j/driver/exceptions/ServiceUnavailableException.java b/driver/src/main/java/org/neo4j/driver/exceptions/ServiceUnavailableException.java
@@ -20,9 +20,10 @@
 
 /**
  * An <em>ServiceUnavailableException</em> indicates that the driver cannot communicate with the cluster.
+ *
  * @since 1.1
  */
-public class ServiceUnavailableException extends Neo4jException
+public class ServiceUnavailableException extends Neo4jException implements RetryableException
 {
     public ServiceUnavailableException( String message )
     {
@@ -31,6 +32,6 @@ public ServiceUnavailableException( String message )
 
     public ServiceUnavailableException( String message, Throwable throwable )
     {
-        super( message, throwable);
+        super( message, throwable );
     }
 }
diff --git a/driver/src/main/java/org/neo4j/driver/exceptions/SessionExpiredException.java b/driver/src/main/java/org/neo4j/driver/exceptions/SessionExpiredException.java
@@ -19,14 +19,14 @@
 package org.neo4j.driver.exceptions;
 
 /**
- * A <em>SessionExpiredException</em> indicates that the session can no longer satisfy the criteria under which it
- * was acquired, e.g. a server no longer accepts write requests. A new session needs to be acquired from the driver
- * and all actions taken on the expired session must be replayed.
+ * A <em>SessionExpiredException</em> indicates that the session can no longer satisfy the criteria under which it was acquired, e.g. a server no longer accepts
+ * write requests. A new session needs to be acquired from the driver and all actions taken on the expired session must be replayed.
+ *
  * @since 1.1
  */
-public class SessionExpiredException extends Neo4jException
+public class SessionExpiredException extends Neo4jException implements RetryableException
 {
-    public SessionExpiredException( String message)
+    public SessionExpiredException( String message )
     {
         super( message );
     }

diff --git a/driver/src/main/java/org/neo4j/driver/exceptions/TransientException.java b/driver/src/main/java/org/neo4j/driver/exceptions/TransientException.java
@@ -19,11 +19,12 @@
 package org.neo4j.driver.exceptions;
 
 /**
- * A <em>TransientException</em> signals a temporary fault that may be worked around by retrying.
- * The error code provided can be used to determine further detail for the problem.
+ * A <em>TransientException</em> signals a temporary fault that may be worked around by retrying. The error code provided can be used to determine further
+ * detail for the problem.
+ *
  * @since 1.0
  */
-public class TransientException extends Neo4jException
+public class TransientException extends Neo4jException implements RetryableException
 {
     public TransientException( String code, String message )
     {

diff --git a/driver/src/main/java/org/neo4j/driver/internal/retry/ExponentialBackoffRetryLogic.java b/driver/src/main/java/org/neo4j/driver/internal/retry/ExponentialBackoffRetryLogic.java
@@ -38,15 +38,11 @@
 
 import org.neo4j.driver.Logger;
 import org.neo4j.driver.Logging;
-import org.neo4j.driver.exceptions.AuthorizationExpiredException;
 import org.neo4j.driver.exceptions.ClientException;
 import org.neo4j.driver.exceptions.Neo4jException;
-import org.neo4j.driver.exceptions.ServiceUnavailableException;
-import org.neo4j.driver.exceptions.SessionExpiredException;
-import org.neo4j.driver.exceptions.TransientException;
+import org.neo4j.driver.exceptions.RetryableException;
 import org.neo4j.driver.internal.util.Clock;
 import org.neo4j.driver.internal.util.Futures;
-import org.neo4j.driver.util.Experimental;
 
 import static java.util.concurrent.TimeUnit.SECONDS;
 
@@ -148,14 +144,7 @@ public <T> Publisher<T> retryRx( Publisher<T> work )
 
     protected boolean canRetryOn( Throwable error )
     {
-        return isRetryable( error );
-    }
-
-    @Experimental
-    public static boolean isRetryable( Throwable error )
-    {
-        return error instanceof SessionExpiredException || error instanceof ServiceUnavailableException || error instanceof AuthorizationExpiredException ||
-               isTransientError( error );
+        return error instanceof RetryableException;
     }
 
     /**
@@ -351,25 +340,6 @@ private void verifyAfterConstruction()
         }
     }
 
-    private static boolean isTransientError( Throwable error )
-    {
-        if ( error instanceof TransientException )
-        {
-            String code = ((TransientException) error).code();
-            // Retries should not happen when transaction was explicitly terminated by the user.
-            // Termination of transaction might result in two different error codes depending on where it was
-            // terminated. These are really client errors but classification on the server is not entirely correct and
-            // they are classified as transient.
-            if ( "Neo.TransientError.Transaction.Terminated".equals( code ) ||
-                 "Neo.TransientError.Transaction.LockClientStopped".equals( code ) )
-            {
-                return false;
-            }
-            return true;
-        }
-        return false;
-    }
-
     private static List<Throwable> recordError( Throwable error, List<Throwable> errors )
     {
         if ( errors == 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
@@ -100,7 +100,20 @@ else if ( code.equalsIgnoreCase( "Neo.ClientError.Security.TokenExpired" ) )
                 }
             }
         case "TransientError":
-            return new TransientException( code, message );
+            // Since 5.0 these 2 errors have been moved to ClientError class.
+            // This mapping is required if driver is connection to earlier server versions.
+            if ( "Neo.TransientError.Transaction.Terminated".equals( code ) )
+            {
+                return new ClientException( "Neo.ClientError.Transaction.Terminated", message );
+            }
+            else if ( "Neo.TransientError.Transaction.LockClientStopped".equals( code ) )
+            {
+                return new ClientException( "Neo.ClientError.Transaction.LockClientStopped", message );
+            }
+            else
+            {
+                return new TransientException( code, message );
+            }
         default:
             return new DatabaseException( code, message );
         }

diff --git a/driver/src/test/java/org/neo4j/driver/internal/retry/ExponentialBackoffRetryLogicTest.java b/driver/src/test/java/org/neo4j/driver/internal/retry/ExponentialBackoffRetryLogicTest.java
@@ -471,7 +471,7 @@ void throwsWhenTransactionTerminatedError() throws Exception
         ExponentialBackoffRetryLogic logic = newRetryLogic( 1, 13, 1, 0, clock );
 
         Supplier<Void> workMock = newWorkMock();
-        TransientException error = new TransientException( "Neo.TransientError.Transaction.Terminated", "" );
+        ClientException error = new ClientException( "Neo.ClientError.Transaction.Terminated", "" );
         when( workMock.get() ).thenThrow( error ).thenReturn( null );
 
         Exception e = assertThrows( Exception.class, () -> logic.retry( workMock ) );
@@ -489,7 +489,7 @@ void doesNotRetryOnTransactionTerminatedErrorAsync()
         ExponentialBackoffRetryLogic retryLogic = newRetryLogic( 1, 13, 1, 0, clock );
 
         Supplier<CompletionStage<Object>> workMock = newWorkMock();
-        TransientException error = new TransientException( "Neo.TransientError.Transaction.Terminated", "" );
+        ClientException error = new ClientException( "Neo.ClientError.Transaction.Terminated", "" );
         when( workMock.get() ).thenReturn( failedFuture( error ) );
 
         Exception e = assertThrows( Exception.class, () -> await( retryLogic.retryAsync( workMock ) ) );
@@ -506,7 +506,7 @@ void throwsWhenTransactionLockClientStoppedError() throws Exception
         ExponentialBackoffRetryLogic logic = newRetryLogic( 1, 13, 1, 0, clock );
 
         Supplier<Void> workMock = newWorkMock();
-        TransientException error = new TransientException( "Neo.TransientError.Transaction.LockClientStopped", "" );
+        ClientException error = new ClientException( "Neo.ClientError.Transaction.LockClientStopped", "" );
         when( workMock.get() ).thenThrow( error ).thenReturn( null );
 
         Exception e = assertThrows( Exception.class, () -> logic.retry( workMock ) );
@@ -524,7 +524,7 @@ void doesNotRetryOnTransactionLockClientStoppedErrorAsync()
         ExponentialBackoffRetryLogic retryLogic = newRetryLogic( 1, 15, 1, 0, clock );
 
         Supplier<CompletionStage<Object>> workMock = newWorkMock();
-        TransientException error = new TransientException( "Neo.TransientError.Transaction.LockClientStopped", "" );
+        ClientException error = new ClientException( "Neo.ClientError.Transaction.LockClientStopped", "" );
         when( workMock.get() ).thenReturn( failedFuture( error ) );
 
         Exception e = assertThrows( Exception.class, () -> await( retryLogic.retryAsync( workMock ) ) );
@@ -1437,8 +1437,8 @@ private static Stream<Exception> cannotBeRetriedErrors()
     {
         return Stream.of(
                 new IllegalStateException(),
-                new TransientException( "Neo.TransientError.Transaction.Terminated", "" ),
-                new TransientException( "Neo.TransientError.Transaction.LockClientStopped", "" )
+                new ClientException( "Neo.ClientError.Transaction.Terminated", "" ),
+                new ClientException( "Neo.ClientError.Transaction.LockClientStopped", "" )
         );
     }
 }
diff --git a/driver/src/test/java/org/neo4j/driver/internal/util/ErrorUtilTest.java b/driver/src/test/java/org/neo4j/driver/internal/util/ErrorUtilTest.java
@@ -189,4 +189,30 @@ void shouldCreateTokenExpiredException()
         assertEquals( code, error.code() );
         assertEquals( message, error.getMessage() );
     }
+
+    @Test
+    void shouldMapTransientTransactionTerminatedToClientException()
+    {
+        String code = "Neo.TransientError.Transaction.Terminated";
+        String message = "message";
+
+        Neo4jException error = newNeo4jError( code, message );
+
+        assertThat( error, instanceOf( ClientException.class ) );
+        assertEquals( "Neo.ClientError.Transaction.Terminated", error.code() );
+        assertEquals( message, error.getMessage() );
+    }
+
+    @Test
+    void shouldMapTransientTransactionLockClientStoppedToClientException()
+    {
+        String code = "Neo.TransientError.Transaction.LockClientStopped";
+        String message = "message";
+
+        Neo4jException error = newNeo4jError( code, message );
+
+        assertThat( error, instanceOf( ClientException.class ) );
+        assertEquals( "Neo.ClientError.Transaction.LockClientStopped", error.code() );
+        assertEquals( message, error.getMessage() );
+    }
 }
diff --git a/...c/main/java/neo4j/org/testkit/backend/channel/handler/TestkitRequestProcessorHandler.java b/...c/main/java/neo4j/org/testkit/backend/channel/handler/TestkitRequestProcessorHandler.java
@@ -89,7 +89,6 @@ public void channelRead( ChannelHandlerContext ctx, Object msg )
                                                                             {
                                                                                 if ( throwable != null )
                                                                                 {
-//                                                                                    throwable.printStackTrace();
                                                                                     ctx.writeAndFlush( createErrorResponse( throwable ) );
                                                                                 }
                                                                                 else if ( response != null )

diff --git a/...n/java/neo4j/org/testkit/backend/channel/handler/TestkitRequestResponseMapperHandler.java b/...n/java/neo4j/org/testkit/backend/channel/handler/TestkitRequestResponseMapperHandler.java
@@ -36,7 +36,6 @@ public class TestkitRequestResponseMapperHandler extends ChannelDuplexHandler
     public void channelRead( ChannelHandlerContext ctx, Object msg )
     {
         String testkitMessage = (String) msg;
-        System.out.println( testkitMessage );
         TestkitRequest testkitRequest;
         try
         {
@@ -54,7 +53,6 @@ public void write( ChannelHandlerContext ctx, Object msg, ChannelPromise promise
     {
         TestkitResponse testkitResponse = (TestkitResponse) msg;
         String responseStr = objectMapper.writeValueAsString( testkitResponse );
-        System.out.println( responseStr );
         ctx.writeAndFlush( responseStr, promise );
     }