diff --git a/.release-please-manifest.json b/.release-please-manifest.json index b568f9c356..a12a93406d 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "5.10.3" + ".": "5.10.4" } diff --git a/CHANGELOG.md b/CHANGELOG.md index db23d620df..bf51a9e88b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## [5.10.4](https://github.com/opennextjs/opennextjs-netlify/compare/v5.10.3...v5.10.4) (2025-04-07) + + +### Bug Fixes + +* add debug information around potential html/rsc response mismatches ([#2816](https://github.com/opennextjs/opennextjs-netlify/issues/2816)) ([70f9b15](https://github.com/opennextjs/opennextjs-netlify/commit/70f9b152ace8ad36911c5018eac89c6336b5c5df)) +* handle case of zero-length cacheable route handler responses ([#2819](https://github.com/opennextjs/opennextjs-netlify/issues/2819)) ([530d2c5](https://github.com/opennextjs/opennextjs-netlify/commit/530d2c5bab7c2bda2f5157226e84d7ee050afb86)) + ## [5.10.3](https://github.com/opennextjs/opennextjs-netlify/compare/v5.10.2...v5.10.3) (2025-04-03) diff --git a/e2e-report/package-lock.json b/e2e-report/package-lock.json index 35c4ab9106..3113b69467 100644 --- a/e2e-report/package-lock.json +++ b/e2e-report/package-lock.json @@ -8,7 +8,7 @@ "name": "e2e-test-site", "version": "0.2.0", "dependencies": { - "@netlify/plugin-nextjs": "^5.10.2", + "@netlify/plugin-nextjs": "^5.10.3", "next": "^14.2.3", "react": "^18.3.1", "react-dom": "^18.3.1" @@ -263,9 +263,9 @@ } }, "node_modules/@netlify/plugin-nextjs": { - "version": "5.10.2", - "resolved": "https://registry.npmjs.org/@netlify/plugin-nextjs/-/plugin-nextjs-5.10.2.tgz", - "integrity": "sha512-UWf5yA8sumJsF5XF5fvk3S4O1SN1/mEhtWR5UQgDP1kz49hSWeQrR5cdDQYjRAXTrXr/uCxchgug8h42RmSxhA==", + "version": "5.10.3", + "resolved": "https://registry.npmjs.org/@netlify/plugin-nextjs/-/plugin-nextjs-5.10.3.tgz", + "integrity": "sha512-0c4Y6Td2YcWjVaIwvJNNWqV8RJJ6uV/8DvKL2Czbp5Pp6KZ9TEIevexfzFSEZG1VgU6bgUUDAIPI2xFKN2Op8A==", "license": "MIT", "engines": { "node": ">=18.0.0" diff --git a/e2e-report/package.json b/e2e-report/package.json index bd125546be..f197f3cf44 100644 --- a/e2e-report/package.json +++ b/e2e-report/package.json @@ -9,7 +9,7 @@ "lint": "next lint" }, "dependencies": { - "@netlify/plugin-nextjs": "^5.10.2", + "@netlify/plugin-nextjs": "^5.10.3", "next": "^14.2.3", "react": "^18.3.1", "react-dom": "^18.3.1" diff --git a/package-lock.json b/package-lock.json index bd04a964be..5a0501bf04 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@netlify/plugin-nextjs", - "version": "5.10.3", + "version": "5.10.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@netlify/plugin-nextjs", - "version": "5.10.3", + "version": "5.10.4", "license": "MIT", "devDependencies": { "@fastly/http-compute-js": "1.1.5", diff --git a/package.json b/package.json index 07de174715..920c4b32b4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@netlify/plugin-nextjs", - "version": "5.10.3", + "version": "5.10.4", "description": "Run Next.js seamlessly on Netlify", "main": "./dist/index.js", "type": "module", diff --git a/src/run/handlers/cache.cts b/src/run/handlers/cache.cts index 4e9b61ce4a..37ade76962 100644 --- a/src/run/handlers/cache.cts +++ b/src/run/handlers/cache.cts @@ -332,6 +332,11 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions { } } case 'APP_PAGE': { + const requestContext = getRequestContext() + if (requestContext && blob.value?.kind === 'APP_PAGE') { + requestContext.isCacheableAppPage = true + } + const { revalidate, rscData, ...restOfPageValue } = blob.value span.addEvent(blob.value?.kind, { lastModified: blob.lastModified, revalidate, ttl }) @@ -410,6 +415,13 @@ export class NetlifyCacheHandler implements CacheHandlerForMultipleVersions { await this.cacheStore.set(key, { lastModified, value }, 'blobStore.set') + if (data?.kind === 'APP_PAGE') { + const requestContext = getRequestContext() + if (requestContext) { + requestContext.isCacheableAppPage = true + } + } + if ((!data && !isDataReq) || data?.kind === 'PAGE' || data?.kind === 'PAGES') { const requestContext = getRequestContext() if (requestContext?.didPagesRouterOnDemandRevalidate) { diff --git a/src/run/handlers/request-context.cts b/src/run/handlers/request-context.cts index b351984729..9aabb7ca0a 100644 --- a/src/run/handlers/request-context.cts +++ b/src/run/handlers/request-context.cts @@ -35,6 +35,7 @@ export type RequestContext = { backgroundWorkPromise: Promise logger: SystemLogger requestID: string + isCacheableAppPage?: boolean } type RequestContextAsyncLocalStorage = AsyncLocalStorage diff --git a/src/run/handlers/server.ts b/src/run/handlers/server.ts index 62c7c8f8b8..975f6993fb 100644 --- a/src/run/handlers/server.ts +++ b/src/run/handlers/server.ts @@ -17,7 +17,7 @@ import { nextResponseProxy } from '../revalidate.js' import { setFetchBeforeNextPatchedIt } from '../storage/storage.cjs' import { getLogger, type RequestContext } from './request-context.cjs' -import { getTracer } from './tracer.cjs' +import { getTracer, recordWarning } from './tracer.cjs' import { setupWaitUntil } from './wait-until.cjs' setFetchBeforeNextPatchedIt(globalThis.fetch) @@ -117,11 +117,6 @@ export default async ( const nextCache = response.headers.get('x-nextjs-cache') const isServedFromNextCache = nextCache === 'HIT' || nextCache === 'STALE' - topLevelSpan.setAttributes({ - 'x-nextjs-cache': nextCache ?? undefined, - isServedFromNextCache, - }) - if (isServedFromNextCache) { await adjustDateHeader({ headers: response.headers, @@ -136,6 +131,41 @@ export default async ( setVaryHeaders(response.headers, request, nextConfig) setCacheStatusHeader(response.headers, nextCache) + const netlifyVary = response.headers.get('netlify-vary') ?? undefined + const netlifyCdnCacheControl = response.headers.get('netlify-cdn-cache-control') ?? undefined + topLevelSpan.setAttributes({ + 'x-nextjs-cache': nextCache ?? undefined, + isServedFromNextCache, + netlifyVary, + netlifyCdnCacheControl, + }) + + if (requestContext.isCacheableAppPage) { + const isRSCRequest = request.headers.get('rsc') === '1' + const contentType = response.headers.get('content-type') ?? undefined + + const isExpectedContentType = + ((isRSCRequest && contentType?.includes('text/x-component')) || + (!isRSCRequest && contentType?.includes('text/html'))) ?? + false + + topLevelSpan.setAttributes({ + isRSCRequest, + isCacheableAppPage: true, + contentType, + isExpectedContentType, + }) + + if (!isExpectedContentType) { + recordWarning( + new Error( + `Unexpected content type was produced for App Router page response (isRSCRequest: ${isRSCRequest}, contentType: ${contentType})`, + ), + topLevelSpan, + ) + } + } + async function waitForBackgroundWork() { // it's important to keep the stream open until the next handler has finished await nextHandlerPromise diff --git a/src/run/storage/request-scoped-in-memory-cache.cts b/src/run/storage/request-scoped-in-memory-cache.cts index 8a046c3fe1..ddd803b991 100644 --- a/src/run/storage/request-scoped-in-memory-cache.cts +++ b/src/run/storage/request-scoped-in-memory-cache.cts @@ -25,44 +25,70 @@ 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 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() { @@ -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 })) + } }, } } 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..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', @@ -508,4 +509,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('') + }) })