NoRollbackFor rule causes TransactionAspectSupport to log unwarranted "exception overridden" error on WebSphere

[fralalonde]

Observed using Spring 4.3.26, expecting same behavior from Spring 5.x after code comparison.

The application code throws custom NoDataFoundException checked exception in normal conditions to report absence of data, which is then handled outside the transaction. Certainly not the best pattern, but unsolvable for now due to a myriad of possible side effects.

To alleviate the issue, we're using NoRollbackRule attribute in the form of +NoDataFoundException on TransactionProxyFactoryBean like so:

<bean id="MyBean" class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean">
    <property name="transactionAttributes">
        <props>
            <property key="method1">PROPAGATION_REQUIRED,+NoDataFoundException</prop>

This results in NoDataFoundException being correctly rethrown outside tx, but still emitting an error log statement like so:

org.springframework.transaction.interceptor.TransactionAspectSupport invokeWithinTransaction Application exception overridden by commit exception
                                 <com.app.NoDataFoundException>

Stepping into invokeWithinTransaction code, it appears that the checked exception being rethrown is mistaken as a "new" transaction commit exception, overriding... itself.

Initial capture of the checked exception:

if (txAttr.rollbackOn(ex)) {
 [...]
}
else {
	// A normal return value: will lead to a commit.
	throwableHolder.throwable = ex;
	return null;
}

Rethrowing outside of tx:

// Check result state: It might indicate a Throwable to rethrow.
if (throwableHolder.throwable != null) {
	throw throwableHolder.throwable;
}

Exception re-handled on exit, triggering extraneous logging:

catch (Throwable ex2) {
	if (throwableHolder.throwable != null) {
		logger.error("Application exception overridden by commit exception", throwableHolder.throwable);
	}
	throw ex2;
}

Fixing this situation should be as simple as comparing ex2 and throwableHolder.throwable for equality (in addition to the null check already in place), or resetting throwableHolder.throwable to null upon rethrowing the checked exception. A PR can be submitted to this effect if requested.

A quick workaround would be to disable logging for the TransactionProxyFactoryBean, which is unpalatable because of the many other serious error conditions that could go unreported. A more sophisticated patch would apply a targeted logging filter to ignore error entries mentioning the checked exception class.

[jhoeller]

I suppose you're running on WebSphere since this only seems to affect the CallbackPreferringPlatformTransactionManager code path (with WebSphereUowTransactionManager being the only common implementation of that variant)?

In any case, it is indeed falling over its own feet in the exception handling code path there, that log statement needs to be avoided. We'll also backport this to 5.1.x, 5.0.x and 4.3.x once resolved here.

[fralalonde]

Yes, this is on WebSphere.