Skip to content

Commit

Permalink
Ensure we don't poll page in development when notFound: true is retur…
Browse files Browse the repository at this point in the history
…ned (#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>
  • Loading branch information
balazsorban44 and ijjk committed Feb 16, 2022
1 parent 7e93a89 commit 9639fe7
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 1 deletion.
12 changes: 11 additions & 1 deletion packages/next/client/dev/on-demand-entries-client.js
Expand Up @@ -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)
})
}
Expand Down
3 changes: 3 additions & 0 deletions packages/next/server/base-server.ts
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions packages/next/server/render.tsx
Expand Up @@ -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
Expand Down Expand Up @@ -1374,6 +1376,7 @@ export async function renderToHTML(
defaultLocale,
domainLocales,
isPreview: isPreview === true ? true : undefined,
notFoundSrcPage: notFoundSrcPage && dev ? notFoundSrcPage : undefined,
},
buildManifest: filteredBuildManifest,
docComponentsRendered,
Expand Down
1 change: 1 addition & 0 deletions packages/next/server/request-meta.ts
Expand Up @@ -53,6 +53,7 @@ export function addRequestMeta<K extends keyof RequestMeta>(
}

type NextQueryMetadata = {
__nextNotFoundSrcPage?: string
__nextDefaultLocale?: string
__nextFallback?: 'true'
__nextLocale?: string
Expand Down
1 change: 1 addition & 0 deletions packages/next/shared/lib/utils.ts
Expand Up @@ -107,6 +107,7 @@ export type NEXT_DATA = {
domainLocales?: DomainLocale[]
scriptLoader?: any[]
isPreview?: boolean
notFoundSrcPage?: string
rsc?: boolean
}

Expand Down
34 changes: 34 additions & 0 deletions 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)
})
})

0 comments on commit 9639fe7

Please sign in to comment.