useRetry hook leaves loading stuck true after a successful retry

[abayomicornelius]

useRetry hook leaves loading stuck true after a successful retry
In src/hooks/useRetry.ts, the run callback's success path does await fn(); setAttempt(0); return inside the for-loop, which exits the function before the trailing setLoading(false) after the loop is ever reached.

Any component relying on useRetry's loading flag to disable a button or show a spinner will have it stuck true forever after a successful call. Move setLoading(false) into a finally block or explicitly set it before each return.

Additional Notes

Precision on the control flow bug: the loop structure is effectively:

setLoading(true);
for (let i = 0; i <= maxRetries; i++) {
  try {
    const result = await fn();
    setAttempt(0);
    return result;       // <-- early return skips setLoading(false) below the loop
  } catch (e) {
    if (i === maxRetries) { setLoading(false); throw e; }
    await delay(backoff(i));
  }
}

Note the asymmetry: the failure path (when retries are exhausted) does correctly call setLoading(false) before throwing, but the success path does not — meaning this bug is more subtle than a totally missing reset; a naive read of the code could easily conclude loading is handled because it clearly is handled somewhere in the same function.

Implementation sketch: wrap the entire loop body in a try { ... } finally { setLoading(false); } at the top level of run, and remove the now-redundant setLoading(false) from the failure branch to avoid it running twice. Double-check that setAttempt state isn't read anywhere during the brief window between the successful await fn() resolving and finally running, since finally runs after the return value is computed but before it's returned to the caller — this ordering is safe for React state setters (they're not synchronous effects) but worth a comment for future readers.

Edge cases: a component that calls run() again while loading is (incorrectly) still true from a prior successful call — does the UI currently prevent double-submission via loading, and would fixing this change reveal a previously-masked race in a caller? Search all call sites of useRetry (e.g., any retry-wrapped fetches in useTransactions/useStellarAccount) to confirm none of them relied on the stuck-true state as an accidental "already succeeded, don't retry" guard.

Testing strategy: a unit test that resolves fn successfully on the first call and asserts loading === false immediately after await run() resolves (this is the one directly missing today); a second test exhausting retries to confirm the existing failure-path behavior is unchanged; a third test using fake timers to verify backoff delays between attempts are not affected by the fix.

Cross-references: this is one of several hook-level state-machine bugs bundled in this bounty batch (see also #5's error-swallowing and #12's module-scoped counter in the new-issues batch) — all are prime candidates to fix together once issue #1 (test runner) lands, since none currently have any regression coverage.

[YfengJ]

Hi maintainers, I’d like to take this if it is available.

If assigned, I’ll keep the change scoped to the useRetry loading-state bug: reproduce the success-path stuck-loading case, update the hook so loading reliably resets after success and exhausted failures, and add focused hook tests around success, retry, and final failure behavior.

I’ll wait for maintainer or GrantFox assignment before starting work.