fix: stop notification polling from multiplying API requests#3066
Merged
Conversation
…ests TanStack Query schedules refetchInterval per mounted observer, not per query, so every useNotifications consumer (each notification row, repo group, sidebar, filter panel, etc.) ran its own staggered poll timer. A full notification list produced dozens of full fetch + enrichment cycles per interval instead of one, hammering the GitHub/GHES API. Gate refetchInterval, refetchOnMount, refetchOnReconnect and refetchOnWindowFocus behind withSideEffects so only the singleton GlobalEffects host owns polling; all other consumers share the cache.
onRendererEvent registered ipcRenderer listeners with no way to remove them, so every listener was permanent. Return an unsubscribe function from onRendererEvent, onResetApp, onSystemWake and onAuthCallback so callers can clean up.
Every useNotifications consumer (each notification row, repo group, sidebar, etc.) registered a permanent SYSTEM_WAKE ipcRenderer listener, piling up stale closures that all fired refetches on every wake. The OAuth web flow likewise accumulated one permanent AUTH_CALLBACK listener per login attempt. Gate the wake listener behind withSideEffects so only the singleton GlobalEffects host registers it, unsubscribe on effect cleanup everywhere, and drop the auth callback listener once the flow settles.
A failed poll retried almost immediately, doubling request volume against an already-struggling GitHub/GHES instance. Keep the single retry but wait 30 seconds first so outages are not amplified.
|
setchy
reviewed
Jul 23, 2026
| refetchOnWindowFocus: true, | ||
| // Only the singleton side-effects host polls. Other consumers share the | ||
| // cached data and would otherwise each schedule their own refetch timer. | ||
| refetchInterval: withSideEffects && fetchType === FetchType.INTERVAL ? fetchIntervalMs : false, |
Member
There was a problem hiding this comment.
A comment for future us... I was thinking we can drop the FetchType setting now that we're using Tanstack Query since it's better at managing the fetch interval, background fetch, fetch on reconnect, etc
Member
|
Interesting - I wonder why/how I didn't have this within Atlassify. Must be handling this in a different way. |
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
2 participants
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.
Summary
Upgrading to v7 could multiply notification API traffic by an order of magnitude against GitHub/GHES (15k+ extra connections reported). The TanStack Query refactor made every
useNotificationsconsumer (each notification row, repo group, sidebar, header, filter panel) a query observer, and TanStack Query schedulesrefetchIntervalper mounted observer, not per query. Each observer polled on its own staggered timer, so their fetches never deduped, andrefetchIntervalInBackground: truekept all of them firing while the menubar sat hidden. A full notification list produced dozens of complete fetch + GraphQL-enrichment cycles per interval instead of one.Three fixes, in commit order:
refetchInterval,refetchOnMount,refetchOnReconnectandrefetchOnWindowFocusare now gated behind the existingwithSideEffectsflag, so only the app-levelGlobalEffectshost schedules refetches. All other consumers share the cached data, exactly as they already did for sound/native alerts.SYSTEM_WAKElistener per mounted row (the preload bridge offered no unsubscribe at all), piling up stale closures that each fired a refetch on every wake; the OAuth web flow likewise leaked oneAUTH_CALLBACKlistener per login attempt. Preload event registrations now return unsubscribe functions, effects clean up on unmount, the wake listener is host-only, and the auth listener is removed once the flow settles.Validation
ipcRendererlistener.vp check(format + lint + tsc) clean.Closes #3065