Skip to content

fix: handle case of zero-length cacheable route handler responses #2819

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

Merged
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
88 changes: 64 additions & 24 deletions src/run/storage/request-scoped-in-memory-cache.cts
Original file line number Diff line number Diff line change
Expand Up @@ -25,44 +25,70 @@ export function setInMemoryCacheMaxSizeFromNextConfig(size: unknown) {
}
}

const estimateBlobSize = (valueToStore: BlobType | null | Promise<unknown>): number => {
type PositiveNumber = number & { __positive: true }
const isPositiveNumber = (value: unknown): value is PositiveNumber => {
return typeof value === 'number' && value > 0
}

const BASE_BLOB_SIZE = 25 as PositiveNumber

const estimateBlobKnownTypeSize = (
valueToStore: BlobType | null | Promise<unknown>,
): number | undefined => {
// very approximate size calculation to avoid expensive exact size calculation
// inspired by https://github.com/vercel/next.js/blob/ed10f7ed0246fcc763194197eb9beebcbd063162/packages/next/src/server/lib/incremental-cache/file-system-cache.ts#L60-L79
if (valueToStore === null || isPromise(valueToStore) || isTagManifest(valueToStore)) {
return 25
return BASE_BLOB_SIZE
}
if (isHtmlBlob(valueToStore)) {
return valueToStore.html.length
return BASE_BLOB_SIZE + valueToStore.html.length
}

if (valueToStore.value?.kind === 'FETCH') {
return BASE_BLOB_SIZE + valueToStore.value.data.body.length
}
if (valueToStore.value?.kind === 'APP_PAGE') {
return (
BASE_BLOB_SIZE + valueToStore.value.html.length + (valueToStore.value.rscData?.length ?? 0)
)
}
if (valueToStore.value?.kind === 'PAGE' || valueToStore.value?.kind === 'PAGES') {
return (
BASE_BLOB_SIZE +
valueToStore.value.html.length +
JSON.stringify(valueToStore.value.pageData).length
)
}
let knownKindFailed = false
if (valueToStore.value?.kind === 'ROUTE' || valueToStore.value?.kind === 'APP_ROUTE') {
return BASE_BLOB_SIZE + valueToStore.value.body.length
Copy link
Contributor Author

@pieh pieh Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This in particular is the fix for added test case.

But using it as example, I did add more adjustments:

  • ensure we add "base" blob size to all known cases
  • ensure estimateBlobSize returns "PositiveNumber"
  • make using in-memory store non-fatal - this is optimization, but failures related to it should NOT cause crashes. I did add recording of failures there so we can adjust code if problems happen still

}
}

const estimateBlobSize = (valueToStore: BlobType | null | Promise<unknown>): PositiveNumber => {
let estimatedKnownTypeSize: number | undefined
let estimateBlobKnownTypeSizeError: unknown
try {
if (valueToStore.value?.kind === 'FETCH') {
return valueToStore.value.data.body.length
}
if (valueToStore.value?.kind === 'APP_PAGE') {
return valueToStore.value.html.length + (valueToStore.value.rscData?.length ?? 0)
estimatedKnownTypeSize = estimateBlobKnownTypeSize(valueToStore)
if (isPositiveNumber(estimatedKnownTypeSize)) {
return estimatedKnownTypeSize
}
if (valueToStore.value?.kind === 'PAGE' || valueToStore.value?.kind === 'PAGES') {
return valueToStore.value.html.length + JSON.stringify(valueToStore.value.pageData).length
}
if (valueToStore.value?.kind === 'ROUTE' || valueToStore.value?.kind === 'APP_ROUTE') {
return valueToStore.value.body.length
}
} catch {
// size calculation rely on the shape of the value, so if it's not what we expect, we fallback to JSON.stringify
knownKindFailed = true
} catch (error) {
estimateBlobKnownTypeSizeError = error
}

// fallback for not known kinds or known kinds that did fail to calculate size
// fallback for not known kinds or known kinds that did fail to calculate positive size
const calculatedSize = JSON.stringify(valueToStore).length

// we should also monitor cases when fallback is used because it's not the most efficient way to calculate/estimate size
// and might indicate need to make adjustments or additions to the size calculation
recordWarning(
new Error(
`Blob size calculation did fallback to JSON.stringify. Kind: KnownKindFailed: ${knownKindFailed}, ${valueToStore.value?.kind ?? 'undefined'}`,
`Blob size calculation did fallback to JSON.stringify. EstimatedKnownTypeSize: ${estimatedKnownTypeSize}, CalculatedSize: ${calculatedSize}, ValueToStore: ${JSON.stringify(valueToStore)}`,
estimateBlobKnownTypeSizeError ? { cause: estimateBlobKnownTypeSizeError } : undefined,
),
)

return JSON.stringify(valueToStore).length
return isPositiveNumber(calculatedSize) ? calculatedSize : BASE_BLOB_SIZE
}

function getInMemoryLRUCache() {
Expand Down Expand Up @@ -98,12 +124,26 @@ export const getRequestScopedInMemoryCache = (): RequestScopedInMemoryCache => {
return {
get(key) {
if (!requestContext) return
const value = inMemoryLRUCache?.get(`${requestContext.requestID}:${key}`)
return value === NullValue ? null : value
try {
const value = inMemoryLRUCache?.get(`${requestContext.requestID}:${key}`)
return value === NullValue ? null : value
} catch (error) {
// using in-memory store is perf optimization not requirement
// trying to use optimization should NOT cause crashes
// so we just record warning and return undefined
recordWarning(new Error('Failed to get value from memory cache', { cause: error }))
}
},
set(key, value) {
if (!requestContext) return
inMemoryLRUCache?.set(`${requestContext?.requestID}:${key}`, value ?? NullValue)
try {
inMemoryLRUCache?.set(`${requestContext?.requestID}:${key}`, value ?? NullValue)
} catch (error) {
// using in-memory store is perf optimization not requirement
// trying to use optimization should NOT cause crashes
// so we just record warning and return undefined
recordWarning(new Error('Failed to store value in memory cache', { cause: error }))
}
},
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export async function GET() {
return new Response('')
}

export const dynamic = 'force-static'
11 changes: 11 additions & 0 deletions tests/integration/cache-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ describe('plugin', () => {
'/api/revalidate-handler',
'/api/static/first',
'/api/static/second',
'/api/zero-length-response',
'/index',
'/product/事前レンダリング,test',
'/revalidate-fetch',
Expand Down Expand Up @@ -508,4 +509,14 @@ describe('route', () => {

expect(call2.body).toBe('{"params":{"slug":"not-in-generateStaticParams"}}')
})

test<FixtureTestContext>('cacheable route handler response with 0 length response is served correctly', async (ctx) => {
await createFixture('server-components', ctx)
await runPlugin(ctx)

const call = await invokeFunction(ctx, { url: '/api/zero-length-response' })

expect(call.statusCode).toBe(200)
expect(call.body).toBe('')
})
})
Loading