[#16455] java.lang.Error should rollback the JTA transaction

extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestException.java

@@ -0,0 +1,4 @@
+package io.quarkus.narayana.interceptor;
+
+public class TestException extends Exception {
+}

extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestXAResource.java

@@ -0,0 +1,60 @@
+package io.quarkus.narayana.interceptor;
+
+import javax.transaction.xa.XAException;
+import javax.transaction.xa.XAResource;
+import javax.transaction.xa.Xid;
+
+public class TestXAResource implements XAResource {
+    final TxAssertionData txAssertionData;
+
+    TestXAResource(TxAssertionData txAssertionData) {
+        this.txAssertionData = txAssertionData;
+    }
+
+    @Override
+    public void commit(Xid xid, boolean b) throws XAException {
+        txAssertionData.addCommit();
+    }
+
+    @Override
+    public void end(Xid xid, int i) throws XAException {
+    }
+
+    @Override
+    public void forget(Xid xid) throws XAException {
+    }
+
+    @Override
+    public int getTransactionTimeout() throws XAException {
+        return 0;
+    }
+
+    @Override
+    public boolean isSameRM(XAResource xaResource) throws XAException {
+        return false;
+    }
+
+    @Override
+    public int prepare(Xid xid) throws XAException {
+        return XA_OK;
+    }
+
+    @Override
+    public Xid[] recover(int i) throws XAException {
+        return new Xid[0];
+    }
+
+    @Override
+    public void rollback(Xid xid) throws XAException {
+        txAssertionData.addRollback();
+    }
+
+    @Override
+    public boolean setTransactionTimeout(int i) throws XAException {
+        return false;
+    }
+
+    @Override
+    public void start(Xid xid, int i) throws XAException {
+    }
+}

extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TransactionalTest.java

@@ -0,0 +1,210 @@
+package io.quarkus.narayana.interceptor;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Inject;
+import javax.transaction.RollbackException;
+import javax.transaction.Status;
+import javax.transaction.SystemException;
+import javax.transaction.TransactionManager;
+import javax.transaction.Transactional;
+import javax.transaction.UserTransaction;
+
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+public class TransactionalTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
+                    .addClasses(TransactionalTest.TransactionalBean.class, TestXAResource.class,
+                            TxAssertionData.class, TestException.class));
+
+    @Inject
+    private TransactionManager tm;
+
+    @Inject
+    private UserTransaction userTransaction;
+
+    @Inject
+    private TransactionalTest.TransactionalBean testTransactionalBean;
+
+    @Inject
+    private TxAssertionData txAssertionData;
+
+    @AfterEach
+    public void tearDown() {
+        try {
+            userTransaction.rollback();
+        } catch (Exception e) {
+            // do nothing
+        } finally {
+            txAssertionData.reset();
+        }
+    }
+
+    @Test
+    public void transactionalRequiresToCommit() throws Exception {
+        assertTransactionInactive();
+        testTransactionalBean.executeTransactional();
+        assertTransactionInactive();
+        Assertions.assertEquals(1, txAssertionData.getCommit());
+        Assertions.assertEquals(0, txAssertionData.getRollback());
+    }
+
+    @Test
+    public void transactionalThrowRuntimeException() {
+        assertTransactionInactive();
+        try {
+            testTransactionalBean.executeTransactionalThrowException(RuntimeException.class);
+            Assertions.fail("Expecting RuntimeException to be thrown and the execution does not reach this point");
+        } catch (Throwable expected) {
+        }
+        assertTransactionInactive();
+        Assertions.assertEquals(0, txAssertionData.getCommit());
+        Assertions.assertEquals(1, txAssertionData.getRollback());
+    }
+
+    @Test
+    public void transactionalThrowApplicationException() {
+        assertTransactionInactive();
+        try {
+            testTransactionalBean.executeTransactionalThrowException(TestException.class);
+            Assertions.fail("Expecting TestException to be thrown and the execution does not reach this point");
+        } catch (Throwable expected) {
+        }
+        assertTransactionInactive();
+        Assertions.assertEquals(1, txAssertionData.getCommit());
+        Assertions.assertEquals(0, txAssertionData.getRollback());
+    }
+
+    @Test
+    public void transactionalThrowError() {
+        assertTransactionInactive();
+        try {
+            testTransactionalBean.executeTransactionalThrowException(Error.class);
+            Assertions.fail("Expecting Error to be thrown and the execution does not reach this point");
+        } catch (Throwable expected) {
+        }
+        assertTransactionInactive();
+        Assertions.assertEquals(0, txAssertionData.getCommit());
+        Assertions.assertEquals(1, txAssertionData.getRollback());
+    }
+
+    @Test
+    public void transactionalThrowApplicationExceptionWithRollbackOn() {
+        assertTransactionInactive();
+        try {
+            testTransactionalBean.executeTransactionalRollbackOnException(TestException.class);
+            Assertions.fail("Expecting TestException to be thrown and the execution does not reach this point");
+        } catch (Throwable expected) {
+        }
+        assertTransactionInactive();
+        Assertions.assertEquals(0, txAssertionData.getCommit());
+        Assertions.assertEquals(1, txAssertionData.getRollback());
+    }
+
+    @Test
+    public void transactionalThrowRuntimeExceptionWithDontRollbackOn() {
+        assertTransactionInactive();
+        try {
+            testTransactionalBean.executeTransactionalDontRollbackOnRuntimeException(RuntimeException.class);
+            Assertions.fail("Expecting RuntimeException to be thrown and the execution does not reach this point");
+        } catch (Throwable expected) {
+        }
+        assertTransactionInactive();
+        Assertions.assertEquals(1, txAssertionData.getCommit());
+        Assertions.assertEquals(0, txAssertionData.getRollback());
+    }
+
+    @Test
+    public void transactionalThrowErrorWithDontRollbackOn() {
+        assertTransactionInactive();
+        try {
+            testTransactionalBean.executeTransactionalDontRollbackOnError(Error.class);
+            Assertions.fail("Expecting Error to be thrown and the execution does not reach this point");
+        } catch (Throwable expected) {
+        }
+        assertTransactionInactive();
+        Assertions.assertEquals(1, txAssertionData.getCommit());
+        Assertions.assertEquals(0, txAssertionData.getRollback());
+    }
+
+    @Test
+    public void transactionalThrowApplicationExceptionDontRollbackOnPriority() {
+        assertTransactionInactive();
+        try {
+            testTransactionalBean.executeTransactionalRollbackOnPriority(TestException.class);
+            Assertions.fail("Expecting TestException to be thrown and the execution does not reach this point");
+        } catch (Throwable expected) {
+        }
+        assertTransactionInactive();
+        Assertions.assertEquals(1, txAssertionData.getCommit());
+        Assertions.assertEquals(0, txAssertionData.getRollback());
+    }
+
+    private void assertTransactionInactive() {
+        try {
+            if (tm.getTransaction() != null) {
+                Assertions.assertNotEquals(Status.STATUS_ACTIVE, tm.getTransaction().getStatus());
+            }
+        } catch (Exception e) {
+            Assertions.fail(e.getMessage());
+        }
+    }
+
+    @ApplicationScoped
+    static class TransactionalBean {
+        @Inject
+        private TransactionManager transactionManager;
+
+        @Inject
+        private TxAssertionData txAssertionData;
+
+        private void enlist() throws SystemException, RollbackException {
+            transactionManager.getTransaction()
+                    .enlistResource(new TestXAResource(txAssertionData));
+        }
+
+        @Transactional
+        public void executeTransactional() throws Exception {
+            enlist();
+        }
+
+        @Transactional
+        public void executeTransactionalThrowException(Class<? extends Throwable> throwable) throws Throwable {
+            enlist();
+            throw throwable.newInstance();
+        }
+
+        @Transactional(rollbackOn = Exception.class)
+        public void executeTransactionalRollbackOnException(Class<? extends Throwable> throwable) throws Throwable {
+            enlist();
+            throw throwable.newInstance();
+        }
+
+        @Transactional(dontRollbackOn = RuntimeException.class)
+        public void executeTransactionalDontRollbackOnRuntimeException(Class<? extends Throwable> throwable) throws Throwable {
+            enlist();
+            throw throwable.newInstance();
+        }
+
+        @Transactional(dontRollbackOn = Error.class)
+        public void executeTransactionalDontRollbackOnError(Class<? extends Throwable> throwable) throws Throwable {
+            enlist();
+            throw throwable.newInstance();
+        }
+
+        @Transactional(dontRollbackOn = Exception.class, rollbackOn = Exception.class)
+        public void executeTransactionalRollbackOnPriority(Class<? extends Throwable> throwable) throws Throwable {
+            enlist();
+            throw throwable.newInstance();
+        }
+    }
+}

extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TxAssertionData.java

@@ -0,0 +1,32 @@
+package io.quarkus.narayana.interceptor;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.enterprise.context.ApplicationScoped;
+
+@ApplicationScoped
+public class TxAssertionData {
+    private AtomicInteger commitNumber = new AtomicInteger();
+    private AtomicInteger rollbackNumber = new AtomicInteger();
+
+    public void reset() {
+        commitNumber.set(0);
+        rollbackNumber.set(0);
+    }
+
+    public int addCommit() {
+        return commitNumber.incrementAndGet();
+    }
+
+    public int addRollback() {
+        return rollbackNumber.incrementAndGet();
+    }
+
+    public int getCommit() {
+        return commitNumber.get();
+    }
+
+    public int getRollback() {
+        return rollbackNumber.get();
+    }
+}

extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/interceptor/TransactionalInterceptorBase.java

@@ -125,9 +125,9 @@ protected Object invokeInOurTx(InvocationContext ic, TransactionManager tm, Runn
 
         try {
             ret = ic.proceed();
-        } catch (Exception e) {
+        } catch (Throwable t) {
             throwing = true;
-            handleException(ic, e, tx);
+            handleException(ic, t, tx);
         } finally {
             // handle asynchronously if not throwing
             if (!throwing && ret != null) {
@@ -250,8 +250,8 @@ protected Object invokeInCallerTx(InvocationContext ic, Transaction tx) throws E
         try {
             checkConfiguration(ic);
             return ic.proceed();
-        } catch (Exception e) {
-            handleException(ic, e, tx);
+        } catch (Throwable t) {
+            handleException(ic, t, tx);
         }
         throw new RuntimeException("UNREACHABLE");
     }
@@ -269,34 +269,35 @@ private void checkConfiguration(InvocationContext ic) {
         }
     }
 
-    protected void handleExceptionNoThrow(InvocationContext ic, Throwable e, Transaction tx)
+    protected void handleExceptionNoThrow(InvocationContext ic, Throwable t, Transaction tx)
             throws IllegalStateException, SystemException {
 
         Transactional transactional = getTransactional(ic);
 
         for (Class<?> dontRollbackOnClass : transactional.dontRollbackOn()) {
-            if (dontRollbackOnClass.isAssignableFrom(e.getClass())) {
+            if (dontRollbackOnClass.isAssignableFrom(t.getClass())) {
                 return;
             }
         }
 
         for (Class<?> rollbackOnClass : transactional.rollbackOn()) {
-            if (rollbackOnClass.isAssignableFrom(e.getClass())) {
+            if (rollbackOnClass.isAssignableFrom(t.getClass())) {
                 tx.setRollbackOnly();
                 return;
             }
         }
 
-        if (e instanceof RuntimeException) {
+        // RuntimeException and Error are un-checked exceptions and rollback is expected
+        if (t instanceof RuntimeException || t instanceof Error) {
             tx.setRollbackOnly();
             return;
         }
     }
 
-    protected void handleException(InvocationContext ic, Exception e, Transaction tx) throws Exception {
+    protected void handleException(InvocationContext ic, Throwable t, Transaction tx) throws Exception {
 
-        handleExceptionNoThrow(ic, e, tx);
-        throw e;
+        handleExceptionNoThrow(ic, t, tx);
+        sneakyThrow(t);
     }
 
     protected void endTransaction(TransactionManager tm, Transaction tx, RunnableWithException afterEndTransaction)
@@ -327,4 +328,16 @@ protected boolean setUserTransactionAvailable(boolean available) {
     protected void resetUserTransactionAvailability(boolean previousUserTransactionAvailability) {
         ServerVMClientUserTransaction.setAvailability(previousUserTransactionAvailability);
     }
+
+    /**
+     * An utility method to throw any exception as a {@link RuntimeException}.
+     * We may throw a checked exception (subtype of {@code Throwable} or {@code Exception}) as un-checked exception.
+     * This considers the Java 8 inference rule that states that a {@code throws E} is inferred as {@code RuntimeException}.
+     *
+     * This method can be used in {@code throw} statement such as: {@code throw sneakyThrow(exception);}.
+     */
+    @SuppressWarnings("unchecked")
+    private static <E extends Throwable> void sneakyThrow(Throwable e) throws E {
+        throw (E) e;
+    }
 }