Update fallback 404 handling to prevent reload loop (#18119)

This updates the fallback 404 handling to render the correct 404 page on the client when a 404 is returned from fetching the data route on a fallback page on the client. This prevents us from having to rely on a cache to be updated by the time we reload the page to prevent non-stop reloading. 

This also adds handling in serverless mode to ensure the correct 404 page is rendered when leveraging fallback: 'blocking' mode. 

Additional tests for the fallback: 'blocking' 404 handling will be added in a follow-up where returning notFound from `getServerSideProps` is also added.

packages/next/build/entries.ts

@@ -79,6 +79,7 @@ export function createEntrypoints(
     absoluteAppPath: pages['/_app'],
     absoluteDocumentPath: pages['/_document'],
     absoluteErrorPath: pages['/_error'],
+    absolute404Path: pages['/404'] || '',
     distDir: DOT_NEXT_ALIAS,
     buildId,
     assetPrefix: config.assetPrefix,

packages/next/build/webpack/loaders/next-serverless-loader.ts

@@ -19,6 +19,7 @@ export type ServerlessLoaderQuery = {
   absoluteAppPath: string
   absoluteDocumentPath: string
   absoluteErrorPath: string
+  absolute404Path: string
   buildId: string
   assetPrefix: string
   generateEtags: string
@@ -44,6 +45,7 @@ const nextServerlessLoader: loader.Loader = function () {
     absoluteAppPath,
     absoluteDocumentPath,
     absoluteErrorPath,
+    absolute404Path,
     generateEtags,
     poweredByHeader,
     basePath,
@@ -494,6 +496,7 @@ const nextServerlessLoader: loader.Loader = function () {
     export async function renderReqToHTML(req, res, renderMode, _renderOpts, _params) {
       let Document
       let Error
+      let NotFound
       ;[
         getStaticProps,
         getServerSideProps,
@@ -502,7 +505,8 @@ const nextServerlessLoader: loader.Loader = function () {
         App,
         config,
         { default: Document },
-        { default: Error }
+        { default: Error },
+        ${absolute404Path ? `{ default: NotFound }, ` : ''}
       ] = await Promise.all([
         getStaticProps,
         getServerSideProps,
@@ -511,7 +515,8 @@ const nextServerlessLoader: loader.Loader = function () {
         App,
         config,
         require('${absoluteDocumentPath}'),
-        require('${absoluteErrorPath}')
+        require('${absoluteErrorPath}'),
+        ${absolute404Path ? `require("${absolute404Path}"),` : ''}
       ])
 
       const fromExport = renderMode === 'export' || renderMode === true;
@@ -767,14 +772,41 @@ const nextServerlessLoader: loader.Loader = function () {
 
         if (!renderMode) {
           if (_nextData || getStaticProps || getServerSideProps) {
-            sendPayload(req, res, _nextData ? JSON.stringify(renderOpts.pageData) : result, _nextData ? 'json' : 'html', ${
-              generateEtags === 'true' ? true : false
-            }, {
-              private: isPreviewMode,
-              stateful: !!getServerSideProps,
-              revalidate: renderOpts.revalidate,
-            })
-            return null
+            if (renderOpts.ssgNotFound) {
+              res.statusCode = 404
+
+              const NotFoundComponent = ${
+                absolute404Path ? 'NotFound' : 'Error'
+              }
+
+              const errPathname = "${absolute404Path ? '/404' : '/_error'}"
+
+              const result = await renderToHTML(req, res, errPathname, parsedUrl.query, Object.assign({}, options, {
+                getStaticProps: undefined,
+                getStaticPaths: undefined,
+                getServerSideProps: undefined,
+                Component: NotFoundComponent,
+                err: undefined
+              }))
+
+              sendPayload(req, res, result, 'html', ${
+                generateEtags === 'true' ? true : false
+              }, {
+                private: isPreviewMode,
+                stateful: !!getServerSideProps,
+                revalidate: renderOpts.revalidate,
+              })
+              return null
+            } else {
+              sendPayload(req, res, _nextData ? JSON.stringify(renderOpts.pageData) : result, _nextData ? 'json' : 'html', ${
+                generateEtags === 'true' ? true : false
+              }, {
+                private: isPreviewMode,
+                stateful: !!getServerSideProps,
+                revalidate: renderOpts.revalidate,
+              })
+              return null
+            }
           }
         } else if (isPreviewMode) {
           res.setHeader(

packages/next/next-server/lib/router/router.ts

@@ -337,7 +337,7 @@ function fetchNextData(dataHref: string, isServerRender: boolean) {
     // on a client-side transition. Otherwise, we'd get into an infinite
     // loop.
 
-    if (!isServerRender || err.message === 'SSG Data NOT_FOUND') {
+    if (!isServerRender) {
       markLoadingError(err)
     }
     throw err
@@ -907,13 +907,6 @@ export default class Router implements BaseRouter {
       //  3. Internal error while loading the page
 
       // So, doing a hard reload is the proper way to deal with this.
-      if (process.env.NODE_ENV === 'development') {
-        // append __next404 query to prevent fallback from being re-served
-        // on reload in development
-        if (err.message === SSG_DATA_NOT_FOUND_ERROR && this.isSsr) {
-          as += `${as.indexOf('?') > -1 ? '&' : '?'}__next404=1`
-        }
-      }
       window.location.href = as
 
       // Changing the URL doesn't block executing the current code path.
@@ -922,14 +915,34 @@ export default class Router implements BaseRouter {
     }
 
     try {
-      const { page: Component, styleSheets } = await this.fetchComponent(
-        '/_error'
-      )
+      let Component: ComponentType
+      let styleSheets: StyleSheetTuple[]
+      const ssg404 = err.message === SSG_DATA_NOT_FOUND_ERROR
+
+      if (ssg404) {
+        try {
+          ;({ page: Component, styleSheets } = await this.fetchComponent(
+            '/404'
+          ))
+        } catch (_err) {
+          // non-fatal fallback to _error
+        }
+      }
+
+      if (
+        typeof Component! === 'undefined' ||
+        typeof styleSheets! === 'undefined'
+      ) {
+        ;({ page: Component, styleSheets } = await this.fetchComponent(
+          '/_error'
+        ))
+      }
+
       const routeInfo: PrivateRouteInfo = {
         Component,
         styleSheets,
-        err,
-        error: err,
+        err: ssg404 ? undefined : err,
+        error: ssg404 ? undefined : err,
       }
 
       try {
@@ -938,6 +951,11 @@ export default class Router implements BaseRouter {
           pathname,
           query,
         } as any)
+
+        if (ssg404 && routeInfo.props && routeInfo.props.pageProps) {
+          routeInfo.props.pageProps.statusCode = 404
+        }
+        console.log(routeInfo)
       } catch (gipErr) {
         console.error('Error in error page `getInitialProps`: ', gipErr)
         routeInfo.props = {}

packages/next/next-server/server/next-server.ts

@@ -1144,7 +1144,6 @@ export default class Server {
             ...(components.getStaticProps
               ? {
                   amp: query.amp,
-                  __next404: query.__next404,
                   _nextDataReq: query._nextDataReq,
                   __nextLocale: query.__nextLocale,
                 }
@@ -1270,15 +1269,6 @@ export default class Server {
             query.amp ? '.amp' : ''
           }`
 
-    // In development we use a __next404 query to allow signaling we should
-    // render the 404 page after attempting to fetch the _next/data for a
-    // fallback page since the fallback page will always be available after
-    // reload and we don't want to re-serve it and instead want to 404.
-    if (this.renderOpts.dev && isSSG && query.__next404) {
-      delete query.__next404
-      throw new NoFallbackError()
-    }
-
     // Complete the response with cached data if its present
     const cachedData = ssgCacheKey
       ? await this.incrementalCache.get(ssgCacheKey)

packages/next/next-server/server/render.tsx

@@ -414,7 +414,6 @@ export async function renderToHTML(
   const isFallback = !!query.__nextFallback
   delete query.__nextFallback
   delete query.__nextLocale
-  delete query.__next404
 
   const isSSG = !!getStaticProps
   const isBuildTimeSSG = isSSG && renderOpts.nextExport

test/integration/build-output/test/index.test.js

@@ -95,16 +95,16 @@ describe('Build Output', () => {
       expect(indexSize.endsWith('B')).toBe(true)
 
       // should be no bigger than 60.8 kb
-      expect(parseFloat(indexFirstLoad) - 61).toBeLessThanOrEqual(0)
+      expect(parseFloat(indexFirstLoad) - 61.2).toBeLessThanOrEqual(0)
       expect(indexFirstLoad.endsWith('kB')).toBe(true)
 
       expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0)
       expect(err404Size.endsWith('kB')).toBe(true)
 
-      expect(parseFloat(err404FirstLoad) - 64.2).toBeLessThanOrEqual(0)
+      expect(parseFloat(err404FirstLoad) - 64.4).toBeLessThanOrEqual(0)
       expect(err404FirstLoad.endsWith('kB')).toBe(true)
 
-      expect(parseFloat(sharedByAll) - 60.8).toBeLessThanOrEqual(0)
+      expect(parseFloat(sharedByAll) - 61).toBeLessThanOrEqual(0)
       expect(sharedByAll.endsWith('kB')).toBe(true)
 
       if (_appSize.endsWith('kB')) {

test/integration/i18n-support/test/index.test.js

@@ -697,7 +697,7 @@ function runTests(isDev) {
       true
     )
     expect(parsedUrl.pathname).toBe('/en/not-found/fallback/first')
-    expect(parsedUrl.query).toEqual(isDev ? { __next404: '1' } : {})
+    expect(parsedUrl.query).toEqual({})
 
     if (isDev) {
       // make sure page doesn't reload un-necessarily in development