From c5037d7a90c8adaaa3fc49dc3abddb306437b8bf Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 7 Apr 2025 08:50:02 +0200 Subject: [PATCH 1/4] test: add test case for route with 0-length response --- .../app/api/zero-length-response/route.ts | 5 +++++ tests/integration/cache-handler.test.ts | 10 ++++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/fixtures/server-components/app/api/zero-length-response/route.ts diff --git a/tests/fixtures/server-components/app/api/zero-length-response/route.ts b/tests/fixtures/server-components/app/api/zero-length-response/route.ts new file mode 100644 index 0000000000..45d6078ce5 --- /dev/null +++ b/tests/fixtures/server-components/app/api/zero-length-response/route.ts @@ -0,0 +1,5 @@ +export async function GET() { + return new Response('') +} + +export const dynamic = 'force-static' diff --git a/tests/integration/cache-handler.test.ts b/tests/integration/cache-handler.test.ts index 22ba4397b8..9050a1f0d1 100644 --- a/tests/integration/cache-handler.test.ts +++ b/tests/integration/cache-handler.test.ts @@ -508,4 +508,14 @@ describe('route', () => { expect(call2.body).toBe('{"params":{"slug":"not-in-generateStaticParams"}}') }) + + test('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('') + }) }) From 05e328ce83f82c57ab7932710a36f73758dfbf99 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 7 Apr 2025 09:32:17 +0200 Subject: [PATCH 2/4] test: add new blob key to test checking prerendered blobs after changing fixture --- tests/integration/cache-handler.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/cache-handler.test.ts b/tests/integration/cache-handler.test.ts index 9050a1f0d1..b3910f8e62 100644 --- a/tests/integration/cache-handler.test.ts +++ b/tests/integration/cache-handler.test.ts @@ -367,6 +367,7 @@ describe('plugin', () => { '/api/revalidate-handler', '/api/static/first', '/api/static/second', + '/api/zero-length-response', '/index', '/product/事前レンダリング,test', '/revalidate-fetch', From 006ce406a8bef6c875689dfd25fe42a90ad338cb Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 7 Apr 2025 09:32:57 +0200 Subject: [PATCH 3/4] fix: ensure size calculation returns positive numbers and make in-memory cache failures not fatal --- .../request-scoped-in-memory-cache.cts | 89 ++++++++++++++----- 1 file changed, 65 insertions(+), 24 deletions(-) diff --git a/src/run/storage/request-scoped-in-memory-cache.cts b/src/run/storage/request-scoped-in-memory-cache.cts index 8a046c3fe1..91d9a53616 100644 --- a/src/run/storage/request-scoped-in-memory-cache.cts +++ b/src/run/storage/request-scoped-in-memory-cache.cts @@ -25,44 +25,71 @@ export function setInMemoryCacheMaxSizeFromNextConfig(size: unknown) { } } -const estimateBlobSize = (valueToStore: BlobType | null | Promise): 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, +): 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 + } +} + +const estimateBlobSize = (valueToStore: BlobType | null | Promise): PositiveNumber => { + let knownTypeFailed = false + 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) - } - 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 + estimatedKnownTypeSize = estimateBlobKnownTypeSize(valueToStore) + if (isPositiveNumber(estimatedKnownTypeSize)) { + return estimatedKnownTypeSize } - } 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) { + knownTypeFailed = true + 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 // 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. KnownTypeFailed: ${knownTypeFailed}, EstimatedKnownTypeSize: ${estimatedKnownTypeSize}, ValueToStore: ${JSON.stringify(valueToStore)}`, + estimateBlobKnownTypeSizeError ? { cause: estimateBlobKnownTypeSizeError } : undefined, ), ) - return JSON.stringify(valueToStore).length + const calculatedSize = JSON.stringify(valueToStore).length + return isPositiveNumber(calculatedSize) ? calculatedSize : BASE_BLOB_SIZE } function getInMemoryLRUCache() { @@ -98,12 +125,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 })) + } }, } } From f1ab6281390205b32b23960a79306dc33cb2d5ad Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Mon, 7 Apr 2025 11:52:03 +0200 Subject: [PATCH 4/4] fix: drop unneded extra variable and also capture calculated size in warning --- src/run/storage/request-scoped-in-memory-cache.cts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/run/storage/request-scoped-in-memory-cache.cts b/src/run/storage/request-scoped-in-memory-cache.cts index 91d9a53616..ddd803b991 100644 --- a/src/run/storage/request-scoped-in-memory-cache.cts +++ b/src/run/storage/request-scoped-in-memory-cache.cts @@ -65,7 +65,6 @@ const estimateBlobKnownTypeSize = ( } const estimateBlobSize = (valueToStore: BlobType | null | Promise): PositiveNumber => { - let knownTypeFailed = false let estimatedKnownTypeSize: number | undefined let estimateBlobKnownTypeSizeError: unknown try { @@ -74,21 +73,21 @@ const estimateBlobSize = (valueToStore: BlobType | null | Promise): Pos return estimatedKnownTypeSize } } catch (error) { - knownTypeFailed = true estimateBlobKnownTypeSizeError = error } // 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. KnownTypeFailed: ${knownTypeFailed}, EstimatedKnownTypeSize: ${estimatedKnownTypeSize}, ValueToStore: ${JSON.stringify(valueToStore)}`, + `Blob size calculation did fallback to JSON.stringify. EstimatedKnownTypeSize: ${estimatedKnownTypeSize}, CalculatedSize: ${calculatedSize}, ValueToStore: ${JSON.stringify(valueToStore)}`, estimateBlobKnownTypeSizeError ? { cause: estimateBlobKnownTypeSizeError } : undefined, ), ) - const calculatedSize = JSON.stringify(valueToStore).length return isPositiveNumber(calculatedSize) ? calculatedSize : BASE_BLOB_SIZE }