Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds Sentry integration to the docs app: dependency, Next.js config wrapper, server and edge initializers, client and shared instrumentation hooks, and an App Router global error component that captures exceptions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
🍈 Lychee Link Check Report3660 links: ✅ All links are working!Full Statistics Table
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/docs/next.config.mjs (1)
1-55:⚠️ Potential issue | 🟡 MinorFix the trailing backslash typo on line 27; authToken gating is optional.
Your Sentry configuration is well-structured per
@sentry/nextjs@10.38.0. A couple of clarifications:
- Line 27 typo: Remove the trailing
\from the comment (CI\→CI).webpackoptions placement: Your nestedwebpack: { automaticVercelMonitors, treeshake: ... }is the correct modern location for these options (not mixing buckets). BothautomaticVercelMonitorsandtreeshake.removeDebugLoggingare valid here and won't cause issues.authTokenundefined behavior: Whenprocess.env.SENTRY_AUTH_TOKENis missing in non-CI environments,authTokenis simplyundefined, and the build proceeds without attempting uploads—no failure. Explicit CI gating (as suggested below) is defensive and good practice, but not strictly required.Optional defensive CI gating
- authToken: process.env.SENTRY_AUTH_TOKEN, + authToken: process.env.CI ? process.env.SENTRY_AUTH_TOKEN : undefined,Fix the typo. The authToken suggestion is optional; the rest is correct as-is.
apps/docs/src/instrumentation-client.ts (1)
2-29:⚠️ Potential issue | 🟠 MajorLock down Sentry config: env-based DSN, lower sampling rates, and disable PII by default.
The current config poses three real risks on a public docs site:
Hardcoded DSN prevents separate environment configs. Preview/staging deployments will send errors to the same production project, polluting signal and making it harder to troubleshoot issues in isolation. Use
process.env.NEXT_PUBLIC_SENTRY_DSNinstead, so environments can be configured independently.
sendDefaultPii: truecaptures IP addresses, headers, and user-like context by default. This is a privacy concern for a public site—users haven't consented to PII collection by Sentry. The Sentry default isfalsefor good reason. Set it tofalseunless you have explicit consent and documented need.
tracesSampleRate: 1means 100% of transactions (all page navigation, API calls, etc.) are sent to Sentry. On a docs site with meaningful traffic, this will quickly exceed your quota (5M spans/month on standard plans) and trigger pay-as-you-go overage charges. Production should use a much lower rate (~0.1 = 10% is a common starting point), configurable via env var.As for
enableLogs: true—it's a valid option that pipes console logs into Sentry. There's no inherent problem, but gating it to dev-only (process.env.NODE_ENV !== "production") reduces noise in production events.Apply this pattern:
Suggested fix
Sentry.init({ - dsn: "https://e83ce4699e59051fdeaa330bf4a0dfb9@o4510879743737856.ingest.us.sentry.io/4510879744000000", + dsn: process.env.NEXT_PUBLIC_SENTRY_DSN, - tracesSampleRate: 1, + tracesSampleRate: + process.env.NODE_ENV === "production" + ? Number(process.env.NEXT_PUBLIC_SENTRY_TRACES_SAMPLE_RATE ?? 0.1) + : 1, - enableLogs: true, + enableLogs: process.env.NODE_ENV !== "production", - sendDefaultPii: true, + sendDefaultPii: false, });If
NEXT_PUBLIC_SENTRY_DSNcan be absent in dev/preview, guard the init:if (process.env.NEXT_PUBLIC_SENTRY_DSN) { Sentry.init({ ... }); }
🤖 Fix all issues with AI agents
In `@apps/docs/sentry.edge.config.ts`:
- Around line 1-20: Replace the hardcoded Sentry configuration in Sentry.init:
read dsn from an environment variable (e.g., process.env.SENTRY_DSN) instead of
the literal string, drive tracesSampleRate from an env var with a safe
production default (e.g., parseFloat(process.env.SENTRY_TRACES_SAMPLE_RATE) ||
0.1), set enableLogs based on NODE_ENV (true only in development), and set
sendDefaultPii from an env var defaulting to false (e.g.,
!!JSON.parse(process.env.SENTRY_SEND_DEFAULT_PII || "false")); apply the same
env-driven changes to sentry.server.config.ts so both edge and server configs
use the same safe defaults.
In `@apps/docs/src/app/global-error.tsx`:
- Around line 1-27: Replace the Pages Router-only NextError usage in GlobalError
with a minimal custom error UI and recovery button: inside the GlobalError
component (function GlobalError) remove NextError and render a simple message
and a "Try again" button that calls the reset() function (destructure reset from
the component props) to trigger client-side recovery; when reporting to Sentry
use Sentry.captureException(error) together with Sentry.captureContext (or
Sentry.setContext) to tag the event with error.digest for correlation (ensure
you pass error.digest in the context before captureException) so production logs
can be tied to the UI error digest.
🧹 Nitpick comments (1)
apps/docs/src/instrumentation.ts (1)
1-13: Useelse ifto clarify mutually exclusive runtime branches.Since
NEXT_RUNTIMEis mutually exclusive, switching toelse ifmakes the intent clearer and avoids unnecessary condition checks:Runtime branch tightening
export async function register() { if (process.env.NEXT_RUNTIME === "nodejs") { await import("../sentry.server.config"); - } - - if (process.env.NEXT_RUNTIME === "edge") { + } else if (process.env.NEXT_RUNTIME === "edge") { await import("../sentry.edge.config"); } }The direct assignment of
Sentry.captureRequestErrortoonRequestErroris correct and intentional for Next.js 16.1.1 with@sentry/nextjs10.38.0—Sentry explicitly documents this pattern as the recommended integration approach for error capture in instrumentation files.
Summary by CodeRabbit