Repository navigation
feat(nextjs): Warn when Next.js production builds miss the instrumentation file - #25603
Conversation
size-limit report 📦Compared: 9ac677a (PR test merge using develop at 9927b7c) Against: 9927b7c (develop) · base run #63346 ✅ Baseline matches: this diff contains only this PR's changes.
|
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 26e519d. Configure here.
| getBuildLogger(userSentryOptions.silent).warn( | ||
| `[@sentry/nextjs] Found ${misplacedFile}, but Next.js production builds only pick up the instrumentation file next to your ${routerDir} directory. Move it to ${path.join(loadedDir, path.basename(misplacedFile))}, or the Sentry SDK may not be initialized on the server in production.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Feat lacks integration or E2E test
Low Severity
This feat is covered only by unit tests that call warnIfMisplacedInstrumentationFile against a temp fixture. This is flagged because the review rules ask feat PRs to include at least one integration or E2E test, for example asserting that next build actually emits the warning for a misplaced instrumentation.ts.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 26e519d. Configure here.
nicohrubec
left a comment
nicohrubec
left a comment
There was a problem hiding this comment.
there also seems to be a build time log for webpack that checks for missing instrumentation files. we could probably remove this check then and only keep this one so we don't get double warnings?
| return; | ||
| } | ||
|
|
||
| const loadedDir = path.dirname(routerDir); |
There was a problem hiding this comment.
do we need to show the user in what wrong location the instrumentation file is? as in could we not just simplify this logic to:
- get the path where we expect the instrumentation file to live
- check whether its there
- if not log something like "Could not find a server instrumentation file next to
{router_dir}. Create{router_dir}/instrumentation.{extension}, or move your existing instrumentation file there."
I'd assume a user will know where he placed this file or can easily find out and less logic that might be buggy
logaretm
left a comment
logaretm
left a comment
There was a problem hiding this comment.
Seems fine to me, although it feels like something Vercel should do.
s1gr1d
changed the title
With
src/appand a rootinstrumentation.ts(orapp/andsrc/instrumentation.ts),next builddoes not pick up the file. Dev andnext startwith Turbopack still load it, so local testing passes. But on Vercel and in standalone output, the server SDK never initializes.pages/app. It uses the same lookup as Next.js (next build, withpageExtensions).SENTRY_SUPPRESS_INSTRUMENTATION_FILE_WARNINGstill turns it off.This is only a warning because Next.js decides which file it deploys. We can only warn users.
Upstream: vercel/next.js#68740 (other users hit the same cause)
Fixes #17024
Docs: getsentry/sentry-docs#20125