Skip to content

fix(firestore): retry commits when the transaction has expired #19066

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.

Sign up for GitHub

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

Open
pcbeingused333 wants to merge 1 commit into
base: main
Choose a base branch
from
+227 −8
Open

fix(firestore): retry commits when the transaction has expired #19066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions packages/google-cloud-firestore/google/cloud/firestore_v1/async_transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
MAX_ATTEMPTS,
BaseTransaction,
_BaseTransactional,
_is_retryable_commit_error,
)

# Types needed only for Type Hints
Expand Down Expand Up @@ -329,9 +330,6 @@ async def __call__(
``max_attempts``.
"""
self._reset()
retryable_exceptions = (
(exceptions.Aborted) if not transaction._read_only else ()
)
last_exc = None

try:
Expand All @@ -340,7 +338,11 @@ async def __call__(
try:
await transaction._commit()
return result
except retryable_exceptions as exc:
except (exceptions.Aborted, exceptions.InvalidArgument) as exc:
# Read-only transactions are never retried. InvalidArgument
# restarts the transaction only when the id has expired.
if transaction._read_only or not _is_retryable_commit_error(exc):
raise
last_exc = exc
# Retry attempts that result in retryable exceptions
# Subsequent requests will use the failed transaction ID as part of
Expand Down
18 changes: 18 additions & 0 deletions packages/google-cloud-firestore/google/cloud/firestore_v1/base_transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
Union,
)

from google.api_core import exceptions
from google.api_core import retry as retries

from google.cloud.firestore_v1 import types
Expand All @@ -50,6 +51,23 @@
_WRITE_READ_ONLY: str = "Cannot perform write operation in read-only transaction."
_EXCEED_ATTEMPTS_TEMPLATE: str = "Failed to commit transaction in {:d} attempts."
_CANT_RETRY_READ_ONLY: str = "Only read-write transactions can be retried."
# Firestore reports an expired transaction id as InvalidArgument. That status
# is not generally retryable; this substring is, matching the Node and Java
# clients (error.message / getMessage() contains "transaction has expired").
_EXPIRED_TRANSACTION_MESSAGE = "transaction has expired"


def _is_retryable_commit_error(exc: BaseException) -> bool:
"""Return whether a commit failure should restart the transaction.

``Aborted`` is always retryable. ``InvalidArgument`` is retryable only
when the message says the transaction has expired.
"""
if isinstance(exc, exceptions.Aborted):
return True
if isinstance(exc, exceptions.InvalidArgument):
return _EXPIRED_TRANSACTION_MESSAGE in str(exc)
Comment on lines +68 to +69

@gemini-code-assist gemini-code-assist Bot Nov 13, 2026

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.

medium

To make the check robust against potential casing variations in the backend error message (e.g., "Transaction has expired"), it is safer to perform a case-insensitive substring search by converting the exception string to lowercase.

Suggested change
if isinstance(exc, exceptions.InvalidArgument):
return _EXPIRED_TRANSACTION_MESSAGE in str(exc)
if isinstance(exc, exceptions.InvalidArgument):
return _EXPIRED_TRANSACTION_MESSAGE in str(exc).lower()

return False


class BaseTransaction(object):
Expand Down
10 changes: 6 additions & 4 deletions packages/google-cloud-firestore/google/cloud/firestore_v1/transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
MAX_ATTEMPTS,
BaseTransaction,
_BaseTransactional,
_is_retryable_commit_error,
)
from google.cloud.firestore_v1.document import DocumentReference
from google.cloud.firestore_v1.query import Query
Expand Down Expand Up @@ -305,9 +306,6 @@ def __call__(self, transaction: Transaction, *args, **kwargs):
``max_attempts``.
"""
self._reset()
retryable_exceptions = (
(exceptions.Aborted) if not transaction._read_only else ()
)
last_exc = None

try:
Expand All @@ -316,7 +314,11 @@ def __call__(self, transaction: Transaction, *args, **kwargs):
try:
transaction._commit()
return result
except retryable_exceptions as exc:
except (exceptions.Aborted, exceptions.InvalidArgument) as exc:
# Read-only transactions are never retried. InvalidArgument
# restarts the transaction only when the id has expired.
if transaction._read_only or not _is_retryable_commit_error(exc):
raise
last_exc = exc
# Retry attempts that result in retryable exceptions
# Subsequent requests will use the failed transaction ID as part of
Expand Down
91 changes: 91 additions & 0 deletions packages/google-cloud-firestore/tests/unit/v1/test_async_transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,97 @@ async def test_asynctransactional___call__success_second_attempt():
assert firestore_api.commit.mock_calls == [commit_call, commit_call]


@pytest.mark.asyncio
async def test_asynctransactional___call__success_second_attempt_expired_transaction():
from google.api_core import exceptions

from google.cloud.firestore_v1.types import common, firestore, write

to_wrap = AsyncMock(return_value=mock.sentinel.result, spec=[])
wrapped = _make_async_transactional(to_wrap)

txn_id = b"whole-enchilada"
transaction = _make_transaction(txn_id)

# An expired transaction id is InvalidArgument, not Aborted.
exc = exceptions.InvalidArgument(
"The referenced transaction has expired or is no longer valid."
)
firestore_api = transaction._client._firestore_api
firestore_api.commit.side_effect = [
exc,
firestore.CommitResponse(write_results=[write.WriteResult()]),
]

result = await wrapped(transaction, "a", b="c")
assert result is mock.sentinel.result

assert transaction._id is None
assert wrapped.current_id == txn_id
assert wrapped.retry_id == txn_id

wrapped_call = mock.call(transaction, "a", b="c")
assert to_wrap.mock_calls == [wrapped_call, wrapped_call]
db_str = transaction._client._database_string
options_ = common.TransactionOptions(
read_write=common.TransactionOptions.ReadWrite(retry_transaction=txn_id)
)
expected_calls = [
mock.call(
request={"database": db_str, "options": None},
metadata=transaction._client._rpc_metadata,
),
mock.call(
request={"database": db_str, "options": options_},
metadata=transaction._client._rpc_metadata,
),
]
assert firestore_api.begin_transaction.mock_calls == expected_calls
firestore_api.rollback.assert_not_called()
commit_call = mock.call(
request={"database": db_str, "writes": [], "transaction": txn_id},
metadata=transaction._client._rpc_metadata,
)
assert firestore_api.commit.mock_calls == [commit_call, commit_call]


@pytest.mark.asyncio
async def test_asynctransactional___call__invalid_argument_not_expired_does_not_retry():
from google.api_core import exceptions

to_wrap = AsyncMock(return_value=mock.sentinel.result, spec=[])
wrapped = _make_async_transactional(to_wrap)

txn_id = b"not-expired"
transaction = _make_transaction(txn_id)

exc = exceptions.InvalidArgument("other")
firestore_api = transaction._client._firestore_api
firestore_api.commit.side_effect = exc

with pytest.raises(exceptions.InvalidArgument) as exc_info:
await wrapped(transaction, "a", b="c")

assert exc_info.value is exc
to_wrap.assert_called_once_with(transaction, "a", b="c")
firestore_api.commit.assert_called_once_with(
request={
"database": transaction._client._database_string,
"writes": [],
"transaction": txn_id,
},
metadata=transaction._client._rpc_metadata,
)
firestore_api.rollback.assert_called_once_with(
request={
"database": transaction._client._database_string,
"transaction": txn_id,
},
metadata=transaction._client._rpc_metadata,
)
firestore_api.begin_transaction.assert_called_once()


@pytest.mark.parametrize("max_attempts", [1, 5])
@pytest.mark.asyncio
async def test_asynctransactional___call__failure_max_attempts(max_attempts):
Expand Down
15 changes: 15 additions & 0 deletions packages/google-cloud-firestore/tests/unit/v1/test_base_transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,18 @@ def test__basetransactional_reset():

assert wrapped.current_id is None
assert wrapped.retry_id is None


def test_is_retryable_commit_error():
from google.api_core import exceptions

from google.cloud.firestore_v1.base_transaction import _is_retryable_commit_error

assert _is_retryable_commit_error(exceptions.Aborted("Contention."))
assert _is_retryable_commit_error(
exceptions.InvalidArgument(
"The referenced transaction has expired or is no longer valid."
)
)
assert not _is_retryable_commit_error(exceptions.InvalidArgument("other"))
assert not _is_retryable_commit_error(ValueError("transaction has expired"))
91 changes: 91 additions & 0 deletions packages/google-cloud-firestore/tests/unit/v1/test_transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,97 @@ def test__transactional___call__success_second_attempt(database):
assert firestore_api.commit.mock_calls == [commit_call, commit_call]


@pytest.mark.parametrize("database", [None, "somedb"])
def test__transactional___call__success_second_attempt_expired_transaction(database):
from google.api_core import exceptions

from google.cloud.firestore_v1.types import common, firestore, write

to_wrap = mock.Mock(return_value=mock.sentinel.result, spec=[])
wrapped = _make__transactional(to_wrap)

txn_id = b"whole-enchilada"
transaction = _make_transaction_pb(txn_id, database=database)

# An expired transaction id is InvalidArgument, not Aborted.
exc = exceptions.InvalidArgument(
"The referenced transaction has expired or is no longer valid."
)
firestore_api = transaction._client._firestore_api
firestore_api.commit.side_effect = [
exc,
firestore.CommitResponse(write_results=[write.WriteResult()]),
]

result = wrapped(transaction, "a", b="c")
assert result is mock.sentinel.result

assert transaction._id is None
assert wrapped.current_id == txn_id
assert wrapped.retry_id == txn_id

wrapped_call = mock.call(transaction, "a", b="c")
assert to_wrap.mock_calls == [wrapped_call, wrapped_call]
db_str = transaction._client._database_string
options_ = common.TransactionOptions(
read_write=common.TransactionOptions.ReadWrite(retry_transaction=txn_id)
)
expected_calls = [
mock.call(
request={"database": db_str, "options": None},
metadata=transaction._client._rpc_metadata,
),
mock.call(
request={"database": db_str, "options": options_},
metadata=transaction._client._rpc_metadata,
),
]
assert firestore_api.begin_transaction.mock_calls == expected_calls
firestore_api.rollback.assert_not_called()
commit_call = mock.call(
request={"database": db_str, "writes": [], "transaction": txn_id},
metadata=transaction._client._rpc_metadata,
)
assert firestore_api.commit.mock_calls == [commit_call, commit_call]


@pytest.mark.parametrize("database", [None, "somedb"])
def test__transactional___call__invalid_argument_not_expired_does_not_retry(database):
from google.api_core import exceptions

to_wrap = mock.Mock(return_value=mock.sentinel.result, spec=[])
wrapped = _make__transactional(to_wrap)

txn_id = b"not-expired"
transaction = _make_transaction_pb(txn_id, database=database)

exc = exceptions.InvalidArgument("other")
firestore_api = transaction._client._firestore_api
firestore_api.commit.side_effect = exc

with pytest.raises(exceptions.InvalidArgument) as exc_info:
wrapped(transaction, "a", b="c")

assert exc_info.value is exc
to_wrap.assert_called_once_with(transaction, "a", b="c")
firestore_api.commit.assert_called_once_with(
request={
"database": transaction._client._database_string,
"writes": [],
"transaction": txn_id,
},
metadata=transaction._client._rpc_metadata,
)
firestore_api.rollback.assert_called_once_with(
request={
"database": transaction._client._database_string,
"transaction": txn_id,
},
metadata=transaction._client._rpc_metadata,
)
firestore_api.begin_transaction.assert_called_once()


@pytest.mark.parametrize("database", [None, "somedb"])
@pytest.mark.parametrize("max_attempts", [1, 5])
def test_transactional___call__failure_max_attempts(database, max_attempts):
Expand Down
Loading