-
Notifications
You must be signed in to change notification settings - Fork 0
fix: fix cache analysis not rendering when edge cache miss occurs #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
7b4deee
22ae765
18d3517
fb52dda
2c05b12
7f33d83
fc87dc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
export enum ServedBySource { | ||
CDN = 'CDN', | ||
CdnEdge = 'CDN edge', | ||
CdnOrigin = 'CDN origin', | ||
DurableCache = 'Durable Cache', | ||
Function = 'Function', | ||
EdgeFunction = 'Edge Function', | ||
|
@@ -40,16 +41,28 @@ const getServedBySource = ( | |
// So, the first cache hit (starting from the user) is the one that served the request. | ||
// But we don't quite want to return exactly the same concept of "caches" as in `Cache-Status`, so | ||
// we need a bit of extra logic to map to other sources. | ||
|
||
// First, check for cache hits | ||
for (const { | ||
cacheName, | ||
parameters: { hit }, | ||
} of cacheStatus) { | ||
if (!hit) continue | ||
|
||
if (cacheName === 'Netlify Edge') return ServedBySource.CDN | ||
if (cacheName === 'Netlify Edge') return ServedBySource.CdnEdge | ||
if (cacheName === 'Netlify Durable') return ServedBySource.DurableCache | ||
} | ||
|
||
// Check for the specific case of a single Netlify Edge miss - this handles the weird | ||
// Netlify Cache-Status behavior where a miss on the CDN edge means the request was | ||
// forwarded to CDN origin. According to Netlify's cache behavior, when there's a miss | ||
// on "Netlify Edge", the request gets forwarded to the CDN origin to fetch the static asset. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's one detail missing here. this only applies when there's a miss on the edge node, but a hit at the CDN origin. This can only be detected by the presence of a cache status entry for Netlify Edge that is a miss and no entries at all for subsequent caches. There's a miss on the edge node, but a hit at the CDN origin. This can only be detected by the presence of a cache status entry for Netlify Edge that is a miss and no entries at all for subsequent caches. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the CdnOrigin detection logic to properly handle the case you described. Now checks for Netlify Edge miss AND no subsequent cache hits (not just single entry). This correctly detects when there's a miss on edge but hit at CDN origin. (fc87dc2) |
||
if (cacheStatus.length === 1 | ||
&& cacheStatus[0]?.cacheName === 'Netlify Edge' | ||
&& !cacheStatus[0]?.parameters.hit) { | ||
return ServedBySource.CdnOrigin | ||
} | ||
|
||
// NOTE: the order is important here, since a response can be served by a Function even | ||
// though one or more Edge Functions are also invoked (as middleware). | ||
if (cacheHeaders.has('Debug-X-NF-Function-Type')) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one is realistic, but while we're refactoring the logic let's update the fixture in the other tests to be realistic. for example the durable cache one should have a Netlify Edge miss, the function one should have Netlify Edge and Durable Cache misses, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated test fixtures to be realistic! The durable cache test now includes a Netlify Edge miss, and the function/edge function tests include both Netlify Edge and Netlify Durable misses. (fc87dc2)