Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close stream when fatal error occurs #31164

Merged
merged 3 commits into from Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/next/build/entries.ts
Expand Up @@ -162,6 +162,7 @@ export function createEntrypoints(
page,
absoluteAppPath: pages['/_app'],
absoluteDocumentPath: pages['/_document'],
absoluteErrorPath: pages['/_error'],
absolutePagePath,
isServerComponent: isFlight,
buildId,
Expand Down
Expand Up @@ -5,6 +5,7 @@ export default async function middlewareRSCLoader(this: any) {
absolutePagePath,
absoluteAppPath,
absoluteDocumentPath,
absoluteErrorPath,
basePath,
isServerComponent: isServerComponentQuery,
assetPrefix,
Expand All @@ -14,23 +15,28 @@ export default async function middlewareRSCLoader(this: any) {
const isServerComponent = isServerComponentQuery === 'true'
const stringifiedAbsolutePagePath = stringifyRequest(this, absolutePagePath)
const stringifiedAbsoluteAppPath = stringifyRequest(this, absoluteAppPath)
const stringifiedAbsoluteErrorPath = stringifyRequest(this, absoluteErrorPath)
const stringified500PagePath = stringifyRequest(this, './pages/500')
const stringifiedAbsoluteDocumentPath = stringifyRequest(
this,
absoluteDocumentPath
)

let appDefinition = `const App = require(${stringifiedAbsoluteAppPath}).default`
let documentDefinition = `const Document = require(${stringifiedAbsoluteDocumentPath}).default`

const transformed = `
import { adapter } from 'next/dist/server/web/adapter'

import { RouterContext } from 'next/dist/shared/lib/router-context'
import { renderToHTML } from 'next/dist/server/web/render'

${appDefinition}
${documentDefinition}

import App from ${stringifiedAbsoluteAppPath}
import Document from ${stringifiedAbsoluteDocumentPath}

let ErrorPage
try {
ErrorPage = require(${stringified500PagePath}).default
} catch (_) {
ErrorPage = require(${stringifiedAbsoluteErrorPath}).default
}

const {
default: Page,
config,
Expand Down Expand Up @@ -66,6 +72,7 @@ export default async function middlewareRSCLoader(this: any) {
}
delete query.__flight__

const req = { url: pathname }
const renderOpts = {
Component,
pageConfig: config || {},
Expand Down Expand Up @@ -97,32 +104,51 @@ export default async function middlewareRSCLoader(this: any) {
const transformStream = new TransformStream()
const writer = transformStream.writable.getWriter()
const encoder = new TextEncoder()

let result
let renderError
let statusCode = 200
try {
const result = await renderToHTML(
{ url: pathname },
result = await renderToHTML(
req,
{},
pathname,
query,
renderOpts
)
result.pipe({
write: str => writer.write(encoder.encode(str)),
end: () => writer.close(),
// Not implemented: cork/uncork/on/removeListener
})
} catch (err) {
return new Response(
(err || 'An error occurred while rendering ' + pathname + '.').toString(),
{
status: 500,
headers: { 'x-middleware-ssr': '1' }
}
)
renderError = err
statusCode = 500
}
if (renderError) {
try {
const errorRes = { statusCode, err: renderError }
result = await renderToHTML(
req,
errorRes,
pathname,
query,
{ ...renderOpts, Component: ErrorPage }
)
} catch (err) {
return new Response(
(err || 'An error occurred while rendering ' + pathname + '.').toString(),
{
status: 500,
headers: { 'x-middleware-ssr': '1' }
}
)
}
}

result.pipe({
write: str => writer.write(encoder.encode(str)),
end: () => writer.close(),
// Not implemented: cork/uncork/on/removeListener
})

return new Response(transformStream.readable, {
headers: { 'x-middleware-ssr': '1' }
headers: { 'x-middleware-ssr': '1' },
status: statusCode
})
}

Expand Down
1 change: 1 addition & 0 deletions packages/next/server/dev/hot-reloader.ts
Expand Up @@ -528,6 +528,7 @@ export default class HotReloader {
page,
absoluteAppPath: this.pagesMapping['/_app'],
absoluteDocumentPath: this.pagesMapping['/_document'],
absoluteErrorPath: this.pagesMapping['/_error'],
absolutePagePath,
isServerComponent,
buildId: this.buildId,
Expand Down
@@ -0,0 +1,5 @@
const page = () => 'page with err'
page.getInitialProps = () => {
throw new Error('oops')
}
export default page
@@ -0,0 +1,7 @@
let did = false
export default function Error() {
if (!did && typeof window === 'undefined') {
did = true
throw new Error('oops')
}
}
@@ -0,0 +1,20 @@
import { Suspense } from 'react'

let did = false
function Error() {
if (!did && typeof window === 'undefined') {
did = true
throw new Error('broken page')
}
}

export default function page() {
return (
<>
<h1>Hey Error</h1>
<Suspense>
<Error />
</Suspense>
</>
)
}
Expand Up @@ -24,6 +24,7 @@ const nativeModuleTestAppDir = join(__dirname, '../unsupported-native-module')
const distDir = join(__dirname, '../app/.next')
const documentPage = new File(join(appDir, 'pages/_document.jsx'))
const appPage = new File(join(appDir, 'pages/_app.js'))
const error500Page = new File(join(appDir, 'pages/500.js'))

const documentWithGip = `
import { Html, Head, Main, NextScript } from 'next/document'
Expand Down Expand Up @@ -55,6 +56,12 @@ function App({ Component, pageProps }) {
export default App
`

const page500 = `
export default function Page500() {
return 'custom-500-page'
}
`

async function nextBuild(dir) {
return await _nextBuild(dir, [], {
stdout: true,
Expand Down Expand Up @@ -100,11 +107,13 @@ describe('concurrentFeatures - prod', () => {
const context = { appDir }

beforeAll(async () => {
error500Page.write(page500)
context.appPort = await findPort()
await nextBuild(context.appDir)
context.server = await nextStart(context.appDir, context.appPort)
})
afterAll(async () => {
error500Page.delete()
await killApp(context.server)
})

Expand Down Expand Up @@ -155,10 +164,12 @@ describe('concurrentFeatures - dev', () => {
const context = { appDir }

beforeAll(async () => {
error500Page.write(page500)
context.appPort = await findPort()
context.server = await nextDev(context.appDir, context.appPort)
})
afterAll(async () => {
error500Page.delete()
await killApp(context.server)
})

Expand Down Expand Up @@ -217,6 +228,7 @@ async function runBasicTests(context) {
)

const path404HTML = await renderViaHTTP(context.appPort, '/404')
const path500HTML = await renderViaHTTP(context.appPort, '/err')
const pathNotFoundHTML = await renderViaHTTP(
context.appPort,
'/this-is-not-found'
Expand All @@ -230,6 +242,7 @@ async function runBasicTests(context) {
expect(dynamicRouteHTML2).toContain('[pid]')

expect(path404HTML).toContain('custom-404-page')
expect(path500HTML).toContain('custom-500-page')
expect(pathNotFoundHTML).toContain('custom-404-page')
})

Expand Down