Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Commit

Permalink
Disable prerendering /500 when _error has getServerSideProps (vercel#…
Browse files Browse the repository at this point in the history
…23586)

This prevents unexpected errors occurring from users leveraging `getServerSideProps` in `_error` with the new `/500` prerendering as we are currently only checking for a custom `getInitialProps` in `_error` since it opts out of the static optimization although `getServerSideProps` also opts out of the optimization. 

Fixes: vercel#23541
Fixes: vercel#23128
Fixes: vercel#23541
Fixes: vercel#24206

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
  • Loading branch information
ijjk authored Jun 11, 2021
1 parent b1153cb commit a9f8a12
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 6 deletions.
30 changes: 26 additions & 4 deletions packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ export default async function build(
const nonStaticErrorPageSpan = staticCheckSpan.traceChild(
'check-static-error-page'
)
const nonStaticErrorPagePromise = nonStaticErrorPageSpan.traceAsyncFn(
const errorPageHasCustomGetInitialProps = nonStaticErrorPageSpan.traceAsyncFn(
async () =>
hasCustomErrorPage &&
(await staticCheckWorkers.hasCustomGetInitialProps(
Expand All @@ -687,6 +687,20 @@ export default async function build(
false
))
)

const errorPageStaticResult = nonStaticErrorPageSpan.traceAsyncFn(
async () =>
hasCustomErrorPage &&
staticCheckWorkers.isPageStatic(
'/_error',
distDir,
isLikeServerless,
runtimeEnvConfig,
config.i18n?.locales,
config.i18n?.defaultLocale
)
)

// we don't output _app in serverless mode so use _app export
// from _error instead
const appPageToCheck = isLikeServerless ? '/_error' : '/_app'
Expand Down Expand Up @@ -847,12 +861,18 @@ export default async function build(
})
})
)

const errorPageResult = await errorPageStaticResult
const nonStaticErrorPage =
(await errorPageHasCustomGetInitialProps) ||
(errorPageResult && errorPageResult.hasServerProps)

const returnValue = {
customAppGetInitialProps: await customAppGetInitialPropsPromise,
namedExports: await namedExportsPromise,
isNextImageImported,
hasSsrAmpPages,
hasNonStaticErrorPage: await nonStaticErrorPagePromise,
hasNonStaticErrorPage: nonStaticErrorPage,
}

staticCheckWorkers.end()
Expand Down Expand Up @@ -989,13 +1009,15 @@ export default async function build(
mappedPages[page] && mappedPages[page].startsWith('private-next-pages')
)
usedStaticStatusPages.forEach((page) => {
if (!ssgPages.has(page)) {
if (!ssgPages.has(page) && !customAppGetInitialProps) {
staticPages.add(page)
}
})

const hasPages500 = usedStaticStatusPages.includes('/500')
const useDefaultStatic500 = !hasPages500 && !hasNonStaticErrorPage
const useDefaultStatic500 =
!hasPages500 && !hasNonStaticErrorPage && !customAppGetInitialProps

const combinedPages = [...staticPages, ...ssgPages]

if (combinedPages.length > 0 || useStatic404 || useDefaultStatic500) {
Expand Down
3 changes: 2 additions & 1 deletion packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,7 @@ export default class Server {
opts: RenderOptsPartial
): Promise<string | null> {
const is404Page = pathname === '/404'
const is500Page = pathname === '/500'

const isLikeServerless =
typeof components.Component === 'object' &&
Expand Down Expand Up @@ -1560,7 +1561,7 @@ export default class Server {
: resolvedUrlPathname
}${query.amp ? '.amp' : ''}`

if (is404Page && isSSG) {
if ((is404Page || is500Page) && isSSG) {
ssgCacheKey = `${locale ? `/${locale}` : ''}${pathname}${
query.amp ? '.amp' : ''
}`
Expand Down
121 changes: 120 additions & 1 deletion test/integration/500-page/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ describe('500 Page Support', () => {
runTests('serverless')
})

it('still builds 500 statically with getInitialProps in _app', async () => {
it('does not build 500 statically with getInitialProps in _app', async () => {
await fs.writeFile(
pagesApp,
`
Expand All @@ -127,8 +127,71 @@ describe('500 Page Support', () => {
stderr: true,
stdout: true,
})

await fs.remove(pagesApp)

expect(stderr).not.toMatch(gip500Err)
expect(buildStdout).not.toContain('rendered 500')
expect(code).toBe(0)
expect(
await fs.pathExists(join(appDir, '.next/server/pages/500.html'))
).toBe(false)

let appStdout = ''
const appPort = await findPort()
const app = await nextStart(appDir, appPort, {
onStdout(msg) {
appStdout += msg || ''
},
onStderr(msg) {
appStdout += msg || ''
},
})

await renderViaHTTP(appPort, '/err')
await killApp(app)

expect(appStdout).toContain('rendered 500')
})

it('does build 500 statically with getInitialProps in _app and getStaticProps in pages/500', async () => {
await fs.writeFile(
pagesApp,
`
import App from 'next/app'
const page = ({ Component, pageProps }) => <Component {...pageProps} />
page.getInitialProps = (ctx) => App.getInitialProps(ctx)
export default page
`
)
await fs.rename(pages500, `${pages500}.bak`)
await fs.writeFile(
pages500,
`
const page = () => {
console.log('rendered 500')
return 'custom 500 page'
}
export default page
export const getStaticProps = () => {
return {
props: {}
}
}
`
)
await fs.remove(join(appDir, '.next'))
const { stderr, stdout: buildStdout, code } = await nextBuild(appDir, [], {
stderr: true,
stdout: true,
})

await fs.remove(pagesApp)
await fs.remove(pages500)
await fs.rename(`${pages500}.bak`, pages500)

expect(stderr).not.toMatch(gip500Err)
expect(buildStdout).toContain('rendered 500')
expect(code).toBe(0)
Expand All @@ -139,6 +202,9 @@ describe('500 Page Support', () => {
let appStdout = ''
const appPort = await findPort()
const app = await nextStart(appDir, appPort, {
onStdout(msg) {
appStdout += msg || ''
},
onStderr(msg) {
appStdout += msg || ''
},
Expand Down Expand Up @@ -270,6 +336,59 @@ describe('500 Page Support', () => {
expect(appStderr).toContain('called _error.getInitialProps')
})

it('does not build 500 statically with no pages/500 and getServerSideProps in _error', async () => {
await fs.rename(pages500, `${pages500}.bak`)
await fs.writeFile(
pagesError,
`
function Error({ statusCode }) {
return <p>Error status: {statusCode}</p>
}
export const getServerSideProps = ({ req, res, err }) => {
console.error('called _error getServerSideProps')
if (req.url === '/500') {
throw new Error('should not export /500')
}
return {
props: {
statusCode: res && res.statusCode ? res.statusCode : err ? err.statusCode : 404
}
}
}
export default Error
`
)
await fs.remove(join(appDir, '.next'))
const { stderr: buildStderr, code } = await nextBuild(appDir, [], {
stderr: true,
})
await fs.rename(`${pages500}.bak`, pages500)
await fs.remove(pagesError)
console.log(buildStderr)
expect(buildStderr).not.toMatch(gip500Err)
expect(code).toBe(0)
expect(
await fs.pathExists(join(appDir, '.next/server/pages/500.html'))
).toBe(false)

let appStderr = ''
const appPort = await findPort()
const app = await nextStart(appDir, appPort, {
onStderr(msg) {
appStderr += msg || ''
},
})

await renderViaHTTP(appPort, '/err')
await killApp(app)

expect(appStderr).toContain('called _error getServerSideProps')
})

it('does not build 500 statically with no pages/500 and custom getInitialProps in _error and _app', async () => {
await fs.rename(pages500, `${pages500}.bak`)
await fs.writeFile(
Expand Down

0 comments on commit a9f8a12

Please sign in to comment.