[2026 Reliability] Cancel stale Horizon requests during account changes - #920
Open
solaawojobi00-bit wants to merge 1 commit into
Open
Conversation
Slower responses for a previously selected account or network could resolve
after a newer request and overwrite current state, showing the wrong account's
data. Guarding with a local isActive flag only covered a single useEffect and
left the connect handler and cross-component reads unprotected.
Add a lane-scoped request coordinator implementing latest-request-wins. Each
lane names a state slot; starting new work on a lane aborts the previous lease
and marks it inactive, so a superseded response is both cancelled where the
transport allows and discarded if it still arrives.
- src/lib/requestCancellation.ts: RequestCoordinator, RequestLease,
StaleRequestError and cancellation predicates. Degrades to token-only
invalidation where AbortController is unavailable, preserving correctness.
- src/lib/stellar.ts: optional trailing HorizonRequestOptions ({ signal }) on
fetchAccount, fetchTransactions, fetchOperations, fetchAccountOffers,
fetchAccountCreationDate and fetchClaimableBalances. Backwards compatible.
fetchAccountCreationDate now rethrows aborts instead of resolving null.
- ConnectPanel: the whole connect flow runs under one lease and pins the
network, so a mid-flight switch cannot mix networks.
- Account: offers and creation-date reads use leases and abort on cleanup.
- store.setNetwork: aborts in-flight account reads and clears the loading
flags those cancelled requests can no longer clear themselves.
Repository repair required to run any of this: package.json and
package-lock.json were both invalid JSON, and optionalDependencies declared
@stellar/ledger@^1.0.0, which does not exist on npm and had no lock entry, so
npm ci failed for every job. Removes the duplicated lines, the phantom
dependency and two duplicate keys.
Tests: 42 new tests covering the primary flow, boundary cases (already-aborted
signals, unsupported environments, idle lanes) and failure paths (genuine
errors versus cancellations, invalid lane input).
Docs: docs/REQUEST_CANCELLATION.md with compatibility, security and migration
notes, linked from docs/contributing.md.
|
@solaawojobi00-bit Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
@solaawojobi00-bit is attempting to deploy a commit to the nanle-code's projects Team on Vercel. A member of the Team first needs to authorize it. |
Collaborator
|
@solaawojobi00-bit please fix all CI checks failure |
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.
Cancel stale Horizon requests during account changes
Prevents a slower response for an old account or network from overwriting current state.
Problem
Account data is loaded asynchronously from Horizon. When the user switches account or network while a read is in flight, two responses race and nothing guarantees they land in request order:
The existing
isActiveboolean inAccount.tsxcovered oneuseEffectonly. It did not protect the connect handler inConnectPanel.tsx, did not cancel the request, and did not coordinate between components reading the same account.Scenarios
setNetworkfinallyruns after new one startscommit()refuses; only the newest clears itcreatedAtreset to the old account's valueSolution
A lane-scoped coordinator implementing latest-request-wins.
A lane names a state slot; a lease is one unit of work on it. Starting new work on a lane invalidates every earlier lease there:
lease.signalfetchlease.active/lease.commit()The distinction is load-bearing: the Stellar SDK's
CallBuilderandloadAccount()accept noAbortSignal, so those requests cannot always be stopped on the wire. The active check is what actually fixes the bug; abort is the optimisation on top.Key design point: a lane keys on the state slot being written, not the account being read. There is one
accountDatain the store, so all account loads share one lane and the newest always wins. Keying by address would give A and B separate lanes that never cancel each other - the bug itself.Changes by file
src/lib/requestCancellation.ts(new)RequestCoordinator,RequestLease,StaleRequestError, and theisStaleRequestError/isAbortError/isCancellationpredicates.run()rejects if the lease goes stale before or while the work runs, so a superseded response can never reach the caller. Once a lease is stale,run()reports any outcome asStaleRequestError- including a genuine network failure - so an error belonging to an abandoned account cannot surface over current data.src/lib/stellar.tsOptional trailing
HorizonRequestOptions({ signal }) on the six account-scoped readers:fetchAccount,fetchTransactions,fetchOperations,fetchAccountOffers,fetchAccountCreationDate,fetchClaimableBalances. Each rejects withAbortErrorbefore issuing a request if already aborted, and if the signal fires mid-flight.fetchAccountCreationDatepreviously swallowed every error and returnednull. It now rethrows aborts - otherwise a cancelled read would resolve as "no creation date" and clear the current account's value:fetchClaimableBalancesgoes throughfetch, so its signal cancels on the wire.src/components/dashboard/ConnectPanel.tsxThe whole connect flow (resolve -> account -> transactions -> operations) runs under one lease, and the network is pinned for its duration so a mid-flight switch cannot mix an address resolved on one network with data fetched from another.
src/components/dashboard/Account.tsxOffers and creation-date reads replace the
isActiveflag with leases, aborting on effect cleanup. Also fixes a latent type error:setCreatedAtreceived thestringreturned byfetchAccountCreationDatewhile the state isDate | null.src/lib/store.tssetNetworkaborts all account lanes, and clearsaccountLoading/txLoading/opsLoading. Those cancelled requests' ownfinallyhandlers are lease-guarded and will no longer fire, which would otherwise leave a spinner stuck on permanently. Found while reviewing the first cut of this change; covered by a regression test.Regression tests
42 new tests. Mapped to the acceptance criteria:
run()resolves; lanes independentrequestCancellation.test.tsstellar.cancellation.test.tsrequestCancellation.test.tsstore.networkCancellation.test.tsstellar.cancellation.test.tsabort()on an idle lane is a no-oprequestCancellation.test.tsstore.networkCancellation.test.tsAbortControllerdeleted -> token-only invalidation still correctrequestCancellation.test.tsrequestCancellation.test.tsTypeErrorrequestCancellation.test.tsTesting
Targeted before/after comparison over every test file touching the changed modules:
The failure set is identical in both runs - the same three pre-existing failures in
tests/unit/lib/stellar.test.ts, which usesrequire()inside an ESM test file:+42 passing, 0 new failures.
New tests in isolation:
Static checks:
Documentation
docs/REQUEST_CANCELLATION.md- the race, the lane/lease model, API reference, how to choose a lane name, error handling, plus the required compatibility, security and migration notes. Linked fromdocs/contributing.mdunder a new "Async data loading" convention.Compatibility highlights:
optionsis optional and trailing on every reader; existing call sites are untouched.AbortController(older webviews, SSR shims): detected once and exported assupportsAbortController. Degrades to token-only invalidation -lease.signalisundefinedand nothing is cancelled on the wire, but stale responses are still discarded, so correctness holds.publicKeyandnetwork, so a cancelled read for one account can never serve another.Notes for reviewers
Prerequisite repair (please read)
mastercould not install at all.package.jsonandpackage-lock.jsonwere both invalid JSON - commit 4405a8c duplicated lines without commas - sonpm cifailed and every CI job was red on every open PR.optionalDependenciesalso declared@stellar/ledger@^1.0.0, which 404s on npm and had no lock entry, sonpm cirefused as out of sync even once the JSON parsed.This PR includes the minimum needed to make the repo installable again:
@ledgerhq/hw-transport-webusbline (both files)hoist-non-react-statics/react-is/ truncatedhtml-encoding-snifferblock from the lock@stellar/ledgerfromoptionalDependenciesnpm cidocs:validate-driftand@tensorflow/tfjs-nodekeysnpm cinow succeeds (1301 packages). Happy to split this into its own PR if you would rather keep #745 scope-pure.Pre-existing breakage this PR does not fix
So CI results are not misread as regressions from this change:
lint- 51 pre-existing errors, e.g.DataSourceBadgeis rendered atAccount.tsx:188but never imported.format:checkalso fails oneslint.config.js,tsconfig.json,ci.ymland.prettierrc.json, none of which this PR touches.type-check- 1429 pre-existing errors across the codebase.docs-scripts/validate-docs-drift.mjsis two complete, different implementations concatenated into one 331-line file, so it is a syntax error and the workflow cannot run. Deciding which implementation survives is a maintainer call, so it is left untouched. The new doc was validated by hand instead (relative links resolve; no references to nonexistent scripts).npm test- hangs indefinitely onmastersomewhere intests/unit/lib(killed at 20+ min, twice). This is why verification above uses a scoped before/after comparison rather than a whole-suite number.Each of these is happy to become its own issue if useful.
Review pointers
docs/REQUEST_CANCELLATION.mdsection Choosing a lane name explains why lanes must not be keyed by address.withAbort()instellar.tsdetaches the caller from an SDK promise that cannot itself be cancelled. The HTTP request may still complete; the point is that its result can no longer reach state.setNetworkloading-flag reset is easy to miss but necessary - without it, cancelling an in-flight connect leaves the spinner on forever.Closes #745