diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/TransactionRunner.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/TransactionRunner.java index 57bc4c3b0..3c479f0fc 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/TransactionRunner.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/TransactionRunner.java @@ -145,28 +145,35 @@ public void run() { * Invokes the user callback on the user callback executor and returns the user-provided result. */ private SettableApiFuture invokeUserCallback() { - final SettableApiFuture callbackResult = SettableApiFuture.create(); + final SettableApiFuture returnedResult = SettableApiFuture.create(); + userCallbackExecutor.execute( new Runnable() { @Override public void run() { + ApiFuture userCallbackResult; + try { + userCallbackResult = userCallback.updateCallback(transaction); + } catch (Exception e) { + userCallbackResult = ApiFutures.immediateFailedFuture(e); + } ApiFutures.addCallback( - userCallback.updateCallback(transaction), + userCallbackResult, new ApiFutureCallback() { @Override public void onFailure(Throwable t) { - callbackResult.setException(t); + returnedResult.setException(t); } @Override public void onSuccess(T result) { - callbackResult.set(result); + returnedResult.set(result); } }, - MoreExecutors.directExecutor()); + firestoreExecutor); } }); - return callbackResult; + return returnedResult; } /** A callback that invokes the BeginTransaction callback. */ diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java index a8405972a..5638bcda0 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/TransactionTest.java @@ -253,7 +253,7 @@ public String updateCallback(Transaction transaction) throws Exception { } @Test - public void rollbackOnCallbackErrorAsync() { + public void rollbackOnCallbackApiFutureErrorAsync() { doReturn(beginResponse()) .doReturn(rollbackResponse()) .when(firestoreMock) @@ -339,6 +339,31 @@ public ApiFuture updateCallback(Transaction transaction) { assertEquals(1, requests.size()); } + @Test + public void noRollbackOnThrownExceptionAsync() { + doReturn(beginResponse()) + .doReturn(rollbackResponse()) + .when(firestoreMock) + .sendRequest(requestCapture.capture(), Matchers.>any()); + + ApiFuture transaction = + firestoreMock.runAsyncTransaction( + new Transaction.AsyncFunction() { + @Override + public ApiFuture updateCallback(Transaction transaction) { + throw new RuntimeException("User exception"); + } + }, + options); + + try { + transaction.get(); + fail(); + } catch (Exception e) { + assertTrue(e.getMessage().endsWith("User exception")); + } + } + @Test public void limitsRetriesWithFailure() { ResponseStubber responseStubber = diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java index a5572efbe..fd17dab44 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java @@ -688,6 +688,24 @@ public String updateCallback(Transaction transaction) { } } + @Test + public void asyncTxFailsWithUserError() throws Exception { + try { + firestore + .runAsyncTransaction( + new Transaction.AsyncFunction() { + @Override + public ApiFuture updateCallback(Transaction transaction) { + throw new RuntimeException("User exception"); + } + }) + .get(); + fail(); + } catch (Exception e) { + assertTrue(e.getMessage().endsWith("User exception")); + } + } + @Test public void doesNotRetryTransactionsWithFailedPreconditions() { final DocumentReference documentReference = randomColl.document();