Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RetryableException interface #1219

Merged
merged 2 commits into from
May 13, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.";

Expand Down
Original file line number Diff line number Diff line change
@@ -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
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
{
Expand All @@ -31,6 +32,6 @@ public ServiceUnavailableException( String message )

public ServiceUnavailableException( String message, Throwable throwable )
{
super( message, throwable);
super( message, throwable );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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 )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) );
Expand All @@ -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 ) ) );
Expand All @@ -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 ) );
Expand All @@ -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 ) ) );
Expand Down Expand Up @@ -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", "" )
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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 );
}

Expand Down