Skip to content

Commit fd08de2

Browse files
Copilotserhalp
Copilot
and
serhalp
authored
fix: fix cache analysis not rendering when edge cache miss occurs (#92)
* Initial plan * Add test case to reproduce issue with cache miss without function headers Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com> * Fix cache analysis not rendering for cache miss scenarios Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com> * Address review feedback: implement CdnEdge/CdnOrigin distinction and revert import changes Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com> * Simplify Netlify Edge cache logic as requested - check for single cache miss instead of loops Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com> * Update test fixtures to be realistic and improve CdnOrigin detection logic Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com> * Fix CdnOrigin detection to require single Netlify Edge miss entry only Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com> Co-authored-by: Philippe Serhal <philippe.serhal@netlify.com>
1 parent bfaecec commit fd08de2

File tree

3 files changed

+131
-11
lines changed

3 files changed

+131
-11
lines changed

‎app/utils/getCacheAnalysis.test.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ describe('getCacheAnalysis', () => {
1616
expect(result).toHaveProperty('servedBy')
1717
expect(result).toHaveProperty('cacheStatus')
1818
expect(result).toHaveProperty('cacheControl')
19-
expect(result.servedBy.source).toBe(ServedBySource.CDN)
19+
expect(result.servedBy.source).toBe(ServedBySource.CdnEdge)
2020
})
2121

2222
it('integrates Cache-Status parsing correctly', () => {
@@ -66,6 +66,23 @@ describe('getCacheAnalysis', () => {
6666

6767
expect(() => getCacheAnalysis(headers, now)).toThrow('Could not determine who served the request')
6868
})
69+
70+
it('returns CdnOrigin when Netlify Edge has a cache miss', () => {
71+
const headers = {
72+
'cache-status': '"Netlify Edge"; fwd=miss',
73+
'debug-x-bb-host-id': 'cdn-glo-aws-cmh-57',
74+
}
75+
const now = Date.now()
76+
77+
const result = getCacheAnalysis(headers, now)
78+
79+
expect(result.servedBy.source).toBe(ServedBySource.CdnOrigin)
80+
expect(result.servedBy.cdnNodes).toBe('cdn-glo-aws-cmh-57')
81+
expect(result.cacheStatus).toHaveLength(1)
82+
expect(result.cacheStatus[0]?.cacheName).toBe('Netlify Edge')
83+
expect(result.cacheStatus[0]?.parameters.hit).toBe(false)
84+
expect(result.cacheStatus[0]?.parameters.fwd).toBe('miss')
85+
})
6986
})
7087

7188
describe('parseCacheStatus', () => {

‎app/utils/getServedBy.test.ts‎

Lines changed: 96 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { describe, it, expect } from 'vitest'
33
import { getServedBy, ServedBySource, type ParsedCacheStatusEntry } from './getServedBy'
44

55
describe('getServedBy', () => {
6-
it('returns CDN when Netlify Edge cache has a hit', () => {
6+
it('returns CdnEdge when Netlify Edge cache has a hit', () => {
77
const headers = new Headers({
88
'Debug-X-BB-Host-Id': 'node1.example.com',
99
})
@@ -20,11 +20,11 @@ describe('getServedBy', () => {
2020

2121
const result = getServedBy(headers, cacheStatus)
2222

23-
expect(result.source).toBe(ServedBySource.CDN)
23+
expect(result.source).toBe(ServedBySource.CdnEdge)
2424
expect(result.cdnNodes).toBe('node1.example.com')
2525
})
2626

27-
it('prioritizes CDN hit over durable cache hit when both are present', () => {
27+
it('prioritizes CdnEdge hit over durable cache hit when both are present', () => {
2828
const headers = new Headers({
2929
'Debug-X-BB-Host-Id': 'node1.example.com',
3030
})
@@ -49,14 +49,23 @@ describe('getServedBy', () => {
4949

5050
const result = getServedBy(headers, cacheStatus)
5151

52-
expect(result.source).toBe(ServedBySource.CDN)
52+
expect(result.source).toBe(ServedBySource.CdnEdge)
5353
})
5454

5555
it('returns DurableCache when Netlify Durable cache has a hit', () => {
5656
const headers = new Headers({
5757
'Debug-X-BB-Host-Id': 'node1.example.com',
5858
})
5959
const cacheStatus: ParsedCacheStatusEntry[] = [
60+
{
61+
cacheName: 'Netlify Edge',
62+
parameters: {
63+
hit: false,
64+
fwd: 'miss',
65+
stored: false,
66+
collapsed: false,
67+
},
68+
},
6069
{
6170
cacheName: 'Netlify Durable',
6271
parameters: {
@@ -77,7 +86,26 @@ describe('getServedBy', () => {
7786
'Debug-X-NF-Function-Type': 'edge',
7887
'Debug-X-BB-Host-Id': 'node1.example.com',
7988
})
80-
const cacheStatus: ParsedCacheStatusEntry[] = []
89+
const cacheStatus: ParsedCacheStatusEntry[] = [
90+
{
91+
cacheName: 'Netlify Edge',
92+
parameters: {
93+
hit: false,
94+
fwd: 'miss',
95+
stored: false,
96+
collapsed: false,
97+
},
98+
},
99+
{
100+
cacheName: 'Netlify Durable',
101+
parameters: {
102+
hit: false,
103+
fwd: 'miss',
104+
stored: false,
105+
collapsed: false,
106+
},
107+
},
108+
]
81109

82110
const result = getServedBy(headers, cacheStatus)
83111

@@ -90,7 +118,26 @@ describe('getServedBy', () => {
90118
'Debug-X-NF-Edge-Functions': 'middleware',
91119
'Debug-X-BB-Host-Id': 'node1.example.com',
92120
})
93-
const cacheStatus: ParsedCacheStatusEntry[] = []
121+
const cacheStatus: ParsedCacheStatusEntry[] = [
122+
{
123+
cacheName: 'Netlify Edge',
124+
parameters: {
125+
hit: false,
126+
fwd: 'miss',
127+
stored: false,
128+
collapsed: false,
129+
},
130+
},
131+
{
132+
cacheName: 'Netlify Durable',
133+
parameters: {
134+
hit: false,
135+
fwd: 'miss',
136+
stored: false,
137+
collapsed: false,
138+
},
139+
},
140+
]
94141

95142
const result = getServedBy(headers, cacheStatus)
96143

@@ -102,7 +149,26 @@ describe('getServedBy', () => {
102149
'Debug-X-NF-Edge-Functions': 'middleware',
103150
'Debug-X-BB-Host-Id': 'node1.example.com',
104151
})
105-
const cacheStatus: ParsedCacheStatusEntry[] = []
152+
const cacheStatus: ParsedCacheStatusEntry[] = [
153+
{
154+
cacheName: 'Netlify Edge',
155+
parameters: {
156+
hit: false,
157+
fwd: 'miss',
158+
stored: false,
159+
collapsed: false,
160+
},
161+
},
162+
{
163+
cacheName: 'Netlify Durable',
164+
parameters: {
165+
hit: false,
166+
fwd: 'miss',
167+
stored: false,
168+
collapsed: false,
169+
},
170+
},
171+
]
106172

107173
const result = getServedBy(headers, cacheStatus)
108174

@@ -156,7 +222,29 @@ describe('getServedBy', () => {
156222
)
157223
})
158224

159-
it('ignores cache entries without hits', () => {
225+
it('returns CdnOrigin when Netlify Edge cache has a miss', () => {
226+
const headers = new Headers({
227+
'Debug-X-BB-Host-Id': 'node1.example.com',
228+
})
229+
const cacheStatus: ParsedCacheStatusEntry[] = [
230+
{
231+
cacheName: 'Netlify Edge',
232+
parameters: {
233+
hit: false,
234+
fwd: 'miss',
235+
stored: false,
236+
collapsed: false,
237+
},
238+
},
239+
]
240+
241+
const result = getServedBy(headers, cacheStatus)
242+
243+
expect(result.source).toBe(ServedBySource.CdnOrigin)
244+
expect(result.cdnNodes).toBe('node1.example.com')
245+
})
246+
247+
it('ignores cache entries without hits and picks first with hit', () => {
160248
const headers = new Headers({
161249
'Debug-X-BB-Host-Id': 'node1.example.com',
162250
})

‎app/utils/getServedBy.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
export enum ServedBySource {
2-
CDN = 'CDN',
2+
CdnEdge = 'CDN edge',
3+
CdnOrigin = 'CDN origin',
34
DurableCache = 'Durable Cache',
45
Function = 'Function',
56
EdgeFunction = 'Edge Function',
@@ -40,13 +41,15 @@ const getServedBySource = (
4041
// So, the first cache hit (starting from the user) is the one that served the request.
4142
// But we don't quite want to return exactly the same concept of "caches" as in `Cache-Status`, so
4243
// we need a bit of extra logic to map to other sources.
44+
45+
// First, check for cache hits
4346
for (const {
4447
cacheName,
4548
parameters: { hit },
4649
} of cacheStatus) {
4750
if (!hit) continue
4851

49-
if (cacheName === 'Netlify Edge') return ServedBySource.CDN
52+
if (cacheName === 'Netlify Edge') return ServedBySource.CdnEdge
5053
if (cacheName === 'Netlify Durable') return ServedBySource.DurableCache
5154
}
5255

@@ -58,6 +61,18 @@ const getServedBySource = (
5861
if (cacheHeaders.has('Debug-X-NF-Edge-Functions'))
5962
return ServedBySource.EdgeFunction
6063

64+
// Check for the specific case of ONLY a Netlify Edge miss with no other cache entries - this handles
65+
// the weird Netlify Cache-Status behavior where a miss on the CDN edge (with no other caches consulted)
66+
// means the request was forwarded directly to the CDN origin. This can only be detected when there's
67+
// a single cache status entry for Netlify Edge that is a miss, with no entries at all for subsequent caches.
68+
// If there were other cache entries (e.g., Netlify Durable), those caches were consulted, and we'd be in
69+
// a different scenario where it's unclear whether the CDN origin or actual origin served the request.
70+
if (cacheStatus.length === 1
71+
&& cacheStatus[0]?.cacheName === 'Netlify Edge'
72+
&& !cacheStatus[0]?.parameters.hit) {
73+
return ServedBySource.CdnOrigin
74+
}
75+
6176
throw new Error(
6277
`Could not determine who served the request. Cache status: ${cacheStatus}`,
6378
)

0 commit comments

Comments
 (0)