mirror of
https://github.com/hanzoai/next.js.git
synced 2026-09-24 17:00:51 +00:00
### What?
When `node_modules/next` is briefly unresolvable mid-session (for
example because pnpm is mid-install of a package with a `next` peer
dependency, reshuffling its symlinks) Turbopack would:
- Emit a `MissingNextFolderIssue` with severity `Fatal`
- Propagate `anyhow!("Next.js package not found")` through ~10 levels of
the build graph
- Surface the failure at the napi boundary as a
`TurbopackInternalError`, which the Next.js JS-side treats as fatal and
shuts the dev server down
The reported user impact was a dev session that became wedged and could
not recover even after restarting `next dev`, because the persistent
cache had stored the failed operation state. Users reported it as
"stuck", "catastrophic", and required wiping `.next/` to recover.
Unfortunately while i was able to reproduce a number of bad outcomes, i
couldn't actually reproduce the issue of `next dev` getting stuck in a
bad state. Still this PR will improve a number of apis and error
messages
See internal discussion:
https://vercel.slack.com/archives/C046HAU4H7F/p1778792876550309
### Why?
A transient filesystem race during an install is the canonical
recoverable error — once the install finishes, `next` is resolvable
again. There is no reason this should bring down the dev server or
poison the persistent cache.
The bug had two contributing causes:
1. The error message was poor. It assumed the only cause was a mis-set
`turbopack.root`, with the entire message in `title()`. It didn't
acknowledge the install-race case or any of the other realistic causes
(broken symlink, monorepo hoisting, removed `node_modules/next`, global
install). Users had no way to know that the right response was to wait
or refresh.
2. The Rust→napi boundary did not treat this as a recoverable error. Two
specific code paths propagated `Err` instead of catching it:
- `issue_filter_from_endpoint` in `endpoint.rs` calls
`endpoint_op.connect().await?` — if the upstream endpoint resolution
errors (as it does when `directory_tree_to_loader_tree` fails), this `?`
propagates **before** the surrounding
`strongly_consistent_catch_collectables` gets a chance to convert the
failure into Issues.
- `Project::hmr_version_state` is called bare from `project_hmr_events`
at the napi boundary. There is no `_with_issues_operation` wrapper, so
any error from the underlying
`hmr_content`/`versioned_content_map.compute_entry` chain propagates
straight to JS.
### How?
Three changes:
**1. Rewrite `MissingNextFolderIssue`**
([next_import_map.rs](crates/next-core/src/next_import_map.rs))
- Severity downgraded from `Fatal` to `Error`. `Fatal` triggers an
explicit JS-side server shutdown in `print-build-errors.ts`; `Error`
lets the dev server recover.
- Title is a single line: `"Could not find the Next.js package
(next/package.json)"`.
- Description lists all realistic causes (concurrent install,
missing/broken symlink, mis-set workspace root, monorepo hoisting,
global install).
- Doc link moved to `documentation_link()` so it doesn't pollute the
inline message.
**2. Make `issue_filter_from_endpoint` Err-tolerant**
([endpoint.rs](crates/next-napi-bindings/src/next_api/endpoint.rs))
- Wrap the `endpoint_op.connect().await` in a `match` and fall back to
`IssueFilter::warnings_and_foreign_errors()` on `Err`.
- This unblocks the existing `strongly_consistent_catch_collectables`
recovery path, which then absorbs the upstream failure as Issues that
surface through the napi boundary normally.
**3. Add `hmr_version_state_with_issues_operation`**
([project.rs](crates/next-napi-bindings/src/next_api/project.rs))
- Mirrors the existing `hmr_update_with_issues_operation` pattern.
- Drives `hmr_version_operation` as an `OperationVc`, catches any `Err`
and falls back to `NotFoundVersion`, and collects issues from the
operation via `peek_issues`.
- The napi `project_hmr_events` handler now uses this wrapper instead of
calling `Project::hmr_version_state` bare. Issues from both
`hmr_version_state` and `hmr_update` are merged before being sent to JS.
- Required lifting the inner `hmr_version_operation` out of
`Project::hmr_version_state`'s closure to make it callable from the napi
layer.
A new regression test at
[test/development/app-dir/concurrent-install/](test/development/app-dir/concurrent-install/)
reproduces the failure by moving `node_modules/next` aside
mid-HMR-session and asserts that the dev server does not emit `FATAL: An
unexpected Turbopack error occurred` or `TurbopackInternalError`, and
that the friendly Issue text surfaces. The test is skipped under
`NEXT_SKIP_ISOLATE=1` since that mode doesn't produce a manipulable
`node_modules`.
Out of scope: after recovery, the dev server still returns 500 on
subsequent requests until the user manually refreshes (the JS-side
per-route error state isn't invalidated by Turbopack's successful
recompile). That's a separate dev-server caching issue.
<!-- NEXT_JS_LLM_PR -->
181 lines
5.6 KiB
TypeScript
181 lines
5.6 KiB
TypeScript
import { nextTestSetup } from 'e2e-utils'
|
|
import { retry } from 'next-test-utils'
|
|
import fs from 'fs/promises'
|
|
import path from 'path'
|
|
|
|
/**
|
|
* Simulates the failure mode where a concurrent package manager install
|
|
* reorganizes `node_modules` while the Next.js dev server is running.
|
|
*
|
|
* Specifically, when `next` (re)moves under `node_modules` mid-HMR, Turbopack
|
|
* fails to resolve `next/package.json` and emits a `MissingNextFolderIssue`.
|
|
* The dev server must:
|
|
* - surface the issue (recoverable, not Fatal)
|
|
* - NOT crash with a `TurbopackInternalError` / "FATAL" log
|
|
* - recover once `node_modules/next` is restored
|
|
*/
|
|
const describeMaybe = process.env.NEXT_SKIP_ISOLATE ? describe.skip : describe
|
|
|
|
describeMaybe('concurrent-install', () => {
|
|
const { next, isTurbopack } = nextTestSetup({
|
|
files: __dirname,
|
|
})
|
|
|
|
const itTurbopack = isTurbopack ? it : it.skip
|
|
|
|
async function getNextPath(): Promise<string> {
|
|
const nextPath = path.join(next.testDir, 'node_modules', 'next')
|
|
// sanity check
|
|
await fs.lstat(nextPath)
|
|
return nextPath
|
|
}
|
|
|
|
async function moveNextAside(): Promise<{ original: string; stash: string }> {
|
|
const original = await getNextPath()
|
|
const stash = `${original}.stash-${Date.now()}`
|
|
await fs.rename(original, stash)
|
|
return { original, stash }
|
|
}
|
|
|
|
async function restoreNext({
|
|
original,
|
|
stash,
|
|
}: {
|
|
original: string
|
|
stash: string
|
|
}): Promise<void> {
|
|
await fs.rename(stash, original)
|
|
}
|
|
|
|
itTurbopack(
|
|
'does not crash when node_modules/next is moved mid-session',
|
|
async () => {
|
|
await next.browser('/')
|
|
|
|
const getOutput = next.getCliOutputFromHere()
|
|
const stashInfo = await moveNextAside()
|
|
try {
|
|
// Force a recompile while next is missing. Not strickly necessary, but important to ensure
|
|
// we do recover with the new content eventually
|
|
await next.patchFile(
|
|
'app/page.tsx',
|
|
`export default function Page() {
|
|
return <p>hello world (edited)</p>
|
|
}
|
|
`
|
|
)
|
|
|
|
// Give the dev server time to react. We're not asserting on a specific
|
|
// user-visible behavior here — we just want the failure path to fire.
|
|
await retry(
|
|
async () => {
|
|
// The friendly Issue should be surfaced.
|
|
expect(getOutput()).toContain('Could not find the Next.js package')
|
|
},
|
|
5000,
|
|
500
|
|
)
|
|
} finally {
|
|
await restoreNext(stashInfo)
|
|
}
|
|
|
|
// The dev server must not have died from a TurbopackInternalError.
|
|
// (Whether the page itself recovers without a manual reload is a separate
|
|
// dev-server caching concern; the catastrophic failure mode is the crash.)
|
|
expect(getOutput()).not.toContain(
|
|
'FATAL: An unexpected Turbopack error occurred'
|
|
)
|
|
expect(getOutput()).not.toContain('TurbopackInternalError')
|
|
}
|
|
)
|
|
|
|
itTurbopack(
|
|
'surfaces a friendly issue when node_modules/next is missing',
|
|
async () => {
|
|
await next.browser('/')
|
|
|
|
const getOutput = next.getCliOutputFromHere()
|
|
const stashInfo = await moveNextAside()
|
|
try {
|
|
await next.patchFile(
|
|
'app/page.tsx',
|
|
`export default function Page() {
|
|
return <p>hello world (while-missing)</p>
|
|
}
|
|
`
|
|
)
|
|
|
|
// Wait for the Issue to be rendered to stdout.
|
|
await retry(
|
|
async () => {
|
|
expect(getOutput()).toContain('Could not find the Next.js package')
|
|
},
|
|
10000,
|
|
500
|
|
)
|
|
|
|
// The full issue text. Normalize path-like values that vary per run
|
|
// (the test-dir is a random tmpdir).
|
|
expect(getOutput()).toContain(
|
|
'Could not find the Next.js package (next/package.json)'
|
|
)
|
|
|
|
expect(getOutput()).not.toContain(
|
|
'FATAL: An unexpected Turbopack error occurred'
|
|
)
|
|
expect(getOutput()).not.toContain('TurbopackInternalError')
|
|
} finally {
|
|
await restoreNext(stashInfo)
|
|
}
|
|
}
|
|
)
|
|
|
|
itTurbopack(
|
|
'does not crash when navigating to an uncompiled route while node_modules/next is missing',
|
|
async () => {
|
|
// Compile `/` so the harness has at least one warm chunk.
|
|
await next.browser('/')
|
|
|
|
const getOutput = next.getCliOutputFromHere()
|
|
const stashInfo = await moveNextAside()
|
|
try {
|
|
// Navigating to `/late-route` (never compiled in this session) forces
|
|
// a fresh `hmr_version_state` evaluation for that chunk. That path
|
|
// hits `get_next_package`, which Errs when `node_modules/next` is
|
|
// missing. The dev server must surface the failure as an Issue and
|
|
// keep the HMR subscription alive — not crash.
|
|
await next.fetch('/late-route').catch(() => {
|
|
// The request itself may fail (500) while next is missing. That's
|
|
// expected. We only care that the dev server stays alive.
|
|
})
|
|
|
|
await retry(
|
|
async () => {
|
|
expect(getOutput()).toContain('Could not find the Next.js package')
|
|
},
|
|
10000,
|
|
500
|
|
)
|
|
|
|
expect(getOutput()).not.toContain(
|
|
'FATAL: An unexpected Turbopack error occurred'
|
|
)
|
|
expect(getOutput()).not.toContain('TurbopackInternalError')
|
|
} finally {
|
|
await restoreNext(stashInfo)
|
|
}
|
|
|
|
// Once next is restored, `/late-route` should eventually render.
|
|
await retry(
|
|
async () => {
|
|
const res = await next.fetch('/late-route')
|
|
expect(res.status).toBe(200)
|
|
expect(await res.text()).toContain('late route')
|
|
},
|
|
15000,
|
|
500
|
|
)
|
|
}
|
|
)
|
|
})
|