Skip to content

fix(spanner): ensure active transaction is rolled back on exception - #9416

Merged
bshaffer merged 4 commits into
mainfrom
fix-transaction-rollback
Aug 7, 2026
Merged

fix(spanner): ensure active transaction is rolled back on exception#9416
bshaffer merged 4 commits into
mainfrom
fix-transaction-rollback

Conversation

@cy-yun

@cy-yun cy-yun commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

This PR addresses an issue in Cloud Spanner where an active transaction might leak if an exception is thrown during the Database::runTransaction callback.

Previously, if a user-provided callback threw an exception, the exception would bubble up without explicitly rolling back the transaction. This PR catches any \Throwable thrown by the callback, checks if the transaction is still active, and explicitly issues a rollback to the Spanner backend before re-throwing the exception.

Additional Fixes

While adding this, a latent bug was discovered and fixed:
Database::runTransaction was passing its configuration $options directly into $transaction->rollback($options). Since runTransaction options (like tag, isRetry, singleUse) are not valid CallOptions for Operation::rollback, this would cause an InvalidArgumentException (e.g., Unexpected option(s) provided: tag). This exception was masking the rollback failure and leaving the transaction un-rolled back on the server.
This PR fixes this by calling $transaction->rollback() without passing the runTransaction configuration options, ensuring the rollback completes successfully.

Testing

  • testRunTransactionRollsBackOnException: Verifies that an exception in the callback triggers a rollback.
  • testRunTransactionNoCommitWithTag: Verifies that passing tag to runTransaction no longer causes an InvalidArgumentException when the transaction must be rolled back at the end of execution.

@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Jul 30, 2026
@cy-yun
cy-yun marked this pull request as ready for review July 30, 2026 00:57
@cy-yun
cy-yun requested a review from a team as a code owner July 30, 2026 00:57
Comment thread Spanner/tests/Unit/DatabaseTest.php Outdated

@bshaffer bshaffer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, looking at this, I am not honestly sure this is the correct behavior. I see that we automatically roll back transactions already, so this seems appropriate

EDIT: oops, did not mean to request changes.

Comment thread Spanner/src/Database.php
Comment thread Spanner/src/Database.php
@bshaffer
bshaffer force-pushed the fix-transaction-rollback branch from 9152851 to 9ed14ef Compare August 6, 2026 14:20
@bshaffer
bshaffer enabled auto-merge (squash) August 6, 2026 14:20
@bshaffer
bshaffer merged commit 7912a78 into main Aug 7, 2026
50 of 55 checks passed
@bshaffer
bshaffer deleted the fix-transaction-rollback branch August 7, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants