feat: surface transaction abort reason on AbortedError - #307
Open
rahst12 wants to merge 2 commits into
Open
Conversation
Same correction as dgraph4j dgraph-io#290. No behaviour change is needed - no category was added or renamed, and the prefix parser already handles every message the server now sends - but the fixtures and docstrings had gone stale. - the stale-startts fixture quoted "Transaction has been aborted due to a leader change", which the server no longer emits. Zero raises its validation floor on a leader change and also when trimming its conflict map at a snapshot, which is not a leader change, so the message names both and so does the docstring now - UNKNOWN was documented as meaning "an older server". A current server also returns uncategorized aborts on purpose: where no published category fits it sends the explanation without a code rather than borrowing one that implies the wrong remedy - the conflict docstring now notes the collision may be on an index or count key derived from what was written, and that @upsert excludes the uid Fixtures now hold the verbatim server strings as module constants, so the wire format is recorded in one place. Adds coverage for two ways parsing could go wrong: the conflict and completed-move messages carry colons of their own, so only the first may delimit the category; and the out-of-band abort opens with the exact sentence a pre-feature server sent for every abort, where a false CONFLICT would tell a caller to retry something that cannot succeed. 13 tests pass, and test_retry (which asserts on the unchanged default message) still passes. ruff check and ruff format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When Dgraph aborts a transaction, pydgraph collapses every cause into a single opaque
exception:
AbortedErrorexposes no way to tell why the transaction aborted, so an applicationcannot distinguish the cases that warrant different responses:
temporarily blocked, so back off and retry once the move completes;
change); retry with a fresh transaction.
The server now reports the category (see the companion
dgraphPR), encoding it as a"<code>: <detail>"prefix on the gRPCABORTEDstatus. Previously pydgraph discardedeven the server's message — it raised a bare
AbortedErrorwith a hardcoded defaultstring — so that information never reached the caller at all.
Fix
Surface the category as a typed attribute on
AbortedError, parsed from the message theserver already sends.
AbortReasonenum —CONFLICT,PREDICATE_MOVE,STALE_STARTTS,UNKNOWN(exported from the top-level
pydgraphpackage).AbortedError.reason— the parsedAbortReason, derived viaparse_abort_reason()from the
"<code>: <detail>"prefix. The full server text remains available viastr(error).txn.py,async_txn.py) nowpass the server's message into
AbortedErrorinstead of raising a reasonless default;util.abort_error_message()extracts it from the gRPC error (preferringerror.details(), falling back tostr(error)).Backward compatible by design:
reasonisAbortReason.UNKNOWN, so callers degrade gracefully.except AbortedError/ retry logic is unaffected.Tests
tests/test_abort_reason.py— unit tests forparse_abort_reason()andAbortedError.reason: each category parses correctly, and an empty or prefix-lessmessage degrades to
UNKNOWN.tests/test_abort_reason_live.py— end-to-end test that drives a real (locallypatched) Dgraph cluster, forces each abort category, and asserts it propagates to
AbortedError.reason. Skips gracefully when cluster prerequisites are unavailable.tests/docker-compose.multigroup.ymlstands up the multi-group cluster needed for thepredicate-move case.
Future work
This change surfaces the category only — the slice the server provides today. Once the
server enriches aborts with the contended predicate and UID/token (planned
dgraphfollow-up via the gRPC rich-error model, then a first-class
TxnContextfield), thisclient can add typed accessors (e.g.
conflict_predicates) without breaking thereasonattribute introduced here. A later phase may also expand
AbortReason(e.g. splittingnon-move
predicate-movecases intoPREDICATE_UNAVAILABLE/INTERNAL); unknown codesalready map to
UNKNOWN, so adding values stays backward compatible.Example
Before, every abort looked the same — there was no way to branch on the cause:
Now the category is available via
error.reason:Checklist
Conventional Commits syntax, leading
with
fix:,feat:,chore:,ci:, etc.