Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exceptions not propagated from async function back to async transaction #514

Closed
elefeint opened this issue Oct 15, 2020 · 0 comments · Fixed by #516
Closed

Exceptions not propagated from async function back to async transaction #514

elefeint opened this issue Oct 15, 2020 · 0 comments · Fixed by #516
Assignees
Labels
api: spanner

Comments

@elefeint
Copy link
Contributor

@elefeint elefeint commented Oct 15, 2020

Environment details

  1. Java version: 11
  2. spanner version(s): 1.59.0

Steps to reproduce

In the Async API, TransactionContext.executeUpdateAsync() correctly propagates errors through ApiFuture callbacks. However, in an asynchronous transaction, the .then() operator does not get the same exceptions propagated to it from the transaction function.

Code example

The following test (just add a custom databaseId) hangs because neither the success nor failure callback is called, and the latch is never released. If the SQL statement were corrected, the test would pass, as success is propagated correctly.

@Test
  public void clientLibraryTransactionNotPropagatingFailure()
      throws ExecutionException, InterruptedException {
    Executor executor = Executors.newFixedThreadPool(4);

    SpannerOptions options = SpannerOptions.newBuilder().build();
    DatabaseClient dbClient = options.getService().getDatabaseClient(databaseId);
    AsyncTransactionManager transactionManager = dbClient.transactionManagerAsync();
    TransactionContextFuture txnContextFuture = transactionManager.beginAsync();


    // Statement has garbled SQL syntax intentionally
    Statement statement =
        Statement.newBuilder("INSERT INTO BOOKS (UUID, TITLE) VALUES ('123', 'Test book')jljlk")
            .build();
    ApiFuture<Long> updateFuture =
        txnContextFuture.then(
            (txnContext, unusedVoid) -> txnContext.executeUpdateAsync(statement),
            executor);

    CountDownLatch latch = new CountDownLatch(1);
    ApiFutures.addCallback(
        updateFuture,
        new ApiFutureCallback<Long>() {
          @Override
          public void onFailure(Throwable throwable) {
            // Expecting the failure callback to be called, but it's not, so the latch hangs.
            System.out.println("Exception: " + throwable);
            latch.countDown();
          }

          @Override
          public void onSuccess(Long aLong) {
            System.out.println("Success: " + aLong);
            latch.countDown();
          }
        }, executor);

    latch.await();
  }

Thanks!

@product-auto-label product-auto-label bot added the api: spanner label Oct 15, 2020
@olavloite olavloite self-assigned this Oct 15, 2020
olavloite added a commit that referenced this issue Oct 15, 2020
Invalid statements or other statements that would cause an error would not cause the
returned ApiFuture to fail.

Fixes #514
olavloite added a commit that referenced this issue Oct 16, 2020
* fix: AsyncTransactionManager did not propagate statement errors

Invalid statements or other statements that would cause an error would not cause the
returned ApiFuture to fail.

Fixes #514

* test: use existing invalid statement in test
elefeint added a commit to GoogleCloudPlatform/cloud-spanner-r2dbc that referenced this issue Nov 16, 2020
Bring in the latest client library, with the fix for googleapis/java-spanner#514.

Fixes #282.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants