Repository navigation
feat(txn): surface transaction abort reason on AbortedError - #288
Open
matthewmcneely wants to merge 3 commits into
Open
matthewmcneely wants to merge 3 commits into
matthewmcneely wants to merge 3 commits into
Conversation
Dgraph now reports why it aborted a transaction as a "<code>: <detail>" prefix on the gRPC Aborted status (dgraph-io/dgraph#9747). Txn flattened every Aborted status to the bare ErrAborted sentinel, so that reason never reached the caller. Txn.Commit and the mutation paths of Txn.Do/Mutate now return an *AbortedError carrying the parsed Reason and the server's message. It unwraps to ErrAborted, so errors.Is(err, dgo.ErrAborted) keeps working; a direct == comparison against ErrAborted no longer matches. Parsing matches dgraph4j#290 and pydgraph#307: the text before the first colon, trimmed and case-insensitive, mapped onto conflict, predicate-move or stale-startts, with anything else (including every message from an older server) reported as AbortReasonUnknown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
status.Code(err) on an *AbortedError returned codes.Unknown, so code that classifies errors by gRPC status (the dgraph live loader, for one) didn't see an abort. GRPCStatus reports the Aborted status with the server's message. Test fixtures now use the server's final framing, where every abort message carries "Transaction has been aborted. Please retry" after the category. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
matthewmcneely
added a commit
to rahst12/dgraph
that referenced
this pull request
Oct 22, 2026
dgraph-io/dgo#288 makes Txn return an *AbortedError that wraps ErrAborted, carrying the server's abort reason. A direct == comparison against the sentinel stops matching once dgraph moves to that dgo, so the live loader would print aborts as "Error while mutating" and the mutations systests would treat an abort as a failure. errors.Is matches both the current sentinel and the wrapped error, so this is safe ahead of the dgo bump. The edgraph comparisons are left as is: they check worker.CommitOverNetwork's return, not a dgo client error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
matthewmcneely
added a commit
to rahst12/dgraph
that referenced
this pull request
Oct 22, 2026
- Abort details from checkPreds name the predicate as the user wrote it. Preds entries carry the namespaced attribute, so clients were about to receive "0-name" in a wire message that has never been published before. displayPred strips the namespace with a guarded strings.Cut, since Preds is client-supplied and x.ParseAttr would panic on an entry without a separator. - CommitOverNetwork emits its "Commit status" span event for server-decided aborts too, now with the reason, so the aborts most worth tracing don't go dark. - The pre-aborted detail lists a concurrent client Discard as a fourth cause and stops reading as exhaustive. proposeTxn's race comment no longer claims a predicate move aborts pending transactions: a move blocks commits in checkPreds, and the move path waits with WaitForTs rather than calling TryAbort. - Comments that described dgraph4j and pydgraph behavior in the present tense now name the open client PRs, including dgraph-io/dgo#288. - TestStaleStartTsAbortReason polls the commit itself until it returns codes.Aborted. A Zero leader existing doesn't mean the Alpha has a connection to it yet, and in that window the commit fails with codes.Unknown. - Copyright headers on the new test files use 2017-2026. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
matthewmcneely
added a commit
to rahst12/dgraph
that referenced
this pull request
Oct 22, 2026
Abort messages can now name a predicate, so a predicate-move abort on a predicate called e.g. x509cert would hit the x509 case in handleError and make the live loader exit. The abort check now comes first and also matches dgo.ErrAborted via errors.Is, since dgo hands the loader that rather than the raw status. TestTxnDiscardBeforeCommit asserted NotEqual(err, dgo.ErrAborted), which passes vacuously once dgo returns a wrapping error; it now requires no error. Test comments no longer state that dgo always discards the abort reason, since dgraph-io/dgo#288 keeps it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 22, 2026
staticcheck ST1023 flagged the explicit error type on a fmt.Errorf result. Co-Authored-By: Claude Opus 5.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.
Description
This PR surfaces the server's transaction abort reason in dgo, matching what dgraph-io/dgraph4j#290 and dgraph-io/pydgraph#307 do for the Java and Python clients.
dgraph-io/dgraph#9747 has Zero report why it aborted a transaction as a
"<code>: <detail>"prefix on the gRPCAbortedstatus (conflict,predicate-move, orstale-startts). TodayTxnflattens everyAbortedstatus to the baredgo.ErrAbortedsentinel, so callers can't tell a write-write conflict (retry now) from a predicate move (back off first).Changes
AbortReasontype withAbortReasonConflict,AbortReasonPredicateMove,AbortReasonStaleStartTs, andAbortReasonUnknown.*AbortedError{Reason, Message}, returned byTxn.Commitand by the mutation paths ofTxn.Mutate/Txn.Doin place ofErrAborted.Error()returns the server's message verbatim, same asgetMessage()in dgraph4j andstr(e)in pydgraph.AbortedErrorimplementsGRPCStatus(), sostatus.Code(err)reportscodes.Abortedinstead ofcodes.Unknown.ParseAbortReason(msg)is exported for callers using the rawapi.DgraphClient. Parsing follows the other two clients: text before the first colon, trimmed and case-insensitive. Anything unrecognized maps toAbortReasonUnknown, which covers every message from a server without #9747 as well as causes the server deliberately leaves uncategorized.errors.Is(it also had a strayy.ErrAbortedtypo), plus a shorterrors.Asexample.Compatibility
AbortedErrorunwraps toErrAborted, soerrors.Is(err, dgo.ErrAborted)keeps working, and every abort message from the server still contains "Transaction has been aborted. Please retry", so substring checks onerr.Error()keep matching too. A directerr == dgo.ErrAbortedcomparison no longer matches. The Java and Python clients could hang the reason off their existing exception type, but Go can't attach data to a sentinel value, so this is the trade. It should get a call-out in the release notes.The dgraph repo had a handful of
== dgo.ErrAbortedcomparisons in the live loader and systests. Those are switched toerrors.Isin #9747, so bumping dgraph to this dgo is safe.Testing
abort_test.go: unit tests for prefix parsing (including colons inside the detail, unknown categories, and pre-#9747 messages) and forerrors.Is/errors.Asthrough a wrapped error.TestTxnErrAbortednow actually checks the error. The oldrequire.Error(t, err, dgo.ErrAborted, ...)only checked for non-nil, since the sentinel landed inmsgAndArgs. It assertsErrorIs,ErrorAs, and a reason of conflict or unknown, so it passes against dgraphmainbefore and after #9747 merges.AbortReasonConflictwith the server's full conflict message.Checklist