You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
src/hooks/useSorobanEvents.test.ts (240 lines) exercises the hook's happy path and some failure handling, but — matching the bug described elsewhere in this batch — there is no test that specifically simulates the RPC rejecting getEvents because startLedger is outside the node's retention window (the most realistic real-world failure mode for a polling hook that can run for an arbitrarily long session), and no test asserting what startLedgerRef does afterward. Whatever the intended recovery behavior is decided to be (re-anchoring to the latest ledger, exponential backoff, a visible 'live updates paused' state), it currently has zero regression protection, meaning a future refactor could easily reintroduce or worsen the permanent-stall behavior without any test failing.
Acceptance Criteria
Add a test to src/hooks/useSorobanEvents.test.ts that mocks getEvents rejecting with an out-of-range/retention-window-style error and asserts the hook recovers on a subsequent tick (re-anchors startLedgerRef) rather than repeating an identically-doomed request forever.
Add a test asserting repeated transient failures trigger backoff (not a fixed 5s retry against a failing endpoint).
Add a test asserting that once recovered, the hook correctly resumes invalidating QUERY_KEYS.POOLS/QUERY_KEYS.USER_POSITION on the next genuine event.
Wire this test file into CI if it isn't already part of the default vitest run suite (verify via package.json's test script).
Relevant Files
src/hooks/useSorobanEvents.test.ts — existing test suite lacks coverage for the retention-window/permanent-stall failure mode
src/hooks/useSorobanEvents.ts — the implementation under test, whose recovery behavior needs to be defined and then locked in by these tests
Additional Notes
Confirmed against current source (src/hooks/useSorobanEvents.ts, 109 lines)
The polling loop lives entirely inside the init() closure created in the useEffect at lines 37-108:
init() (L52-99) first calls server.getLatestLedger() once to seed startLedgerRef.current (L54-59). If this initial call throws, the effect just returns (L58) — the interval is never even created, so a transient failure at mount time produces a useSorobanEvents instance that polls nothing, silently, forever, with no user-visible signal and no retry of the seed step either. That's a second, narrower stall mode worth covering alongside the main one.
The recurring poll is setInterval(async () => {...}, 5000) (L61-98). Inside it:
server.getEvents({ startLedger: startLedgerRef.current, filters: [...], limit: 100 }) (L63-67) is the call that will reject when startLedger falls outside the RPC's retention window (Soroban RPC returns a JSON-RPC error, typically surfaced by @stellar/stellar-sdk's rpc.Server.getEvents as a thrown error with a message like "start is before oldest ledger" — exact wording depends on RPC provider).
The catch {} block at L95-97 is a bare, typeless catch that discards the error entirely — no console.error, no state update, nothing. Because startLedgerRef.current is only ever advanced at L94 (response.latestLedger + 1), a caught exception leaves it untouched, so the next tick issues the identical doomed request. Since retention-window rejections are permanent (the ledger will never come back into range), this is an infinite loop of no-op requests every 5s, indistinguishable from "everything is fine" from the UI's perspective — QUERY_KEYS.POOLS/QUERY_KEYS.USER_POSITION simply stop being invalidated by events (they still get refetched by whatever staleTime/refetchInterval those queries have configured elsewhere, but the "immediate on-event" real-time path silently dies).
Note this hook has no exposed error/status output at all — it returns void (L28) — so today there is no hook-level state a component could even observe to show a "live updates paused" banner without a source change. Tests should make this concrete: after adding a re-anchor/backoff mechanism, decide whether that mechanism needs to be exposed via a return value (e.g. { status: "live" | "stalled" | "recovering" }) for issue useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72's fix to have a UI hook point at all.
This issue's companion, #72 ("useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call"), is the implementation-side bug report for exactly this code path (L95-97's silent catch + L94's conditional advance). The two issues should land together or at least reference each other in their PRs: #72's fix (whatever recovery strategy is chosen — re-anchor to getLatestLedger() on failure, exponential backoff via a failure counter, or both) is what issue #91's tests need to pin down. Landing #91's tests without #72's fix means the tests must be written to fail against current main (red) and pass once #72 lands (green) — a genuine regression test, not a tautological one, mirroring the same requirement called out explicitly in #94 for the leaderboard race fix.
Implementation sketch (for whoever picks up #72, informing what #91 should assert)
Track consecutive-failure count in a ref (e.g. failureCountRef), incremented in the catch block.
On catch, instead of leaving startLedgerRef.current untouched, re-fetch getLatestLedger() and reset startLedgerRef.current to that value once failures exceed some threshold (e.g. 1, since retention-window errors are non-transient) — this is the "re-anchoring" behavior AC Integrate Frontend with Deployed Soroban Contracts via RPC #1 wants tested.
Use failureCountRef to compute the next interval delay (simple doubling capped at some max, e.g. 5s → 10s → 20s → capped at 60s) rather than a fixed 5000ms setInterval — this likely requires switching from setInterval to a self-rescheduling setTimeout chain, since setInterval's period can't be changed once created. That refactor is itself worth flagging in the PR review since it changes the effect's cleanup logic (L103-106 currently just does clearInterval).
Reset failureCountRef to 0 on any successful getEvents call (alongside the existing startLedgerRef.current = response.latestLedger + 1 at L94).
Testing strategy
Test file: src/hooks/useSorobanEvents.test.ts (existing, 240 lines) — follow the existing pattern using vi.useFakeTimers() (already set up in beforeEach, L30) and mockRpc: SorobanEventsRpc objects with vi.fn() for getLatestLedger/getEvents (see the existing lockEvent test at L52-88 for the exact wrapper/renderHook setup with QueryClientProvider).
For the retention-window rejection test: make getEvents reject once (vi.fn().mockRejectedValueOnce(new Error("start is before oldest ledger"))) then resolve on the next call with a fresh getLatestLedger-derived startLedger; advance fake timers with vi.advanceTimersByTimeAsync(5000) twice and assert the second getEvents call's startLedger argument differs from the first (proving re-anchoring occurred) rather than being byte-identical.
For backoff: mock several consecutive rejections and assert the wall-clock gap between successive getEvents calls grows (inspectable via vi.getTimerCount()/vi.advanceTimersByTimeAsync sequencing, or by asserting setTimeout/setInterval was called with increasing delays if the implementation is refactored to a self-rescheduling setTimeout).
For the recovery-then-invalidate assertion: after the mocked recovery tick, feed a normal lockEvent-shaped event (same shape as the existing L56-70 fixture) on the next tick and assert invalidateQueries is called with QUERY_KEYS.USER_POSITION/QUERY_KEYS.POOLS, exactly like the existing passing test — this proves the recovery path doesn't leave the hook in some half-broken state that merely stops throwing but never resumes real invalidation.
CI: confirm via package.json's "test": "vitest run" (L14) that this file is already part of the default suite with no separate include/exclude glob gating it out — it is (no per-file vitest config found), so AC Implement Global Error Handling with Toast Notifications #4 is effectively a no-op verification step, not new wiring.
Problem
src/hooks/useSorobanEvents.test.ts(240 lines) exercises the hook's happy path and some failure handling, but — matching the bug described elsewhere in this batch — there is no test that specifically simulates the RPC rejectinggetEventsbecausestartLedgeris outside the node's retention window (the most realistic real-world failure mode for a polling hook that can run for an arbitrarily long session), and no test asserting whatstartLedgerRefdoes afterward. Whatever the intended recovery behavior is decided to be (re-anchoring to the latest ledger, exponential backoff, a visible 'live updates paused' state), it currently has zero regression protection, meaning a future refactor could easily reintroduce or worsen the permanent-stall behavior without any test failing.Acceptance Criteria
src/hooks/useSorobanEvents.test.tsthat mocksgetEventsrejecting with an out-of-range/retention-window-style error and asserts the hook recovers on a subsequent tick (re-anchorsstartLedgerRef) rather than repeating an identically-doomed request forever.QUERY_KEYS.POOLS/QUERY_KEYS.USER_POSITIONon the next genuine event.vitest runsuite (verify viapackage.json'stestscript).Relevant Files
src/hooks/useSorobanEvents.test.ts— existing test suite lacks coverage for the retention-window/permanent-stall failure modesrc/hooks/useSorobanEvents.ts— the implementation under test, whose recovery behavior needs to be defined and then locked in by these testsAdditional Notes
Confirmed against current source (
src/hooks/useSorobanEvents.ts, 109 lines)The polling loop lives entirely inside the
init()closure created in theuseEffectat lines 37-108:init()(L52-99) first callsserver.getLatestLedger()once to seedstartLedgerRef.current(L54-59). If this initial call throws, the effect justreturns (L58) — the interval is never even created, so a transient failure at mount time produces auseSorobanEventsinstance that polls nothing, silently, forever, with no user-visible signal and no retry of the seed step either. That's a second, narrower stall mode worth covering alongside the main one.setInterval(async () => {...}, 5000)(L61-98). Inside it:server.getEvents({ startLedger: startLedgerRef.current, filters: [...], limit: 100 })(L63-67) is the call that will reject whenstartLedgerfalls outside the RPC's retention window (Soroban RPC returns a JSON-RPC error, typically surfaced by@stellar/stellar-sdk'srpc.Server.getEventsas a thrown error with a message like"start is before oldest ledger"— exact wording depends on RPC provider).catch {}block at L95-97 is a bare, typeless catch that discards the error entirely — noconsole.error, no state update, nothing. BecausestartLedgerRef.currentis only ever advanced at L94 (response.latestLedger + 1), a caught exception leaves it untouched, so the next tick issues the identical doomed request. Since retention-window rejections are permanent (the ledger will never come back into range), this is an infinite loop of no-op requests every 5s, indistinguishable from "everything is fine" from the UI's perspective —QUERY_KEYS.POOLS/QUERY_KEYS.USER_POSITIONsimply stop being invalidated by events (they still get refetched by whateverstaleTime/refetchIntervalthose queries have configured elsewhere, but the "immediate on-event" real-time path silently dies).void(L28) — so today there is no hook-level state a component could even observe to show a "live updates paused" banner without a source change. Tests should make this concrete: after adding a re-anchor/backoff mechanism, decide whether that mechanism needs to be exposed via a return value (e.g.{ status: "live" | "stalled" | "recovering" }) for issue useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72's fix to have a UI hook point at all.Interaction with #72
This issue's companion, #72 ("useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call"), is the implementation-side bug report for exactly this code path (L95-97's silent catch + L94's conditional advance). The two issues should land together or at least reference each other in their PRs: #72's fix (whatever recovery strategy is chosen — re-anchor to
getLatestLedger()on failure, exponential backoff via a failure counter, or both) is what issue #91's tests need to pin down. Landing #91's tests without #72's fix means the tests must be written to fail against currentmain(red) and pass once #72 lands (green) — a genuine regression test, not a tautological one, mirroring the same requirement called out explicitly in #94 for the leaderboard race fix.Implementation sketch (for whoever picks up #72, informing what #91 should assert)
failureCountRef), incremented in thecatchblock.startLedgerRef.currentuntouched, re-fetchgetLatestLedger()and resetstartLedgerRef.currentto that value once failures exceed some threshold (e.g. 1, since retention-window errors are non-transient) — this is the "re-anchoring" behavior AC Integrate Frontend with Deployed Soroban Contracts via RPC #1 wants tested.failureCountRefto compute the next interval delay (simple doubling capped at some max, e.g. 5s → 10s → 20s → capped at 60s) rather than a fixed 5000mssetInterval— this likely requires switching fromsetIntervalto a self-reschedulingsetTimeoutchain, sincesetInterval's period can't be changed once created. That refactor is itself worth flagging in the PR review since it changes the effect's cleanup logic (L103-106 currently just doesclearInterval).failureCountRefto 0 on any successfulgetEventscall (alongside the existingstartLedgerRef.current = response.latestLedger + 1at L94).Testing strategy
src/hooks/useSorobanEvents.test.ts(existing, 240 lines) — follow the existing pattern usingvi.useFakeTimers()(already set up inbeforeEach, L30) andmockRpc: SorobanEventsRpcobjects withvi.fn()forgetLatestLedger/getEvents(see the existinglockEventtest at L52-88 for the exact wrapper/renderHooksetup withQueryClientProvider).getEventsreject once (vi.fn().mockRejectedValueOnce(new Error("start is before oldest ledger"))) then resolve on the next call with a freshgetLatestLedger-derivedstartLedger; advance fake timers withvi.advanceTimersByTimeAsync(5000)twice and assert the secondgetEventscall'sstartLedgerargument differs from the first (proving re-anchoring occurred) rather than being byte-identical.getEventscalls grows (inspectable viavi.getTimerCount()/vi.advanceTimersByTimeAsyncsequencing, or by assertingsetTimeout/setIntervalwas called with increasing delays if the implementation is refactored to a self-reschedulingsetTimeout).lockEvent-shaped event (same shape as the existing L56-70 fixture) on the next tick and assertinvalidateQueriesis called withQUERY_KEYS.USER_POSITION/QUERY_KEYS.POOLS, exactly like the existing passing test — this proves the recovery path doesn't leave the hook in some half-broken state that merely stops throwing but never resumes real invalidation.package.json's"test": "vitest run"(L14) that this file is already part of the default suite with no separate include/exclude glob gating it out — it is (no per-file vitest config found), so AC Implement Global Error Handling with Toast Notifications #4 is effectively a no-op verification step, not new wiring.