Skip to content

Commit eb6106b

Browse files
refactor: Proper wrap retryable exceptions in TransactionSystemException.
This avoids an attempted rollback when the commit already failed which will lead to an illegal state exception otherwise, which will completely throw off the transaction system. Discovered through an internal card.
1 parent 673c3b5 commit eb6106b

File tree

4 files changed

+23
-6
lines changed

4 files changed

+23
-6
lines changed

src/main/java/org/springframework/data/neo4j/core/support/RetryExceptionPredicate.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.neo4j.driver.exceptions.SessionExpiredException;
2626
import org.neo4j.driver.exceptions.TransientException;
2727
import org.springframework.dao.TransientDataAccessResourceException;
28+
import org.springframework.transaction.TransactionSystemException;
2829

2930

3031
/**
@@ -52,13 +53,17 @@ public boolean test(Throwable throwable) {
5253
return true;
5354
}
5455

56+
if (throwable instanceof TransactionSystemException && throwable.getCause() != null) {
57+
throwable = throwable.getCause();
58+
}
59+
5560
if (throwable instanceof IllegalStateException) {
5661
String msg = throwable.getMessage();
5762
return msg != null && RETRYABLE_ILLEGAL_STATE_MESSAGES.contains(msg);
5863
}
5964

6065
Throwable ex = throwable;
61-
if (throwable instanceof TransientDataAccessResourceException) {
66+
if (throwable instanceof TransientDataAccessResourceException || throwable instanceof TransactionSystemException) {
6267
ex = throwable.getCause();
6368
}
6469

src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.neo4j.driver.Session;
2424
import org.neo4j.driver.Transaction;
2525
import org.neo4j.driver.TransactionConfig;
26+
import org.neo4j.driver.exceptions.Neo4jException;
27+
import org.neo4j.driver.exceptions.RetryableException;
2628
import org.springframework.beans.BeansException;
2729
import org.springframework.context.ApplicationContext;
2830
import org.springframework.context.ApplicationContextAware;
@@ -338,10 +340,17 @@ protected void doResume(@Nullable Object transaction, Object suspendedResources)
338340
@Override
339341
protected void doCommit(DefaultTransactionStatus status) throws TransactionException {
340342

341-
Neo4jTransactionObject transactionObject = extractNeo4jTransaction(status);
342-
Neo4jTransactionHolder transactionHolder = transactionObject.getRequiredResourceHolder();
343-
Collection<Bookmark> newBookmarks = transactionHolder.commit();
344-
this.bookmarkManager.resolve().updateBookmarks(transactionHolder.getBookmarks(), newBookmarks);
343+
try {
344+
Neo4jTransactionObject transactionObject = extractNeo4jTransaction(status);
345+
Neo4jTransactionHolder transactionHolder = transactionObject.getRequiredResourceHolder();
346+
Collection<Bookmark> newBookmarks = transactionHolder.commit();
347+
this.bookmarkManager.resolve().updateBookmarks(transactionHolder.getBookmarks(), newBookmarks);
348+
} catch (Neo4jException ex) {
349+
if (ex instanceof RetryableException) {
350+
throw new TransactionSystemException(ex.getMessage(), ex);
351+
}
352+
throw ex;
353+
}
345354
}
346355

347356
@Override

src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.apiguardian.api.API;
2222
import org.neo4j.driver.Driver;
2323
import org.neo4j.driver.TransactionConfig;
24+
import org.neo4j.driver.exceptions.RetryableException;
2425
import org.neo4j.driver.reactivestreams.ReactiveSession;
2526
import org.neo4j.driver.reactivestreams.ReactiveTransaction;
2627
import org.springframework.beans.BeansException;
@@ -35,6 +36,7 @@
3536
import org.springframework.transaction.NoTransactionException;
3637
import org.springframework.transaction.TransactionDefinition;
3738
import org.springframework.transaction.TransactionException;
39+
import org.springframework.transaction.TransactionSystemException;
3840
import org.springframework.transaction.reactive.AbstractReactiveTransactionManager;
3941
import org.springframework.transaction.reactive.GenericReactiveTransaction;
4042
import org.springframework.transaction.reactive.TransactionSynchronizationManager;
@@ -326,6 +328,7 @@ protected Mono<Void> doCommit(TransactionSynchronizationManager transactionSynch
326328
.getRequiredResourceHolder();
327329
return holder.commit()
328330
.doOnNext(bookmark -> bookmarkManager.resolve().updateBookmarks(holder.getBookmarks(), bookmark))
331+
.onErrorMap(e -> e instanceof RetryableException, e -> new TransactionSystemException(e.getMessage(), e))
329332
.then();
330333
}
331334

src/test/java/org/springframework/data/neo4j/integration/imperative/QuerydslNeo4jPredicateExecutorIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ void fluentFindAllWithLimitShouldWork(@Autowired QueryDSLPersonRepository reposi
323323
Predicate predicate = Expressions.predicate(Ops.EQ, firstNamePath, Expressions.asString("Helge"))
324324
.or(Expressions.predicate(Ops.EQ, lastNamePath, Expressions.asString("B.")));
325325
List<Person> people = repository.findBy(predicate,
326-
q -> q.limit(1)).all();
326+
q -> q.sortBy(Sort.by("firstName").descending()).limit(1)).all();
327327

328328
assertThat(people).hasSize(1);
329329
assertThat(people).extracting(Person::getFirstName).containsExactly("Helge");

0 commit comments

Comments
 (0)