Don't popTransaction unless it was pushed

transactionoutbox-core/src/main/java/com/gruelbox/transactionoutbox/SimpleTransactionManager.java

@@ -56,22 +56,22 @@ private <T, E extends Exception> T processAndCommitOrRollback(
   private <T, E extends Exception> T withTransaction(ThrowingTransactionalSupplier<T, E> work)
       throws E {
     try (Connection connection = connectionProvider.obtainConnection();
-        SimpleTransaction transaction = pushTransaction(new SimpleTransaction(connection, null))) {
+        SimpleTransaction transaction = new SimpleTransaction(connection, null)) {
       log.debug("Got connection {}", connection);
-      boolean autoCommit = transaction.connection().getAutoCommit();
+      boolean autoCommit = connection.getAutoCommit();
       if (autoCommit) {
         log.debug("Setting auto-commit false");
-        Utils.uncheck(() -> transaction.connection().setAutoCommit(false));
+        Utils.uncheck(() -> connection.setAutoCommit(false));
       }
+      pushTransaction(transaction);
       try {
         return work.doWork(transaction);
       } finally {
+        popTransaction();
         connection.setAutoCommit(autoCommit);
       }
     } catch (SQLException e) {
       throw new RuntimeException(e);
-    } finally {
-      popTransaction();
     }
   }
 }

transactionoutbox-core/src/test/java/com/gruelbox/transactionoutbox/SimpleTransactionManagerTest.java

@@ -0,0 +1,49 @@
+package com.gruelbox.transactionoutbox;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.sql.SQLException;
+import java.util.NoSuchElementException;
+import org.junit.jupiter.api.Test;
+
+class SimpleTransactionManagerTest {
+
+  @Test
+  void connectionFailureIsNotMaskedByPop() {
+    SQLException connectionFailure = new SQLException("boom");
+    ConnectionProvider failingProvider =
+        () -> {
+          throw new UncheckedException(connectionFailure);
+        };
+
+    SimpleTransactionManager transactionManager =
+        SimpleTransactionManager.builder().connectionProvider(failingProvider).build();
+
+    RuntimeException thrown =
+        assertThrows(RuntimeException.class, () -> transactionManager.inTransaction(() -> {}));
+
+    assertFalse(
+        containsCause(thrown, NoSuchElementException.class),
+        "Real connection failure was masked by an empty-stack pop");
+    assertSame(connectionFailure, rootCause(thrown), "Original SQLException should be preserved");
+  }
+
+  private static boolean containsCause(Throwable throwable, Class<? extends Throwable> type) {
+    for (Throwable t = throwable; t != null; t = t.getCause()) {
+      if (type.isInstance(t)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private static Throwable rootCause(Throwable throwable) {
+    Throwable t = throwable;
+    while (t.getCause() != null && t.getCause() != t) {
+      t = t.getCause();
+    }
+    return t;
+  }
+}