fix(chain): preserve last error when TxSettler exhausts retries#2532
Merged
Conversation
TxSettler.transact() raised a bare "Failed to send transaction after N retries" on retry exhaustion, dropping the underlying RPC error that drove every retry. Callers that classify failures by inspecting the error string (e.g. mapping a gas/funds RPC rejection to a typed insufficient-funds error) never saw the cause, so a diagnosable insufficient-gas failure surfaced as an opaque timeout. Capture the last caught exception and include it in the ChainTimeoutError message and as the exception cause (`raise ... from last_error`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aad78e9 to
0dfcaa1
Compare
jmoreira-valory
approved these changes
Jun 9, 2026
jmoreira-valory
left a comment
Contributor
There was a problem hiding this comment.
The fix is tight and well-scoped: 9 lines in tx.py + copyright bump + a 42-line test, touching exactly 3 files. Nothing extraneous.
Key points verified:
raise ChainTimeoutError(...) from last_errorwithlast_error is Noneon the deadline path is valid PEP-3134 (raise X from Nonesuppresses context but is legal and intentional here — the caller gets a clean timeout error with no misleading implicit chain when no retry ever ran).- The reprice branch correctly assigns
last_error = ebeforecontinue, so the final timeout always carries the most recent underlying error rather thanNoneor a stale one. - The existing
test_timeouttest already covers deadline-driven exhaustion with zero exceptions raised; the newtest_timeout_preserves_last_errorcloses the gap for the case where every attempt raises.
One NIT filed (non-blocking): the time.sleep mock comment reads as load-bearing for correctness when it is actually just a speed optimization. Cosmetic only.
LGTM.
LOCKhart07
approved these changes
Jun 9, 2026
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
TxSettler.transact()exhausts its retry/timeout budget, it raises:This drops the underlying RPC error that drove every retry. Callers that classify failures by inspecting the error string cannot see the cause, so a diagnosable failure (e.g. an insufficient-gas / insufficient-funds RPC rejection that was repriced on every attempt) surfaces as an opaque timeout.
Concrete impact
A downstream consumer maps gas/funds RPC rejections to a typed insufficient-funds error by matching the RPC error string. When the same condition happens to return a non-retryable wording, it propagates via
ChainInteractionError(carrying the original RPC payload) and is classified correctly. But when every attempt returns a retryable wording (e.g.intrinsic gas too low: gas 0→ reprice), the loop runs to exhaustion and raises the bareChainTimeoutError, whose message contains none of the original error — so the same root cause is misclassified and returned as a generic 500 instead of a typed 4xx.Fix
last_errorinside the retry loop.ChainTimeoutErrormessage and chain it as the cause (raise ChainTimeoutError(message) from last_error).Before:
After:
…with
__cause__set for a full traceback.The success path, signature, and return type are unchanged. The existing
test_same_tx_sent_twiceassertion usesre.searchon the prefix, which the appended suffix preserves.Tests
test_timeout_preserves_last_error: drives the reprice→exhaustion path and asserts the timeout message includes the underlying error and that__cause__is set.Linters
black ✓ · isort ✓ · flake8 ✓ · darglint ✓ · mypy (
autonomy/chain/tx.py) ✓ — no new issues introduced.