feat: add SDK transaction timeout classification (#208) - #327
Merged
El-swaggerito merged 1 commit intoJul 27, 2026
Merged
Conversation
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.
Closes #208
Summary
Classifies SDK timeouts by the stage they interrupt, and routes the two stages
whose outcome is undetermined to
TX_STATUS_UNKNOWNso consumers know whenretrying is unsafe.
The gap
Every timeout in the SDK came from a single
timeoutError()insrc/network/index.ts, which threw the codeREQUEST_TIMEOUT— absent from theERROR_CODESregistry — and fed 33withTimeoutcall sites spanning accountloads, simulation, submission and confirmation polling.
The stage was already available: it arrives as
withTimeout's first argument.It was only interpolated into the message and then discarded.
Why it mattered
payments/index.tswrapsserver.submitTransactioninwithTimeout, and itsrecovery-hint block matched
REQUEST_TIMEOUTto pushaction: 'retry',retryable: true,suggestedDelayMs: 3000— for a payment whose outcome wasunknown. Advising a retry there risks paying twice.
TX_STATUS_UNKNOWNandisUnknownStatusError()exist for exactly this case,but the helper matches only that code, so it returned
falsefor every timeoutthe SDK threw.
Mapping timeouts to
NET_TIMEOUTwould have been worse: it isretryable: true,so any automation reading
retryablewould resend the payment.Changes
src/types/index.ts—TimeoutStage(preparation|submission|confirmation|unknown) andTimeoutMetadata;PocketPayError.timeoutcarries stage, operation and budget.
src/network/index.ts—inferTimeoutStage()derives the stage from theoperation label already passed;
withTimeouttakes an optional explicitstagethat wins over inference. Submission and confirmation timeouts reportTX_STATUS_UNKNOWN; every other stage keepsREQUEST_TIMEOUT.src/errors/codes.ts—REQUEST_TIMEOUTadded to the registry, with adeveloper hint pointing at
error.timeout.stage.src/payments/index.ts— aTX_STATUS_UNKNOWNfailure now gets acheck_statushint withretryable: false, instead of the plain retry hint.docs/timeout-classification.md(new) — stages, recovery per stage, andwhy
NET_TIMEOUTwas not used.Existing consumers remain compatible
REQUEST_TIMEOUTkeeps its exact code string and message format, and is now aregistered code rather than an unknown one, so
describeError()returns realguidance instead of the generic fallback.
The four existing assertions on
REQUEST_TIMEOUT— intests/fund.test.ts,tests/mockHorizon.test.tsandtests/transactions.test.ts(×2) — all staygreen, because none of them is a submission or confirmation timeout: they cover
account lookup, Friendbot and plain read requests. Only those two stages change
code.
Behaviour change: submission and confirmation timeouts report
TX_STATUS_UNKNOWNinstead ofREQUEST_TIMEOUT. Consumers branching onREQUEST_TIMEOUTfor those stages should useisUnknownStatusError().Tests
17 new tests in
tests/timeout-classification.test.ts: stage inference acrossthe real operation labels used in the codebase; explicit stage overriding
inference; metadata attached to every timeout; each of the four stages covered;
isUnknownStatusError()true after submission and confirmation timeouts andfalse after preparation; the unknown-outcome timeout never reported as retryable
and never mapped to
NET_TIMEOUT;REQUEST_TIMEOUTnow registry-known; andsuccessful work plus non-timeout rejections passing through unchanged.
Verification
lint,check:circular(46 modules, no cycles) andbuildall pass.Full suite: 46 failed · 765 passed · 1 skipped. The 46 failures are
pre-existing on the base commit — verified against a clean checkout of
ef46a9a, which gives 46 failed · 748 passed · 1 skipped: same failures inthe same five files. This branch adds exactly the 17 new passing tests and
introduces no regressions.