-
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?
Conversation
…ders Co-authored-by: serhalp <[email protected]>
Co-authored-by: serhalp <[email protected]>
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 project uses pnpm. delete the lockfile.
…revert import changes Co-authored-by: serhalp <[email protected]>
Done! Deleted the package-lock.json file since the project uses pnpm. (fb52dda) |
…he miss instead of loops Co-authored-by: serhalp <[email protected]>
const cacheStatus: ParsedCacheStatusEntry[] = [ | ||
{ | ||
cacheName: 'Netlify Edge', | ||
parameters: { | ||
hit: false, | ||
fwd: 'miss', | ||
stored: false, | ||
collapsed: false, | ||
}, | ||
}, | ||
] |
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)
app/utils/getServedBy.ts
Outdated
// 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 comment
The 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 comment
The 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)
…logic Co-authored-by: serhalp <[email protected]>
This PR fixes an issue where sometimes no cache analysis would be rendered at all, causing the UI to fall back to displaying raw headers instead of a proper cache analysis.
Problem
The issue occurred when a request had the following characteristics:
"Netlify Edge"; fwd=miss
)Debug-X-NF-Function-Type
,Debug-X-NF-Edge-Functions
)Debug-X-BB-Host-Id
header)In this scenario, the
getServedBySource
function would throw an error instead of handling this valid case gracefully.Root Cause
The
getServedBySource
function was missing a fallback case for when:hit: false
)Solution
Added a fallback in
getServedBySource
that checks for the presence ofDebug-X-BB-Host-Id
header when no cache hits or function headers are found. This header indicates CDN involvement, so the CDN served the request (likely a cache miss that was forwarded to origin).Testing
getServedBy
Impact
Users will no longer encounter the "raw headers only" display when cache misses occur without function headers. The cache inspector will properly analyze and display cache information in all valid scenarios.
Fixes #91.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.