fix: skip 304 short-circuit when get bypasses cache#140
Merged
Conversation
Returning null/undefined from .get() is a documented cache bypass. After #138 stopped the null crash, matching If-None-Match still triggered a 304 with an empty body because the etag was derived from the {} fallback while send() was never called. Only return 304 when memoize produced a stored value (raw is truthy). Co-authored-by: kikohumanbeatbox <kikohumanbeatbox@gmail.com>
The 304 revalidation should fire only when memoize actually had a stored value. `hasValue` is the semantic flag from @keyvhq/memoize for exactly that, already destructured at the call site. Gating on `raw` truthiness worked for the documented bypass case only because compress(undefined) returns undefined, an implicit coupling; hasValue also stays correct when .get() returns a falsy primitive. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Bug and impact
Returning
nullorundefinedfrom.get()is a documented cache bypass. After #138 fixed the null crash, a follow-up request with a matchingIf-None-Matchheader (browser refresh, CDN, API client) still returned 304 with an empty body instead of callingsend(). Bypass routes silently lost their response body.Root cause
When
.get()bypasses the cache, memoize returnsraw = undefined. The handler falls back to{}for etag computation, producing a deterministic ETag. The 304 branch only checked!forceExpiration && !isModified, so a matchingIf-None-Matchskippedsend()even though nothing was cached.Fix
Only return 304 when memoize produced a stored value (
rawis truthy). Cache revalidation after a cleared cache still works becauseget()re-fetches and returns a truthyraw.Validation
MISS for null data value with If-None-MatchIf-None-Matchnow returns 200 with body instead of empty 304Note
Low Risk
Single guard on the 304 branch in the HTTP cache handler; behavior change is limited to documented cache-bypass paths and is covered by a new test.
Overview
Fixes empty 304 responses on cache-bypass routes when clients send
If-None-Match.When
.get()returnsnull/undefined(documented bypass), memoize does not store a payload, but the handler could still treat a matching ETag as “not modified” and return 304 without callingsend(), so refreshes/CDN revalidation got an empty body.The 304 path now requires
hasValue(a real memoized entry) in addition to!forceExpirationand a matching ETag, so bypass requests always fall through tosend()with 200 and the body. Normal cached revalidation is unchanged whenget()returns a stored object.Adds regression coverage in
test/status.jsfor null.get()plusIf-None-Match.Reviewed by Cursor Bugbot for commit fa54f4a. Bugbot is set up for automated code reviews on this repo. Configure here.