feat: reduce 404 caching scope to just the 404 page itself

Author: orinokai

src/run/headers.ts

@@ -261,10 +261,9 @@ export const setCacheControlHeaders = (
     }
 
     if (
-      process.env.ENABLE_404_CACHING &&
-      ['GET', 'HEAD'].includes(request.method) &&
-      !headers.has('cdn-cache-control') &&
-      !headers.has('netlify-cdn-cache-control')
+      process.env.CACHE_404_PAGE &&
+      request.url.endsWith('/404') &&
+      ['GET', 'HEAD'].includes(request.method)
     ) {
       // handle CDN Cache Control on 404 Page responses
       setCacheControlFromRequestContext(headers, requestContext.pageHandlerRevalidate)

tests/integration/page-router.test.ts

@@ -152,15 +152,10 @@ test<FixtureTestContext>('Should serve correct locale-aware custom 404 pages', a
   ).toBe('fr')
 })
 
-describe('404 caching', () => {
-  beforeAll(() => {
-    process.env.ENABLE_404_CACHING = 'true'
-  })
-
-  afterAll(() => {
-    delete process.env.ENABLE_404_CACHING
-  })
-
+// These tests describe how the 404 caching should work, but unfortunately it doesn't work like
+// this in v5 and a fix would represent a breaking change so we are skipping them for now, but
+// leaving them here for future reference when considering the next major version
+describe.skip('404 caching', () => {
   describe('404 without getStaticProps', () => {
     test<FixtureTestContext>('not matching dynamic paths should be cached permanently', async (ctx) => {
       await createFixture('page-router', ctx)
@@ -177,7 +172,7 @@ describe('404 caching', () => {
         'should be cached permanently',
       ).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
     })
-    test<FixtureTestContext>('matching dynamic path with revalidate should be cached permanently', async (ctx) => {
+    test<FixtureTestContext>('matching dynamic path with revalidate should be cached for revalidate time', async (ctx) => {
       await createFixture('page-router', ctx)
       await runPlugin(ctx)
 
@@ -189,8 +184,8 @@ describe('404 caching', () => {
 
       expect(
         notExistingPage.headers['netlify-cdn-cache-control'],
-        'should be cached permanently',
-      ).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
+        'should be cached for revalidate time',
+      ).toBe('s-maxage=600, stale-while-revalidate=31536000, durable')
     })
   })
 
@@ -210,7 +205,7 @@ describe('404 caching', () => {
         'should be cached permanently',
       ).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
     })
-    test<FixtureTestContext>('matching dynamic path with revalidate should be cached permanently', async (ctx) => {
+    test<FixtureTestContext>('matching dynamic path with revalidate should be cached for revalidate time', async (ctx) => {
       await createFixture('page-router-base-path-i18n', ctx)
       await runPlugin(ctx)
 
@@ -222,8 +217,8 @@ describe('404 caching', () => {
 
       expect(
         notExistingPage.headers['netlify-cdn-cache-control'],
-        'should be cached permanently',
-      ).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
+        'should be cached for revalidate time',
+      ).toBe('s-maxage=600, stale-while-revalidate=31536000, durable')
     })
   })
 
@@ -251,7 +246,7 @@ describe('404 caching', () => {
       ).toBe('s-maxage=300, stale-while-revalidate=31536000, durable')
     })
 
-    test<FixtureTestContext>('matching dynamic path with revalidate should be cached for 404 page revalidate', async (ctx) => {
+    test<FixtureTestContext>('matching dynamic path with revalidate should be cached for revalidate time', async (ctx) => {
       await createFixture('page-router-404-get-static-props-with-revalidate', ctx)
       await runPlugin(ctx)
 
@@ -269,8 +264,76 @@ describe('404 caching', () => {
 
       expect(
         notExistingPage.headers['netlify-cdn-cache-control'],
-        'should be cached for 404 page revalidate',
-      ).toBe('s-maxage=300, stale-while-revalidate=31536000, durable')
+        'should be cached for revalidate time',
+      ).toBe('s-maxage=600, stale-while-revalidate=31536000, durable')
+    })
+  })
+})
+
+// This is a temporary fix to ensure that the 404 page itself is cached correctly when requested
+// directly. This is a workaround for a specific customer and should be removed once the 404 caching
+// is fixed in the next major version.
+describe.only('404 page caching', () => {
+  beforeAll(() => {
+    process.env.CACHE_404_PAGE = 'true'
+  })
+
+  afterAll(() => {
+    delete process.env.CACHE_404_PAGE
+  })
+
+  test<FixtureTestContext>('404 without getStaticProps', async (ctx) => {
+    await createFixture('page-router', ctx)
+    await runPlugin(ctx)
+
+    const notExistingPage = await invokeFunction(ctx, {
+      url: '/404',
+    })
+
+    expect(notExistingPage.statusCode).toBe(404)
+
+    expect(
+      notExistingPage.headers['netlify-cdn-cache-control'],
+      'should be cached permanently',
+    ).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
+  })
+
+  test<FixtureTestContext>('404 with getStaticProps without revalidate', async (ctx) => {
+    await createFixture('page-router-base-path-i18n', ctx)
+    await runPlugin(ctx)
+
+    const notExistingPage = await invokeFunction(ctx, {
+      url: '/base/404',
+    })
+
+    expect(notExistingPage.statusCode).toBe(404)
+
+    expect(
+      notExistingPage.headers['netlify-cdn-cache-control'],
+      'should be cached permanently',
+    ).toBe('s-maxage=31536000, stale-while-revalidate=31536000, durable')
+  })
+
+  test<FixtureTestContext>('404 with getStaticProps with revalidate', async (ctx) => {
+    await createFixture('page-router-404-get-static-props-with-revalidate', ctx)
+    await runPlugin(ctx)
+
+    // ignoring initial stale case
+    await invokeFunction(ctx, {
+      url: '/404',
     })
+
+    await new Promise((res) => setTimeout(res, 100))
+
+    const notExistingPage = await invokeFunction(ctx, {
+      url: '/404',
+    })
+
+    expect(notExistingPage.statusCode).toBe(404)
+
+    expect(
+      notExistingPage.headers['netlify-cdn-cache-control'],
+      'should be cached for 404 page revalidate',
+    ).toBe('s-maxage=300, stale-while-revalidate=31536000, durable')
   })
 })