fix: handle pages router notFound pages (#318)

* test: add page router not found case

* test: add app router not found case

* test: add i18n basePath case

* fix: don't try to upload not found routes

* fix: app router notFound() should have 404 status

Author: pieh

src/build/content/prerendered.ts

@@ -46,12 +46,21 @@ const buildPagesCacheValue = async (path: string): Promise<CachedPageValue> => (
   status: undefined,
 })
 
-const buildAppCacheValue = async (path: string): Promise<CachedPageValue> => ({
-  kind: 'PAGE',
-  html: await readFile(`${path}.html`, 'utf-8'),
-  pageData: await readFile(`${path}.rsc`, 'utf-8'),
-  ...JSON.parse(await readFile(`${path}.meta`, 'utf-8')),
-})
+const buildAppCacheValue = async (path: string): Promise<CachedPageValue> => {
+  const meta = JSON.parse(await readFile(`${path}.meta`, 'utf-8'))
+  const rsc = await readFile(`${path}.rsc`, 'utf-8')
+
+  if (!meta.status && rsc.includes('NEXT_NOT_FOUND')) {
+    meta.status = 404
+  }
+
+  return {
+    kind: 'PAGE',
+    html: await readFile(`${path}.html`, 'utf-8'),
+    pageData: rsc,
+    ...meta,
+  }
+}
 
 const buildRouteCacheValue = async (path: string): Promise<CachedRouteValue> => ({
   kind: 'ROUTE',
@@ -83,6 +92,10 @@ export const copyPrerenderedContent = async (ctx: PluginContext): Promise<void>
             !existsSync(join(ctx.publishDir, 'server/app', `${key}.html`)):
             return
           case meta.dataRoute?.endsWith('.json'):
+            if (manifest.notFoundRoutes.includes(route)) {
+              // if pages router returns 'notFound: true', build won't produce html and json files
+              return
+            }
             value = await buildPagesCacheValue(join(ctx.publishDir, 'server/pages', key))
             break
           case meta.dataRoute?.endsWith('.rsc'):

tests/e2e/page-router.test.ts

@@ -208,6 +208,22 @@ test.describe('Simple Page Router (no basePath, no i18n)', () => {
     expect(headers['cache-control']).toBe('no-cache,no-store,max-age=0,must-revalidate')
   })
 
+  test('requesting a non existing page route that needs to be fetched from the blob store like 404.html (notFound: true)', async ({
+    page,
+    pageRouter,
+  }) => {
+    const response = await page.goto(new URL('static/not-found', pageRouter.url).href)
+    const headers = response?.headers() || {}
+    expect(response?.status()).toBe(404)
+
+    expect(await page.textContent('h1')).toBe('404')
+
+    expect(headers['netlify-cdn-cache-control']).toBe(
+      's-maxage=31536000, stale-while-revalidate=31536000',
+    )
+    expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
+  })
+
   test('requesting a page with a very long name works', async ({ page, pageRouter }) => {
     const response = await page.goto(
       new URL(
@@ -740,4 +756,36 @@ test.describe('Page Router with basePath and i18n', () => {
       expect(data3?.pageProps?.time).toBe(date3)
     })
   })
+
+  test('requesting a non existing page route that needs to be fetched from the blob store like 404.html', async ({
+    page,
+    pageRouter,
+  }) => {
+    const response = await page.goto(new URL('non-exisitng', pageRouter.url).href)
+    const headers = response?.headers() || {}
+    expect(response?.status()).toBe(404)
+
+    expect(await page.textContent('h1')).toBe('404')
+
+    expect(headers['netlify-cdn-cache-control']).toBe(
+      'no-cache, no-store, max-age=0, must-revalidate',
+    )
+    expect(headers['cache-control']).toBe('no-cache,no-store,max-age=0,must-revalidate')
+  })
+
+  test('requesting a non existing page route that needs to be fetched from the blob store like 404.html (notFound: true)', async ({
+    page,
+    pageRouter,
+  }) => {
+    const response = await page.goto(new URL('static/not-found', pageRouter.url).href)
+    const headers = response?.headers() || {}
+    expect(response?.status()).toBe(404)
+
+    expect(await page.textContent('h1')).toBe('404')
+
+    expect(headers['netlify-cdn-cache-control']).toBe(
+      's-maxage=31536000, stale-while-revalidate=31536000',
+    )
+    expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
+  })
 })

tests/e2e/simple-app.test.ts

@@ -170,3 +170,35 @@ test.describe('next/image is using Netlify Image CDN', () => {
     await expect(await nextImageResponse.headerValue('content-type')).toEqual('image/avif')
   })
 })
+
+test('requesting a non existing page route that needs to be fetched from the blob store like 404.html', async ({
+  page,
+  simpleNextApp,
+}) => {
+  const response = await page.goto(new URL('non-exisitng', simpleNextApp.url).href)
+  const headers = response?.headers() || {}
+  expect(response?.status()).toBe(404)
+
+  expect(await page.textContent('h1')).toBe('404')
+
+  expect(headers['netlify-cdn-cache-control']).toBe(
+    'private, no-cache, no-store, max-age=0, must-revalidate',
+  )
+  expect(headers['cache-control']).toBe('private,no-cache,no-store,max-age=0,must-revalidate')
+})
+
+test('requesting a non existing page route that needs to be fetched from the blob store like 404.html (notFound())', async ({
+  page,
+  simpleNextApp,
+}) => {
+  const response = await page.goto(new URL('not-found', simpleNextApp.url).href)
+  const headers = response?.headers() || {}
+  expect(response?.status()).toBe(404)
+
+  expect(await page.textContent('h1')).toBe('404')
+
+  expect(headers['netlify-cdn-cache-control']).toBe(
+    's-maxage=31536000, stale-while-revalidate=31536000',
+  )
+  expect(headers['cache-control']).toBe('public,max-age=0,must-revalidate')
+})

tests/fixtures/page-router-base-path-i18n/pages/static/not-found.js

@@ -0,0 +1,9 @@
+const Show = () => <div>Won't be used</div>
+
+export async function getStaticProps() {
+  return {
+    notFound: true,
+  }
+}
+
+export default Show

tests/fixtures/page-router/pages/static/not-found.js

@@ -0,0 +1,9 @@
+const Show = () => <div>Won't be used</div>
+
+export async function getStaticProps() {
+  return {
+    notFound: true,
+  }
+}
+
+export default Show

tests/fixtures/simple-next-app/app/not-found/page.js

@@ -0,0 +1,5 @@
+import { notFound } from 'next/navigation'
+
+export default async function Page() {
+  notFound()
+}

tests/integration/simple-app.test.ts

@@ -42,6 +42,7 @@ test<FixtureTestContext>('Test that the simple next app is working', async (ctx)
     '/image/remote-pattern-1',
     '/image/remote-pattern-2',
     '/index',
+    '/not-found',
     '/other',
     '/redirect',
     '/redirect/response',
@@ -60,7 +61,11 @@ test<FixtureTestContext>('Test that the simple next app is working', async (ctx)
 
   const notFound = await invokeFunction(ctx, { url: 'not-found' })
   expect(notFound.statusCode).toBe(404)
-  expect(load(notFound.body)('h1').text()).toBe('404')
+  expect(notFound.body).toContain('NEXT_NOT_FOUND')
+
+  const notExisting = await invokeFunction(ctx, { url: 'non-exisitng' })
+  expect(notExisting.statusCode).toBe(404)
+  expect(load(notExisting.body)('h1').text()).toBe('404')
 })
 
 test<FixtureTestContext>('Should warn if publish dir is root', async (ctx) => {