Don't wrap exceptions thrown inside Cloud SQL transactions (#338)

* Don't wrap exceptions thrown inside Cloud SQL transactions

There's no reason to wrap them in PersistenceException, and it makes dealing
with thrown exceptions harder as seen in the diffs on test classes in this
commit.

core/src/main/java/google/registry/model/registry/RegistryLockDao.java

@@ -41,6 +41,7 @@ public final class RegistryLockDao {
                           Long.class)
                       .setParameter("verificationCode", verificationCode)
                       .getSingleResult();
+              // TODO(gbrodman): Don't throw NPE here. Maybe NoResultException fits better?
               checkNotNull(revisionId, "No registry lock with this code");
               return em.find(RegistryLock.class, revisionId);
             });

core/src/main/java/google/registry/model/transaction/JpaTransactionManagerImpl.java

@@ -79,17 +79,14 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
       T result = work.run();
       txn.commit();
       return result;
-    } catch (Throwable transactionException) {
-      String rollbackMessage;
+    } catch (RuntimeException e) {
       try {
         txn.rollback();
-        rollbackMessage = "transaction rolled back";
+        logger.atWarning().log("Error during transaction; transaction rolled back");
       } catch (Throwable rollbackException) {
-        logger.atSevere().withCause(rollbackException).log("Rollback failed, suppressing error");
-        rollbackMessage = "transaction rollback failed";
+        logger.atSevere().withCause(rollbackException).log("Rollback failed; suppressing error");
       }
-      throw new PersistenceException(
-          "Error during transaction, " + rollbackMessage, transactionException);
+      throw e;
     } finally {
       txnInfo.clear();
     }

core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java

@@ -25,7 +25,6 @@ import google.registry.schema.domain.RegistryLock.Action;
 import google.registry.testing.AppEngineRule;
 import java.util.Optional;
 import java.util.UUID;
-import javax.persistence.PersistenceException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -54,15 +53,11 @@ public final class RegistryLockDaoTest {
   public void testSaveAndLoad_failure_differentCode() {
     RegistryLock lock = createLock();
     RegistryLockDao.save(lock);
-    PersistenceException exception =
+    NullPointerException thrown =
         assertThrows(
-            PersistenceException.class,
+            NullPointerException.class,
             () -> RegistryLockDao.getByVerificationCode(UUID.randomUUID().toString()));
-    assertThat(exception)
-        .hasCauseThat()
-        .hasMessageThat()
-        .isEqualTo("No registry lock with this code");
-    assertThat(exception).hasCauseThat().isInstanceOf(NullPointerException.class);
+    assertThat(thrown).hasMessageThat().isEqualTo("No registry lock with this code");
   }
 
   @Test

core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.java

@@ -22,7 +22,6 @@ import google.registry.model.transaction.JpaTransactionManagerRule;
 import google.registry.schema.tmch.ClaimsList;
 import google.registry.testing.FakeClock;
 import javax.persistence.NoResultException;
-import javax.persistence.PersistenceException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -71,9 +70,7 @@ public class ClaimsListDaoTest {
 
   @Test
   public void getCurrent_throwsNoResultExceptionIfTableIsEmpty() {
-    PersistenceException thrown =
-        assertThrows(PersistenceException.class, () -> ClaimsListDao.getCurrent());
-    assertThat(thrown).hasCauseThat().isInstanceOf(NoResultException.class);
+    assertThrows(NoResultException.class, ClaimsListDao::getCurrent);
   }
 
   @Test

core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java

@@ -75,11 +75,7 @@ public class CurrencyUnitConverterTest {
                 jpaTm()
                     .transact(
                         () -> jpaTm().getEntityManager().find(TestEntity.class, "id").currency));
-    assertThat(thrown)
-        .hasCauseThat()
-        .hasCauseThat()
-        .hasMessageThat()
-        .isEqualTo("Unknown currency 'XXXX'");
+    assertThat(thrown).hasCauseThat().hasMessageThat().isEqualTo("Unknown currency 'XXXX'");
   }
 
   @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference.

core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java

@@ -21,7 +21,6 @@ import static google.registry.testing.JUnitBackports.assertThrows;
 import com.google.common.collect.ImmutableMap;
 import google.registry.model.transaction.JpaTransactionManagerRule;
 import java.math.BigDecimal;
-import javax.persistence.PersistenceException;
 import org.joda.money.CurrencyUnit;
 import org.junit.Rule;
 import org.junit.Test;
@@ -68,16 +67,13 @@ public class PremiumListDaoTest {
   @Test
   public void saveNew_throwsWhenPremiumListAlreadyExists() {
     PremiumListDao.saveNew(PremiumList.create("testlist", CurrencyUnit.USD, TEST_PRICES));
-    PersistenceException thrown =
+    IllegalArgumentException thrown =
         assertThrows(
-            PersistenceException.class,
+            IllegalArgumentException.class,
             () ->
                 PremiumListDao.saveNew(
                     PremiumList.create("testlist", CurrencyUnit.USD, TEST_PRICES)));
-    assertThat(thrown)
-        .hasCauseThat()
-        .hasMessageThat()
-        .contains("A premium list of this name already exists");
+    assertThat(thrown).hasMessageThat().contains("A premium list of this name already exists");
   }
 
   @Test