Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 530d2c5

Browse filesBrowse files
piehpieh
authored
fix: handle case of zero-length cacheable route handler responses (#2819)
* test: add test case for route with 0-length response * test: add new blob key to test checking prerendered blobs after changing fixture * fix: ensure size calculation returns positive numbers and make in-memory cache failures not fatal * fix: drop unneded extra variable and also capture calculated size in warning
1 parent 8a63ac2 commit 530d2c5
Copy full SHA for 530d2c5

File tree

Expand file treeCollapse file tree

3 files changed

+80
-24
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+80
-24
lines changed

‎src/run/storage/request-scoped-in-memory-cache.cts

Copy file name to clipboardExpand all lines: src/run/storage/request-scoped-in-memory-cache.cts
+64-24Lines changed: 64 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,44 +25,70 @@ export function setInMemoryCacheMaxSizeFromNextConfig(size: unknown) {
2525
}
2626
}
2727

28-
const estimateBlobSize = (valueToStore: BlobType | null | Promise<unknown>): number => {
28+
type PositiveNumber = number & { __positive: true }
29+
const isPositiveNumber = (value: unknown): value is PositiveNumber => {
30+
return typeof value === 'number' && value > 0
31+
}
32+
33+
const BASE_BLOB_SIZE = 25 as PositiveNumber
34+
35+
const estimateBlobKnownTypeSize = (
36+
valueToStore: BlobType | null | Promise<unknown>,
37+
): number | undefined => {
2938
// very approximate size calculation to avoid expensive exact size calculation
3039
// inspired by https://github.com/vercel/next.js/blob/ed10f7ed0246fcc763194197eb9beebcbd063162/packages/next/src/server/lib/incremental-cache/file-system-cache.ts#L60-L79
3140
if (valueToStore === null || isPromise(valueToStore) || isTagManifest(valueToStore)) {
32-
return 25
41+
return BASE_BLOB_SIZE
3342
}
3443
if (isHtmlBlob(valueToStore)) {
35-
return valueToStore.html.length
44+
return BASE_BLOB_SIZE + valueToStore.html.length
45+
}
46+
47+
if (valueToStore.value?.kind === 'FETCH') {
48+
return BASE_BLOB_SIZE + valueToStore.value.data.body.length
49+
}
50+
if (valueToStore.value?.kind === 'APP_PAGE') {
51+
return (
52+
BASE_BLOB_SIZE + valueToStore.value.html.length + (valueToStore.value.rscData?.length ?? 0)
53+
)
54+
}
55+
if (valueToStore.value?.kind === 'PAGE' || valueToStore.value?.kind === 'PAGES') {
56+
return (
57+
BASE_BLOB_SIZE +
58+
valueToStore.value.html.length +
59+
JSON.stringify(valueToStore.value.pageData).length
60+
)
3661
}
37-
let knownKindFailed = false
62+
if (valueToStore.value?.kind === 'ROUTE' || valueToStore.value?.kind === 'APP_ROUTE') {
63+
return BASE_BLOB_SIZE + valueToStore.value.body.length
64+
}
65+
}
66+
67+
const estimateBlobSize = (valueToStore: BlobType | null | Promise<unknown>): PositiveNumber => {
68+
let estimatedKnownTypeSize: number | undefined
69+
let estimateBlobKnownTypeSizeError: unknown
3870
try {
39-
if (valueToStore.value?.kind === 'FETCH') {
40-
return valueToStore.value.data.body.length
41-
}
42-
if (valueToStore.value?.kind === 'APP_PAGE') {
43-
return valueToStore.value.html.length + (valueToStore.value.rscData?.length ?? 0)
71+
estimatedKnownTypeSize = estimateBlobKnownTypeSize(valueToStore)
72+
if (isPositiveNumber(estimatedKnownTypeSize)) {
73+
return estimatedKnownTypeSize
4474
}
45-
if (valueToStore.value?.kind === 'PAGE' || valueToStore.value?.kind === 'PAGES') {
46-
return valueToStore.value.html.length + JSON.stringify(valueToStore.value.pageData).length
47-
}
48-
if (valueToStore.value?.kind === 'ROUTE' || valueToStore.value?.kind === 'APP_ROUTE') {
49-
return valueToStore.value.body.length
50-
}
51-
} catch {
52-
// size calculation rely on the shape of the value, so if it's not what we expect, we fallback to JSON.stringify
53-
knownKindFailed = true
75+
} catch (error) {
76+
estimateBlobKnownTypeSizeError = error
5477
}
5578

56-
// fallback for not known kinds or known kinds that did fail to calculate size
79+
// fallback for not known kinds or known kinds that did fail to calculate positive size
80+
const calculatedSize = JSON.stringify(valueToStore).length
81+
5782
// we should also monitor cases when fallback is used because it's not the most efficient way to calculate/estimate size
5883
// and might indicate need to make adjustments or additions to the size calculation
5984
recordWarning(
6085
new Error(
61-
`Blob size calculation did fallback to JSON.stringify. Kind: KnownKindFailed: ${knownKindFailed}, ${valueToStore.value?.kind ?? 'undefined'}`,
86+
`Blob size calculation did fallback to JSON.stringify. EstimatedKnownTypeSize: ${estimatedKnownTypeSize}, CalculatedSize: ${calculatedSize}, ValueToStore: ${JSON.stringify(valueToStore)}`,
87+
estimateBlobKnownTypeSizeError ? { cause: estimateBlobKnownTypeSizeError } : undefined,
6288
),
6389
)
6490

65-
return JSON.stringify(valueToStore).length
91+
return isPositiveNumber(calculatedSize) ? calculatedSize : BASE_BLOB_SIZE
6692
}
6793

6894
function getInMemoryLRUCache() {
@@ -98,12 +124,26 @@ export const getRequestScopedInMemoryCache = (): RequestScopedInMemoryCache => {
98124
return {
99125
get(key) {
100126
if (!requestContext) return
101-
const value = inMemoryLRUCache?.get(`${requestContext.requestID}:${key}`)
102-
return value === NullValue ? null : value
127+
try {
128+
const value = inMemoryLRUCache?.get(`${requestContext.requestID}:${key}`)
129+
return value === NullValue ? null : value
130+
} catch (error) {
131+
// using in-memory store is perf optimization not requirement
132+
// trying to use optimization should NOT cause crashes
133+
// so we just record warning and return undefined
134+
recordWarning(new Error('Failed to get value from memory cache', { cause: error }))
135+
}
103136
},
104137
set(key, value) {
105138
if (!requestContext) return
106-
inMemoryLRUCache?.set(`${requestContext?.requestID}:${key}`, value ?? NullValue)
139+
try {
140+
inMemoryLRUCache?.set(`${requestContext?.requestID}:${key}`, value ?? NullValue)
141+
} catch (error) {
142+
// using in-memory store is perf optimization not requirement
143+
// trying to use optimization should NOT cause crashes
144+
// so we just record warning and return undefined
145+
recordWarning(new Error('Failed to store value in memory cache', { cause: error }))
146+
}
107147
},
108148
}
109149
}

‎tests/fixtures/server-components/app/api/zero-length-response/route.ts

Copy file name to clipboard
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export async function GET() {
2+
return new Response('')
3+
}
4+
5+
export const dynamic = 'force-static'

‎tests/integration/cache-handler.test.ts

Copy file name to clipboardExpand all lines: tests/integration/cache-handler.test.ts
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,7 @@ describe('plugin', () => {
367367
'/api/revalidate-handler',
368368
'/api/static/first',
369369
'/api/static/second',
370+
'/api/zero-length-response',
370371
'/index',
371372
'/product/事前レンダリング,test',
372373
'/revalidate-fetch',
@@ -508,4 +509,14 @@ describe('route', () => {
508509

509510
expect(call2.body).toBe('{"params":{"slug":"not-in-generateStaticParams"}}')
510511
})
512+
513+
test<FixtureTestContext>('cacheable route handler response with 0 length response is served correctly', async (ctx) => {
514+
await createFixture('server-components', ctx)
515+
await runPlugin(ctx)
516+
517+
const call = await invokeFunction(ctx, { url: '/api/zero-length-response' })
518+
519+
expect(call.statusCode).toBe(200)
520+
expect(call.body).toBe('')
521+
})
511522
})

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.