From 9639fe704cf5c4a5a477bdc0c43219514c811601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Wed, 16 Feb 2022 19:53:48 +0100 Subject: [PATCH] Ensure we don't poll page in development when notFound: true is returned (#34352) Fixes: #34342 Visiting the following page will call gSSP indefinitely in a loop and logs errors from `on-demand-entries-client`: ```js const Home = () => null export default Home export function getServerSideProps() { console.log("gssp called") return { notFound: true } } ``` We should not keep fetching the page if it returns 404 as it can introduce unnecessary data requests. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com> --- .../client/dev/on-demand-entries-client.js | 12 ++++++- packages/next/server/base-server.ts | 3 ++ packages/next/server/render.tsx | 3 ++ packages/next/server/request-meta.ts | 1 + packages/next/shared/lib/utils.ts | 1 + test/development/gssp-notfound/index.test.ts | 34 +++++++++++++++++++ 6 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 test/development/gssp-notfound/index.test.ts diff --git a/packages/next/client/dev/on-demand-entries-client.js b/packages/next/client/dev/on-demand-entries-client.js index 349f95f5c22de..c68a9dd287e4e 100644 --- a/packages/next/client/dev/on-demand-entries-client.js +++ b/packages/next/client/dev/on-demand-entries-client.js @@ -11,7 +11,17 @@ export default async (page) => { } else { Router.ready(() => { setInterval(() => { - sendMessage(JSON.stringify({ event: 'ping', page: Router.pathname })) + // when notFound: true is returned we should use the notFoundPage + // as the Router.pathname will point to the 404 page but we want + // to ping the source page that returned notFound: true instead + const notFoundSrcPage = self.__NEXT_DATA__.notFoundSrcPage + const pathname = + (Router.pathname === '/404' || Router.pathname === '/_error') && + notFoundSrcPage + ? notFoundSrcPage + : Router.pathname + + sendMessage(JSON.stringify({ event: 'ping', page: pathname })) }, 2500) }) } diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index 6b4a6ace6885b..77160619770fe 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -1550,6 +1550,9 @@ export default abstract class Server { res.body('{"notFound":true}').send() return null } else { + if (this.renderOpts.dev) { + query.__nextNotFoundSrcPage = pathname + } await this.render404( req, res, diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index e52b91eeabc26..87b45ab0b44af 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -528,9 +528,11 @@ export async function renderToHTML( const headTags = (...args: any) => callMiddleware('headTags', args) const isFallback = !!query.__nextFallback + const notFoundSrcPage = query.__nextNotFoundSrcPage delete query.__nextFallback delete query.__nextLocale delete query.__nextDefaultLocale + delete query.__nextIsNotFound const isSSG = !!getStaticProps const isBuildTimeSSG = isSSG && renderOpts.nextExport @@ -1374,6 +1376,7 @@ export async function renderToHTML( defaultLocale, domainLocales, isPreview: isPreview === true ? true : undefined, + notFoundSrcPage: notFoundSrcPage && dev ? notFoundSrcPage : undefined, }, buildManifest: filteredBuildManifest, docComponentsRendered, diff --git a/packages/next/server/request-meta.ts b/packages/next/server/request-meta.ts index e2fed315088dc..f906e5f5f1f05 100644 --- a/packages/next/server/request-meta.ts +++ b/packages/next/server/request-meta.ts @@ -53,6 +53,7 @@ export function addRequestMeta( } type NextQueryMetadata = { + __nextNotFoundSrcPage?: string __nextDefaultLocale?: string __nextFallback?: 'true' __nextLocale?: string diff --git a/packages/next/shared/lib/utils.ts b/packages/next/shared/lib/utils.ts index 6c22ce7ff852d..94196af4f5971 100644 --- a/packages/next/shared/lib/utils.ts +++ b/packages/next/shared/lib/utils.ts @@ -107,6 +107,7 @@ export type NEXT_DATA = { domainLocales?: DomainLocale[] scriptLoader?: any[] isPreview?: boolean + notFoundSrcPage?: string rsc?: boolean } diff --git a/test/development/gssp-notfound/index.test.ts b/test/development/gssp-notfound/index.test.ts new file mode 100644 index 0000000000000..9ad292b974051 --- /dev/null +++ b/test/development/gssp-notfound/index.test.ts @@ -0,0 +1,34 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { waitFor } from 'next-test-utils' +import webdriver from 'next-webdriver' + +describe('getServerSideProps returns notFound: true', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: { + 'pages/index.js': ` + const Home = () => null + export default Home + + export function getServerSideProps() { + console.log("gssp called") + return { notFound: true } + } + `, + }, + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('should not poll indefinitely', async () => { + const browser = await webdriver(next.appPort, '/') + await waitFor(3000) + await browser.close() + const logOccurrences = next.cliOutput.split('gssp called').length - 1 + expect(logOccurrences).toBe(1) + }) +})