Skip to content

fix(notifications): abort inflight fetch on unmount (#555) - #893

Open
TCROWN10 wants to merge 1 commit into
Iris-IV:mainfrom
TCROWN10:fix/555-abort-inflight-notifications
Open

fix(notifications): abort inflight fetch on unmount (#555)#893
TCROWN10 wants to merge 1 commit into
Iris-IV:mainfrom
TCROWN10:fix/555-abort-inflight-notifications

Conversation

@TCROWN10

Copy link
Copy Markdown

Summary

  • Abort in-flight fetchNotifications on unmount/wallet change via AbortController so remounts cannot call setNotifications after cleanup
  • Propagate abort signals through remote notification fetch (no local fallback on abort)
  • Add regression tests for unmount and wallet-change abort paths

Closes #555

Type of Change

  • Bug fix

Test plan

  • npm test -- --runTestsByPath src/__tests__/hooks/useNotifications.test.tsx
  • Confirm notification bell still polls while connected
  • Rapidly navigate away/back while notifications are loading and verify no console stale-state warnings

Use AbortController in useNotifications refresh/cleanup so a remount cannot
apply setNotifications from a previous poll (Iris-IV#555).

Co-authored-by: Cursor <cursoragent@cursor.com>
@drips-wave

drips-wave Bot commented Jul 30, 2026

Copy link
Copy Markdown

@TCROWN10 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! 🚀

Learn more about application limits

@sshdopey

Copy link
Copy Markdown
Contributor

Auto-review failed (API error). Leaving PR for human review.

@davidmaronio

Copy link
Copy Markdown
Contributor

well executed fix. the AbortController lives in the effect, is aborted in cleanup, and the signal is threaded all the way through fetchNotifications into the actual fetch call. the decision to rethrow aborts from fetchRemoteNotifications so callers do not fall back to local data on abort (src/lib/notifications.ts:139) shows real care, and the two regression tests (unmount and wallet change) cover exactly the paths in #555.

two small notes, neither blocking:

  1. src/hooks/useNotifications.ts:31 the catch now silently swallows every non-abort error too (previously they surfaced as unhandled rejections). fine to keep quiet, but consider a dev-only console.warn so real network failures are not invisible.
  2. src/hooks/useNotifications.ts:44 each interval tick reuses the effect's single controller. that is correct today since a tick's response for the same wallet is never stale, just noting the signal is per-effect, not per-request, in case someone later adds per-tick timeouts.

the red checks (typecheck, unit, build, lint) match the stale pre-#875 base. please rebase onto current main and confirm green, then good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] useNotifications setInterval doesn't abort inflight fetch on unmount — stale state update on remount

3 participants