Option 2: drop the unwrapping, propagate RuntimeException

Author: EnricSala

spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/R2dbcTransactionManagerUnitTests.java

@@ -256,7 +256,9 @@ void testCommitFails() {
 				.doOnNext(connection -> connection.createStatement("foo")).then()
 				.as(operator::transactional)
 				.as(StepVerifier::create)
-				.verifyError(BadSqlGrammarException.class);
+				.verifyErrorSatisfies(ex -> assertThat(ex)
+						.isExactlyInstanceOf(RuntimeException.class)
+						.hasCauseInstanceOf(BadSqlGrammarException.class));
 
 		verify(connectionMock).isAutoCommit();
 		verify(connectionMock).beginTransaction();
@@ -309,7 +311,9 @@ void testRollbackFails() {
 			return ConnectionFactoryUtils.getConnection(connectionFactoryMock)
 					.doOnNext(connection -> connection.createStatement("foo")).then();
 		}).as(StepVerifier::create)
-				.verifyError(BadSqlGrammarException.class);
+				.verifyErrorSatisfies(ex -> assertThat(ex)
+						.isExactlyInstanceOf(RuntimeException.class)
+						.hasCauseInstanceOf(BadSqlGrammarException.class));
 
 		verify(connectionMock).isAutoCommit();
 		verify(connectionMock).beginTransaction();

spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java

@@ -921,8 +921,7 @@ public Object invokeWithinTransaction(Method method, @Nullable Class<?> targetCl
 								},
 								this::commitTransactionAfterReturning,
 								this::completeTransactionAfterThrowing,
-								this::rollbackTransactionOnCancel)
-							.onErrorMap(this::unwrapIfResourceCleanupFailure))
+								this::rollbackTransactionOnCancel))
 						.contextWrite(TransactionContextManager.getOrCreateContext())
 						.contextWrite(TransactionContextManager.getOrCreateContextHolder());
 			}
@@ -941,8 +940,7 @@ public Object invokeWithinTransaction(Method method, @Nullable Class<?> targetCl
 							},
 							this::commitTransactionAfterReturning,
 							this::completeTransactionAfterThrowing,
-							this::rollbackTransactionOnCancel)
-						.onErrorMap(this::unwrapIfResourceCleanupFailure))
+							this::rollbackTransactionOnCancel))
 					.contextWrite(TransactionContextManager.getOrCreateContext())
 					.contextWrite(TransactionContextManager.getOrCreateContextHolder()));
 		}
@@ -1024,9 +1022,6 @@ private Mono<Void> completeTransactionAfterThrowing(@Nullable ReactiveTransactio
 								if (ex2 instanceof TransactionSystemException) {
 									((TransactionSystemException) ex2).initApplicationException(ex);
 								}
-								else {
-									ex2.addSuppressed(ex);
-								}
 								return ex2;
 							}
 					);
@@ -1039,30 +1034,13 @@ private Mono<Void> completeTransactionAfterThrowing(@Nullable ReactiveTransactio
 								if (ex2 instanceof TransactionSystemException) {
 									((TransactionSystemException) ex2).initApplicationException(ex);
 								}
-								else {
-									ex2.addSuppressed(ex);
-								}
 								return ex2;
 							}
 					);
 				}
 			}
 			return Mono.empty();
 		}
-
-		/**
-		 * Unwrap the cause of a throwable, if produced by a failure
-		 * during the async resource cleanup in {@link Flux#usingWhen}.
-		 * @param ex the throwable to try to unwrap
-		 */
-		private Throwable unwrapIfResourceCleanupFailure(Throwable ex) {
-			if (ex instanceof RuntimeException &&
-					ex.getCause() != null &&
-					ex.getMessage().startsWith("Async resource cleanup failed")) {
-				return ex.getCause();
-			}
-			return ex;
-		}
 	}
 
 

spring-tx/src/main/java/org/springframework/transaction/reactive/TransactionalOperatorImpl.java

@@ -79,8 +79,7 @@ public <T> Flux<T> execute(TransactionCallback<T> action) throws TransactionExce
 				action::doInTransaction,
 				this.transactionManager::commit,
 				this::rollbackOnException,
-				this.transactionManager::rollback)
-			.onErrorMap(this::unwrapIfResourceCleanupFailure))
+				this.transactionManager::rollback))
 		.contextWrite(TransactionContextManager.getOrCreateContext())
 		.contextWrite(TransactionContextManager.getOrCreateContextHolder());
 	}
@@ -98,28 +97,11 @@ private Mono<Void> rollbackOnException(ReactiveTransaction status, Throwable ex)
 					if (ex2 instanceof TransactionSystemException) {
 						((TransactionSystemException) ex2).initApplicationException(ex);
 					}
-					else {
-						ex2.addSuppressed(ex);
-					}
 					return ex2;
 				}
 		);
 	}
 
-	/**
-	 * Unwrap the cause of a throwable, if produced by a failure
-	 * during the async resource cleanup in {@link Flux#usingWhen}.
-	 * @param ex the throwable to try to unwrap
-	 */
-	private Throwable unwrapIfResourceCleanupFailure(Throwable ex) {
-		if (ex instanceof RuntimeException &&
-				ex.getCause() != null &&
-				ex.getMessage().startsWith("Async resource cleanup failed")) {
-			return ex.getCause();
-		}
-		return ex;
-	}
-
 
 	@Override
 	public boolean equals(@Nullable Object other) {

spring-tx/src/test/java/org/springframework/transaction/interceptor/AbstractReactiveTransactionAspectTests.java

@@ -256,7 +256,9 @@ public boolean rollbackOn(Throwable t) {
 				.as(StepVerifier::create)
 				.expectErrorSatisfies(actual -> {
 					if (rollbackException) {
-						assertThat(actual).isEqualTo(tex);
+						assertThat(actual)
+								.isExactlyInstanceOf(RuntimeException.class)
+								.hasCause(tex);
 					}
 					else {
 						assertThat(actual).isEqualTo(ex);
@@ -335,7 +337,9 @@ public void cannotCommitTransaction() throws Exception {
 
 		Mono.from(itb.setName(name))
 				.as(StepVerifier::create)
-				.verifyErrorSatisfies(actual -> assertThat(actual).isEqualTo(ex));
+				.verifyErrorSatisfies(actual -> assertThat(actual)
+						.isExactlyInstanceOf(RuntimeException.class)
+						.hasCause(ex));
 
 		// Should have invoked target and changed name
 

spring-tx/src/test/java/org/springframework/transaction/reactive/TransactionalOperatorTests.java

@@ -119,7 +119,9 @@ public void commitFailureWithMono() {
 		TransactionalOperator operator = TransactionalOperator.create(tm, new DefaultTransactionDefinition());
 		Mono.just(true).as(operator::transactional)
 				.as(StepVerifier::create)
-				.verifyError(IOException.class);
+				.verifyErrorSatisfies(ex ->	assertThat(ex)
+						.isExactlyInstanceOf(RuntimeException.class)
+						.hasCauseInstanceOf(IOException.class));
 		assertThat(commit.subscribeCount()).isEqualTo(1);
 		rollback.assertWasNotSubscribed();
 	}
@@ -138,7 +140,8 @@ public void rollbackFailureWithMono() {
 		Mono.error(actionFailure).as(operator::transactional)
 				.as(StepVerifier::create)
 				.verifyErrorSatisfies(ex -> assertThat(ex)
-						.isInstanceOf(IOException.class)
+						.isExactlyInstanceOf(RuntimeException.class)
+						.hasCauseInstanceOf(IOException.class)
 						.hasSuppressedException(actionFailure));
 		commit.assertWasNotSubscribed();
 		assertThat(rollback.subscribeCount()).isEqualTo(1);
@@ -178,7 +181,9 @@ public void commitFailureWithFlux() {
 		Flux.just(1, 2, 3, 4).as(operator::transactional)
 				.as(StepVerifier::create)
 				.expectNextCount(4)
-				.verifyError(IOException.class);
+				.verifyErrorSatisfies(ex ->	assertThat(ex)
+						.isExactlyInstanceOf(RuntimeException.class)
+						.hasCauseInstanceOf(IOException.class));
 		assertThat(commit.subscribeCount()).isEqualTo(1);
 		rollback.assertWasNotSubscribed();
 	}
@@ -198,7 +203,8 @@ public void rollbackFailureWithFlux() {
 				.as(StepVerifier::create)
 				.expectNextCount(3)
 				.verifyErrorSatisfies(ex -> assertThat(ex)
-						.isInstanceOf(IOException.class)
+						.isExactlyInstanceOf(RuntimeException.class)
+						.hasCauseInstanceOf(IOException.class)
 						.hasSuppressedException(actionFailure));
 		commit.assertWasNotSubscribed();
 		assertThat(rollback.subscribeCount()).isEqualTo(1);

spring-tx/src/test/kotlin/org/springframework/transaction/interceptor/AbstractCoroutinesTransactionAspectTests.kt

@@ -218,7 +218,11 @@ abstract class AbstractCoroutinesTransactionAspectTests {
 			}
 			catch (actual: Exception) {
 				if (rollbackException) {
-					Assertions.assertThat(actual).hasMessage(tex.message).isInstanceOf(tex::class.java)
+					Assertions.assertThat(actual)
+						.isExactlyInstanceOf(RuntimeException::class.java)
+						.cause
+						.isExactlyInstanceOf(RuntimeException::class.java)
+						.hasCause(tex)
 				} else {
 					Assertions.assertThat(actual).hasMessage(ex.message).isInstanceOf(ex::class.java)
 				}
@@ -291,7 +295,11 @@ abstract class AbstractCoroutinesTransactionAspectTests {
 				itb.setName(name)
 			}
 			catch (actual: Exception) {
-				Assertions.assertThat(actual).isEqualTo(ex)
+				Assertions.assertThat(actual)
+					.isExactlyInstanceOf(RuntimeException::class.java)
+					.cause
+					.isExactlyInstanceOf(RuntimeException::class.java)
+					.hasCause(ex)
 			}
 			// Should have invoked target and changed name
 			Assertions.assertThat(itb.getName()).isEqualTo(name)
@@ -352,4 +360,4 @@ abstract class AbstractCoroutinesTransactionAspectTests {
 			}
 		}
 	}
-}
+}