Skip to content
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

Fix fetch cache for subsequent 404 responses #70259

Open
wants to merge 2 commits into
base: canary
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions packages/next/src/server/lib/patch-fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,146 @@ describe('createPatchedFetcher', () => {
// Setting a lower timeout than default, because the test will fail with a
// timeout when we regress and buffer the response.
}, 1000)

it('should overwrite existing status 200 cache after 404 response', async () => {
// make http cache with res 200 and cache
const mockFetch: jest.MockedFunction<typeof fetch> = jest.fn()
mockFetch.mockResolvedValue(
new Response('{"data": "hello"}', {
status: 200,
headers: {
'Content-Type': 'application/json',
},
})
)

const staticGenerationAsyncStorage =
new AsyncLocalStorage<StaticGenerationStore>()

const prerenderAsyncStorage = new AsyncLocalStorage<PrerenderStore>()

const patchedFetch = createPatchedFetcher(mockFetch, {
// requestAsyncStorage does not need to provide a store for this test.
requestAsyncStorage: new AsyncLocalStorage<RequestStore>(),
staticGenerationAsyncStorage,
prerenderAsyncStorage,
})

let resolveIncrementalCacheSet: () => void

let incrementalCacheSetPromise = new Promise<void>((resolve) => {
resolveIncrementalCacheSet = resolve
})

const incrementalCache = {
get: jest.fn(),
set: jest.fn(() => resolveIncrementalCacheSet()),
generateCacheKey: jest.fn(() => 'test-cache-key'),
lock: jest.fn(() => resolveIncrementalCacheSet),
} as unknown as IncrementalCache

// We only need to provide a few of the StaticGenerationStore properties.
const staticGenerationStore: Partial<StaticGenerationStore> = {
page: '/',
route: '/',
incrementalCache,
}

await staticGenerationAsyncStorage.run(
staticGenerationStore as StaticGenerationStore,
async () => {
const response = await patchedFetch('https://example.com', {
cache: 'force-cache',
})

if (!response.body) {
throw new Error(`Response body is ${JSON.stringify(response.body)}.`)
}

const reader = response.body.getReader()
let result = await reader.read()
expect(result.done).toBe(false)
expect(new TextDecoder().decode(result.value)).toBe('{"data": "hello"}')

await incrementalCacheSetPromise

expect(incrementalCache.set).toHaveBeenCalledTimes(1)
expect(incrementalCache.set).toHaveBeenCalledWith(
'test-cache-key',
{
data: {
body: btoa('{"data": "hello"}'),
headers: {
'content-type': 'application/json',
},
status: 200,
url: '', // the mocked response does not have a URL
},
kind: 'FETCH',
revalidate: 31536000, // default of one year
},
{
fetchCache: true,
fetchIdx: 1,
fetchUrl: 'https://example.com/',
revalidate: false,
tags: [],
}
)

incrementalCacheSetPromise = new Promise<void>((resolve) => {
resolveIncrementalCacheSet = resolve
})

mockFetch.mockResolvedValue(
new Response('{"data": "none"}', {
status: 404,
headers: {
'Content-Type': 'application/json',
},
})
)

const response2 = await patchedFetch('https://example.com', {
cache: 'force-cache',
})

if (!response2.body) {
throw new Error(`Response body is ${JSON.stringify(response.body)}.`)
}

const reader2 = response2.body.getReader()
const result2 = await reader2.read()
expect(result2.done).toBe(false)
expect(new TextDecoder().decode(result2.value)).toBe('{"data": "none"}')

await incrementalCacheSetPromise

expect(incrementalCache.set).toHaveBeenCalledTimes(2)

expect(incrementalCache.set).toHaveBeenCalledWith(
'test-cache-key',
{
data: {
body: btoa('{"data": "none"}'),
headers: {
'content-type': 'application/json',
},
status: 404,
url: '', // the mocked response does not have a URL
},
kind: 'FETCH',
revalidate: 31536000, // default of one year
},
{
fetchCache: true,
fetchIdx: 2,
fetchUrl: 'https://example.com/',
revalidate: false,
tags: [],
}
)
}
)
})
})
2 changes: 1 addition & 1 deletion packages/next/src/server/lib/patch-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ export function createPatchedFetcher(
})
}
if (
res.status === 200 &&
(res.status === 200 || res.status === 404) &&
incrementalCache &&
cacheKey &&
(isCacheableRevalidate || requestStore?.serverComponentsHmrCache)
Expand Down
Loading