Fix middleware notFound: true handling (vercel#46759)

This ensures we properly handle rendering the `404` page when `notFound: true` is returned and middleware is present. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) Fixes: vercel#38239 x-ref: vercel#43772 --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
MLH-Fellowship · Mar 4, 2023 · 3e9a99c · 3e9a99c
1 parent 6357de0
commit 3e9a99c
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 3 deletions.
diff --git a/packages/next/src/shared/lib/router/router.ts b/packages/next/src/shared/lib/router/router.ts
@@ -535,7 +535,7 @@ function fetchNextData({
               return { dataHref, response, text, json: {}, cacheKey }
             }
 
-            if (!hasMiddleware && response.status === 404) {
+            if (response.status === 404) {
               if (tryToParseAsJSON(text)?.notFound) {
                 return {
                   dataHref,
@@ -1647,6 +1647,7 @@ export default class Router implements BaseRouter {
             routeProps: { shallow: false },
             locale: nextState.locale,
             isPreview: nextState.isPreview,
+            isNotFound: true,
           })
 
           if ('type' in routeInfo) {
@@ -1913,6 +1914,7 @@ export default class Router implements BaseRouter {
     unstable_skipClientCache,
     isQueryUpdating,
     isMiddlewareRewrite,
+    isNotFound,
   }: {
     route: string
     pathname: string
@@ -1926,6 +1928,7 @@ export default class Router implements BaseRouter {
     unstable_skipClientCache?: boolean
     isQueryUpdating?: boolean
     isMiddlewareRewrite?: boolean
+    isNotFound?: boolean
   }) {
     /**
      * This `route` binding can change if there's a rewrite
@@ -1959,7 +1962,7 @@ export default class Router implements BaseRouter {
         dataHref: this.pageLoader.getDataHref({
           href: formatWithValidation({ pathname, query }),
           skipInterpolation: true,
-          asPath: resolvedAs,
+          asPath: isNotFound ? '/404' : resolvedAs,
           locale,
         }),
         hasMiddleware: true,
@@ -1981,7 +1984,7 @@ export default class Router implements BaseRouter {
           ? null
           : await withMiddlewareEffects({
               fetchData: () => fetchNextData(fetchNextDataParams),
-              asPath: resolvedAs,
+              asPath: isNotFound ? '/404' : resolvedAs,
               locale: locale,
               router: this,
             }).catch((err) => {
@@ -1995,6 +1998,12 @@ export default class Router implements BaseRouter {
               throw err
             })
 
+      // when rendering error routes we don't apply middleware
+      // effects
+      if (data && (pathname === '/_error' || pathname === '/404')) {
+        data.effect = undefined
+      }
+
       if (isQueryUpdating) {
         if (!data) {
           data = { json: self.__NEXT_DATA__.props }

diff --git a/test/e2e/middleware-general/app/pages/ssg/[slug].js b/test/e2e/middleware-general/app/pages/ssg/[slug].js
@@ -8,6 +8,11 @@ export default function Page(props) {
     router.isReady ? router.asPath : router.href
   )
 
+  if (!props.params) {
+    console.error('props', props)
+    throw new Error('missing props!!!')
+  }
+
   useEffect(() => {
     if (router.isReady) {
       setAsPath(router.asPath)
@@ -26,6 +31,12 @@ export default function Page(props) {
 }
 
 export function getStaticProps({ params }) {
+  if (params.slug.includes('not-found')) {
+    return {
+      notFound: true,
+    }
+  }
+
   return {
     props: {
       now: Date.now(),

diff --git a/test/e2e/middleware-general/test/index.test.ts b/test/e2e/middleware-general/test/index.test.ts
@@ -70,6 +70,18 @@ describe('Middleware Runtime', () => {
   }
 
   function runTests({ i18n }: { i18n?: boolean }) {
+    it('should work with notFound: true correctly', async () => {
+      const browser = await next.browser('/ssr-page')
+      await browser.eval('window.beforeNav = 1')
+      await browser.eval('window.next.router.push("/ssg/not-found-1")')
+
+      await check(
+        () => browser.eval('document.documentElement.innerHTML'),
+        /This page could not be found/
+      )
+      expect(await browser.eval('window.beforeNav')).toBe(1)
+    })
+
     it('should be able to rewrite on _next/static/chunks/pages/ 404', async () => {
       const res = await fetchViaHTTP(
         next.url,