feat: add cacheGet probe API and null-sentinel caching#486
Open
funkypenguin wants to merge 1 commit into
Open
Conversation
Three related changes to cache hygiene:
1. cacheWrap / cacheWrapGlobal now persist null/undefined results as a
`{__null_sentinel: true}` entry. This prevents hot re-fetch loops
when an upstream provider legitimately returns nothing (e.g. fanart
for a series that doesn't have it). Sentinels expire per the result
classifier's EMPTY_RESULT TTL, raised from 60s to 15min to match.
2. Fix an in-flight deduplication race: previously the in-flight
promise was registered AFTER `await redis.get()`, so concurrent
callers for the same key could each get past the `has()` check and
start their own producer call. Wrap the entire flow in an IIFE
async promise and register it before any await.
3. Add cacheGet / cacheGetGlobal — read-only probe functions that
return the cached value or null on miss WITHOUT invoking any
producer or writing to Redis. Migrate the four call sites in
getCatalog.ts (Trakt up-next, Trakt unwatched) that had been using
`cacheWrap(key, async () => null, ttl)` as a probe.
The probe pattern at those four sites was load-bearing before this
change because nothing caused a sentinel write. With (1) in place,
that same pattern poisons the cache: on a miss the no-op producer
returns null, the sentinel is written, and the next cacheWrap call to
populate the key sees the sentinel and returns null without invoking
its real producer. Trakt up-next / unwatched catalogs would stop
updating until the sentinel TTL expired. cacheGet keeps the
read-only intent explicit and avoids that interaction.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
PR Guard
Maintainers may still close PRs that do not match project direction or review capacity. |
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
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
Three related changes to cache hygiene in
addon/lib/getCache.js:Null-sentinel caching —
cacheWrap/cacheWrapGlobalnow persist null/undefined results as a{__null_sentinel: true}entry. This prevents hot re-fetch loops when an upstream provider legitimately returns nothing (e.g. fanart for a series that doesn't have it). Sentinels expire per the result classifier'sEMPTY_RESULTTTL, raised from 60s to 15min to match.In-flight deduplication race fix — previously the in-flight promise was registered after
await redis.get(), so concurrent callers for the same key could each get past thehas()check and start their own producer call. The whole flow is now wrapped in an IIFE async promise registered before any await.New
cacheGet/cacheGetGlobalread-only probe API — returns the cached value or null on miss without invoking any producer or writing to Redis. Four call sites inaddon/lib/getCatalog.ts(Trakt up-next and Trakt unwatched) had been usingcacheWrap(key, async () => null, ttl)as a probe; those are migrated tocacheGet.Why the migration matters
The probe pattern at those four sites was load-bearing before this change because nothing caused a sentinel write. With (1) in place, that same pattern poisons the cache:
cacheWrapcall meant to populate the key sees the sentinel → returns null without invoking its real producercacheGetkeeps the read-only intent explicit and avoids that interaction. The JSDoc oncacheGetexplicitly warns against the old probe pattern so future contributors don't re-introduce it.Test plan
[Fanart]should drop for known-missing IDscacheWrap*call sites in the codebase — they're all real producers, so the sentinel-write path is the desired behavior for them🤖 Generated with Claude Code