Ensure SSG data 404 handles correctly for non-notFound (#20622)

Follow-up to #20594 this ensures non-notFound SSG data 404s do cause a hard navigation as this signals a new deployment has occurred and a hard navigation will load the new deployment's version of the page. Closes: #20623
vercel · Dec 30, 2020 · 5324e8b · 5324e8b
1 parent 74909ec
commit 5324e8b
Show file tree

Hide file tree

Showing 8 changed files with 178 additions and 16 deletions.
diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts
@@ -312,6 +312,11 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) {
           if (renderOpts.isNotFound) {
             res.statusCode = 404
 
+            if (_nextData) {
+              res.end('{"notFound":true}')
+              return null
+            }
+
             const NotFoundComponent = notFoundMod ? notFoundMod.default : Error
             const errPathname = notFoundMod ? '/404' : '/_error'
 

diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts
@@ -368,7 +368,12 @@ function fetchRetry(url: string, attempts: number): Promise<any> {
         return fetchRetry(url, attempts - 1)
       }
       if (res.status === 404) {
-        return { notFound: SSG_DATA_NOT_FOUND }
+        return res.json().then((data) => {
+          if (data.notFound) {
+            return { notFound: SSG_DATA_NOT_FOUND }
+          }
+          throw new Error(`Failed to load static props`)
+        })
       }
       throw new Error(`Failed to load static props`)
     }
@@ -977,8 +982,6 @@ export default class Router implements BaseRouter {
             as,
             { shallow: false }
           )
-
-          console.log('using routeInfo', routeInfo)
         }
       }
 

diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts
@@ -1490,10 +1490,15 @@ export default class Server {
         if (revalidateOptions) {
           setRevalidateHeaders(res, revalidateOptions)
         }
-        await this.render404(req, res, {
-          pathname,
-          query,
-        } as UrlWithParsedQuery)
+        if (isDataReq) {
+          res.statusCode = 404
+          res.end('{"notFound":true}')
+        } else {
+          await this.render404(req, res, {
+            pathname,
+            query,
+          } as UrlWithParsedQuery)
+        }
       } else {
         sendPayload(
           req,
@@ -1737,12 +1742,15 @@ export default class Server {
       if (revalidateOptions) {
         setRevalidateHeaders(res, revalidateOptions)
       }
-      await this.render404(
-        req,
-        res,
-        { pathname, query } as UrlWithParsedQuery,
-        !!revalidateOptions
-      )
+      if (isDataReq) {
+        res.statusCode = 404
+        res.end('{"notFound":true}')
+      } else {
+        await this.render404(req, res, {
+          pathname,
+          query,
+        } as UrlWithParsedQuery)
+      }
     }
     return resHtml
   }

diff --git a/test/integration/i18n-support/test/shared.js b/test/integration/i18n-support/test/shared.js
@@ -187,7 +187,6 @@ export function runTests(ctx) {
             '/fr/gsp.json',
             '/fr/gsp/fallback/first.json',
             '/fr/gsp/fallback/hello.json',
-            '/fr/gsp/no-fallback/first.json',
           ]
         )
         return 'yes'
@@ -753,7 +752,6 @@ export function runTests(ctx) {
             '/fr/gsp.json',
             '/fr/gsp/fallback/first.json',
             '/fr/gsp/fallback/hello.json',
-            '/fr/gsp/no-fallback/first.json',
           ]
         )
         return 'yes'
@@ -966,7 +964,6 @@ export function runTests(ctx) {
             '/fr/gsp.json',
             '/fr/gsp/fallback/first.json',
             '/fr/gsp/fallback/hello.json',
-            '/fr/gsp/no-fallback/first.json',
           ]
         )
         return 'yes'

diff --git a/test/integration/ssg-data-404/pages/gsp.js b/test/integration/ssg-data-404/pages/gsp.js
@@ -0,0 +1,17 @@
+export const getStaticProps = () => {
+  return {
+    props: {
+      hello: 'world',
+      random: Math.random(),
+    },
+  }
+}
+
+export default function Page(props) {
+  return (
+    <>
+      <p id="gsp">getStaticProps page</p>
+      <p id="props">{JSON.stringify(props)}</p>
+    </>
+  )
+}
diff --git a/test/integration/ssg-data-404/pages/gssp.js b/test/integration/ssg-data-404/pages/gssp.js
@@ -0,0 +1,17 @@
+export const getServerSideProps = () => {
+  return {
+    props: {
+      hello: 'world',
+      random: Math.random(),
+    },
+  }
+}
+
+export default function Page(props) {
+  return (
+    <>
+      <p id="gssp">getServerSideProps page</p>
+      <p id="props">{JSON.stringify(props)}</p>
+    </>
+  )
+}
diff --git a/test/integration/ssg-data-404/pages/index.js b/test/integration/ssg-data-404/pages/index.js
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p id="index">Index page</p>
+}
diff --git a/test/integration/ssg-data-404/test/index.test.js b/test/integration/ssg-data-404/test/index.test.js
@@ -0,0 +1,112 @@
+/* eslint-env jest */
+
+import { join } from 'path'
+import http from 'http'
+import httpProxy from 'http-proxy'
+import webdriver from 'next-webdriver'
+import {
+  findPort,
+  killApp,
+  launchApp,
+  nextBuild,
+  nextStart,
+} from 'next-test-utils'
+
+jest.setTimeout(1000 * 60 * 1)
+const appDir = join(__dirname, '..')
+
+let app
+let appPort
+let proxyServer
+let proxyPort
+let should404Data = false
+
+const runTests = () => {
+  it('should hard navigate when a new deployment occurs', async () => {
+    const browser = await webdriver(proxyPort, '/')
+
+    await browser.eval('window.beforeNav = 1')
+    expect(await browser.elementByCss('#index').text()).toBe('Index page')
+
+    should404Data = true
+
+    await browser.eval(`(function() {
+      window.next.router.push('/gsp')
+    })()`)
+    await browser.waitForElementByCss('#gsp')
+
+    expect(await browser.eval('window.beforeNav')).toBe(null)
+
+    await browser.eval('window.beforeNav = 1')
+    await browser.eval(`(function() {
+      window.next.router.push('/gssp')
+    })()`)
+    await browser.waitForElementByCss('#gssp')
+
+    expect(await browser.eval('window.beforeNav')).toBe(null)
+  })
+}
+
+describe('SSG data 404', () => {
+  describe('dev mode', () => {
+    beforeAll(async () => {
+      appPort = await findPort()
+      app = await launchApp(appDir, appPort)
+
+      const proxy = httpProxy.createProxyServer({
+        target: `http://localhost:${appPort}`,
+      })
+      proxyPort = await findPort()
+
+      proxyServer = http.createServer((req, res) => {
+        if (should404Data && req.url.match(/\/_next\/data/)) {
+          res.statusCode = 404
+          return res.end('not found')
+        }
+        proxy.web(req, res)
+      })
+
+      await new Promise((resolve) => {
+        proxyServer.listen(proxyPort, () => resolve())
+      })
+    })
+    afterAll(async () => {
+      await killApp(app)
+      proxyServer.close()
+    })
+
+    runTests()
+  })
+
+  describe('production mode', () => {
+    beforeAll(async () => {
+      await nextBuild(appDir)
+
+      appPort = await findPort()
+      app = await nextStart(appDir, appPort)
+
+      const proxy = httpProxy.createProxyServer({
+        target: `http://localhost:${appPort}`,
+      })
+      proxyPort = await findPort()
+
+      proxyServer = http.createServer((req, res) => {
+        if (should404Data && req.url.match(/\/_next\/data/)) {
+          res.statusCode = 404
+          return res.end('not found')
+        }
+        proxy.web(req, res)
+      })
+
+      await new Promise((resolve) => {
+        proxyServer.listen(proxyPort, () => resolve())
+      })
+    })
+    afterAll(async () => {
+      await killApp(app)
+      proxyServer.close()
+    })
+
+    runTests()
+  })
+})