WebSphereUowTransactionManager logs overridden application exceptions

Issue: SPR-16102

Author: jhoeller

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

@@ -306,6 +306,8 @@ protected Object invokeWithinTransaction(Method method, @Nullable Class<?> targe
 		}
 
 		else {
+			final ThrowableHolder throwableHolder = new ThrowableHolder();
+
 			// It's a CallbackPreferringPlatformTransactionManager: pass a TransactionCallback in.
 			try {
 				Object result = ((CallbackPreferringPlatformTransactionManager) tm).execute(txAttr, status -> {
@@ -325,25 +327,37 @@ protected Object invokeWithinTransaction(Method method, @Nullable Class<?> targe
 						}
 						else {
 							// A normal return value: will lead to a commit.
-							return new ThrowableHolder(ex);
+							throwableHolder.throwable = ex;
+							return null;
 						}
 					}
 					finally {
 						cleanupTransactionInfo(txInfo);
 					}
 				});
 
-				// Check result: It might indicate a Throwable to rethrow.
-				if (result instanceof ThrowableHolder) {
-					throw ((ThrowableHolder) result).getThrowable();
-				}
-				else {
-					return result;
+				// Check result state: It might indicate a Throwable to rethrow.
+				if (throwableHolder.throwable != null) {
+					throw throwableHolder.throwable;
 				}
+				return result;
 			}
 			catch (ThrowableHolderException ex) {
 				throw ex.getCause();
 			}
+			catch (TransactionSystemException ex2) {
+				if (throwableHolder.throwable != null) {
+					logger.error("Application exception overridden by commit exception", throwableHolder.throwable);
+					ex2.initApplicationException(throwableHolder.throwable);
+				}
+				throw ex2;
+			}
+			catch (Throwable ex2) {
+				if (throwableHolder.throwable != null) {
+					logger.error("Application exception overridden by commit exception", throwableHolder.throwable);
+				}
+				throw ex2;
+			}
 		}
 	}
 
@@ -540,14 +554,10 @@ protected void completeTransactionAfterThrowing(@Nullable TransactionInfo txInfo
 					ex2.initApplicationException(ex);
 					throw ex2;
 				}
-				catch (RuntimeException ex2) {
+				catch (RuntimeException | Error ex2) {
 					logger.error("Application exception overridden by rollback exception", ex);
 					throw ex2;
 				}
-				catch (Error err) {
-					logger.error("Application exception overridden by rollback error", ex);
-					throw err;
-				}
 			}
 			else {
 				// We don't roll back on this exception.
@@ -560,14 +570,10 @@ protected void completeTransactionAfterThrowing(@Nullable TransactionInfo txInfo
 					ex2.initApplicationException(ex);
 					throw ex2;
 				}
-				catch (RuntimeException ex2) {
+				catch (RuntimeException | Error ex2) {
 					logger.error("Application exception overridden by commit exception", ex);
 					throw ex2;
 				}
-				catch (Error err) {
-					logger.error("Application exception overridden by commit error", ex);
-					throw err;
-				}
 			}
 		}
 	}
@@ -679,20 +685,12 @@ protected interface InvocationCallback {
 
 
 	/**
-	 * Internal holder class for a Throwable, used as a return value
-	 * from a TransactionCallback (to be subsequently unwrapped again).
+	 * Internal holder class for a Throwable in a callback transaction model.
 	 */
 	private static class ThrowableHolder {
 
-		private final Throwable throwable;
-
-		public ThrowableHolder(Throwable throwable) {
-			this.throwable = throwable;
-		}
-
-		public final Throwable getThrowable() {
-			return this.throwable;
-		}
+		@Nullable
+		public Throwable throwable;
 	}
 
 

spring-tx/src/main/java/org/springframework/transaction/jta/WebSphereUowTransactionManager.java

@@ -261,7 +261,8 @@ public <T> T execute(@Nullable TransactionDefinition definition, TransactionCall
 						"Transaction propagation 'nested' not supported for WebSphere UOW transactions");
 			}
 			if (pb == TransactionDefinition.PROPAGATION_SUPPORTS ||
-					pb == TransactionDefinition.PROPAGATION_REQUIRED || pb == TransactionDefinition.PROPAGATION_MANDATORY) {
+					pb == TransactionDefinition.PROPAGATION_REQUIRED ||
+					pb == TransactionDefinition.PROPAGATION_MANDATORY) {
 				joinTx = true;
 				newSynch = (getTransactionSynchronization() != SYNCHRONIZATION_NEVER);
 			}
@@ -279,7 +280,8 @@ else if (pb == TransactionDefinition.PROPAGATION_NOT_SUPPORTED) {
 						"Transaction propagation 'mandatory' but no existing transaction found");
 			}
 			if (pb == TransactionDefinition.PROPAGATION_SUPPORTS ||
-					pb == TransactionDefinition.PROPAGATION_NOT_SUPPORTED || pb == TransactionDefinition.PROPAGATION_NEVER) {
+					pb == TransactionDefinition.PROPAGATION_NOT_SUPPORTED ||
+					pb == TransactionDefinition.PROPAGATION_NEVER) {
 				uowType = UOWSynchronizationRegistry.UOW_TYPE_LOCAL_TRANSACTION;
 				newSynch = (getTransactionSynchronization() == SYNCHRONIZATION_ALWAYS);
 			}
@@ -293,26 +295,31 @@ else if (pb == TransactionDefinition.PROPAGATION_NOT_SUPPORTED) {
 			logger.debug("Creating new transaction with name [" + definition.getName() + "]: " + definition);
 		}
 		SuspendedResourcesHolder suspendedResources = (!joinTx ? suspend(null) : null);
+		UOWActionAdapter<T> action = null;
 		try {
 			if (definition.getTimeout() > TransactionDefinition.TIMEOUT_DEFAULT) {
 				uowManager.setUOWTimeout(uowType, definition.getTimeout());
 			}
 			if (debug) {
 				logger.debug("Invoking WebSphere UOW action: type=" + uowType + ", join=" + joinTx);
 			}
-			UOWActionAdapter<T> action = new UOWActionAdapter<>(
+			action = new UOWActionAdapter<>(
 					definition, callback, (uowType == UOWManager.UOW_TYPE_GLOBAL_TRANSACTION), !joinTx, newSynch, debug);
 			uowManager.runUnderUOW(uowType, joinTx, action);
 			if (debug) {
 				logger.debug("Returned from WebSphere UOW action: type=" + uowType + ", join=" + joinTx);
 			}
 			return action.getResult();
 		}
-		catch (UOWException ex) {
-			throw new TransactionSystemException("UOWManager transaction processing failed", ex);
-		}
-		catch (UOWActionException ex) {
-			throw new TransactionSystemException("UOWManager threw unexpected UOWActionException", ex);
+		catch (UOWException | UOWActionException ex) {
+			TransactionSystemException tse =
+					new TransactionSystemException("UOWManager transaction processing failed", ex);
+			Throwable appEx = action.getException();
+			if (appEx != null) {
+				logger.error("Application exception overridden by rollback exception", appEx);
+				tse.initApplicationException(appEx);
+			}
+			throw tse;
 		}
 		finally {
 			if (suspendedResources != null) {
@@ -368,12 +375,15 @@ public void run() {
 			}
 			catch (Throwable ex) {
 				this.exception = ex;
+				if (status.isDebug()) {
+					logger.debug("Rolling back on application exception from transaction callback", ex);
+				}
 				uowManager.setRollbackOnly();
 			}
 			finally {
 				if (status.isLocalRollbackOnly()) {
 					if (status.isDebug()) {
-						logger.debug("Transactional code has requested rollback");
+						logger.debug("Transaction callback has explicitly requested rollback");
 					}
 					uowManager.setRollbackOnly();
 				}
@@ -396,6 +406,11 @@ public T getResult() {
 			return this.result;
 		}
 
+		@Nullable
+		public Throwable getException() {
+			return this.exception;
+		}
+
 		@Override
 		public boolean isRollbackOnly() {
 			return obtainUOWManager().getRollbackOnly();

spring-tx/src/main/java/org/springframework/transaction/support/TransactionTemplate.java

@@ -172,14 +172,10 @@ private void rollbackOnException(TransactionStatus status, Throwable ex) throws
 			ex2.initApplicationException(ex);
 			throw ex2;
 		}
-		catch (RuntimeException ex2) {
+		catch (RuntimeException | Error ex2) {
 			logger.error("Application exception overridden by rollback exception", ex);
 			throw ex2;
 		}
-		catch (Error err) {
-			logger.error("Application exception overridden by rollback error", ex);
-			throw err;
-		}
 	}
 
 }