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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve logging for _devPagesManifest.json loading failures #38046

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
18 changes: 18 additions & 0 deletions errors/failed-to-fetch-devpagesmanifest.md
@@ -0,0 +1,18 @@
# Failed to fetch devPagesManifest

#### Why This Error Occurred

The network request to load `_devPagesManifest.json` did not succeed.

The dev pages manifest file is used by `next/link` to retrieve the list of pages to be (pre-)loaded by Next.js.
If it fails, Next.js cannot properly navigate and link between pages.

#### Possible Ways to Fix It

- Make sure your browser developer tools, extensions, and any other network tools aren't blocking that request.
- If you're running your Next.js application through a proxy, nginx, or other network layer, make sure links like `/_next/*` are configured to be allowed.

### Useful Links

- [Original GitHub Issue Thread](https://github.com/vercel/next.js/issues/16874)
- [GitHub Issue Thread With Reproduction](https://github.com/vercel/next.js/issues/38047)
4 changes: 4 additions & 0 deletions errors/manifest.json
Expand Up @@ -715,6 +715,10 @@
{
"title": "invalid-next-config",
"path": "/errors/invalid-next-config.md"
},
{
"title": "failed-to-fetch-devpagesmanifest",
"path": "/errors/failed-to-fetch-devpagesmanifest.md"
}
]
}
Expand Down
33 changes: 16 additions & 17 deletions packages/next/client/page-loader.ts
Expand Up @@ -58,23 +58,22 @@ export default class PageLoader {
if (window.__DEV_PAGES_MANIFEST) {
return window.__DEV_PAGES_MANIFEST.pages
} else {
if (!this.promisedDevPagesManifest) {
// TODO: Decide what should happen when fetching fails instead of asserting
// @ts-ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Psst - may I suggest using @ts-expect-error instead? I would be very happy to send a PR migrating over! 馃檪

More broadly: I'd also suggest enabling typescript-eslint's prefer-ts-expect-error rule and, even more broadly, much more of typescript-eslint's rules.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely open to migrating this over to ts-expect-error and investigating better TypeScript lint checks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super! For investigating more lint checks, I'd been chatting with @elsigh to try to get a more formal effort set up. Will check back in tomorrow!

this.promisedDevPagesManifest = fetch(
`${this.assetPrefix}/_next/static/development/_devPagesManifest.json`
)
.then((res) => res.json())
.then((manifest: { pages: string[] }) => {
window.__DEV_PAGES_MANIFEST = manifest
return manifest.pages
})
.catch((err) => {
console.log(`Failed to fetch devPagesManifest`, err)
})
}
// TODO Remove this assertion as this could be undefined
return this.promisedDevPagesManifest!
this.promisedDevPagesManifest ||= fetch(
`${this.assetPrefix}/_next/static/development/_devPagesManifest.json`
)
.then((res) => res.json())
.then((manifest: { pages: string[] }) => {
window.__DEV_PAGES_MANIFEST = manifest
return manifest.pages
})
.catch((err) => {
console.log(`Failed to fetch devPagesManifest:`, err)
throw new Error(
`Failed to fetch _devPagesManifest.json. Is something blocking that network request?\n` +
'Read more: https://nextjs.org/docs/messages/failed-to-fetch-devpagesmanifest'
)
})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to throwing on failure would be to make the getPageList function return Promise<string | undefined>. I personally prefer informative return types rather than mysterious error throwing. However, I think this code path is only ever called in dev mode within code that already try/catches.

return this.promisedDevPagesManifest
}
}
}
Expand Down