Avoid message listener recovery in case of persistence exceptions from external transaction manager

[drewtul]

If we get an exception, but the transaction completed, log to indicate
we rolled back the transaction, but do not re-throw the exception.
If the transaction did not complete, then throw the exception as there
was likely an infrastructure failure and this listener needs to retry.

[drewtul]

@jhoeller Given your thoughts on #1778 I thought this might be a better more generic approach for handling exceptions on transaction manager commit to avoid recycling listeners unless the exception was fatal from the transaction managers perspective i.e. it failed to complete the transaction.

[bipster]

Should this be wrapped in an illustrative unchecked exception? As-is, this shouldn't compile because Throwable is checked, right?

[drewtul]

It looks like it should be a syntax error, this is:

public class TestThrowable {

    public static void main(String[] args) {
        throw new Throwable("Throw me");
    }
}

However catching and re-throwing throwable is valid:

public class TestThrowable {

    public static void main(String[] args) {
        try {
            System.out.println("But in a try?");
        } catch (Throwable e) {
            if (true) {
                throw e;
            }
        }
    }
}

Outputs:

But in a try?

Since I don't know what the infrastructure exception is, the only super class is throwable. I don't think I should wrap it in anything in case anything above might handle the original cause and not my wrapper, I was trying to avoid changing anything more behaviour than I had to here.

[bipster]

Ahh, right, since PlatformTransactionManager.commit(..) doesn't declare any checked exceptions, the compiler knows that (in this case) Throwable must necessarily be an unchecked exception. If you added something to the try-block that could throw a checked exception, only then throw e; would fail to compile. Forgot about that nicety; thanks for the patience!

[drewtul]

@jhoeller Did you have any further thoughts about this issue and this PR changing the exception handling to throw less infrastructure failure exceptions when the transaction completed with a rollback?

[jhoeller]

Revisit this topic, I've arrived at a straightforward change where we can process non-TransactionExceptions from transactionManager.commit as listener exceptions instead of infrastructure exceptions, assuming that they are typically late-triggered persistence exceptions from a listener-used resource (and therefore to be handled like similar exceptions coming out of the listener invocation itself).

I'll repurpose this PR ticket for the specific purpose suggested but with a custom commit of mine that aligns it with listener exception processing. Thanks for raising the pull request, in any case - and sorry that it took so long to act here.

[drewtul]

Thanks for coming back to this! Hopefully it fixes the problem, I'm no longer working on the project in question so can't test I'm afraid.