Skip to content

No test coverage for useSorobanEvents' out-of-range / ledger-retention-window getEvents failure path #91

Description

@prodbycorne

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 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.

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 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)

  1. Track consecutive-failure count in a ref (e.g. failureCountRef), incremented in the catch block.
  2. 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.
  3. 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).
  4. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial CampaignCampaign: Official CampaignOfficial Campaign | FWC26Campaign: Official Campaign | FWC26sorobanSoroban smart-contract integration (XDR, RPC, transaction building)testingUnit, integration, or e2e test coveragevery hardExtremely hard — deep expertise, careful design, and significant time required

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions