-
Notifications
You must be signed in to change notification settings - Fork 27.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
When we prerender in `dynamicIO` the async APIs that provide access to Request data return promises that never resolve. This is how we stall the prerender in that particular slot while letting the rest of the App continue. This is genernally fine however these promises can leak to other contexts via setTimeout and after. After is particularly concerning because it won't stop until the callbacks have finished and if the callback is waiting on a promise that never resolves we end up with a deadlock. To resolves this we now instrument these hanging promises with an AbortSignal. When this signal aborts the promise rejects. This works as long as the abort signal is always aborted at or after when the prerender is aborted. In this implementation's case we actually use the same abortSignal for both which provides the necessary guarantee. I believe this is fine even when we synchronously abort a render such as when you do something sync dynamic like call `Math.random()`. If it turns out that we don't want to reject immediately in these cases we can always switch to a second AbortController that is only ever aborted after the prerender is completed.
- Loading branch information
Showing
14 changed files
with
285 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,26 @@ | ||
function hangForever() {} | ||
|
||
/** | ||
* This function constructs a promise that will never resolve. This is primarily | ||
* useful for dynamicIO where we use promise resolution timing to determine which | ||
* parts of a render can be included in a prerender. | ||
* | ||
* @internal | ||
*/ | ||
export function makeHangingPromise<T>(): Promise<T> { | ||
return new Promise(hangForever) | ||
export function makeHangingPromise<T>( | ||
signal: AbortSignal, | ||
expression: string | ||
): Promise<T> { | ||
const hangingPromise = new Promise<T>((_, reject) => { | ||
signal.addEventListener('abort', () => { | ||
reject( | ||
new Error( | ||
`During prerendering, ${expression} rejects when the prerender is complete. Typically these errors are handled by React but if you move ${expression} to a different context by using \`setTimeout\`, \`after\`, or similar functions you may observe this error and you should handle it in that context.` | ||
) | ||
) | ||
}) | ||
}) | ||
// We are fine if no one actually awaits this promise. We shouldn't consider this an unhandled rejection so | ||
// we attach a noop catch handler here to suppress this warning. If you actually await somewhere or construct | ||
// your own promise out of it you'll need to ensure you handle the error when it rejects. | ||
hangingPromise.catch(() => {}) | ||
return hangingPromise | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
test/e2e/app-dir/dynamic-io-request-apis/dynamic-io-request-apis.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
import { nextTestSetup } from 'e2e-utils' | ||
|
||
const WITH_PPR = !!process.env.__NEXT_EXPERIMENTAL_PPR | ||
|
||
const stackStart = /\s+at / | ||
|
||
function createExpectError(cliOutput: string) { | ||
let cliIndex = 0 | ||
return function expectError( | ||
containing: string, | ||
withStackContaining?: string | ||
) { | ||
const initialCliIndex = cliIndex | ||
let lines = cliOutput.slice(cliIndex).split('\n') | ||
|
||
let i = 0 | ||
while (i < lines.length) { | ||
let line = lines[i++] + '\n' | ||
cliIndex += line.length | ||
if (line.includes(containing)) { | ||
if (typeof withStackContaining !== 'string') { | ||
return | ||
} else { | ||
while (i < lines.length) { | ||
let stackLine = lines[i++] + '\n' | ||
if (!stackStart.test(stackLine)) { | ||
expect(stackLine).toContain(withStackContaining) | ||
} | ||
if (stackLine.includes(withStackContaining)) { | ||
return | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
expect(cliOutput.slice(initialCliIndex)).toContain(containing) | ||
} | ||
} | ||
|
||
describe(`Request Promises`, () => { | ||
describe('On Prerender Completion', () => { | ||
const { next, isNextDev, skipped } = nextTestSetup({ | ||
files: __dirname + '/fixtures/reject-hanging-promises-static', | ||
skipStart: true, | ||
}) | ||
|
||
if (skipped) { | ||
return | ||
} | ||
|
||
if (isNextDev) { | ||
it('does not run in dev', () => {}) | ||
return | ||
} | ||
|
||
it('should reject request APIs after the prerender is complete when it finishes naturally', async () => { | ||
try { | ||
await next.start() | ||
} catch { | ||
throw new Error('expected build not to fail for fully static project') | ||
} | ||
const expectError = createExpectError(next.cliOutput) | ||
|
||
if (WITH_PPR) { | ||
expectError( | ||
'Error: During prerendering, `params` rejects when the prerender is complete' | ||
) | ||
} | ||
expectError( | ||
'Error: During prerendering, `searchParams` rejects when the prerender is complete' | ||
) | ||
expectError( | ||
'Error: During prerendering, `cookies()` rejects when the prerender is complete' | ||
) | ||
expectError( | ||
'Error: During prerendering, `headers()` rejects when the prerender is complete' | ||
) | ||
expectError( | ||
'Error: During prerendering, `connection()` rejects when the prerender is complete' | ||
) | ||
}) | ||
}) | ||
describe('On Prerender Interruption', () => { | ||
const { next, isNextDev, skipped } = nextTestSetup({ | ||
files: __dirname + '/fixtures/reject-hanging-promises-dynamic', | ||
skipStart: true, | ||
}) | ||
|
||
if (skipped) { | ||
return | ||
} | ||
|
||
if (isNextDev) { | ||
it('does not run in dev', () => {}) | ||
return | ||
} | ||
|
||
it('should reject request APIs after the prerender is interrupted with synchronously dynamic APIs', async () => { | ||
try { | ||
await next.start() | ||
} catch { | ||
throw new Error('expected build not to fail for fully static project') | ||
} | ||
const expectError = createExpectError(next.cliOutput) | ||
|
||
if (WITH_PPR) { | ||
expectError( | ||
'Error: During prerendering, `params` rejects when the prerender is complete' | ||
) | ||
} | ||
expectError( | ||
'Error: During prerendering, `searchParams` rejects when the prerender is complete' | ||
) | ||
expectError( | ||
'Error: During prerendering, `cookies()` rejects when the prerender is complete' | ||
) | ||
expectError( | ||
'Error: During prerendering, `headers()` rejects when the prerender is complete' | ||
) | ||
expectError( | ||
'Error: During prerendering, `connection()` rejects when the prerender is complete' | ||
) | ||
}) | ||
}) | ||
}) |
41 changes: 41 additions & 0 deletions
41
...-dir/dynamic-io-request-apis/fixtures/reject-hanging-promises-dynamic/app/[slug]/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import { cookies, headers, draftMode } from 'next/headers' | ||
import { connection } from 'next/server' | ||
|
||
export function generateStaticParams() { | ||
return [{ slug: 'one' }] | ||
} | ||
|
||
export default async function Page(props: { | ||
params: Promise<{}> | ||
searchParams: Promise<{}> | ||
}) { | ||
setTimeout(async () => await props.params) | ||
setTimeout(async () => await props.searchParams) | ||
let pendingCookies = cookies() | ||
setTimeout(async () => await pendingCookies) | ||
let pendingHeaders = headers() | ||
setTimeout(async () => await pendingHeaders) | ||
let pendingDraftMode = draftMode() | ||
setTimeout(async () => await pendingDraftMode) | ||
let pendingConnection = connection() | ||
setTimeout(async () => await pendingConnection) | ||
return ( | ||
<> | ||
<p> | ||
This page renders statically but it passes all of the Request Data | ||
promises (cookies(), etc...) to a setTimeout scope. This test asserts | ||
that these promises eventually reject even when the route is | ||
synchronously dynamic (which this one is by rendering a Math.random() | ||
value) | ||
</p> | ||
<p> | ||
<TriggerSyncDynamic /> | ||
</p> | ||
</> | ||
) | ||
} | ||
|
||
async function TriggerSyncDynamic() { | ||
await new Promise((r) => process.nextTick(r)) | ||
return Math.random() | ||
} |
13 changes: 13 additions & 0 deletions
13
...e/app-dir/dynamic-io-request-apis/fixtures/reject-hanging-promises-dynamic/app/layout.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { Suspense } from 'react' | ||
|
||
export default function Root({ children }: { children: React.ReactNode }) { | ||
return ( | ||
<html> | ||
<body> | ||
<main> | ||
<Suspense fallback="loading...">{children}</Suspense> | ||
</main> | ||
</body> | ||
</html> | ||
) | ||
} |
13 changes: 13 additions & 0 deletions
13
...e/app-dir/dynamic-io-request-apis/fixtures/reject-hanging-promises-dynamic/next.config.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/** | ||
* @type {import('next').NextConfig} | ||
*/ | ||
const nextConfig = { | ||
experimental: { | ||
ppr: process.env.__NEXT_EXPERIMENTAL_PPR === 'true', | ||
pprFallbacks: process.env.__NEXT_EXPERIMENTAL_PPR === 'true', | ||
dynamicIO: true, | ||
serverMinification: true, | ||
}, | ||
} | ||
|
||
module.exports = nextConfig |
32 changes: 32 additions & 0 deletions
32
...p-dir/dynamic-io-request-apis/fixtures/reject-hanging-promises-static/app/[slug]/page.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
import { cookies, headers, draftMode } from 'next/headers' | ||
import { connection } from 'next/server' | ||
|
||
export function generateStaticParams() { | ||
return [{ slug: 'one' }] | ||
} | ||
|
||
export default async function Page(props: { | ||
params: Promise<{}> | ||
searchParams: Promise<{}> | ||
}) { | ||
setTimeout(async () => await props.params) | ||
setTimeout(async () => await props.searchParams) | ||
let pendingCookies = cookies() | ||
setTimeout(async () => await pendingCookies) | ||
let pendingHeaders = headers() | ||
setTimeout(async () => await pendingHeaders) | ||
let pendingDraftMode = draftMode() | ||
setTimeout(async () => await pendingDraftMode) | ||
let pendingConnection = connection() | ||
setTimeout(async () => await pendingConnection) | ||
return ( | ||
<> | ||
<p> | ||
This page renders statically but it passes all of the Request Data | ||
promises (cookies(), etc...) to a setTimeout scope. This test asserts | ||
that these promises eventually reject even when the route is entirely | ||
static (which this one is) | ||
</p> | ||
</> | ||
) | ||
} |
9 changes: 9 additions & 0 deletions
9
...2e/app-dir/dynamic-io-request-apis/fixtures/reject-hanging-promises-static/app/layout.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Root({ children }: { children: React.ReactNode }) { | ||
return ( | ||
<html> | ||
<body> | ||
<main>{children}</main> | ||
</body> | ||
</html> | ||
) | ||
} |
13 changes: 13 additions & 0 deletions
13
...2e/app-dir/dynamic-io-request-apis/fixtures/reject-hanging-promises-static/next.config.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/** | ||
* @type {import('next').NextConfig} | ||
*/ | ||
const nextConfig = { | ||
experimental: { | ||
ppr: process.env.__NEXT_EXPERIMENTAL_PPR === 'true', | ||
pprFallbacks: process.env.__NEXT_EXPERIMENTAL_PPR === 'true', | ||
dynamicIO: true, | ||
serverMinification: true, | ||
}, | ||
} | ||
|
||
module.exports = nextConfig |