fix: useAggregatedQueryKeys should use an Observer to persist fetch results

[ryan953]

What was happening before is that I was doing some manual tracking of which queryKeys had been requested, and then tracked the 'done' state in the query cache.

This was causing trouble because over time the query cache will have items evicted, so the actual result might be gone, while the record that we used to have a result remained. This is bad and causes us to not re-fetch data.

So the change is minor, big deal if you didn't read all the docs like me earlier... But basically we can setup a QueryObserver which will help hold onto the query results so they don't get evicted quite so fast. We also drop the 'done' state from the cache to unblock re-fetches. The rest of the PR is just a little cleanup function for the observers which fires on unmount... and we can put the inFlight state stuff into the observer callback, replacing the async/await keywords.

---

The repro steps for the bug are simple.

1. Visit this issue stream page: https://sentry.sentry.io/issues/?project=11276&query=useOrganization&referrer=issue-list&statsPeriod=30d
2. Notice how issue JAVASCRIPT-2RZ7 has no replays (no icon, no count)
3. Notice how other issues do have replay counts appearing
4. Click on JAVASCRIPT-2RZ7 to visit the details page
5. Expected: The replays tab on the details page should say 'Replays (0)'. Actual: the count icon is empty, it shows 'Replays ()'
6. Refresh the details page (ctrl+r) and you'll see the count appear 'Replays(0)' as expected

Fixes #66973

[codecov]

Bundle Report

Changes will decrease total bundle size by 150 bytes ⬇️

Bundle name	Size	Change
sentry-webpack-bundle-array-push	25.54MB	150 bytes ⬇️

[malwilley]

Overall a good improvement, just have a couple questions. Didn't know about QueryObserver until now!

[malwilley]

I'm a bit confused about this done key, I see it is set here but did anything read from this cache key?

[ryan953]

stuff was reading from it yeah. We were reading anything with a prefix matching ['aggregate', cacheKey, key] to see if a given queryKey was already in the cache or not. So like we'd get a list of cached "query keys" matching['aggregate', cacheKey, key, *, *] and then look at the list position in the tuple to get the key/id (also called 'queryKey') which is passed into requestPromise.... if that key/id was found in the cache then we wouldn't re-requet the id.

This sort of everlasting cache was the source of the bug.

The whole thing still doesn't accept options where we can set cache duration. This change sets the cache from like 5mins (setQueryData has a fixed duration there...) to cache=0.

[malwilley]

Ahh okay I see now, that makes sense

[malwilley]

Since we unsubscribe on unmount, seems the cache keys can be evicted if you go to another page not rendering this hook (which we probably want since this matches useApiQuery behavior). Are there any possible bugs in this scenario or should this hook handle it gracefully?

[ryan953]

I think the hook will handle it fine, if the keys are evicted then we'll re-fetch.

Also, because it's a buffer, we could often be re-fetching a different list of id's. For example, you're on the issue stream and have issues abc-3, abc-2, abc-1 captured so far. We'll fetch /issue-count/?id=3,2,1. If you navigate away then back a new issue abc-4 might've appeared, we'll either fetch only ?id=4 if the cache is still full, or ?id=4,3,2,1 and get new counts for each.