From baa7fbbcc68c6837892c63325b76e4eab6283e9a Mon Sep 17 00:00:00 2001 From: GregDThomas Date: Tue, 27 Aug 2024 12:33:02 +0100 Subject: [PATCH] fix: Catch retryable exceptions thrown by SDN itself. (#2945) This fix #2944 by making the retryable exception messages consistent by introducing constants for prematurely closed transactions and sessions omitting the trailing dot as agreed within the bigger Spring Data team. --- .../data/neo4j/core/support/RetryExceptionPredicate.java | 8 ++++++-- .../neo4j/core/transaction/Neo4jTransactionHolder.java | 7 ++++--- .../neo4j/core/support/RetryExceptionPredicateTest.java | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicate.java b/src/main/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicate.java index 14223fb98..b2a0c88d5 100644 --- a/src/main/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicate.java +++ b/src/main/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicate.java @@ -26,6 +26,7 @@ import org.neo4j.driver.exceptions.SessionExpiredException; import org.neo4j.driver.exceptions.TransientException; import org.springframework.dao.TransientDataAccessResourceException; + /** * A predicate indicating {@literal true} for {@link Throwable throwables} that can be safely retried and {@literal false} * in any other case. This predicate can be used for example with Resilience4j. @@ -37,9 +38,12 @@ import org.springframework.dao.TransientDataAccessResourceException; @API(status = API.Status.STABLE, since = "6.0") public final class RetryExceptionPredicate implements Predicate { + public static final String TRANSACTION_MUST_BE_OPEN_BUT_HAS_ALREADY_BEEN_CLOSED = "Transaction must be open, but has already been closed"; + public static final String SESSION_MUST_BE_OPEN_BUT_HAS_ALREADY_BEEN_CLOSED = "Session must be open, but has already been closed"; private static final Set RETRYABLE_ILLEGAL_STATE_MESSAGES = Set.of( - "Transaction must be open, but has already been closed.", - "Session must be open, but has already been closed."); + TRANSACTION_MUST_BE_OPEN_BUT_HAS_ALREADY_BEEN_CLOSED, + SESSION_MUST_BE_OPEN_BUT_HAS_ALREADY_BEEN_CLOSED + ); @Override public boolean test(Throwable throwable) { diff --git a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionHolder.java b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionHolder.java index a0df40eb7..5bfedbd5e 100644 --- a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionHolder.java +++ b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionHolder.java @@ -22,6 +22,7 @@ import org.neo4j.driver.Session; import org.neo4j.driver.Transaction; import org.springframework.data.neo4j.core.DatabaseSelection; import org.springframework.data.neo4j.core.UserSelection; +import org.springframework.data.neo4j.core.support.RetryExceptionPredicate; import org.springframework.lang.Nullable; import org.springframework.transaction.support.ResourceHolderSupport; import org.springframework.util.Assert; @@ -68,7 +69,7 @@ final class Neo4jTransactionHolder extends ResourceHolderSupport { Collection commit() { - Assert.state(hasActiveTransaction(), "Transaction must be open, but has already been closed"); + Assert.state(hasActiveTransaction(), RetryExceptionPredicate.TRANSACTION_MUST_BE_OPEN_BUT_HAS_ALREADY_BEEN_CLOSED); Assert.state(!isRollbackOnly(), "Resource must not be marked as rollback only"); transaction.commit(); @@ -79,7 +80,7 @@ final class Neo4jTransactionHolder extends ResourceHolderSupport { void rollback() { - Assert.state(hasActiveTransaction(), "Transaction must be open, but has already been closed"); + Assert.state(hasActiveTransaction(), RetryExceptionPredicate.TRANSACTION_MUST_BE_OPEN_BUT_HAS_ALREADY_BEEN_CLOSED); transaction.rollback(); transaction.close(); @@ -87,7 +88,7 @@ final class Neo4jTransactionHolder extends ResourceHolderSupport { void close() { - Assert.state(hasActiveSession(), "Session must be open, but has already been closed"); + Assert.state(hasActiveSession(), RetryExceptionPredicate.SESSION_MUST_BE_OPEN_BUT_HAS_ALREADY_BEEN_CLOSED); if (hasActiveTransaction()) { transaction.close(); diff --git a/src/test/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicateTest.java b/src/test/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicateTest.java index 77c13df5c..7727b4f2d 100644 --- a/src/test/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicateTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicateTest.java @@ -34,8 +34,8 @@ import org.springframework.data.neo4j.core.Neo4jPersistenceExceptionTranslator; class RetryExceptionPredicateTest { @ParameterizedTest - @ValueSource(strings = { "Transaction must be open, but has already been closed.", - "Session must be open, but has already been closed." }) + @ValueSource(strings = { "Transaction must be open, but has already been closed", + "Session must be open, but has already been closed" }) void shouldRetryOnSomeIllegalStateExceptions(String msg) { RetryExceptionPredicate predicate = new RetryExceptionPredicate();