feat: add Sentry error tracking to all Next.js apps#436
Conversation
WalkthroughIntegrates Sentry across the monorepo: adds environment variables and CI bindings, installs Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (19)
🚧 Files skipped from review as they are similar to previous changes (10)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-18T23:19:10.405ZApplied to files:
📚 Learning: 2025-12-18T23:19:10.405ZApplied to files:
🧬 Code graph analysis (5)apps/docs/sentry.edge.config.ts (1)
apps/marketing/sentry.edge.config.ts (1)
apps/web/sentry.edge.config.ts (1)
apps/marketing/sentry.server.config.ts (1)
apps/docs/sentry.server.config.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
- Add @sentry/nextjs to web, admin, api, docs, marketing apps - Configure server, edge, and client-side error tracking - Add global-error.tsx handlers for each app - Set up /monitoring tunnel route for ad-blocker bypass - Add Sentry env vars to CI/CD workflows - Conditionally enable Sentry based on DSN availability - Allow /monitoring and /ingest routes in proxy middleware 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
saddlepaddle
force-pushed
the
small-badger-2d27f7
branch
from
ff5ba79 to
9244bb3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (5)
apps/web/package.json (1)
15-15: Verify package versions (same as admin app).The same Sentry-related dependencies have been added here. Please verify versions and security status as noted in the
apps/admin/package.jsonreview.Also applies to: 28-28, 37-37
apps/docs/sentry.edge.config.ts (1)
1-12: Same privacy and duplication concerns as admin app.This file exhibits the same issues noted in
apps/admin/sentry.edge.config.ts:
sendDefaultPii: trueposes GDPR/CCPA compliance risks- Code is duplicated across multiple apps and should be extracted to a shared package
Please refer to the review comments on
apps/admin/sentry.edge.config.tsfor detailed recommendations.apps/web/src/instrumentation.ts (1)
1-13: Code duplication flagged in marketing app.This file is identical to
apps/marketing/src/instrumentation.ts. See the review comment on that file for the suggested refactoring to extract this logic to a shared package.apps/api/src/instrumentation.ts (1)
1-13: Code duplication flagged in marketing app.This file is identical to the instrumentation modules in other apps. See the review comment on
apps/marketing/src/instrumentation.tsfor the suggested refactoring.apps/docs/src/instrumentation.ts (1)
1-13: Code duplication flagged in marketing app.This file is identical to the instrumentation modules in other apps. See the review comment on
apps/marketing/src/instrumentation.tsfor the suggested refactoring.
🧹 Nitpick comments (20)
.env.example (1)
64-74: Sentry environment variables properly documented.The Sentry configuration section is well-organized with appropriate default values and clear naming conventions for multi-app DSNs.
Note: The dotenv-linter flags suggest alphabetical ordering, but the current semantic grouping (auth token first, then environment, then DSNs) is more intuitive for developers. This is an optional stylistic preference.
apps/admin/sentry.edge.config.ts (1)
1-12: Consider extracting shared Sentry configuration.The Sentry edge configuration is duplicated across multiple apps (admin, docs, marketing, api, web) with only the DSN variable name changing. This could be extracted to a shared package to reduce duplication and ensure consistency.
💡 Suggested approach
Create
packages/sentry-config/edge.ts:import * as Sentry from "@sentry/nextjs"; export function initSentryEdge(config: { dsn: string | undefined; environment: string; }) { Sentry.init({ dsn: config.dsn, environment: config.environment, enabled: !!config.dsn, tracesSampleRate: config.environment === "production" ? 0.1 : 1.0, sendDefaultPii: false, debug: false, }); }Then in each app:
import { initSentryEdge } from "@superset/sentry-config/edge"; import { env } from "@/env"; initSentryEdge({ dsn: env.NEXT_PUBLIC_SENTRY_DSN_ADMIN, environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, });apps/docs/src/app/global-error.tsx (1)
7-23: Global error handler correctly implemented.The component properly captures exceptions with Sentry and renders a fallback error page. The
useEffectdependency array correctly includeserrorto re-capture if the error changes.Note: This component is duplicated identically across all five apps (admin, api, docs, marketing, web). Consider extracting it to
@superset/uior a shared error handling package to reduce duplication.apps/docs/src/env.ts (1)
20-21: Make NEXT_PUBLIC_SENTRY_ENVIRONMENT optional across all apps for consistency.This pattern appears in
admin,api, anddocsapps:NEXT_PUBLIC_SENTRY_DSN_*is marked optional whileNEXT_PUBLIC_SENTRY_ENVIRONMENTis required. This creates an inconsistency where the environment variable must be set even when Sentry is disabled (no DSN provided). When the SENTRY_ENVIRONMENT environment variable isn't set, Sentry defaults to "production", so making it optional aligns better with this behavior.Apply this change consistently across all affected apps:
client: { NEXT_PUBLIC_POSTHOG_KEY: z.string(), NEXT_PUBLIC_POSTHOG_HOST: z.string().url(), NEXT_PUBLIC_SENTRY_DSN_*: z.string().optional(), - NEXT_PUBLIC_SENTRY_ENVIRONMENT: z.enum(["development", "preview", "production"]), + NEXT_PUBLIC_SENTRY_ENVIRONMENT: z.enum(["development", "preview", "production"]).optional(), },apps/web/sentry.server.config.ts (1)
5-12: Consider extracting shared Sentry configuration.This initialization pattern is duplicated across multiple apps (admin, api, docs, marketing, web) with only the DSN variable name changing. Consider extracting to a shared utility function in
@superset/sharedto reduce duplication and ensure consistent configuration.Example shared utility
// packages/shared/src/sentry/init-server.ts export function initSentryServer(dsn: string | undefined, environment: string) { Sentry.init({ dsn, environment, enabled: !!dsn, tracesSampleRate: environment === "production" ? 0.1 : 1.0, sendDefaultPii: true, debug: false, }); }Then in each app:
import { initSentryServer } from "@superset/shared/sentry/init-server"; import { env } from "@/env"; initSentryServer(env.NEXT_PUBLIC_SENTRY_DSN_WEB, env.NEXT_PUBLIC_SENTRY_ENVIRONMENT);apps/web/sentry.edge.config.ts (1)
5-12: Edge and server configs are identical—consider consolidation.The edge config is identical to the server config. In Next.js, you can often use a single configuration file that works in both runtimes, reducing maintenance overhead.
apps/api/src/app/global-error.tsx (3)
19-19: Consider using a standard error status code.
statusCode={0}is unconventional. For global errors,statusCode={500}is more appropriate and provides clearer semantics for internal server errors.Suggested change
- <NextError statusCode={0} /> + <NextError statusCode={500} />
7-23: Extract shared GlobalError component to reduce duplication.This component is identical across all apps (admin, docs, marketing, web, api). Consider extracting it to
@superset/uior@superset/sharedto maintain consistency and reduce duplication.
17-21: Add suppressHydrationWarning to html element.Global error boundaries render outside the normal React tree and may cause hydration warnings. Add
suppressHydrationWarningto the<html>element to prevent these warnings.Suggested fix
- <html lang="en"> + <html lang="en" suppressHydrationWarning> <body> <NextError statusCode={0} /> </body>apps/docs/sentry.server.config.ts (1)
5-12: Consider extracting shared Sentry configuration.This exact initialization pattern (with only the DSN variable name differing) is duplicated across all five apps. Consider extracting this into a shared helper function to reduce duplication and ensure consistency.
🔎 Example shared configuration helper
Create a shared package function:
// packages/shared/src/sentry.ts export function createSentryConfig(dsn: string | undefined, environment: string) { return { dsn, environment, enabled: !!dsn, tracesSampleRate: environment === "production" ? 0.1 : 1.0, sendDefaultPii: true, debug: false, }; }Then use it in each app:
-Sentry.init({ - dsn: env.NEXT_PUBLIC_SENTRY_DSN_DOCS, - environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, - enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_DOCS, - tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, - sendDefaultPii: true, - debug: false, -}); +Sentry.init(createSentryConfig( + env.NEXT_PUBLIC_SENTRY_DSN_DOCS, + env.NEXT_PUBLIC_SENTRY_ENVIRONMENT +));apps/admin/src/instrumentation.ts (1)
13-13: Consider extracting shared instrumentation logic.This exact file (13 lines) is duplicated across all five apps (admin, api, docs, marketing, web). Since the logic is identical, consider extracting it to a shared package to maintain consistency and reduce duplication.
apps/marketing/sentry.edge.config.ts (1)
5-12: Consider extracting shared Sentry configuration.This configuration is duplicated across multiple edge config files with only the DSN variable differing. See the suggestion in
apps/docs/sentry.server.config.tsfor a shared helper approach.apps/admin/sentry.server.config.ts (1)
5-12: Consider extracting shared Sentry configuration.This configuration is duplicated across multiple server config files. See the suggestion in
apps/docs/sentry.server.config.tsfor a shared helper approach.apps/docs/src/instrumentation-client.ts (1)
26-33: Consider extracting shared Sentry client configuration.This client-side initialization pattern is duplicated across multiple instrumentation-client files. Consider using a shared helper function as suggested in
apps/docs/sentry.server.config.ts.apps/marketing/src/app/global-error.tsx (1)
7-23: Consider extracting shared global error component.This exact component (23 lines) is duplicated identically across all five apps. Consider extracting it to a shared package or at least documenting why it needs to be duplicated per-app.
apps/admin/src/instrumentation-client.ts (1)
26-33: Consider extracting shared Sentry client configuration.This client-side initialization pattern is duplicated across multiple instrumentation-client files. Consider using a shared helper function to reduce duplication and ensure consistency.
apps/marketing/src/instrumentation.ts (1)
3-11: Consider adding error handling for dynamic imports.The dynamic imports could fail if the configuration files are missing or have initialization errors. While Sentry should handle its own errors gracefully, explicit error handling would make debugging easier.
💡 Optional error handling
export async function register() { if (process.env.NEXT_RUNTIME === "nodejs") { - await import("../sentry.server.config"); + await import("../sentry.server.config").catch((error) => { + console.error("Failed to load Sentry server config:", error); + }); } if (process.env.NEXT_RUNTIME === "edge") { - await import("../sentry.edge.config"); + await import("../sentry.edge.config").catch((error) => { + console.error("Failed to load Sentry edge config:", error); + }); } }apps/api/sentry.edge.config.ts (1)
1-12: Extract Sentry configuration to shared utility.The Sentry initialization logic is duplicated across multiple files (server.config.ts and edge.config.ts in all apps). Consider creating a shared configuration factory function.
🔧 Proposed shared configuration
Create
packages/sentry-config/src/index.ts:import * as Sentry from "@sentry/nextjs"; export function initSentry(options: { dsn: string | undefined; environment: string; appName: string; }) { Sentry.init({ dsn: options.dsn, environment: options.environment, enabled: !!options.dsn, tracesSampleRate: options.environment === "production" ? 0.1 : 1.0, sendDefaultPii: false, // Privacy-conscious default debug: false, }); }Then in each config file:
import { initSentry } from "@superset/sentry-config"; import { env } from "@/env"; initSentry({ dsn: env.NEXT_PUBLIC_SENTRY_DSN_API, environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, appName: "api", });apps/api/src/env.ts (1)
20-21: Consider makingNEXT_PUBLIC_SENTRY_ENVIRONMENToptional or providing a default.
NEXT_PUBLIC_SENTRY_DSN_APIis optional (Sentry is conditionally enabled based on DSN presence), butNEXT_PUBLIC_SENTRY_ENVIRONMENTis required. This creates an inconsistency: if Sentry is disabled (no DSN), the environment variable is still mandatory and will cause validation to fail.Consider either:
- Making it optional:
z.enum([...]).optional()- Providing a default:
z.enum([...]).default("development")🔎 Suggested fix
- NEXT_PUBLIC_SENTRY_ENVIRONMENT: z.enum(["development", "preview", "production"]), + NEXT_PUBLIC_SENTRY_ENVIRONMENT: z.enum(["development", "preview", "production"]).optional(),apps/marketing/src/env.ts (1)
23-24: Same inconsistency:NEXT_PUBLIC_SENTRY_ENVIRONMENTis required while DSN is optional.As noted in
apps/api/src/env.ts, this creates a validation requirement even when Sentry is disabled. Consider making this optional or adding a default for consistency with the conditional Sentry enablement pattern.🔎 Suggested fix
- NEXT_PUBLIC_SENTRY_ENVIRONMENT: z.enum(["development", "preview", "production"]), + NEXT_PUBLIC_SENTRY_ENVIRONMENT: z.enum(["development", "preview", "production"]).optional(),
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.env.example(1 hunks).github/workflows/deploy-preview.yml(9 hunks).github/workflows/deploy-production.yml(9 hunks)apps/admin/next.config.ts(2 hunks)apps/admin/package.json(2 hunks)apps/admin/sentry.edge.config.ts(1 hunks)apps/admin/sentry.server.config.ts(1 hunks)apps/admin/src/app/global-error.tsx(1 hunks)apps/admin/src/env.ts(3 hunks)apps/admin/src/instrumentation-client.ts(2 hunks)apps/admin/src/instrumentation.ts(1 hunks)apps/api/next.config.ts(2 hunks)apps/api/package.json(1 hunks)apps/api/sentry.edge.config.ts(1 hunks)apps/api/sentry.server.config.ts(1 hunks)apps/api/src/app/global-error.tsx(1 hunks)apps/api/src/env.ts(1 hunks)apps/api/src/instrumentation.ts(1 hunks)apps/api/src/proxy.ts(2 hunks)apps/docs/next.config.ts(2 hunks)apps/docs/package.json(1 hunks)apps/docs/sentry.edge.config.ts(1 hunks)apps/docs/sentry.server.config.ts(1 hunks)apps/docs/src/app/global-error.tsx(1 hunks)apps/docs/src/env.ts(1 hunks)apps/docs/src/instrumentation-client.ts(2 hunks)apps/docs/src/instrumentation.ts(1 hunks)apps/marketing/next.config.ts(2 hunks)apps/marketing/package.json(2 hunks)apps/marketing/sentry.edge.config.ts(1 hunks)apps/marketing/sentry.server.config.ts(1 hunks)apps/marketing/src/app/global-error.tsx(1 hunks)apps/marketing/src/env.ts(3 hunks)apps/marketing/src/instrumentation-client.ts(2 hunks)apps/marketing/src/instrumentation.ts(1 hunks)apps/marketing/src/proxy.ts(1 hunks)apps/web/next.config.ts(2 hunks)apps/web/package.json(3 hunks)apps/web/sentry.edge.config.ts(1 hunks)apps/web/sentry.server.config.ts(1 hunks)apps/web/src/app/global-error.tsx(1 hunks)apps/web/src/env.ts(3 hunks)apps/web/src/instrumentation-client.ts(2 hunks)apps/web/src/instrumentation.ts(1 hunks)apps/web/src/proxy.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/web/sentry.server.config.tsapps/admin/sentry.edge.config.tsapps/marketing/next.config.tsapps/api/src/app/global-error.tsxapps/admin/src/app/global-error.tsxapps/web/next.config.tsapps/docs/src/app/global-error.tsxapps/marketing/src/instrumentation.tsapps/docs/src/env.tsapps/marketing/src/app/global-error.tsxapps/docs/sentry.server.config.tsapps/api/next.config.tsapps/marketing/src/instrumentation-client.tsapps/api/src/proxy.tsapps/web/src/app/global-error.tsxapps/web/src/env.tsapps/docs/src/instrumentation.tsapps/docs/sentry.edge.config.tsapps/marketing/src/proxy.tsapps/admin/sentry.server.config.tsapps/web/src/proxy.tsapps/web/src/instrumentation-client.tsapps/docs/next.config.tsapps/admin/src/instrumentation-client.tsapps/admin/src/env.tsapps/api/sentry.edge.config.tsapps/marketing/sentry.edge.config.tsapps/docs/src/instrumentation-client.tsapps/api/src/env.tsapps/marketing/sentry.server.config.tsapps/web/src/instrumentation.tsapps/admin/src/instrumentation.tsapps/api/sentry.server.config.tsapps/admin/next.config.tsapps/web/sentry.edge.config.tsapps/api/src/instrumentation.tsapps/marketing/src/env.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/web/sentry.server.config.tsapps/admin/sentry.edge.config.tsapps/marketing/next.config.tsapps/api/src/app/global-error.tsxapps/admin/src/app/global-error.tsxapps/web/next.config.tsapps/docs/src/app/global-error.tsxapps/marketing/src/instrumentation.tsapps/docs/src/env.tsapps/marketing/src/app/global-error.tsxapps/docs/sentry.server.config.tsapps/api/next.config.tsapps/marketing/src/instrumentation-client.tsapps/api/src/proxy.tsapps/web/src/app/global-error.tsxapps/web/src/env.tsapps/docs/src/instrumentation.tsapps/docs/sentry.edge.config.tsapps/marketing/src/proxy.tsapps/admin/sentry.server.config.tsapps/web/src/proxy.tsapps/web/src/instrumentation-client.tsapps/docs/next.config.tsapps/admin/src/instrumentation-client.tsapps/admin/src/env.tsapps/api/sentry.edge.config.tsapps/marketing/sentry.edge.config.tsapps/docs/src/instrumentation-client.tsapps/api/src/env.tsapps/marketing/sentry.server.config.tsapps/web/src/instrumentation.tsapps/admin/src/instrumentation.tsapps/api/sentry.server.config.tsapps/admin/next.config.tsapps/web/sentry.edge.config.tsapps/api/src/instrumentation.tsapps/marketing/src/env.ts
apps/api/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations
Files:
apps/api/src/app/global-error.tsxapps/api/src/proxy.tsapps/api/src/env.tsapps/api/src/instrumentation.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/api/src/app/global-error.tsxapps/admin/src/app/global-error.tsxapps/docs/src/app/global-error.tsxapps/marketing/src/app/global-error.tsxapps/web/src/app/global-error.tsx
🧠 Learnings (4)
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to apps/*/app/**/proxy.ts : Never create middleware.ts in Next.js 16 - always use proxy.ts for request interception
Applied to files:
apps/web/sentry.server.config.tsapps/admin/sentry.edge.config.tsapps/api/src/proxy.tsapps/marketing/src/proxy.tsapps/web/src/proxy.tsapps/api/sentry.server.config.tsapps/web/sentry.edge.config.ts
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/docs/package.jsonapps/marketing/package.jsonapps/admin/package.jsonapps/web/package.json
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to **/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
apps/docs/package.json
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/web/src/env.tsapps/admin/src/env.tsapps/marketing/src/env.ts
🧬 Code graph analysis (25)
apps/web/sentry.server.config.ts (1)
apps/web/src/env.ts (1)
env(5-51)
apps/admin/sentry.edge.config.ts (2)
apps/admin/src/env.ts (1)
env(5-46)apps/docs/src/env.ts (1)
env(5-34)
apps/api/src/app/global-error.tsx (4)
apps/admin/src/app/global-error.tsx (1)
GlobalError(7-23)apps/docs/src/app/global-error.tsx (1)
GlobalError(7-23)apps/marketing/src/app/global-error.tsx (1)
GlobalError(7-23)apps/web/src/app/global-error.tsx (1)
GlobalError(7-23)
apps/admin/src/app/global-error.tsx (4)
apps/api/src/app/global-error.tsx (1)
GlobalError(7-23)apps/docs/src/app/global-error.tsx (1)
GlobalError(7-23)apps/marketing/src/app/global-error.tsx (1)
GlobalError(7-23)apps/web/src/app/global-error.tsx (1)
GlobalError(7-23)
apps/docs/src/app/global-error.tsx (4)
apps/admin/src/app/global-error.tsx (1)
GlobalError(7-23)apps/api/src/app/global-error.tsx (1)
GlobalError(7-23)apps/marketing/src/app/global-error.tsx (1)
GlobalError(7-23)apps/web/src/app/global-error.tsx (1)
GlobalError(7-23)
apps/marketing/src/instrumentation.ts (4)
apps/admin/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/api/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/docs/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/web/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)
apps/marketing/src/app/global-error.tsx (4)
apps/admin/src/app/global-error.tsx (1)
GlobalError(7-23)apps/api/src/app/global-error.tsx (1)
GlobalError(7-23)apps/docs/src/app/global-error.tsx (1)
GlobalError(7-23)apps/web/src/app/global-error.tsx (1)
GlobalError(7-23)
apps/docs/sentry.server.config.ts (1)
apps/docs/src/env.ts (1)
env(5-34)
apps/marketing/src/instrumentation-client.ts (3)
apps/marketing/src/env.ts (1)
env(5-39)apps/docs/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)apps/web/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)
apps/api/src/proxy.ts (1)
apps/api/src/env.ts (1)
env(4-31)
apps/web/src/app/global-error.tsx (4)
apps/admin/src/app/global-error.tsx (1)
GlobalError(7-23)apps/api/src/app/global-error.tsx (1)
GlobalError(7-23)apps/docs/src/app/global-error.tsx (1)
GlobalError(7-23)apps/marketing/src/app/global-error.tsx (1)
GlobalError(7-23)
apps/docs/src/instrumentation.ts (4)
apps/admin/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/api/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/marketing/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/web/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)
apps/docs/sentry.edge.config.ts (1)
apps/docs/src/env.ts (1)
env(5-34)
apps/admin/sentry.server.config.ts (1)
apps/admin/src/env.ts (1)
env(5-46)
apps/web/src/instrumentation-client.ts (4)
apps/web/src/env.ts (1)
env(5-51)apps/admin/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)apps/docs/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)apps/marketing/src/instrumentation-client.ts (1)
onRouterTransitionStart(41-41)
apps/admin/src/instrumentation-client.ts (1)
apps/admin/src/env.ts (1)
env(5-46)
apps/api/sentry.edge.config.ts (1)
apps/api/src/env.ts (1)
env(4-31)
apps/marketing/sentry.edge.config.ts (1)
apps/marketing/src/env.ts (1)
env(5-39)
apps/marketing/sentry.server.config.ts (1)
apps/marketing/src/env.ts (1)
env(5-39)
apps/web/src/instrumentation.ts (4)
apps/admin/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/api/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/docs/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/marketing/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)
apps/admin/src/instrumentation.ts (4)
apps/api/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/docs/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/marketing/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/web/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)
apps/api/sentry.server.config.ts (1)
apps/api/src/env.ts (1)
env(4-31)
apps/admin/next.config.ts (4)
apps/api/src/proxy.ts (1)
config(42-49)apps/marketing/src/proxy.ts (1)
config(12-17)apps/web/src/proxy.ts (1)
config(34-39)apps/admin/src/proxy.ts (1)
config(41-46)
apps/web/sentry.edge.config.ts (1)
apps/web/src/env.ts (1)
env(5-51)
apps/api/src/instrumentation.ts (4)
apps/admin/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/docs/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/marketing/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/web/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 68-68: [UnorderedKey] The NEXT_PUBLIC_SENTRY_ENVIRONMENT key should go before the SENTRY_AUTH_TOKEN key
(UnorderedKey)
[warning] 69-69: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_WEB key should go before the NEXT_PUBLIC_SENTRY_ENVIRONMENT key
(UnorderedKey)
[warning] 70-70: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_MARKETING key should go before the NEXT_PUBLIC_SENTRY_DSN_WEB key
(UnorderedKey)
[warning] 71-71: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_ADMIN key should go before the NEXT_PUBLIC_SENTRY_DSN_MARKETING key
(UnorderedKey)
[warning] 72-72: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_DOCS key should go before the NEXT_PUBLIC_SENTRY_DSN_MARKETING key
(UnorderedKey)
[warning] 73-73: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_API key should go before the NEXT_PUBLIC_SENTRY_DSN_DOCS key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Web
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Build
🔇 Additional comments (33)
apps/web/src/proxy.ts (1)
11-11: LGTM! Monitoring route correctly added to public routes.The addition of
/monitoring(.*)to the public route matcher appropriately bypasses authentication for Sentry's monitoring tunnel, enabling error tracking even when ad-blockers are present.apps/api/src/proxy.ts (1)
1-1: LGTM! Public route handling correctly implemented.The addition of public route matching for
/ingest(.*)and/monitoring(.*)appropriately bypasses CORS processing for observability endpoints (PostHog and Sentry). This allows monitoring tools to function without authentication while maintaining CORS protection for other API routes.Also applies to: 7-7, 21-24
apps/web/src/app/global-error.tsx (1)
1-23: LGTM! Consistent global error handler implementation.The implementation correctly follows Next.js global error handler conventions and matches the pattern used across other apps (admin, api, docs, marketing). The error is captured to Sentry on mount/change, and a generic error UI is rendered.
apps/marketing/src/proxy.ts (1)
1-10: LGTM! Correctly implements public route bypass for Sentry tunneling.The addition of
/ingest(.*)and/monitoring(.*)as public routes allows Sentry's tunnel mechanism to bypass ad-blockers without requiring authentication. This pattern is consistent with similar changes in other apps.apps/admin/src/app/global-error.tsx (1)
1-23: LGTM! Consistent global error handler implementation.The implementation correctly follows Next.js global error handler conventions and matches the pattern used across other apps. Error tracking and UI rendering are properly configured.
.github/workflows/deploy-preview.yml (5)
134-136: LGTM! Correctly configures Sentry environment variables for API preview deployments.The Sentry DSN, auth token, and environment variables are properly passed to both the build environment and Vercel deployment command.
Also applies to: 153-155
229-231: LGTM! Correctly configures Sentry environment variables for Web preview deployments.Sentry configuration is properly propagated to the Web app deployment.
Also applies to: 247-249
306-308: LGTM! Correctly configures Sentry environment variables for Marketing preview deployments.Sentry configuration is properly propagated to the Marketing app deployment.
Also applies to: 319-321
391-393: LGTM! Correctly configures Sentry environment variables for Admin preview deployments.Sentry configuration is properly propagated to the Admin app deployment.
Also applies to: 406-408
463-465: LGTM! Correctly configures Sentry environment variables for Docs preview deployments.Sentry configuration is properly propagated to the Docs app deployment.
Also applies to: 471-473
.github/workflows/deploy-production.yml (5)
84-86: LGTM! Correctly configures Sentry environment variables for API production deployments.The Sentry configuration is properly passed to both the build environment and Vercel deployment command.
Also applies to: 103-105
151-153: LGTM! Correctly configures Sentry environment variables for Web production deployments.Sentry configuration is properly propagated to the Web app production deployment.
Also applies to: 169-171
212-214: LGTM! Correctly configures Sentry environment variables for Marketing production deployments.Sentry configuration is properly propagated to the Marketing app production deployment.
Also applies to: 225-227
270-272: LGTM! Correctly configures Sentry environment variables for Admin production deployments.Sentry configuration is properly propagated to the Admin app production deployment.
Also applies to: 285-287
326-328: LGTM! Correctly configures Sentry environment variables for Docs production deployments.Sentry configuration is properly propagated to the Docs app production deployment.
Also applies to: 334-336
apps/docs/package.json (1)
15-15: Version pinning is consistent and intentional across apps. Bothapps/docsandapps/apiuse the same strategy: exact versions for instrumentation packages (import-in-the-middle,require-in-the-middle) and a semver range for@sentry/nextjs(^10.32.1). This pattern reflects the different stability requirements—infrastructure packages are pinned for predictability, while the higher-level SDK uses a range to receive patch and minor updates safely.apps/api/package.json (1)
16-16: Verify version pinning strategy for instrumentation packages.The
import-in-the-middleandrequire-in-the-middlepackages use exact versions (2.0.1,8.0.1), while@sentry/nextjsuses a semver range (^10.32.1). This inconsistency might be intentional if the instrumentation packages have known compatibility constraints.Please verify whether exact version pinning is required for these Sentry instrumentation dependencies:
#!/bin/bash # Check if other apps use the same versioning strategy rg -n "import-in-the-middle|require-in-the-middle" --type jsonAlso applies to: 24-24, 29-29
apps/marketing/src/instrumentation-client.ts (1)
41-41: LGTM—proper Next.js App Router integration.Exporting
onRouterTransitionStartcorrectly integrates with Next.js router instrumentation for tracking navigation events.apps/marketing/package.json (1)
17-17: @sentry/nextjs 10.32.1 is compatible with Next.js 16 and has no known security advisories.The Sentry Next.js SDK requires a minimum Next.js version of 13.2.0, which is satisfied by Next.js 16. No active security concerns have been identified for this specific Sentry version.
apps/admin/src/instrumentation.ts (1)
3-11: LGTM! Runtime-conditional initialization is correct.The
register()function correctly imports the appropriate Sentry configuration based on the runtime environment (Node.js or Edge). This aligns with Next.js 15+ instrumentation patterns.apps/docs/src/instrumentation-client.ts (1)
35-35: LGTM! Router transition tracking export is correct.The
onRouterTransitionStartexport correctly integrates Sentry's client-side navigation tracking with Next.js 15+ App Router.apps/web/next.config.ts (2)
35-44: LGTM! Sentry Next.js integration is properly configured.The
withSentryConfigwrapper correctly integrates Sentry with the Next.js build. The tunnel route/monitoringhelps bypass ad-blockers, and the configuration is appropriate for production use.
40-40: VerifywidenClientFileUploadnecessity.The
widenClientFileUpload: trueoption uploads more source files to Sentry for better stack traces, but increases build time and upload size. Ensure this is necessary for your debugging needs, as the default behavior may be sufficient.apps/marketing/src/app/global-error.tsx (2)
7-14: LGTM! Error capture logic is correct.The
useEffecthook properly captures errors to Sentry when the error prop changes. This aligns with Next.js 15+ global error handling patterns.
19-19:statusCode={0}is correct for App Router global-error.tsx.The App Router does not expose status codes for errors, so
statusCode={0}is the standard approach to render a generic error message. UsingstatusCode={500}would be inappropriate in this context.apps/admin/src/instrumentation-client.ts (1)
35-35: LGTM! Router transition tracking export is correct.The
onRouterTransitionStartexport correctly integrates Sentry's client-side navigation tracking with Next.js 15+ App Router.apps/marketing/sentry.server.config.ts (1)
9-9: Verify the trace sampling rates align with your observability budget.The configuration samples 10% of traces in production and 100% in other environments. Ensure this aligns with your Sentry quota and budget expectations, especially as traffic scales.
apps/web/src/instrumentation-client.ts (2)
7-24: LGTM - PostHog configuration unchanged.The existing PostHog initialization is preserved correctly, and Sentry is added without interfering with the analytics setup. Good separation of concerns.
35-35: Good addition of router transition instrumentation.Exporting
onRouterTransitionStartprovides a clean API for tracking navigation errors and performance. This follows Sentry's recommended patterns for Next.js App Router.apps/marketing/next.config.ts (1)
36-45: No action required. The/monitoringtunnel route is automatically configured by the Sentry SDK and correctly set up in the marketing app. Since no middleware exists to intercept it and it's explicitly included in public routes, the configuration will function as intended.apps/admin/next.config.ts (1)
35-44: LGTM!The Sentry configuration is consistent with other apps in the monorepo. The options are properly configured for Vercel deployment with source map uploads and monitoring tunnel.
Same consideration as the API app applies: ensure the
/monitoringtunnel route is excluded from proxy middleware inapps/admin/src/proxy.ts.apps/docs/next.config.ts (1)
46-55: LGTM!The Sentry configuration correctly wraps the Nextra-enhanced config. The composition order
withSentryConfig(withNextra(nextConfig), ...)is appropriate, allowing both Nextra and Sentry to apply their transformations.apps/api/next.config.ts (1)
17-26: No changes needed—/monitoringis already properly excluded from proxy middleware.The Sentry tunnel route is already handled correctly. The proxy.ts file uses Clerk's route matcher to explicitly allow
/monitoringas a public route (line 7), which returns early without proxy processing (line 23). This prevents any interception of Sentry tunnel requests.Likely an incorrect or invalid review comment.
| }, | ||
| "dependencies": { | ||
| "@clerk/nextjs": "^6.36.2", | ||
| "@sentry/nextjs": "^10.32.1", |
There was a problem hiding this comment.
Update import-in-the-middle to a valid version—2.0.1 does not exist.
The latest version of import-in-the-middle is 2.0.0, not 2.0.1 as specified in package.json. This will cause installation failures. @sentry/nextjs@10.32.1 is the latest version and is current. require-in-the-middle@8.0.1 is the latest version and has no known vulnerabilities.
🤖 Prompt for AI Agents
In apps/admin/package.json around line 15, the dependency "import-in-the-middle"
is set to a non-existent 2.0.1 which will break installs; change it to the valid
version "2.0.0", then run your package manager (npm/yarn/pnpm) to update
node_modules and the lockfile (package-lock.json / yarn.lock / pnpm-lock.yaml)
and verify installs/builds succeed; also run tests or start the app to confirm
no regressions.
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | ||
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_ADMIN, | ||
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | ||
| sendDefaultPii: true, |
There was a problem hiding this comment.
Privacy concern: sendDefaultPii: true may violate GDPR/CCPA.
Enabling sendDefaultPii sends personally identifiable information (IP addresses, cookies, user agents) to Sentry. This requires explicit user consent under GDPR/CCPA and should be documented in your privacy policy.
Consider:
- Setting
sendDefaultPii: falseby default - Adding a configuration flag to enable it only when compliant
- Documenting this in privacy policies and consent flows
- Using
beforeSendcallbacks to scrub PII from events
🔎 Recommended configuration
Sentry.init({
dsn: env.NEXT_PUBLIC_SENTRY_DSN_ADMIN,
environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT,
enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_ADMIN,
tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0,
- sendDefaultPii: true,
+ sendDefaultPii: false,
debug: false,
+ beforeSend(event) {
+ // Scrub sensitive data if needed
+ return event;
+ },
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sendDefaultPii: true, | |
| Sentry.init({ | |
| dsn: env.NEXT_PUBLIC_SENTRY_DSN_ADMIN, | |
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | |
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_ADMIN, | |
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | |
| sendDefaultPii: false, | |
| debug: false, | |
| beforeSend(event) { | |
| // Scrub sensitive data if needed | |
| return event; | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/admin/sentry.edge.config.ts around line 10, sendDefaultPii is currently
set to true which can transmit PII and conflict with GDPR/CCPA; change the
default to sendDefaultPii: false, add a config flag (e.g., SENTRY_SEND_PII) read
from env to enable it only when explicit consent/compliance is present,
implement a beforeSend hook that strips IPs/user-agent/cookies from events when
the flag is false, and update project privacy/consent docs and any consent-flow
code to toggle the env/config flag accordingly.
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | ||
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_ADMIN, | ||
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | ||
| sendDefaultPii: true, |
There was a problem hiding this comment.
Review PII transmission for compliance requirements.
The sendDefaultPii: true option enables transmission of personally identifiable information to Sentry. Ensure this complies with your privacy policy and data protection regulations.
🤖 Prompt for AI Agents
In apps/admin/sentry.server.config.ts around line 10, the configuration sets
sendDefaultPii: true which enables transmission of PII to Sentry; change this to
a safer default and make it configurable: set sendDefaultPii to false by
default, read an environment variable (e.g., SENTRY_SEND_PII) to enable it only
when explicitly allowed, add server-side gating so it cannot be enabled in
production without documented privacy approval, and update project docs/README
to record the required compliance review and where to change the env flag.
| Sentry.init({ | ||
| dsn: env.NEXT_PUBLIC_SENTRY_DSN_ADMIN, | ||
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | ||
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_ADMIN, | ||
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | ||
| sendDefaultPii: true, | ||
| debug: false, | ||
| }); |
There was a problem hiding this comment.
Review PII transmission for compliance requirements.
The sendDefaultPii: true option on Line 31 enables transmission of personally identifiable information to Sentry. Ensure this complies with your privacy policy and data protection regulations.
🤖 Prompt for AI Agents
In apps/admin/src/instrumentation-client.ts around lines 26 to 33, the Sentry
config uses sendDefaultPii: true which will transmit PII; change this to
sendDefaultPii: false or gate it behind an explicit environment variable (e.g.,
NEXT_PUBLIC_SENTRY_SEND_PII) so PII is only enabled when explicitly allowed,
update the env usage to coerce to a boolean, and add a short code comment
referencing the privacy compliance decision so future reviewers know why PII is
disabled or when it may be enabled.
| import * as Sentry from "@sentry/nextjs"; | ||
|
|
||
| export async function register() { | ||
| if (process.env.NEXT_RUNTIME === "nodejs") { | ||
| await import("../sentry.server.config"); | ||
| } | ||
|
|
||
| if (process.env.NEXT_RUNTIME === "edge") { | ||
| await import("../sentry.edge.config"); | ||
| } | ||
| } | ||
|
|
||
| export const onRequestError = Sentry.captureRequestError; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated instrumentation logic to a shared package.
This exact code is duplicated across all five apps (admin, api, docs, marketing, web). The runtime-based Sentry configuration loading pattern should be extracted to a shared package (e.g., packages/instrumentation or @superset/instrumentation) to eliminate duplication and ensure consistency.
📦 Proposed shared package structure
Create packages/instrumentation/src/sentry.ts:
import * as Sentry from "@sentry/nextjs";
export async function register() {
if (process.env.NEXT_RUNTIME === "nodejs") {
await import("../sentry.server.config");
}
if (process.env.NEXT_RUNTIME === "edge") {
await import("../sentry.edge.config");
}
}
export const onRequestError = Sentry.captureRequestError;Then in each app's instrumentation.ts:
export { register, onRequestError } from "@superset/instrumentation/sentry";| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | ||
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_WEB, | ||
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | ||
| sendDefaultPii: true, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -e ts -e json sentry.edge.config.ts package.jsonRepository: superset-sh/superset
Length of output: 163
🏁 Script executed:
fd sentry.edge.config.ts
fd package.json | head -5Repository: superset-sh/superset
Length of output: 344
🏁 Script executed:
cat -n apps/web/sentry.edge.config.tsRepository: superset-sh/superset
Length of output: 492
🏁 Script executed:
cat -n apps/web/package.json | grep -A 5 -B 5 "@sentry"Repository: superset-sh/superset
Length of output: 456
🏁 Script executed:
cat -n apps/web/sentry.server.config.tsRepository: superset-sh/superset
Length of output: 492
Add privacy documentation comment explaining sendDefaultPii: true configuration.
Setting sendDefaultPii: true enables Sentry to collect personally identifiable information including user IP addresses and cookies. Add a comment in the configuration explaining why this is enabled and confirming it aligns with your privacy policy and GDPR/CCPA requirements. Following Sentry's recommendation, include a link to documentation on what data is being collected so developers understand and can disable it if needed.
🤖 Prompt for AI Agents
In apps/web/sentry.edge.config.ts around line 10, the configuration sets
sendDefaultPii: true without a comment; add a concise privacy documentation
comment above this line stating what PII is being collected (e.g., IP addresses,
cookies), why sendDefaultPii is enabled in this project, and confirm it aligns
with our privacy policy and GDPR/CCPA controls; include a link to Sentry’s
documentation
(https://docs.sentry.io/platforms/javascript/configuration/options/#senddefaultpii)
and a short note on how to disable it if needed for future reference.
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | ||
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_WEB, | ||
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | ||
| sendDefaultPii: true, |
There was a problem hiding this comment.
Verify compliance and SDK version for sendDefaultPii: true.
Setting sendDefaultPii: true enables sending cookies from HTTP requests. While server SDKs remove the IP address of incoming HTTP requests by default, this configuration should be validated against your privacy policy and GDPR/CCPA requirements.
Note: A vulnerability in Sentry Node.js SDK versions 10.11.0 to 10.26.0 caused certain incoming HTTP headers to be added as trace span attributes, and when sendDefaultPii: true was set, headers including Authorization and Cookie were unintentionally allowed through. Ensure your SDK is updated to version 10.27.0 or later to mitigate this risk.
🤖 Prompt for AI Agents
In apps/web/sentry.server.config.ts around line 10, the config sets
sendDefaultPii: true which enables sending cookies and other request PII; verify
this is allowed by your privacy/GDPR/CCPA policy and either set sendDefaultPii
to false or narrow the data you send. Also ensure the Sentry Node SDK is
upgraded to >= 10.27.0 (to avoid the header leaking vulnerability in
10.11.0–10.26.0); update package.json, run npm/yarn install, and re-run tests.
Finally, add a brief comment or changelog entry documenting the privacy decision
and include a quick verification test that cookies/Authorization headers are not
captured if you choose to disable sendDefaultPii.
| Sentry.init({ | ||
| dsn: env.NEXT_PUBLIC_SENTRY_DSN_WEB, | ||
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | ||
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_WEB, | ||
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | ||
| sendDefaultPii: true, | ||
| debug: false, | ||
| }); |
There was a problem hiding this comment.
Critical: sendDefaultPii: true on client-side is a significant privacy risk.
Enabling PII collection in the browser is particularly concerning because:
- Browser environment exposes more user data (localStorage, cookies, user-agent, etc.)
- Harder to control what gets sent to third parties
- Increased GDPR/CCPA compliance risk
- Users expect client-side code to be more privacy-conscious
This setting should almost certainly be false in client-side code unless you have explicit user consent and a documented compliance strategy.
🔒 Recommended fix
Sentry.init({
dsn: env.NEXT_PUBLIC_SENTRY_DSN_WEB,
environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT,
enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_WEB,
tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0,
- sendDefaultPii: true,
+ sendDefaultPii: false,
debug: false,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Sentry.init({ | |
| dsn: env.NEXT_PUBLIC_SENTRY_DSN_WEB, | |
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | |
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_WEB, | |
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | |
| sendDefaultPii: true, | |
| debug: false, | |
| }); | |
| Sentry.init({ | |
| dsn: env.NEXT_PUBLIC_SENTRY_DSN_WEB, | |
| environment: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT, | |
| enabled: !!env.NEXT_PUBLIC_SENTRY_DSN_WEB, | |
| tracesSampleRate: env.NEXT_PUBLIC_SENTRY_ENVIRONMENT === "production" ? 0.1 : 1.0, | |
| sendDefaultPii: false, | |
| debug: false, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/src/instrumentation-client.ts around lines 26 to 33, the Sentry init
enables sendDefaultPii: true which is a client-side privacy risk; change
sendDefaultPii to false for browser builds and only enable it behind an
explicit, auditable condition (e.g., an environment flag plus documented user
consent/consent API) — update the code to default to false, read a secure env
variable or consent state to allow true, and ensure this path is only active
after explicit opt-in and that the change is clearly documented in the
code/comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/marketing/src/instrumentation-client.ts (1)
32-39: Sentry client initialization follows correct pattern.The configuration correctly:
- Conditionally enables Sentry based on DSN presence
- Sets appropriate sample rates (10% in production, 100% otherwise)
- Uses environment variables from the validated env schema
Note: The
sendDefaultPii: trueprivacy concern on line 37 has already been comprehensively addressed in previous review comments, which recommend implementing consent-first data collection and updating privacy disclosures.apps/marketing/sentry.server.config.ts (1)
5-12: Server-side Sentry initialization follows correct pattern.The configuration correctly:
- Conditionally enables Sentry based on DSN presence (line 8)
- Uses validated environment variables from the env schema
- Applies appropriate sampling (10% production, 100% otherwise)
- Mirrors the client-side configuration for consistency
Note: The
sendDefaultPii: truesetting on line 10 has already been flagged in previous review comments, which recommend setting it tofalseand explicitly usingSentry.setUser()with non-PII identifiers when needed to comply with privacy regulations.apps/admin/sentry.server.config.ts (1)
10-10: Review PII transmission for compliance requirements.The
sendDefaultPii: trueoption enables transmission of personally identifiable information to Sentry. Ensure this complies with your privacy policy and data protection regulations.apps/admin/src/instrumentation-client.ts (1)
26-33: Review PII transmission for compliance requirements.The
sendDefaultPii: trueoption enables transmission of personally identifiable information to Sentry. Ensure this complies with your privacy policy and data protection regulations.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
.env.example(1 hunks).github/workflows/deploy-preview.yml(9 hunks).github/workflows/deploy-production.yml(9 hunks)apps/admin/next.config.ts(2 hunks)apps/admin/package.json(2 hunks)apps/admin/sentry.edge.config.ts(1 hunks)apps/admin/sentry.server.config.ts(1 hunks)apps/admin/src/app/global-error.tsx(1 hunks)apps/admin/src/env.ts(3 hunks)apps/admin/src/instrumentation-client.ts(2 hunks)apps/admin/src/instrumentation.ts(1 hunks)apps/api/next.config.ts(2 hunks)apps/api/package.json(1 hunks)apps/api/sentry.edge.config.ts(1 hunks)apps/api/sentry.server.config.ts(1 hunks)apps/api/src/app/global-error.tsx(1 hunks)apps/api/src/env.ts(1 hunks)apps/api/src/instrumentation.ts(1 hunks)apps/api/src/proxy.ts(2 hunks)apps/docs/next.config.ts(2 hunks)apps/docs/package.json(1 hunks)apps/docs/sentry.edge.config.ts(1 hunks)apps/docs/sentry.server.config.ts(1 hunks)apps/docs/src/app/global-error.tsx(1 hunks)apps/docs/src/env.ts(1 hunks)apps/docs/src/instrumentation-client.ts(2 hunks)apps/docs/src/instrumentation.ts(1 hunks)apps/marketing/next.config.ts(2 hunks)apps/marketing/package.json(2 hunks)apps/marketing/sentry.edge.config.ts(1 hunks)apps/marketing/sentry.server.config.ts(1 hunks)apps/marketing/src/app/global-error.tsx(1 hunks)apps/marketing/src/env.ts(3 hunks)apps/marketing/src/instrumentation-client.ts(2 hunks)apps/marketing/src/instrumentation.ts(1 hunks)apps/web/next.config.ts(2 hunks)apps/web/package.json(3 hunks)apps/web/sentry.edge.config.ts(1 hunks)apps/web/sentry.server.config.ts(1 hunks)apps/web/src/app/global-error.tsx(1 hunks)apps/web/src/env.ts(3 hunks)apps/web/src/instrumentation-client.ts(2 hunks)apps/web/src/instrumentation.ts(1 hunks)apps/web/src/proxy.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (32)
- apps/docs/src/app/global-error.tsx
- apps/api/src/instrumentation.ts
- apps/docs/src/instrumentation-client.ts
- apps/api/sentry.server.config.ts
- apps/admin/sentry.edge.config.ts
- apps/docs/sentry.edge.config.ts
- apps/marketing/src/app/global-error.tsx
- apps/marketing/src/instrumentation.ts
- apps/web/src/instrumentation.ts
- apps/api/next.config.ts
- apps/web/next.config.ts
- apps/web/src/app/global-error.tsx
- apps/admin/package.json
- apps/admin/src/instrumentation.ts
- apps/docs/next.config.ts
- apps/marketing/src/env.ts
- apps/api/src/proxy.ts
- apps/api/package.json
- apps/web/src/instrumentation-client.ts
- apps/web/sentry.server.config.ts
- apps/admin/src/env.ts
- apps/marketing/sentry.edge.config.ts
- apps/api/src/env.ts
- apps/docs/sentry.server.config.ts
- apps/marketing/package.json
- apps/marketing/next.config.ts
- apps/web/src/proxy.ts
- apps/web/src/env.ts
- apps/api/sentry.edge.config.ts
- apps/docs/package.json
- apps/api/src/app/global-error.tsx
- apps/web/sentry.edge.config.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/admin/src/app/global-error.tsxapps/admin/sentry.server.config.tsapps/admin/src/instrumentation-client.tsapps/docs/src/instrumentation.tsapps/docs/src/env.tsapps/marketing/src/instrumentation-client.tsapps/marketing/sentry.server.config.tsapps/admin/next.config.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/admin/src/app/global-error.tsxapps/admin/sentry.server.config.tsapps/admin/src/instrumentation-client.tsapps/docs/src/instrumentation.tsapps/docs/src/env.tsapps/marketing/src/instrumentation-client.tsapps/marketing/sentry.server.config.tsapps/admin/next.config.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/admin/src/app/global-error.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development
Applied to files:
apps/web/package.json
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to apps/*/app/**/proxy.ts : Never create middleware.ts in Next.js 16 - always use proxy.ts for request interception
Applied to files:
apps/admin/sentry.server.config.ts
📚 Learning: 2025-12-18T23:19:10.405Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.405Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/admin/sentry.server.config.ts
🧬 Code graph analysis (6)
apps/admin/src/app/global-error.tsx (4)
apps/api/src/app/global-error.tsx (1)
GlobalError(7-23)apps/docs/src/app/global-error.tsx (1)
GlobalError(7-23)apps/marketing/src/app/global-error.tsx (1)
GlobalError(7-23)apps/web/src/app/global-error.tsx (1)
GlobalError(7-23)
apps/admin/sentry.server.config.ts (1)
apps/admin/src/env.ts (1)
env(5-46)
apps/admin/src/instrumentation-client.ts (4)
apps/admin/src/env.ts (1)
env(5-46)apps/docs/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)apps/marketing/src/instrumentation-client.ts (1)
onRouterTransitionStart(41-41)apps/web/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)
apps/docs/src/instrumentation.ts (4)
apps/admin/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/api/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/marketing/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)apps/web/src/instrumentation.ts (2)
register(3-11)onRequestError(13-13)
apps/marketing/src/instrumentation-client.ts (4)
apps/marketing/src/env.ts (1)
env(5-39)apps/admin/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)apps/docs/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)apps/web/src/instrumentation-client.ts (1)
onRouterTransitionStart(35-35)
apps/marketing/sentry.server.config.ts (1)
apps/marketing/src/env.ts (1)
env(5-39)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 68-68: [UnorderedKey] The NEXT_PUBLIC_SENTRY_ENVIRONMENT key should go before the SENTRY_AUTH_TOKEN key
(UnorderedKey)
[warning] 69-69: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_WEB key should go before the NEXT_PUBLIC_SENTRY_ENVIRONMENT key
(UnorderedKey)
[warning] 70-70: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_MARKETING key should go before the NEXT_PUBLIC_SENTRY_DSN_WEB key
(UnorderedKey)
[warning] 71-71: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_ADMIN key should go before the NEXT_PUBLIC_SENTRY_DSN_MARKETING key
(UnorderedKey)
[warning] 72-72: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_DOCS key should go before the NEXT_PUBLIC_SENTRY_DSN_MARKETING key
(UnorderedKey)
[warning] 73-73: [UnorderedKey] The NEXT_PUBLIC_SENTRY_DSN_API key should go before the NEXT_PUBLIC_SENTRY_DSN_DOCS key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Web
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Deploy Admin
- GitHub Check: Build
🔇 Additional comments (10)
apps/web/package.json (1)
15-15: No issues found. All Sentry package versions (@sentry/nextjs@^10.32.1, import-in-the-middle@2.0.1, require-in-the-middle@8.0.1) are current, secure, and fully compatible with Next.js 16 and React 19.apps/marketing/src/instrumentation-client.ts (1)
41-41: LGTM: Export pattern is consistent across apps.The
onRouterTransitionStartexport correctly delegates to Sentry's router transition capture and matches the implementation pattern used in admin, docs, and web apps..env.example (1)
64-74: LGTM - Functional grouping over alphabetical ordering.The Sentry variables are properly organized by functional area. The static analysis warnings about key ordering are pedantic; grouping related configuration together is more maintainable than strict alphabetical ordering.
apps/admin/src/app/global-error.tsx (1)
1-23: LGTM - Global error boundary properly implemented.The implementation follows Next.js best practices for global error boundaries and maintains consistency with other apps in the monorepo. The error is correctly captured via Sentry and displayed using the Next.js error component.
apps/admin/src/instrumentation-client.ts (1)
35-35: LGTM - Router transition tracking properly exported.The export of
onRouterTransitionStartfollows the Sentry Next.js integration pattern and is consistent with other apps in the monorepo..github/workflows/deploy-preview.yml (1)
134-136: LGTM - Sentry environment variables properly propagated.The Sentry configuration variables are consistently added across all deployment targets (API, Web, Marketing, Admin, Docs). The pattern of using secrets for DSNs and auth tokens, with vars for the environment name, follows security best practices.
Also applies to: 153-155, 229-231, 247-249, 306-308, 319-321, 391-393, 406-408, 463-465, 471-473
.github/workflows/deploy-production.yml (1)
84-86: LGTM - Production Sentry configuration mirrors preview setup.The Sentry environment variables are consistently propagated across all production deployments, following the same secure pattern as the preview workflow.
Also applies to: 103-105, 151-153, 169-171, 212-214, 225-227, 270-272, 285-287, 326-328, 334-336
apps/docs/src/instrumentation.ts (2)
3-11: LGTM - Runtime-specific Sentry initialization properly implemented.The conditional loading of Sentry configurations based on
NEXT_RUNTIMEfollows the recommended pattern for Next.js 15 instrumentation. This ensures the correct Sentry configuration loads for each runtime environment.
13-13: LGTM - Request error handler properly exported.Exporting
onRequestErroras an alias toSentry.captureRequestErrorenables centralized error handling and is consistent with other apps in the monorepo.apps/admin/next.config.ts (1)
2-2: Sentry integration is complete—no middleware verification needed, and client config uses correct filename.The
withSentryConfigwrapper is correctly applied with sensible defaults:
- tunnelRoute: "/monitoring" bypasses ad-blockers
silent: !process.env.CIensures verbose logging during buildsauthTokenfrom environment variable follows best practiceAll required Sentry files are properly configured:
- Client-side: instrumentation-client.ts initializes Sentry for the browser, located at apps/admin/src/instrumentation-client.ts with DSN configured
- Server-side: sentry.server.config.ts exists at apps/admin/
- Edge runtime: sentry.edge.config.ts exists at apps/admin/
- CI/CD: SENTRY_AUTH_TOKEN is found in GitHub workflows for source map uploads
- Middleware: No middleware.ts exists in the admin app, so Sentry will automatically skip the tunnel route without additional configuration
If using Turbopack, set the tunnel route to a fixed string and add a negative matcher to exclude it from middleware—verify whether the build uses Turbopack or Webpack.
1 participant
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.