feat: implement sentry, t3-env and instrumentation #1
Conversation
…nfiguration files - Reformatted import arrays in biome.jsonc and tsconfig.json for improved readability and consistency. - Consolidated multiple lines into single lines where applicable to enhance clarity. - Added new Sentry configuration documentation in sentry.mdc for better guidance on exception handling and tracing. - Updated VSCode settings to maintain consistent formatting and organization.
- Updated error handling in Error and GlobalError components to log exceptions to Sentry. - Enhanced metadata generation in layout to include Sentry trace data for improved distributed tracing. - Initialized Sentry on the client-side in providers for comprehensive observability.
- Modified Link component href prop in NavCollapsible, NavItem, and SidebarUserSection to ensure proper type casting using Parameters for enhanced TypeScript type safety. - This change helps prevent potential runtime errors related to incorrect path types.
- Updated the type casting for the router.push method in CommandMenu to use Parameters for improved TypeScript type safety. - This change ensures that the path argument is correctly typed, reducing the risk of runtime errors.
…ata merging - Introduced a new JsonLd component to render JSON-LD structured data in a script tag, ensuring proper escaping for HTML. - Updated createPageMetadata function to use lodash.merge for deep merging of metadata, improving the handling of nested properties.
…on item handling - Added a comment to ignore complexity linting in the getTranslatedRouteConfig function for better clarity on future improvements. - Refactored getNavigationItems to directly return the sorted and filtered navigation groups, enhancing readability and reducing unnecessary variable declarations.
…and observability - Introduced `index.ts` for Next.js config with support for AVIF and WebP image formats. - Added `with-analyzer.ts` to integrate bundle analyzer for size analysis based on the ANALYZE environment variable. - Created `with-observability.ts` to configure Sentry integration for enhanced error tracking and observability, including detailed source map handling and release management.
…ection - Introduced a new `keys.ts` file to manage environment variables using `@t3-oss/env-nextjs` for better configuration handling. - Updated `client.ts` to utilize the new `keys` function for retrieving the `DATABASE_URL`, enhancing code maintainability and clarity.
…nagement - Added "server-only" imports to `check-role.ts`, `email.ts`, and `server-client.ts` for improved server-side execution. - Introduced `keys.ts` to manage environment variables using `@t3-oss/env-nextjs`, updating `config.ts` to utilize this for retrieving `BETTER_AUTH_SECRET` and `BETTER_AUTH_URL`. - Enhanced code maintainability and clarity by centralizing environment variable access.
…servability utilities - Introduced "server-only" imports in `server-queries.ts` and `utils.ts` for improved server-side execution. - Enhanced `handleAPIError` function to utilize observability utilities for consistent error logging and improved error response structure.
- Introduced a new `env.ts` file to manage environment variables using `@t3-oss/env-nextjs`. - Centralized the configuration by extending keys from authentication, database, and observability modules for improved maintainability.
…for enhanced monitoring - Introduced a suite of observability utilities including cache instrumentation, HTTP request tracking, error handling, and metrics management. - Added Sentry integration for both client and server environments, enabling detailed error logging and performance tracing. - Implemented context enrichment functions for user and feature tracking, along with transaction sanitization for improved data handling. - Centralized observability exports in an index file for streamlined access across the application.
…integration - Introduced a new `instrumentation.ts` file to facilitate observability tools integration within Next.js applications. - Implemented a registration function for runtime-specific instrumentation (Node.js and Edge). - Added an error handling function that utilizes Sentry's `captureRequestError` for improved error logging and context management.
- Introduced `sentry.edge.config.ts` and `sentry.server.config.ts` files to initialize Sentry for Edge and server runtimes, respectively. - Each configuration file includes documentation and imports the necessary Sentry initialization functions for error tracking and observability.
…nd observability - Integrated Content Security Policy (CSP) reporting headers for Sentry, enabling enhanced security monitoring. - Merged base configuration from the next-config package and applied observability settings conditionally based on environment variables. - Added support for bundle analysis when the ANALYZE environment variable is set.
…ependencies - Added new dependencies: `@next/bundle-analyzer`, `@sentry/nextjs`, `@sentry/node-native`, `@sentry/profiling-node`, `@t3-oss/env-nextjs`, `lodash`, and `schema-dts`. - Updated existing dependencies to their latest versions in `pnpm-lock.yaml` for improved stability and performance.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a comprehensive observability integration (Sentry) with client/edge/server initializers, many runtime-guarded instrumentation utilities (http, cache, queue, traces, spans, metrics, sampling, fingerprinting, scopes), central env/key modules, CSP nonce/reporting and a Sentry tunnel, plus related config, minor typing fixes and adjusted API error payloads. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant App as App Providers
participant ClientInit as Sentry Client Init
participant SentryLib as `@sentry/nextjs`
Browser->>App: load page
App->>ClientInit: initializeSentry()
ClientInit->>SentryLib: dynamic require & init (integrations, sampler)
SentryLib-->>ClientInit: initialized
ClientInit-->>App: instrumentation available
sequenceDiagram
participant Client as Client
participant NextAPI as /api/monitoring
participant SentrySrv as Sentry Envelope API
Client->>NextAPI: POST envelope (tunnel)
NextAPI->>NextAPI: validate DSN & parse envelope
NextAPI->>SentrySrv: POST https://{orgDomain}/api/{projectId}/envelope/
SentrySrv-->>NextAPI: response
NextAPI-->>Client: relay response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-12-18T18:18:05.202ZApplied to files:
📚 Learning: 2025-12-18T18:18:05.202ZApplied to files:
🧬 Code graph analysis (1)src/lib/observability/queue.ts (2)
⏰ 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). (1)
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting 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 |
|
@sourcery-ai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
110-110: Update Zod string format helpers for v4 compatibility.Zod v4 moved string-format helpers (
url(),email(), etc.) off ZodString. Replacez.string().url()withz.url()in:
src/lib/observability/keys.ts(line 12)src/lib/db/keys.ts(line 7)src/lib/auth/keys.ts(line 8)Additionally, review API route handlers for request validation. Per learnings, backend code should have mandatory Zod validation at request boundaries (currently missing validation logic in API routes).
🤖 Fix all issues with AI agents
In @.cursor/rules/sentry.mdc:
- Around line 88-96: The Sentry.init call contains a hardcoded dsn string;
replace the literal DSN value passed to the dsn option with a secure reference
like process.env.SENTRY_DSN (or the placeholder "YOUR_SENTRY_DSN_HERE") in the
Sentry.init({...}) invocation, and make the same replacement for the other
occurrence of the DSN in this file so both dsn options no longer contain the
hardcoded key; keep enableLogs as-is.
In @next.config.ts:
- Around line 168-203: The CSP currently includes 'unsafe-eval' and
'unsafe-inline' in the Content-Security-Policy-Report-Only header generated in
the securityHeaders array; remove both directives and switch to a per-request
nonce approach: generate a cryptographic nonce in your middleware (e.g.,
middleware.ts), attach it to the request/response (e.g., res.locals or header
like x-csp-nonce), and update next.config.ts where securityHeaders is populated
(the block creating the "Content-Security-Policy-Report-Only" entry) to build
script-src using the per-request nonce and 'strict-dynamic' plus the Sentry CDN
host (no 'unsafe-eval'/'unsafe-inline'); also ensure connect-src only lists
Sentry ingest endpoints and the org domain. Finally update inline usages
(src/lib/seo/json-ld.tsx and src/components/ui/chart.tsx) to read the nonce from
the request/response context and set it on their inline script/style tags so
they are allowed by the new CSP.
In @src/app/global-error.tsx:
- Around line 39-43: The useEffect that calls captureException via
require("@sentry/nextjs") can throw if Sentry isn’t available; wrap the dynamic
require/import in a try/catch (or use dynamic import().catch) inside the
useEffect, check that the returned module exposes captureException before
calling it, and on failure fall back to safe logging (e.g., console.error) so
the error boundary doesn’t itself throw; update the effect around useEffect /
captureException accordingly to handle and log any load errors.
In @src/components/layout/navigation/nav-item.tsx:
- Line 36: Remove the redundant type assertion on the Link href: replace usages
like Link href={route.path as Parameters<typeof Link>[0]["href"]} with Link
href={route.path}. Update this in the NavItem and NavCollapsible components (the
Link component usage and any references to route.path from BaseRoute) so the
plain typed string is passed directly without the unnecessary cast.
In @src/lib/api/utils.ts:
- Around line 106-122: The API error handling is inconsistent: some route
handlers return { error: "..." } while utils.handleAPIError (or the final
Response.json) expects { ok: false, error: ... }; update route handlers (e.g.,
handlers in src/app/api/user/me/route.ts and
src/app/api/admin/users/[id]/route.ts) to either call handleAPIError(error) on
exceptions or return the standardized shape { ok: false, error: <message> } via
Response.json so every endpoint uses the same response shape; locate uses of
Response.json({ error: ... }) and replace them with Response.json({ ok: false,
error: ... }) or delegate to handleAPIError/parseError to preserve consistent
error responses.
In @src/lib/next-config/index.ts:
- Line 5: The formats array currently uses MIME types which Next.js doesn't
recognize; update the formats field in the Next config (the formats property) to
use the format identifiers "avif" and "webp" instead of "image/avif" and
"image/webp" so Next.js can detect and optimize those image formats correctly.
In @src/lib/next-config/with-observability.ts:
- Around line 82-88: The current return object passed to withSentryConfig
overwrites any existing sourceConfig.transpilePackages by setting
transpilePackages: ["@sentry/nextjs"]; change this to merge the arrays instead:
read sourceConfig.transpilePackages (if any), ensure it's an array, concat or
union it with ["@sentry/nextjs"] while preserving unique entries, and pass that
merged array as transpilePackages so withSentryConfig and sourceConfig both keep
their required packages without mutation of sourceConfig.
- Line 23: Replace the direct env access for SENTRY_AUTH_TOKEN with the
validated helper used for SENTRY_ORG and SENTRY_PROJECT: use
keys().SENTRY_AUTH_TOKEN instead of process.env.SENTRY_AUTH_TOKEN in the
withObservability config; if SENTRY_AUTH_TOKEN is not present in the
observability environment schema, add it to the schema returned by keys() so it
becomes part of the validated types.
- Line 44: The config uses env.SENTRY_RELEASE directly (e.g., the property
"name: env.SENTRY_RELEASE") even though SENTRY_RELEASE is optional; update the
assignment to provide a safe fallback or validate before use (for example mirror
the pattern used for SENTRY_ORG/SENTRY_PROJECT or use "|| 'unknown'") so
undefined is never passed into Sentry, and apply the same fix in server.ts,
edge.ts, and client.ts where env.SENTRY_RELEASE is used.
In @src/lib/observability/client.ts:
- Around line 148-150: The code sets release: env.SENTRY_RELEASE in the
client-side Sentry config but SENTRY_RELEASE is server-only and will be
undefined; fix by either moving SENTRY_RELEASE to shared in your env schema or
change the client to use NEXT_PUBLIC_SENTRY_RELEASE (and update the env schema
to expose it) or remove the release assignment/comment it out if undefined is
intentional; update the reference to env.SENTRY_RELEASE in
src/lib/observability/client.ts (the Sentry init/config block that sets
environment/release) to use the chosen approach.
In @src/lib/observability/enrichment.ts:
- Around line 174-191: initializePortalContext currently accesses
window.location.hostname inside the setPortalTags call which will throw during
server-side rendering; update initializePortalContext (the setPortalTags
invocation) to guard access to window by computing a hostname var only if typeof
window !== "undefined" (fallback to "server" or an empty string) and pass that
safe value into setPortalTags so no direct window property is read unguarded.
In @src/lib/observability/http.ts:
- Around line 88-96: The current use of JSON.stringify(body).length in the post
(and similar) wrappers can throw for circular or non-serializable bodies;
replace the inline JSON.stringify usage in the post call that builds the
instrumentHttpRequest payload with a safe helper (e.g., calculateBodySize) that
returns undefined on errors: implement calculateBodySize(body: unknown): number
| undefined that returns JSON.stringify(body).length inside a try/catch
(returning undefined on catch), then use calculateBodySize(body) for requestSize
when calling instrumentHttpRequest in the post and other HTTP method helpers.
- Line 70: Replace the hard-coded numeric status in span.setStatus({ code: 2,
message: "error" }) with Sentry's constant by importing SPAN_STATUS_ERROR from
"@sentry/core" and calling span.setStatus({ code: SPAN_STATUS_ERROR, message:
"error" }) (or simply span.setStatus({ code: SPAN_STATUS_ERROR }) if message is
redundant); update the import list to include SPAN_STATUS_ERROR so the span uses
the named constant instead of 2.
In @src/lib/observability/troubleshooting.ts:
- Around line 20-25: The two regexes in observability/troubleshooting.ts used in
the replace chain (the UUID replace and the hash-like string replace) only match
lowercase hex digits; update them to be case-insensitive so uppercase A-F are
matched (either by adding the i flag to both regex literals or by expanding the
character classes to [A-Fa-f0-9]) and keep the same replacement tokens
("/<uuid>" and "/<hash>") so UUIDs and hashes with uppercase hex are properly
normalized.
- Around line 30-31: The current sanitizer uses /\/[A-Za-z0-9_-]{20,}/g which is
too broad and will redact long hyphenated route segments; update the replacement
to be more specific by (1) increasing the generic length threshold (e.g., to
40+) and adding a negative lookahead to avoid matching segments that contain
hyphens/underscores used in normal routes, and (2) add dedicated patterns for
common token encodings such as base64 (e.g., /\/[A-Za-z0-9+/=]{40,}/g) and hex
(e.g., /\/[0-9a-fA-F]{32,}/g) so only real tokens are replaced; change the
.replace call(s) in src/lib/observability/troubleshooting.ts that reference
/\/[A-Za-z0-9_-]{20,}/g to these tightened patterns.
🧹 Nitpick comments (22)
src/components/command-menu.tsx (1)
56-56: Consider typing the path parameter to match router.push expectations.The type assertion here is similar to the Link href casting pattern in other files. Instead of casting at the call site, consider typing the
handleSelectparameter to match the expected router navigation type, or ensure route path definitions naturally satisfy this type.src/components/layout/sidebar/sidebar-user-section.tsx (1)
84-86: Type casting pattern repeated across navigation components.This follows the same pattern as other Link components in this PR. Consider defining
FooterAction.pathwith the appropriate Next.js href type to avoid runtime type assertions throughout the codebase.src/components/layout/navigation/nav-collapsible.tsx (1)
60-60: Consistent with other navigation components, but consider upstream type fix.This type cast matches the pattern used in
nav-item.tsxand other navigation components. Since child routes inherit from the same route configuration structure, fixing the type definition at the source (in@/lib/routes/types) would eliminate the need for casting in all these locations.src/app/error.tsx (1)
27-29: Prefer static import over dynamic require().Using
require()to dynamically load@sentry/nextjsin a client component can lead to bundler issues and is not the standard Sentry pattern for Next.js. Since this is a client component and Sentry is already being integrated throughout the application, a static import is more reliable and idiomatic.♻️ Proposed refactor
+"use client"; + +import { useEffect } from "react"; +import { useTranslations } from "next-intl"; +import { captureException } from "@sentry/nextjs"; + // ... other imports ... export default function Error({ error, reset, }: { error: Error & { digest?: string }; reset: () => void; }) { const t = useTranslations(); useEffect(() => { - // Log the error to Sentry - const { captureException } = require("@sentry/nextjs"); captureException(error); }, [error]);src/lib/routes/i18n.ts (1)
92-92: Address the TODO or track it formally.The biome-ignore directive with a TODO indicates known technical debt. Consider either refactoring the protected route mapping to reduce cognitive complexity, or creating a tracked issue to address this later.
Would you like me to help refactor this function to reduce complexity, or create an issue to track this technical debt?
package.json (1)
88-88: Consider using lodash-es or specific imports for better tree-shaking.The full
lodashpackage is quite large (~530KB). Consider these alternatives:
- Use
lodash-esfor better tree-shaking with ES modules- Import only specific lodash functions you need (e.g.,
lodash.debounce,lodash.throttle)This helps reduce bundle size, especially important for client-side code.
src/lib/seo/json-ld.tsx (1)
7-13: Unicode line separator escapes are no-ops.Lines 12-13 replace the literal Unicode characters
\u2028and\u2029with the string representations\\u2028and\\u2029. However, the replacement strings are identical to what JSON.stringify already produces for these characters. This is harmless but redundant.Also, the order of replacements matters:
&should be escaped first to avoid double-escaping&in previously escaped sequences like\u003c.Suggested fix
const escapeJsonForHtml = (json: string): string => json + .replace(/&/g, "\\u0026") .replace(/</g, "\\u003c") .replace(/>/g, "\\u003e") - .replace(/&/g, "\\u0026") - .replace(/\u2028/g, "\\u2028") - .replace(/\u2029/g, "\\u2029"); + .replace(/\u2028/g, "\\u2028") + .replace(/\u2029/g, "\\u2029");src/lib/seo/metadata.ts (1)
2-2: Consider importing fromlodash/mergefor better tree-shaking.Importing from the main
lodashpackage bundles the entire library. Use a direct import for smaller bundle size.Suggested fix
-import { merge } from "lodash"; +import merge from "lodash/merge";src/app/layout.tsx (2)
31-37: Inconsistent merge strategy withcreatePageMetadata.This uses shallow spread (
...defaultMetadata) whilecreatePageMetadatainmetadata.tsuseslodash.mergefor deep merging. IfdefaultMetadata.othercontains existing fields, the spread would work correctly here, but consider using the samecreatePageMetadatahelper for consistency.Suggested fix using createPageMetadata
+import { createPageMetadata } from "@/lib/seo"; -import { defaultMetadata } from "@/lib/seo"; export async function generateMetadata(): Promise<Metadata> { const { getTraceData } = await import("@sentry/nextjs"); const traceData = getTraceData(); const validTraceData: Record<string, string> = {}; if (traceData["sentry-trace"]) { validTraceData["sentry-trace"] = traceData["sentry-trace"]; } if (traceData.baggage) { validTraceData.baggage = traceData.baggage; } if (traceData.traceparent) { validTraceData.traceparent = traceData.traceparent; } - return { - ...defaultMetadata, - other: { - ...defaultMetadata.other, - ...validTraceData, - }, - }; + return createPageMetadata({ + other: validTraceData, + }); }
14-29: Trace data filtering can be simplified.The manual filtering of undefined values is verbose. Consider using
Object.entrieswith a filter.Suggested simplification
export async function generateMetadata(): Promise<Metadata> { const { getTraceData } = await import("@sentry/nextjs"); const traceData = getTraceData(); - const validTraceData: Record<string, string> = {}; - if (traceData["sentry-trace"]) { - validTraceData["sentry-trace"] = traceData["sentry-trace"]; - } - if (traceData.baggage) { - validTraceData.baggage = traceData.baggage; - } - if (traceData.traceparent) { - validTraceData.traceparent = traceData.traceparent; - } + const validTraceData = Object.fromEntries( + Object.entries(traceData).filter(([, v]) => v !== undefined) + ) as Record<string, string>;src/instrumentation.ts (1)
18-31: Empty runtime-specific blocks could be removed or marked with TODOs.The
register()function contains empty conditional blocks with example comments. If these are placeholders for future implementation, consider adding explicit TODO comments. If they're not planned, they can be removed to reduce noise.Option 1: Add explicit TODOs
export function register() { if (process.env.NEXT_RUNTIME === "nodejs") { - // Node.js specific instrumentation - // Example: await import('./instrumentation-node') + // TODO: Add Node.js specific instrumentation (e.g., OpenTelemetry) } if (process.env.NEXT_RUNTIME === "edge") { - // Edge runtime specific instrumentation - // Example: await import('./instrumentation-edge') + // TODO: Add Edge runtime specific instrumentation } }src/lib/observability/span.ts (1)
12-23: Consider using dynamic import for consistency.Other files in this PR use
await import("@sentry/nextjs")for dynamic imports, but this file usesrequire(). While both work, dynamic imports are more consistent with the ES module style used elsewhere.Note: If this file needs to remain synchronous,
require()is acceptable. The current implementation works correctly.src/app/providers.tsx (1)
3-6: Consider moving Sentry initialization after imports.While initializing Sentry early is important, placing it before all other imports (including React) is unconventional and can make the code harder to maintain. The standard pattern is to group all imports together, then perform side-effect initialization.
♻️ Suggested refactor
"use client"; -import { initializeSentry } from "@/lib/observability/client"; - -// Initialize Sentry on client-side -initializeSentry(); - import type { ReactNode } from "react"; import { Toaster } from "sonner"; import Link from "next/link"; import { useRouter } from "next/navigation"; import { AuthQueryProvider } from "@daveyplate/better-auth-tanstack"; import { AuthUIProviderTanstack } from "@daveyplate/better-auth-ui/tanstack"; import { QueryClientProvider } from "@tanstack/react-query"; import { ThemeProvider } from "next-themes"; +import { initializeSentry } from "@/lib/observability/client"; import { CommandMenu } from "@/components/command-menu"; import { ReactQueryDevtools } from "@/components/dev-tools"; import { ErrorBoundary } from "@/components/error-boundary"; import { getQueryClient } from "@/lib/api/query-client"; import { useBetterAuthUILocalization } from "@/lib/auth/localization"; import { authClient } from "@/auth/client"; +// Initialize Sentry on client-side +initializeSentry(); + // Note: We use our own QueryClientProvider for general data fetching hooks. // AuthQueryProvider is still needed for Better Auth's TanStack Query integration.src/lib/observability/error.ts (1)
9-19: Improve type safety inparseError.The type guard at line 14 only checks that
messageexists in the object, but doesn't verify it's actually a string. Theas stringcast could produce unexpected results ifmessageis a number, boolean, or object.🔧 Suggested improvement
export const parseError = (error: unknown): string => { if (error instanceof Error) { return error.message; } - if (error && typeof error === "object" && "message" in error) { - return error.message as string; + if ( + error && + typeof error === "object" && + "message" in error && + typeof error.message === "string" + ) { + return error.message; } return String(error); };src/lib/observability/server.ts (2)
10-15: Consider removing unused regex constants.The regex constants
NEXT_STATIC_PATTERNandFAVICON_PATTERNare defined here but only used in theignoreTransactionsarray (lines 124-125), not in theignoreIncomingRequestscallback (lines 45-51). While this is not incorrect, it creates a slight inconsistency with the comment on line 9 stating "Regex constants for performance".♻️ Optional: Consolidate regex patterns
Consider moving these constants closer to their usage or documenting which patterns are for transactions vs. request filtering:
// Regex constants for performance const REQUEST_ABORTED_PATTERN = /Request aborted/; + +// Transaction filtering patterns const HEALTH_CHECK_PATTERN = /^GET \/health/; const API_HEALTH_PATTERN = /^GET \/api\/health/; const MONITORING_PATTERN = /^GET \/monitoring/; const NEXT_STATIC_PATTERN = /^GET \/_next\/static\//; const FAVICON_PATTERN = /^GET \/favicon/;
163-169: Consider documenting the sampling rate trade-offs.The sampling rates (10% in production, 100% in development) are reasonable for cost control, but consider documenting:
- The expected volume reduction and cost savings
- How this might impact debugging of rare production issues
- Whether critical paths (e.g., payment processing, user registration) need higher sampling
src/lib/observability/edge.ts (1)
17-23: Unused regex pattern definitions.Similar to the server configuration, several regex patterns are defined (lines 17-23) but not all are used in the callback functions. The patterns are only used in the
ignoreTransactionsarray (lines 100-106), not in theignoreIncomingRequestscallback (lines 46-52).This is consistent with the server.ts file but consider the same consolidation suggested there.
next.config.ts (1)
172-177: URL parsing error handling could be more robust.The try/catch block (lines 171-202) catches all errors during CSP configuration, including URL parsing failures. However, the catch block only logs a generic warning without details about which step failed.
♻️ Enhanced error logging
try { const dsnUrl = new URL(sentryDsn); const publicKey = dsnUrl.username; const projectId = dsnUrl.pathname.split("/")[1]; const orgDomain = dsnUrl.hostname; + + if (!publicKey || !projectId || !orgDomain) { + throw new Error("Invalid Sentry DSN format: missing required components"); + } const reportUri = `https://${orgDomain}/api/${projectId}/security/?sentry_key=${publicKey}&sentry_environment=${process.env.NODE_ENV}&sentry_release=${process.env.SENTRY_RELEASE || "unknown"}`; // ... rest of the code } catch (error) { - console.warn("Failed to configure CSP reporting:", error); + console.warn("Failed to configure CSP reporting:", error instanceof Error ? error.message : error); }src/lib/observability/fingerprinting.ts (1)
66-89: Import Sentry types from@sentry/typesinstead of defining custom interfaces.Lines 66-89 define custom
SentryEventandSentryHintinterfaces. The official types are available in the@sentry/typespackage (which is a dependency of the already-installed@sentry/nextjs@10.32.1). UsingEventandEventHintfrom@sentry/typeswould provide better type safety and reduce maintenance overhead:Suggested import
import type { Event, EventHint } from "@sentry/types";Then replace the custom interfaces with these official types.
src/lib/observability/cache.ts (1)
73-85: Cache hit detection may incorrectly report misses for falsy values.The fallback
const hit = options.hit ?? Boolean(result)will report a cache miss when the cached value is legitimately falsy (e.g.,0,"",false,null).Consider requiring explicit
hitparameter or using a more explicit check:Suggested improvement
- const hit = options.hit ?? Boolean(result); + // Prefer explicit hit parameter; fallback checks if result is not undefined + const hit = options.hit ?? (result !== undefined);Alternatively, document that callers should always provide
options.hitfor accurate metrics when caching falsy values.src/lib/observability/troubleshooting.ts (2)
67-74: Optional: Remove redundant optional chaining after guard clause.Lines 52-54 already verify that
span.setAttributeexists, so the optional chaining on lines 69, 73, and 74 is redundant. While harmless, removing it would be more consistent.♻️ Proposed refactor
// Set chunked attributes chunks.forEach((chunk, index) => { - span.setAttribute?.(`${key}.${index}`, chunk); + span.setAttribute(`${key}.${index}`, chunk); }); // Set metadata about the chunking - span.setAttribute?.(`${key}._chunks`, chunks.length.toString()); - span.setAttribute?.(`${key}._total_length`, value.length.toString()); + span.setAttribute(`${key}._chunks`, chunks.length.toString()); + span.setAttribute(`${key}._total_length`, value.length.toString());
97-99: Optional: Remove redundant optional chaining after guard clause.Similar to the previous comment, line 84 already verifies that
span.setAttributeexists, making the optional chaining on line 98 redundant.♻️ Proposed refactor
params.forEach((value, key) => { - span.setAttribute?.(`http.url.query.${key}`, value); + span.setAttribute(`http.url.query.${key}`, value); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (55)
.cursor/rules/sentry.mdc.vscode/extensions.json.vscode/settings.jsonbiome.jsoncnext.config.tspackage.jsonsrc/app/(dashboard)/app/(overview)/page.tsxsrc/app/error.tsxsrc/app/global-error.tsxsrc/app/layout.tsxsrc/app/providers.tsxsrc/components/command-menu.tsxsrc/components/layout/navigation/nav-collapsible.tsxsrc/components/layout/navigation/nav-item.tsxsrc/components/layout/sidebar/sidebar-user-section.tsxsrc/env.tssrc/instrumentation.tssrc/lib/api/server-queries.tssrc/lib/api/utils.tssrc/lib/auth/check-role.tssrc/lib/auth/config.tssrc/lib/auth/email.tssrc/lib/auth/keys.tssrc/lib/auth/server-client.tssrc/lib/db/client.tssrc/lib/db/keys.tssrc/lib/next-config/index.tssrc/lib/next-config/with-analyzer.tssrc/lib/next-config/with-observability.tssrc/lib/observability/cache.tssrc/lib/observability/client.tssrc/lib/observability/edge.tssrc/lib/observability/enrichment.tssrc/lib/observability/error.tssrc/lib/observability/fingerprinting.tssrc/lib/observability/http.tssrc/lib/observability/index.tssrc/lib/observability/keys.tssrc/lib/observability/levels.tssrc/lib/observability/log.tssrc/lib/observability/metrics.tssrc/lib/observability/queue.tssrc/lib/observability/sampling.tssrc/lib/observability/scopes.tssrc/lib/observability/server.tssrc/lib/observability/span.tssrc/lib/observability/trace.tssrc/lib/observability/troubleshooting.tssrc/lib/routes/i18n.tssrc/lib/routes/permissions.tssrc/lib/seo/json-ld.tsxsrc/lib/seo/metadata.tssrc/sentry.edge.config.tssrc/sentry.server.config.tstsconfig.json
🧰 Additional context used
🧠 Learnings (18)
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Images must include `width`/`height` and lazy-load when appropriate in frontend
Applied to files:
src/lib/next-config/index.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Remove redundant try/catch blocks (red flag in nuclear mode)
Applied to files:
src/app/error.tsxsrc/app/global-error.tsx
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: When working with Next.js projects, ALWAYS call `init` first to establish documentation-first context, list available runtime tools, and set up proper development environment
Applied to files:
src/sentry.edge.config.tssrc/lib/observability/edge.tssrc/lib/observability/server.tssrc/lib/observability/client.tssrc/sentry.server.config.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/config*.{ts,tsx,js,jsx} : ENV must be validated at startup in backend
Applied to files:
src/env.tssrc/lib/auth/config.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/backend/**/*.{ts,tsx,js,jsx} : Use a single HTTP client and single DB client (no parallel stacks) in backend
Applied to files:
src/lib/db/client.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*route*.{ts,tsx,js,jsx} : Clean, non-duplicated REST routes in backend code
Applied to files:
src/lib/routes/i18n.tsbiome.jsonc
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/error*.{ts,tsx,js,jsx} : Errors must have a uniform shape in backend
Applied to files:
src/lib/observability/error.tssrc/lib/api/utils.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Remove unused imports (nuclear mode)
Applied to files:
biome.jsonc
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx,css} : Spacing, typography, radii, z-index, and motion must come from tokens in design system
Applied to files:
biome.jsonc
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx,css} : Use Tailwind without arbitrary values in frontend
Applied to files:
biome.jsonc
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js projects requiring major upgrades or Cache Components setup, use specialized automation tools: `upgrade_nextjs_16` for version upgrades and `enable_cache_components` for Cache Components configuration
Applied to files:
package.json
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Server Components by default; `use client` only when essential in frontend
Applied to files:
src/lib/auth/server-client.tssrc/lib/auth/check-role.tssrc/sentry.server.config.tstsconfig.json
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/api/**/*.{ts,tsx,js,jsx} : API, DB, auth contracts, or tokens require mandatory review
Applied to files:
src/lib/api/utils.tstsconfig.json
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js development error handling, retry MCP calls and check server availability for network issues, use Better Auth session validation for authentication errors, and fall back to general knowledge if MCP is unavailable
Applied to files:
src/lib/api/utils.ts
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js 16+ runtime diagnostics and live application state inspection, use `nextjs_index` to discover dev servers and `nextjs_call` with toolName parameters like 'get_errors', 'get_page_metadata', or 'get_logs'
Applied to files:
src/instrumentation.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Maintain semantic HTML, reading order, contrast, and keyboard navigation in UI/UX work
Applied to files:
src/components/layout/navigation/nav-collapsible.tsxsrc/components/layout/navigation/nav-item.tsx
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : When using shadcn: composition > modification; no template copying
Applied to files:
tsconfig.json
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/backend/**/*.{ts,tsx,js,jsx} : Mandatory Zod validation at boundaries in backend
Applied to files:
tsconfig.json
🧬 Code graph analysis (18)
src/lib/observability/edge.ts (4)
src/lib/observability/client.ts (1)
initializeSentry(59-283)src/lib/observability/index.ts (1)
initializeSentry(9-9)src/lib/observability/server.ts (1)
initializeSentry(26-177)src/lib/observability/keys.ts (1)
keys(4-20)
src/lib/observability/server.ts (5)
src/lib/observability/client.ts (1)
initializeSentry(59-283)src/lib/observability/edge.ts (1)
initializeSentry(27-149)src/lib/observability/index.ts (1)
initializeSentry(9-9)src/env.ts (1)
env(7-12)src/lib/observability/keys.ts (1)
keys(4-20)
src/app/layout.tsx (5)
src/app/(dashboard)/app/(overview)/page.tsx (1)
generateMetadata(13-16)src/app/page.tsx (1)
generateMetadata(9-15)src/app/auth/[path]/page.tsx (1)
generateMetadata(14-77)src/app/(dashboard)/app/admin/page.tsx (1)
generateMetadata(17-20)src/app/metadata.ts (1)
defaultMetadata(11-11)
src/lib/observability/client.ts (6)
src/lib/observability/edge.ts (1)
initializeSentry(27-149)src/lib/observability/server.ts (1)
initializeSentry(26-177)src/env.ts (1)
env(7-12)src/lib/observability/keys.ts (1)
keys(4-20)src/lib/observability/troubleshooting.ts (1)
initializeTransactionSanitization(114-127)src/lib/observability/fingerprinting.ts (1)
initializeFingerprinting(170-200)
src/lib/db/client.ts (2)
src/env.ts (1)
env(7-12)src/lib/db/keys.ts (1)
keys(4-12)
src/lib/observability/error.ts (3)
src/lib/observability/index.ts (3)
parseError(19-19)captureError(19-19)log(32-32)src/app/error.tsx (1)
Error(17-72)src/lib/observability/log.ts (1)
log(75-75)
src/lib/observability/troubleshooting.ts (1)
src/lib/observability/index.ts (4)
sanitizeTransactionName(66-66)setLongAttribute(67-67)setUrlAttributes(68-68)initializeTransactionSanitization(65-65)
src/lib/observability/fingerprinting.ts (1)
src/lib/observability/index.ts (3)
setFingerprint(23-23)fingerprintPatterns(21-21)initializeFingerprinting(22-22)
src/lib/auth/config.ts (1)
src/lib/auth/keys.ts (1)
keys(4-14)
src/lib/observability/span.ts (1)
src/lib/observability/index.ts (5)
addSpanMetrics(57-57)createMetricSpan(59-59)createManualSpan(58-58)updateSpanName(61-61)spanMetrics(60-60)
src/lib/next-config/with-observability.ts (1)
src/env.ts (1)
env(7-12)
src/lib/observability/http.ts (1)
src/lib/observability/index.ts (2)
instrumentHttpRequest(25-25)httpClient(25-25)
src/instrumentation.ts (1)
src/env.ts (1)
env(7-12)
src/lib/observability/cache.ts (1)
src/lib/observability/index.ts (4)
instrumentCacheSet(7-7)instrumentCacheGet(6-6)calculateCacheItemSize(5-5)cacheConfigs(4-4)
src/lib/observability/sampling.ts (1)
src/lib/observability/index.ts (3)
portalSampler(47-47)samplingPatterns(48-48)errorSamplingRates(46-46)
src/lib/observability/levels.ts (1)
src/lib/observability/index.ts (4)
setLevel(30-30)captureMessageWithLevel(28-28)captureExceptionWithLevel(27-27)levelPatterns(29-29)
src/lib/observability/queue.ts (1)
src/lib/observability/trace.ts (1)
continueTrace(23-39)
next.config.ts (2)
src/lib/next-config/with-observability.ts (1)
withObservability(10-89)src/lib/next-config/with-analyzer.ts (1)
withAnalyzer(8-9)
🪛 ast-grep (0.40.4)
src/lib/seo/json-ld.tsx
[warning] 18-18: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
⏰ 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). (1)
- GitHub Check: Seer Code Review
| // Add CSP reporting headers if Sentry is configured | ||
| const sentryDsn = process.env.NEXT_PUBLIC_SENTRY_DSN; | ||
| if (sentryDsn) { | ||
| try { | ||
| const dsnUrl = new URL(sentryDsn); | ||
| const publicKey = dsnUrl.username; | ||
| const projectId = dsnUrl.pathname.split("/")[1]; | ||
| const orgDomain = dsnUrl.hostname; | ||
|
|
||
| const reportUri = `https://${orgDomain}/api/${projectId}/security/?sentry_key=${publicKey}&sentry_environment=${process.env.NODE_ENV}&sentry_release=${process.env.SENTRY_RELEASE || "unknown"}`; | ||
|
|
||
| // CSP Report-Only for monitoring without blocking | ||
| securityHeaders.push({ | ||
| key: "Content-Security-Policy-Report-Only", | ||
| value: `default-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline' https://js.sentry-cdn.com; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' https://${orgDomain} https://js.sentry-cdn.com; report-uri ${reportUri}; report-to csp-endpoint`, | ||
| }); | ||
|
|
||
| // Modern reporting headers | ||
| securityHeaders.push({ | ||
| key: "Report-To", | ||
| value: JSON.stringify({ | ||
| group: "csp-endpoint", | ||
| max_age: 10_886_400, | ||
| endpoints: [{ url: reportUri }], | ||
| include_subdomains: true, | ||
| }), | ||
| }); | ||
|
|
||
| securityHeaders.push({ | ||
| key: "Reporting-Endpoints", | ||
| value: `csp-endpoint="${reportUri}"`, | ||
| }); | ||
| } catch (error) { | ||
| console.warn("Failed to configure CSP reporting:", error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "next.config.ts" -o -name "next.config.js" | head -5Repository: allthingslinux/portal
Length of output: 81
🏁 Script executed:
git ls-files | grep -E "(next\.config|sentry)" | head -20Repository: allthingslinux/portal
Length of output: 158
🏁 Script executed:
cat -n next.config.ts | head -250Repository: allthingslinux/portal
Length of output: 11444
🏁 Script executed:
rg "eval\(" --type ts --type tsx --type js --type jsxRepository: allthingslinux/portal
Length of output: 92
🏁 Script executed:
rg "unsafe-eval|unsafe-inline" --type ts --type tsx --type js --type jsxRepository: allthingslinux/portal
Length of output: 92
🏁 Script executed:
cat -n src/sentry.server.config.tsRepository: allthingslinux/portal
Length of output: 428
🏁 Script executed:
cat -n src/sentry.edge.config.tsRepository: allthingslinux/portal
Length of output: 435
🏁 Script executed:
rg "process\.env\." next.config.tsRepository: allthingslinux/portal
Length of output: 429
🏁 Script executed:
rg "eval\(" --type js --type tsRepository: allthingslinux/portal
Length of output: 47
🏁 Script executed:
rg "unsafe-eval|unsafe-inline" --type js --type tsRepository: allthingslinux/portal
Length of output: 395
🏁 Script executed:
find . -path ./node_modules -prune -o -name "*observability*" -type f -printRepository: allthingslinux/portal
Length of output: 108
🏁 Script executed:
cat -n src/lib/observability/server.ts 2>/dev/null || cat -n src/lib/observability/server.js 2>/dev/nullRepository: allthingslinux/portal
Length of output: 6488
🏁 Script executed:
rg "dangerouslySetInnerHTML|<script" --type js --type tsRepository: allthingslinux/portal
Length of output: 218
🏁 Script executed:
cat -n .cursor/rules/sentry.mdcRepository: allthingslinux/portal
Length of output: 4884
🌐 Web query:
Sentry CSP requirements script-src policy Next.js
💡 Result:
Short summary and recommended policy:
-
If you bundle Sentry into your app (recommended for Next.js): you do NOT need to allow sentry CDN in script-src, but you must allow outgoing event/report endpoints in connect-src (your DSN host, e.g. o0.ingest.sentry.io). Also configure CSP reporting to send violation reports to Sentry if you want that. [1][2]
-
If you load Sentry from the browser CDN (e.g. https://browser.sentry-cdn.com): add that CDN origin to script-src (or bundle instead). Also allow the ingestion host in connect-src. [2][3]
-
For Next.js itself: Next.js emits inline/hydration scripts, so a strict script-src without nonces will break hydration. Use per-request nonces + 'strict-dynamic' (or as a fallback include https: / 'unsafe-inline' for older browsers). The recommended secure pattern is:
script-src 'nonce-' 'strict-dynamic' https: 'unsafe-inline';
generate a nonce in middleware, inject it into the CSP header and into script tags/Next.js headers. This lets Next.js internal scripts and dynamically loaded scripts run without opening unsafe-inline broadly. [1][4]
Minimal example CSP (set nonce dynamically in middleware and pass it to your pages):
Content-Security-Policy:
default-src 'self';
script-src 'nonce-{NONCE}' 'strict-dynamic' https://browser.sentry-cdn.com;
connect-src 'self' https://o0.ingest.sentry.io;
style-src 'self' 'unsafe-inline';
img-src 'self' data:;
report-uri https://o0.ingest.sentry.io/api/0/security/?sentry_key=;
Notes and links:
- Next.js guide: CSP + nonces and nonce generation in middleware. [1]
- Next.js App-router discussion: Next.js hydration requires nonces / strict-dynamic to avoid 'unsafe-inline'. [4]
- Sentry docs: CSP reporting endpoint / ingest domain and how to configure report-uri. [3]
- Sentry Next.js docs: recommended approach to include Sentry SDK (bundle or CDN) and DSN format. [2]
References:
[1] Next.js Content Security Policy guide (nonces & strict-dynamic).
[2] Sentry Next.js / manual setup docs.
[3] Sentry docs — Set Up Security Policy Reporting (report-uri / ingest host).
[4] Next.js discussion on CSP/hydration and nonce + strict-dynamic.
🌐 Web query:
Sentry JavaScript SDK CSP unsafe-eval unsafe-inline requirements
💡 Result:
Short answer: The Sentry browser/JavaScript SDK does NOT require 'unsafe-eval' or 'unsafe-inline' by default. You only need to allow the Sentry script host(s) in script-src and the Sentry ingest endpoints in connect-src; 'unsafe-eval' is only needed when your app or a dev/build tool (or a library you include) actually uses eval()/new Function(), and 'unsafe-inline' is only needed if you execute inline scripts or event handlers. (blog.sentry.io)
Practical CSP checklist
- script-src: include whichever Sentry CDN/host you load the SDK from (e.g. js.sentry-cdn.com / browser.sentry-cdn.com or your own host). (mintlify.com)
- connect-src: include your Sentry ingest URL (e.g. https://oXXXX.ingest.sentry.io). (forum.sentry.io)
- Do NOT add 'unsafe-eval' or 'unsafe-inline' just for Sentry; instead find the real consumer of eval/inline in your app and remove or replace it, or only enable those keywords as a last resort (or for older browsers). (blog.sentry.io)
If you see CSP reports referencing "inline" or "eval":
- Check which script/resource the browser reports as blocked (blocked_uri) and either add the exact host to script-src/connect-src or fix the offending code (avoid eval/inline). You can also configure Sentry to ignore specific CSP report values while you remediate. (stackoverflow.com)
If you want, tell me the exact CSP header you're using and the Sentry DSN/CDN host you load from and I’ll give the minimal CSP directives to allow Sentry without enabling unsafe-eval/unsafe-inline.
Citations:
- 1: https://blog.sentry.io/how-sentry-captures-csp-violations/?utm_source=openai
- 2: https://www.mintlify.com/docs/guides/csp-configuration?utm_source=openai
- 3: https://forum.sentry.io/t/required-content-security-policy/4484/2?utm_source=openai
- 4: https://blog.sentry.io/how-sentry-captures-csp-violations/?utm_source=openai
- 5: https://stackoverflow.com/questions/56130711/ignore-specific-csp-error-when-using-sentry?utm_source=openai
🏁 Script executed:
cat -n src/lib/seo/json-ld.tsx | head -50Repository: allthingslinux/portal
Length of output: 913
🏁 Script executed:
cat -n src/components/ui/chart.tsx | grep -A 10 "dangerouslySetInnerHTML"Repository: allthingslinux/portal
Length of output: 508
🏁 Script executed:
rg "instrumentationHook\|middleware" --type ts --type jsRepository: allthingslinux/portal
Length of output: 47
Remove 'unsafe-eval' and consider nonce-based CSP instead of 'unsafe-inline'.
The CSP on line 182 includes 'unsafe-eval' and 'unsafe-inline' in script-src, but:
'unsafe-eval'is unnecessary—no eval() usage found in the codebase'unsafe-inline'is used for legitimate purposes (JSON-LD metadata insrc/lib/seo/json-ld.tsxand CSS theme injection insrc/components/ui/chart.tsx), though with Report-Only mode active, violations are only reported, not blocked
For a stronger security posture aligned with Next.js + Turbopack best practices, replace 'unsafe-inline' with a per-request nonce and 'strict-dynamic':
- value: `default-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline' https://js.sentry-cdn.com; ...
+ value: `default-src 'self'; script-src 'nonce-${nonce}' 'strict-dynamic' https://js.sentry-cdn.com; ...This allows Next.js hydration and legitimate inline scripts via nonces without broadly permitting unsafe directives. Generate the nonce in middleware and inject it into the header. The Sentry SDK requires only the CDN host in script-src and ingest endpoints in connect-src—neither 'unsafe-eval' nor 'unsafe-inline' is required.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @next.config.ts around lines 168 - 203, The CSP currently includes
'unsafe-eval' and 'unsafe-inline' in the Content-Security-Policy-Report-Only
header generated in the securityHeaders array; remove both directives and switch
to a per-request nonce approach: generate a cryptographic nonce in your
middleware (e.g., middleware.ts), attach it to the request/response (e.g.,
res.locals or header like x-csp-nonce), and update next.config.ts where
securityHeaders is populated (the block creating the
"Content-Security-Policy-Report-Only" entry) to build script-src using the
per-request nonce and 'strict-dynamic' plus the Sentry CDN host (no
'unsafe-eval'/'unsafe-inline'); also ensure connect-src only lists Sentry ingest
endpoints and the org domain. Finally update inline usages
(src/lib/seo/json-ld.tsx and src/components/ui/chart.tsx) to read the nonce from
the request/response context and set it on their inline script/style tags so
they are allowed by the new CSP.
| useEffect(() => { | ||
| // Log error to error reporting service | ||
| console.error("Global error boundary caught:", error); | ||
| // Log the error to Sentry | ||
| const { captureException } = require("@sentry/nextjs"); | ||
| captureException(error); | ||
| }, [error]); |
There was a problem hiding this comment.
Add error handling for Sentry integration.
The require() call to dynamically load Sentry has no error handling. If @sentry/nextjs is unavailable or fails to load, this will throw an uncaught error in the error boundary itself, potentially causing a broken error experience for users.
🛡️ Recommended fix with error handling
useEffect(() => {
- // Log the error to Sentry
- const { captureException } = require("@sentry/nextjs");
- captureException(error);
+ // Log the error to Sentry
+ try {
+ const { captureException } = require("@sentry/nextjs");
+ captureException(error);
+ } catch (sentryError) {
+ // Fallback to console if Sentry is unavailable
+ console.error("Failed to report error to Sentry:", sentryError);
+ console.error("Original error:", error);
+ }
}, [error]);Alternatively, consider using dynamic import for better TypeScript support:
useEffect(() => {
- // Log the error to Sentry
- const { captureException } = require("@sentry/nextjs");
- captureException(error);
+ // Log the error to Sentry
+ import("@sentry/nextjs")
+ .then(({ captureException }) => {
+ captureException(error);
+ })
+ .catch((sentryError) => {
+ console.error("Failed to report error to Sentry:", sentryError);
+ console.error("Original error:", error);
+ });
}, [error]);📝 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.
| useEffect(() => { | |
| // Log error to error reporting service | |
| console.error("Global error boundary caught:", error); | |
| // Log the error to Sentry | |
| const { captureException } = require("@sentry/nextjs"); | |
| captureException(error); | |
| }, [error]); | |
| useEffect(() => { | |
| // Log the error to Sentry | |
| try { | |
| const { captureException } = require("@sentry/nextjs"); | |
| captureException(error); | |
| } catch (sentryError) { | |
| // Fallback to console if Sentry is unavailable | |
| console.error("Failed to report error to Sentry:", sentryError); | |
| console.error("Original error:", error); | |
| } | |
| }, [error]); |
| useEffect(() => { | |
| // Log error to error reporting service | |
| console.error("Global error boundary caught:", error); | |
| // Log the error to Sentry | |
| const { captureException } = require("@sentry/nextjs"); | |
| captureException(error); | |
| }, [error]); | |
| useEffect(() => { | |
| // Log the error to Sentry | |
| import("@sentry/nextjs") | |
| .then(({ captureException }) => { | |
| captureException(error); | |
| }) | |
| .catch((sentryError) => { | |
| console.error("Failed to report error to Sentry:", sentryError); | |
| console.error("Original error:", error); | |
| }); | |
| }, [error]); |
🤖 Prompt for AI Agents
In @src/app/global-error.tsx around lines 39 - 43, The useEffect that calls
captureException via require("@sentry/nextjs") can throw if Sentry isn’t
available; wrap the dynamic require/import in a try/catch (or use dynamic
import().catch) inside the useEffect, check that the returned module exposes
captureException before calling it, and on failure fall back to safe logging
(e.g., console.error) so the error boundary doesn’t itself throw; update the
effect around useEffect / captureException accordingly to handle and log any
load errors.
| <SidebarMenuItem> | ||
| <SidebarMenuButton asChild isActive={isActive} tooltip={route.label}> | ||
| <Link href={route.path}> | ||
| <Link href={route.path as Parameters<typeof Link>[0]["href"]}> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read the nav-item.tsx file to see the context around line 36
cat -n src/components/layout/navigation/nav-item.tsxRepository: allthingslinux/portal
Length of output: 1969
🏁 Script executed:
#!/bin/bash
# Find the ProtectedRoute type definition
rg -A 5 "type ProtectedRoute\|interface ProtectedRoute" --type=ts --type=tsxRepository: allthingslinux/portal
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Search for route type definitions and usage
rg "path\s*:\s*" --type=ts --type=tsx -A 1 | head -30Repository: allthingslinux/portal
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Find the routes/types file
fd "types" -type f | rg "routes"Repository: allthingslinux/portal
Length of output: 235
🏁 Script executed:
#!/bin/bash
# List files in lib/routes directory
find . -path "*/lib/routes/*" -type fRepository: allthingslinux/portal
Length of output: 313
🏁 Script executed:
#!/bin/bash
# Search for ProtectedRoute type definition
rg "type ProtectedRoute\|interface ProtectedRoute" -A 10Repository: allthingslinux/portal
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check the path property in route definitions
rg "path\s*:" -B 2 -A 1 | head -40Repository: allthingslinux/portal
Length of output: 1339
🏁 Script executed:
#!/bin/bash
# Read the types.ts file to see ProtectedRoute definition
cat -n src/lib/routes/types.tsRepository: allthingslinux/portal
Length of output: 3406
🏁 Script executed:
#!/bin/bash
# Check the route config to see what paths are actually being used
head -50 src/lib/routes/config.tsRepository: allthingslinux/portal
Length of output: 1152
🏁 Script executed:
#!/bin/bash
# Check how Next.js Link is used elsewhere in the codebase without casting
rg "Link href=" -B 2 -A 2 | head -40Repository: allthingslinux/portal
Length of output: 2737
Remove the unnecessary type assertion.
The type assertion as Parameters<typeof Link>[0]["href"] is redundant. Since route.path is already typed as string (from BaseRoute), and Next.js Link accepts href as a string, the types already align. Simply pass route.path directly without casting:
<Link href={route.path}>The same unnecessary casting appears in src/components/layout/navigation/nav-collapsible.tsx and should be removed there as well.
🤖 Prompt for AI Agents
In @src/components/layout/navigation/nav-item.tsx at line 36, Remove the
redundant type assertion on the Link href: replace usages like Link
href={route.path as Parameters<typeof Link>[0]["href"]} with Link
href={route.path}. Update this in the NavItem and NavCollapsible components (the
Link component usage and any references to route.path from BaseRoute) so the
plain typed string is passed directly without the unnecessary cast.
| .replace( | ||
| /\/[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/g, | ||
| "/<uuid>" | ||
| ) | ||
| // Replace hash-like strings (32+ hex chars) | ||
| .replace(/\/[a-f0-9]{32,}/g, "/<hash>") |
There was a problem hiding this comment.
Regex patterns should match case-insensitive hex digits.
The UUID and hash patterns only match lowercase hex digits [a-f0-9]. UUIDs and hashes can contain uppercase letters (A-F) and should be matched case-insensitively to avoid missing valid identifiers.
🔧 Suggested fix
// Replace UUIDs with placeholder
.replace(
- /\/[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/g,
+ /\/[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}/gi,
"/<uuid>"
)
// Replace hash-like strings (32+ hex chars)
- .replace(/\/[a-f0-9]{32,}/g, "/<hash>")
+ .replace(/\/[a-fA-F0-9]{32,}/gi, "/<hash>")📝 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.
| .replace( | |
| /\/[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/g, | |
| "/<uuid>" | |
| ) | |
| // Replace hash-like strings (32+ hex chars) | |
| .replace(/\/[a-f0-9]{32,}/g, "/<hash>") | |
| .replace( | |
| /\/[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}/gi, | |
| "/<uuid>" | |
| ) | |
| // Replace hash-like strings (32+ hex chars) | |
| .replace(/\/[a-fA-F0-9]{32,}/gi, "/<hash>") |
🤖 Prompt for AI Agents
In @src/lib/observability/troubleshooting.ts around lines 20 - 25, The two
regexes in observability/troubleshooting.ts used in the replace chain (the UUID
replace and the hash-like string replace) only match lowercase hex digits;
update them to be case-insensitive so uppercase A-F are matched (either by
adding the i flag to both regex literals or by expanding the character classes
to [A-Fa-f0-9]) and keep the same replacement tokens ("/<uuid>" and "/<hash>")
so UUIDs and hashes with uppercase hex are properly normalized.
Reviewer's GuideIntegrates a comprehensive observability stack (Sentry for server/client/edge, sampling, metrics, spans, logging), centralizes env management with t3-env, and wires it into Next.js config, API error handling, and app metadata, plus adds a bundle analyzer and some small type/formatting fixes. Sequence diagram for API error handling with Sentry observabilitysequenceDiagram
actor Client
participant APIHandler
participant handleAPIError
participant captureError
participant log
Client->>APIHandler: HTTP request
APIHandler->>APIHandler: Business logic
APIHandler-->>APIHandler: Error thrown
APIHandler->>handleAPIError: handleAPIError(error)
alt error is APIError
handleAPIError-->>Client: JSON { ok: false, error } with APIError.status
else error is other
handleAPIError->>captureError: captureError(error, tags: errorType api)
captureError->>log: log.error("Error captured", parsed message)
handleAPIError-->>Client: JSON { ok: false, error } with 500
end
Sequence diagram for Next.js request error capture instrumentationsequenceDiagram
participant NextServer
participant onRequestError
participant keys
participant Sentry as captureRequestError
NextServer-->>NextServer: Request handling error occurs
NextServer->>onRequestError: onRequestError(error, request, context)
onRequestError->>keys: keys()
keys-->>onRequestError: env with NEXT_PUBLIC_SENTRY_DSN
alt Sentry DSN configured
onRequestError->>Sentry: captureRequestError(error, requestInfo, context)
Sentry-->>onRequestError: Recorded in Sentry
else Sentry not configured
onRequestError-->>NextServer: No-op
end
Class diagram for env and observability key managementclassDiagram
class AuthKeys {
+keys() createEnv
<<env module>>
}
class DbKeys {
+keys() createEnv
<<env module>>
}
class ObservabilityKeys {
+keys() createEnv
<<env module>>
}
class EnvAggregator {
+env createEnv
<<env aggregator>>
}
class ObservabilityConfig {
+initializeSentryClient() init
+withObservability(sourceConfig: NextConfig) NextConfig
<<observability>>
}
EnvAggregator --> AuthKeys : extends auth()
EnvAggregator --> DbKeys : extends database()
EnvAggregator --> ObservabilityKeys : extends observability()
ObservabilityConfig --> ObservabilityKeys : reads SENTRY_* and NEXT_PUBLIC_SENTRY_DSN
ObservabilityConfig --> EnvAggregator : can use env
class NextConfigPipeline {
+withNextIntl(config: NextConfig) NextConfig
+withObservability(config: NextConfig) NextConfig
+withAnalyzer(config: NextConfig) NextConfig
}
NextConfigPipeline --> ObservabilityConfig : applies withObservability
NextConfigPipeline --> EnvAggregator : indirect env usage
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 7 issues, and left some high level feedback:
- In
next.config.tsyou’re still readingprocess.env.NEXT_PUBLIC_SENTRY_DSNdirectly for CSP reporting while the rest of the code uses the t3-env helpers; consider using the samekeys()/envhelper there so all Sentry configuration consistently flows through the same validation layer. - Several observability helpers (
client.ts,server.ts,edge.ts,instrumentation.ts) callkeys()each time they run; sincecreateEnvdoes work on each invocation, consider caching the result in a module-level constant to avoid recomputing the env schema on hot paths like error handling. - There are many scattered
require("@sentry/nextjs")calls across the observability utilities for individual functions; you might want to centralize Sentry access behind a small wrapper module so that mocking, tree-shaking, and future SDK upgrades are easier to manage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `next.config.ts` you’re still reading `process.env.NEXT_PUBLIC_SENTRY_DSN` directly for CSP reporting while the rest of the code uses the t3-env helpers; consider using the same `keys()`/`env` helper there so all Sentry configuration consistently flows through the same validation layer.
- Several observability helpers (`client.ts`, `server.ts`, `edge.ts`, `instrumentation.ts`) call `keys()` each time they run; since `createEnv` does work on each invocation, consider caching the result in a module-level constant to avoid recomputing the env schema on hot paths like error handling.
- There are many scattered `require("@sentry/nextjs")` calls across the observability utilities for individual functions; you might want to centralize Sentry access behind a small wrapper module so that mocking, tree-shaking, and future SDK upgrades are easier to manage.
## Individual Comments
### Comment 1
<location> `src/lib/api/utils.ts:111-105` </location>
<code_context>
- console.error("Unknown error:", error);
- return Response.json({ error: "Internal server error" }, { status: 500 });
+ const message = parseError(error);
+ captureError(error, {
+ tags: {
+ errorType: "api",
+ },
+ });
+ log.error(`API Error: ${message}`);
+
+ return Response.json(
+ { ok: false, error: message || "Internal server error" },
+ { status: 500 }
</code_context>
<issue_to_address>
**🚨 issue (security):** Consider avoiding returning fully parsed error messages to clients to prevent leaking internal details.
This now calls `parseError(error)` for all non-`APIError` cases and returns that string in the JSON response. If `error` is an arbitrary object (DB error, auth error, request object, etc.), `parseError` will eventually do `String(error)`, which can leak internal details or object representations.
Instead, keep the detailed `captureError`/`log` behavior for observability, but return only a generic message (or a whitelisted safe message) to clients for non-`APIError` cases, e.g.:
```ts
const message = parseError(error);
captureError(error, { tags: { errorType: "api" } });
log.error(`API Error: ${message}`);
return Response.json(
{ ok: false, error: "Internal server error" },
{ status: 500 }
);
```
Only expose `message` when you know the error type is safe to surface.
</issue_to_address>
### Comment 2
<location> `src/lib/observability/client.ts:59` </location>
<code_context>
+ }
+};
+
+export const initializeSentry = (): ReturnType<typeof init> => {
+ const env = keys();
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the Sentry initializer by extracting shared sampling and integration-building helpers so the top-level configuration is smaller and easier to read.
You can simplify this initializer a lot by extracting the implicit policies into small helpers and by reusing the shared sampler logic instead of hard‑coding it here.
### 1. Delegate `tracesSampler` to a shared sampler
Right now the `tracesSampler` logic is all inline and partially duplicates the “portal sampler” mentioned by the other reviewer. If `sampling.ts` already encapsulates the same policies, delegate to it and only pass environment knobs from here:
```ts
// sampling.ts
export type PortalSamplerConfig = {
isProduction: boolean;
};
export function portalSampler(
samplingContext: Sentry.TracesSamplerContext,
{ isProduction }: PortalSamplerConfig,
): number {
const { name, inheritOrSampleWith } = samplingContext;
if (name.includes("health") || name.includes("metrics")) return 0;
if (
name.includes("auth") ||
name.includes("login") ||
name.includes("signup")
) {
return 1;
}
if (name.includes("/api/")) return isProduction ? 0.3 : 1;
if (name.includes("/_next/") || name.includes("/favicon")) {
return isProduction ? 0.01 : 0.1;
}
return inheritOrSampleWith(isProduction ? 0.1 : 1);
}
```
Then in this file:
```ts
import { portalSampler } from "./sampling";
export const initializeSentry = (): ReturnType<typeof init> => {
const env = keys();
if (!env.NEXT_PUBLIC_SENTRY_DSN) return undefined as ReturnType<typeof init>;
const isProduction = process.env.NODE_ENV === "production";
// ...
return init({
// ...
tracesSampler: (samplingContext) =>
portalSampler(samplingContext, { isProduction }),
// ...
});
};
```
This keeps all behavior but removes a large, hard‑to‑scan function from the initializer.
### 2. Extract integration setup into a helper
The `integrations` array has a lot of inline logic (including try/catch and environment‑based knobs). Pulling that into a dedicated function makes `initializeSentry` much easier to read:
```ts
const buildIntegrations = (isProduction: boolean) => {
const integrations = [
consoleLoggingIntegration({ levels: ["log", "error", "warn"] }),
httpClientIntegration({
failedRequestStatusCodes: [[400, 599]],
failedRequestTargets: [API_ROUTE_REGEX, ATL_DOMAINS_REGEX],
}),
reportingObserverIntegration({
types: ["crash", "deprecation", "intervention"],
}),
zodErrorsIntegration({ limit: 10 }),
];
try {
const { extraErrorDataIntegration } = require("@sentry/nextjs");
integrations.push(
extraErrorDataIntegration({
depth: 5,
captureErrorCause: true,
}),
);
} catch {
// ignore
}
try {
const {
browserTracingIntegration,
browserProfilingIntegration,
} = require("@sentry/nextjs");
integrations.push(
browserTracingIntegration({
shouldCreateSpanForRequest: (url: string) =>
!url.match(HEALTH_METRICS_REGEX),
ignoreResourceSpans: ["resource.css", "resource.font"],
enableInp: true,
interactionsSampleRate: isProduction ? 0.1 : 1,
}),
browserProfilingIntegration(),
);
} catch {
// ignore
}
const replay = getReplayIntegration();
if (replay) integrations.unshift(replay);
return { integrations, replay };
};
```
Usage:
```ts
export const initializeSentry = (): ReturnType<typeof init> => {
const env = keys();
if (!env.NEXT_PUBLIC_SENTRY_DSN) return undefined as ReturnType<typeof init>;
const isProduction = process.env.NODE_ENV === "production";
const { integrations, replay } = buildIntegrations(isProduction);
initializeTransactionSanitization();
initializeFingerprinting();
return init({
// ...
...(replay && {
replaysOnErrorSampleRate: 1,
replaysSessionSampleRate: isProduction ? 0.01 : 0.1,
}),
integrations,
});
};
```
This keeps all current behavior (including the dynamic `require`s and replay injection order) but makes the top‑level initializer mostly configuration.
### 3. Centralize environment‑specific knobs
You currently compute `isProduction` and then inline several ternaries. Collect those into a small config helper so you only define them once and the policies are easy to tweak:
```ts
const buildEnvSamplingConfig = (isProduction: boolean) => ({
defaultTraceSampleRate: isProduction ? 0.1 : 1,
interactionSampleRate: isProduction ? 0.1 : 1,
replaySessionSampleRate: isProduction ? 0.01 : 0.1,
profileSessionSampleRate: isProduction ? 0.1 : 1,
});
```
Applied:
```ts
export const initializeSentry = (): ReturnType<typeof init> => {
// ...
const isProduction = process.env.NODE_ENV === "production";
const samplingConfig = buildEnvSamplingConfig(isProduction);
const { integrations, replay } = buildIntegrations(isProduction);
return init({
// ...
// if you keep inline tracesSampler instead of portalSampler:
tracesSampler: (ctx) =>
portalSampler(ctx, {
isProduction,
// you can pass samplingConfig.defaultTraceSampleRate if needed
}),
profileSessionSampleRate: samplingConfig.profileSessionSampleRate,
...(replay && {
replaysOnErrorSampleRate: 1,
replaysSessionSampleRate: samplingConfig.replaySessionSampleRate,
}),
integrations,
});
};
```
This doesn’t change behavior but reduces scattered ternary logic and makes it trivial to adjust per‑environment sampling later.
</issue_to_address>
### Comment 3
<location> `src/lib/observability/sampling.ts:72` </location>
<code_context>
+ * Portal's intelligent sampling function
+ * Used in client configuration - extracted for reusability
+ */
+export const portalSampler = (isProduction: boolean) => {
+ return (samplingContext: SamplingContext): number => {
+ const { name, attributes, inheritOrSampleWith } = samplingContext;
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing all Sentry sampling behavior through `portalSampler` and removing unused helpers to shrink the number of concepts future readers must understand.
You can reduce the conceptual surface area by making `portalSampler` the single source of truth for sampling and trimming unused abstractions.
**1. Use `portalSampler` directly in `client.ts` instead of reimplementing sampling**
In `client.ts` (or wherever the Sentry client is configured), replace any inline `tracesSampler` logic that mirrors this module with a call to `portalSampler`:
```ts
// client.ts
import { portalSampler } from "./path/to/portalSampler";
// Example: Sentry.init(...)
Sentry.init({
dsn: "...",
tracesSampler: (samplingContext) => {
const isProduction = process.env.NODE_ENV === "production";
return portalSampler(isProduction)({
name: samplingContext.transactionContext.name,
attributes: samplingContext.transactionContext.data,
parentSampled: samplingContext.parentSampled,
parentSampleRate: samplingContext.parentSampleRate,
inheritOrSampleWith: (fallbackRate: number) =>
samplingContext.parentSampled ?? Math.random() < fallbackRate ? 1 : 0,
});
},
});
```
This way the helpers (`shouldSkipTransaction`, `isCriticalAuthFlow`, etc.) genuinely centralize the logic rather than duplicating it.
**2. Trim speculative abstractions if they are unused**
If `samplingPatterns` and `errorSamplingRates` are not currently referenced, remove or inline them later when actually needed. For example, you can start by only exporting what’s actively used:
```ts
// sampling.ts
// Keep this if it's used:
export const portalSampler = (isProduction: boolean) => { /* ... */ };
// Remove for now if unused:
const samplingPatterns = { /* ... */ };
const errorSamplingRates = { /* ... */ };
// export them only when you actually plug them into Sentry config
```
or, if you want a single “mode” helper, keep it minimal and explicit:
```ts
export const samplingPatterns = {
production: portalSampler(true),
development: portalSampler(false),
};
```
This keeps functionality intact while reducing the number of parallel concepts you need to reason about.
</issue_to_address>
### Comment 4
<location> `src/lib/observability/scopes.ts:129` </location>
<code_context>
+/**
+ * Common scope patterns for Portal
+ */
+export const scopePatterns = {
+ // Set user context for request
+ userContext: (user: { id: string; email?: string; tier?: string }) =>
</code_context>
<issue_to_address>
**issue (complexity):** Consider changing `scopePatterns` to accept a work callback and pass it through to the underlying scope helpers instead of invoking them with empty functions that do nothing.
The `scopePatterns` helpers are currently adding an extra layer without meaningful behavior: they invoke `withIsolatedScope` / `withLocalScope` but then run an empty callback. This makes the API look richer while not actually scoping any caller logic.
You can keep the abstraction but make it do something useful by letting callers pass their work function through, and remove the “empty body” pattern.
For example:
```ts
export const scopePatterns = {
// Set user context for request
userContext: <T>(
user: { id: string; email?: string; tier?: string },
fn: () => T,
): T =>
withIsolatedScope({ user }, fn),
// Set API request context
apiContext: <T>(
endpoint: string,
method: string,
fn: () => T,
userId?: string,
): T =>
withLocalScope(
{
tags: { endpoint, method, ...(userId && { user_id: userId }) },
context: { api: { endpoint, method } },
},
fn,
),
// Set background job context
jobContext: <T>(
jobName: string,
jobId: string,
fn: () => T,
): T =>
withIsolatedScope(
{
tags: { job_name: jobName, job_id: jobId },
extra: { job: { name: jobName, id: jobId } },
},
fn,
),
};
```
Call sites then become straightforward and the layering is clearer:
```ts
return scopePatterns.apiContext("/users", "POST", () => {
// actual API handler logic here
});
```
This keeps all your existing utilities (`setGlobalData`, `withIsolatedScope`, `withLocalScope`) while making `scopePatterns` a thin, meaningful domain-specific wrapper instead of an over-abstracted no-op layer.
</issue_to_address>
### Comment 5
<location> `src/lib/observability/queue.ts:63` </location>
<code_context>
+/**
+ * Instrument queue message consumption
+ */
+export const instrumentQueueConsumer = async <T>(
+ options: QueueConsumerOptions,
+ traceHeaders: { "sentry-trace"?: string; baggage?: string },
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared types and helper functions (for trace headers, consumer attributes, and status-wrapped spans) to simplify and clarify the queue instrumentation code, especially in `instrumentQueueConsumer`.
You can reduce complexity without changing behavior by extracting shared types and small helpers, especially in `instrumentQueueConsumer`.
**1. Extract a shared `TraceHeaders` type**
This removes repeated inline types and makes signatures easier to read:
```ts
type TraceHeaders = {
"sentry-trace"?: string;
baggage?: string;
};
interface QueueMessage {
id: string;
body: unknown;
timestamp: number;
retryCount?: number;
}
```
Then use it everywhere:
```ts
export const instrumentQueueProducer = async <T>(
options: QueueProducerOptions,
producer: (traceHeaders: TraceHeaders) => Promise<T>
): Promise<T> => { /* ... */ };
export const instrumentQueueConsumer = async <T>(
options: QueueConsumerOptions,
traceHeaders: TraceHeaders,
consumer: () => Promise<T>
): Promise<T> => { /* ... */ };
export const createQueueMessage = (
id: string,
body: unknown,
traceHeaders: TraceHeaders
): QueueMessage & { sentryTrace?: string; sentryBaggage?: string } => { /* ... */ };
```
**2. Extract attribute construction for the consumer**
Moving the attribute object into a helper makes the core tracing logic easier to follow and avoids inline spread logic:
```ts
const buildQueueConsumerAttributes = (options: QueueConsumerOptions) => ({
"messaging.message.id": options.messageId,
"messaging.destination.name": options.queueName,
"messaging.message.body.size": options.messageSize,
...(options.retryCount !== undefined && {
"messaging.message.retry.count": options.retryCount,
}),
...(options.receiveLatency !== undefined && {
"messaging.message.receive.latency": options.receiveLatency,
}),
});
```
Then:
```ts
const result = await startSpan(
{
name: "queue_consumer",
op: "queue.process",
attributes: buildQueueConsumerAttributes(options),
},
consumer
);
```
**3. Extract the “status-wrapped span” pattern**
The nested `startSpan` with an inline type for `parent` plus the try/catch makes the core behavior harder to scan. You can keep the same span structure but centralize the status handling:
```ts
interface StatusSpan {
setAttribute: (key: string, value: unknown) => void;
setStatus?: (status: { code: number; message: string }) => void;
}
const withStatusSpan = async <T>(
startSpan: (
options: { name: string },
fn: (span: StatusSpan) => Promise<T>
) => Promise<T>,
fn: (span: StatusSpan) => Promise<T>
): Promise<T> => {
return startSpan({ name: "queue_consumer_transaction" }, async span => {
try {
const result = await fn(span);
span.setStatus?.({ code: 1, message: "ok" });
return result;
} catch (error) {
span.setStatus?.({ code: 2, message: "error" });
throw error;
}
});
};
```
Usage inside `instrumentQueueConsumer`:
```ts
return await continueTrace(
{ sentryTrace: traceHeaders["sentry-trace"], baggage: traceHeaders.baggage },
async () => {
return await withStatusSpan(startSpan, async () => {
return await startSpan(
{
name: "queue_consumer",
op: "queue.process",
attributes: buildQueueConsumerAttributes(options),
},
consumer
);
});
}
);
```
This keeps the same span structure and status behavior but makes the main function read as a high-level flow: `continueTrace → withStatusSpan → processing span`, with the noisy details pushed into focused helpers.
</issue_to_address>
### Comment 6
<location> `src/lib/observability/cache.ts:23` </location>
<code_context>
+/**
+ * Instrument cache set operations
+ */
+export const instrumentCacheSet = async <T>(
+ options: CacheSetOptions,
+ setter: () => Promise<T> | T
</code_context>
<issue_to_address>
**issue (complexity):** Consider introducing a small shared helper API and extracting common key/attribute logic to simplify cache instrumentation usage and make each function’s responsibilities clearer.
You can keep all current functionality but reduce cognitive overhead by:
1. **Introducing a simple, higher-level API** for common use cases.
2. **Deduplicating key/attribute logic** shared by `instrumentCacheSet` and `instrumentCacheGet`.
### 1. Simple `withCacheSpan` wrapper
Most callers probably only care about `op`, `key`, and the function. You can provide a tiny wrapper that hides the options objects in the common case and still reuses the existing implementation.
```ts
type CacheOp = "get" | "set";
export const withCacheSpan = async <T>(
op: CacheOp,
key: string | string[],
fn: () => Promise<T> | T,
): Promise<T> => {
if (op === "set") {
return instrumentCacheSet({ key }, fn);
}
return instrumentCacheGet({ key }, fn);
};
```
Call sites that don’t need `hit`, `itemSize`, or `address` can now use:
```ts
await withCacheSpan("get", cacheKey, () => cache.get(cacheKey));
await withCacheSpan("set", cacheKey, () => cache.set(cacheKey, value));
```
This keeps your existing generalized options API available for advanced use, but offers a low-ceremony path for most code.
### 2. Extract shared key/attribute logic
`instrumentCacheSet` and `instrumentCacheGet` both:
- Normalize `options.key` into a primary key and an array key list
- Build overlapping attribute maps (`cache.key`, `network.peer.*`)
You can centralize that to reduce duplication and make the behavior easier to reason about:
```ts
const normalizeKey = (key: string | string[]) => {
const keys = Array.isArray(key) ? key : [key];
return { primaryKey: keys[0], keys };
};
const baseCacheAttributes = (options: CacheOptions) => {
const { keys } = normalizeKey(options.key);
return {
"cache.key": keys,
...(options.address && { "network.peer.address": options.address }),
...(options.port && { "network.peer.port": options.port }),
};
};
```
Then the two functions become more focused:
```ts
export const instrumentCacheSet = async <T>(
options: CacheSetOptions,
setter: () => Promise<T> | T,
): Promise<T> => {
try {
const { startSpan } = require("@sentry/nextjs");
const { primaryKey } = normalizeKey(options.key);
return await startSpan(
{
name: `cache.set ${primaryKey}`,
op: "cache.put",
attributes: {
...baseCacheAttributes(options),
...(options.itemSize && { "cache.item_size": options.itemSize }),
},
},
setter,
);
} catch {
return await setter();
}
};
export const instrumentCacheGet = async <T>(
options: CacheGetOptions,
getter: () => Promise<T> | T,
): Promise<T> => {
try {
const { startSpan } = require("@sentry/nextjs");
const { primaryKey } = normalizeKey(options.key);
return await startSpan(
{
name: `cache.get ${primaryKey}`,
op: "cache.get",
attributes: baseCacheAttributes(options),
},
async (span: { setAttribute: (key: string, value: unknown) => void }) => {
const result = await getter();
const hit = options.hit ?? Boolean(result);
span.setAttribute("cache.hit", hit);
if (hit && options.itemSize) {
span.setAttribute("cache.item_size", options.itemSize);
}
return result;
},
);
} catch {
return await getter();
}
};
```
This:
- Reduces repeated logic around `key` handling and shared attributes.
- Makes each function’s unique behavior (set vs get semantics) more obvious.
- Preserves all current capabilities and public APIs while offering a simpler, opt-in call pattern.
</issue_to_address>
### Comment 7
<location> `src/lib/observability/http.ts:84` </location>
<code_context>
+/**
+ * Common HTTP methods with instrumentation
+ */
+export const httpClient = {
+ get: <T>(url: string, fetcher: () => Promise<T>) =>
+ instrumentHttpRequest({ method: "GET", url }, fetcher),
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared option-building into a helper so the httpClient methods only map inputs to options while leaving behaviour in instrumentHttpRequest.
You can keep the instrumentation behaviour as-is but reduce the “mini client” complexity and duplication in `httpClient` by centralising the common option-building logic. That keeps the API surface unchanged (`get/post/put/delete` still exist) while reducing the mental overhead and chance of inconsistencies.
For example:
```ts
type HttpMethod = "GET" | "POST" | "PUT" | "DELETE";
const buildOptions = (
method: HttpMethod,
url: string,
body?: unknown
): HttpRequestOptions => ({
method,
url,
...(body !== undefined && {
requestSize: JSON.stringify(body).length,
}),
});
export const httpClient = {
get: <T>(url: string, fetcher: () => Promise<T>) =>
instrumentHttpRequest(buildOptions("GET", url), fetcher),
post: <T>(url: string, body: unknown, fetcher: () => Promise<T>) =>
instrumentHttpRequest(buildOptions("POST", url, body), fetcher),
put: <T>(url: string, body: unknown, fetcher: () => Promise<T>) =>
instrumentHttpRequest(buildOptions("PUT", url, body), fetcher),
delete: <T>(url: string, fetcher: () => Promise<T>) =>
instrumentHttpRequest(buildOptions("DELETE", url), fetcher),
};
```
This keeps all existing functionality (including `requestSize` handling) but makes the abstraction thinner and easier to understand: `httpClient` becomes a very small layer that only does method/body → options mapping, and all the actual behaviour lives in one place (`instrumentHttpRequest`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…variable - Changed the Sentry DSN in the configuration to utilize `process.env.SENTRY_DSN`, allowing for better security and flexibility in different environments. - Added a fallback option for the DSN to ensure functionality during development.
- Reformatted the command arguments in `mcp.json` to use a multi-line array for improved readability and consistency across different entries. - Updated the `next-devtools` command to reference the correct package name, ensuring accurate functionality.
- Updated error responses in API routes for GET and DELETE methods to include an `ok: false` field, providing a consistent structure across the application. - Modified routes for API keys, sessions, users, and user profile to enhance clarity in error handling.
…nism - Updated error handling in `Error` and `GlobalError` components to include a try-catch block around Sentry's `captureException`, ensuring that errors are logged even if Sentry is unavailable. - Refactored `generateMetadata` function in `layout.tsx` to use `Object.fromEntries` for filtering undefined trace data and implemented `createPageMetadata` for consistent metadata merging.
- Updated the `handleAPIError` function to return a generic error message, ensuring sensitive internal details are not leaked in API responses. - Added comments to clarify that detailed error information is logged in Sentry for debugging purposes.
… and compatibility - Changed `authToken` to use `env.SENTRY_AUTH_TOKEN` for consistency. - Added fallback for `release.name` to default to "unknown" if not provided. - Updated Webpack tree-shaking settings to replace deprecated options and ensure proper logging behavior. - Merged existing transpile packages to maintain compatibility with `@sentry/nextjs`.
…d key handling - Introduced `normalizeKey` function to standardize cache key processing. - Updated `baseCacheAttributes` to streamline cache attribute construction for both get and set operations. - Refactored `instrumentCacheSet` and `instrumentCacheGet` to utilize normalized keys and improved attribute handling, ensuring consistent cache operation logging.
…TP client documentation - Introduced a new function to safely calculate the size of a request body by stringifying it, returning undefined if serialization fails. - Enhanced the HTTP client methods with detailed documentation, including examples for GET, POST, PUT, and DELETE requests, to improve usability and clarity for developers.
…ility configuration - Implemented a new function to validate observability environment variables using t3-env, ensuring type safety and runtime validation. - This enhancement provides a structured approach to managing environment configurations for observability, specifically for Sentry integration.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/lib/observability/cache.ts:
- Around line 43-66: The startSpan callback in instrumentCacheSet must accept a
Span parameter but setter is typed as () => Promise<T> | T; wrap the existing
setter when calling startSpan so the callback matches the expected signature
(e.g., pass (span) => setter()) while keeping the existing attributes and catch
fallback behavior; update the call site in instrumentCacheSet that invokes
startSpan to use the wrapped callback so TypeScript correctly recognizes the
(span: Span) => T signature.
In @src/lib/observability/http.ts:
- Around line 14-20: The file contains two duplicate declarations of
calculateBodySize which causes the first to be shadowed and is dead code; remove
one of the duplicate function declarations so only a single calculateBodySize
implementation remains (keep the version that is actually used by other
functions in this module), update any nearby references if they expect different
behavior, and ensure imports/exports in this file remain consistent after
removal.
In @src/lib/observability/keys.ts:
- Around line 9-29: Change keys from a function that calls createEnv each time
to a module-level validated object by invoking createEnv at load time (export
const keys = createEnv({...}) instead of export const keys = () =>
createEnv({...})); update all call sites (e.g., places that do keys()) to import
{ keys } and read properties directly (keys.SENTRY_ORG,
keys.NEXT_PUBLIC_SENTRY_DSN, etc.), leaving the existing
server/client/runtimeEnv shape intact so validation runs once at startup.
🧹 Nitpick comments (4)
src/components/dev-tools.tsx (2)
36-39: Helpful documentation on nonce handling limitations.The comment clearly explains the current CSP Report-Only behavior and future plans. The reasoning is sound for deferring nonce implementation.
💡 Optional: Clarify hydration mismatch explanation
Point 2 could be slightly more precise. The issue isn't that passing nonces causes mismatches, but that implementing nonces properly requires client-side nonce access to avoid mismatches. Consider:
- // 2. Passing nonces causes hydration mismatches between server/client + // 2. Nonces require client-side access during hydration to avoid mismatchesThis phrasing better reflects that the challenge is in the implementation pattern rather than nonces being inherently problematic.
43-51: Consider adding SRI for external scripts.The Script components load development tools from unpkg.com without Subresource Integrity (SRI) hashes. While this is dev-only code, adding SRI would provide defense-in-depth against CDN tampering.
🔒 Optional: Add integrity hashes
You can generate SRI hashes for specific versions:
<Script crossOrigin="anonymous" src="https://unpkg.com/react-grab@<version>/dist/index.global.js" integrity="sha384-..." strategy="beforeInteractive" />Pin to specific versions (not
//unpkg.com/without version) and generate hashes using:curl https://unpkg.com/react-grab@<version>/dist/index.global.js | openssl dgst -sha384 -binary | openssl base64 -AThis prevents supply-chain attacks if unpkg.com is compromised or serves malicious content.
src/lib/observability/http.ts (1)
5-9: Consider exporting the interface for better TypeScript support.The
HttpRequestOptionsinterface is used as a parameter type in the exportedinstrumentHttpRequestfunction but is not itself exported. Consider exporting it so consumers can reference this type when callinginstrumentHttpRequestdirectly.📦 Proposed change
-interface HttpRequestOptions { +export interface HttpRequestOptions { method: string; url: string; requestSize?: number; }src/lib/observability/cache.ts (1)
110-125: Consider adding Buffer support for more accurate size calculation.The current implementation is solid and handles common types well. For Buffer instances, you could add a specific check before the string fallback to improve accuracy:
♻️ Optional enhancement for Buffer support
export const calculateCacheItemSize = (value: unknown): number => { if (value === null || value === undefined) { return 0; } if (typeof value === "string") { return value.length; } + if (Buffer.isBuffer(value)) { + return value.length; + } if (typeof value === "object") { try { return JSON.stringify(value).length; } catch { return 0; } } return String(value).length; };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.cursor/mcp.jsonsrc/components/dev-tools.tsxsrc/lib/observability/cache.tssrc/lib/observability/http.tssrc/lib/observability/keys.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: Implement multi-tool workflows for complex development tasks: bug investigation (get_errors → nextjs_docs → browser_eval), feature development (init → nextjs_docs → enable_cache_components), and code review (ultracite rules → nextjs_docs → browser_eval)
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : When using shadcn: composition > modification; no template copying
Applied to files:
src/components/dev-tools.tsx
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/config*.{ts,tsx,js,jsx} : ENV must be validated at startup in backend
Applied to files:
src/lib/observability/keys.ts
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js projects requiring major upgrades or Cache Components setup, use specialized automation tools: `upgrade_nextjs_16` for version upgrades and `enable_cache_components` for Cache Components configuration
Applied to files:
src/lib/observability/cache.ts
⏰ 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). (1)
- GitHub Check: Seer Code Review
🔇 Additional comments (7)
src/lib/observability/http.ts (3)
25-97: Well-designed instrumentation with robust fallback handling.The function properly handles optional Sentry availability with layered fallbacks:
- Outer try-catch falls back to uninstrumented execution if Sentry is unavailable
- Inner error handling sets span status using
SPAN_STATUS_ERRORwith a fallback to the numeric constant- URL parsing and Response introspection are appropriately guarded
The dynamic
requirepattern is intentional and appropriate for making Sentry an optional dependency.
116-126: LGTM!Clean helper function with appropriate type constraints on the
methodparameter and conditionalrequestSizeinclusion.
138-176: Excellent API design with clear documentation.The
httpClientobject provides a clean, intuitive interface with:
- Type-safe generic return values
- Comprehensive JSDoc with usage examples
- Consistent parameter ordering across methods
- Automatic instrumentation without manual span management
.cursor/mcp.json (2)
20-23: LGTM! Formatting improvements enhance readability.The multi-line formatting of the args arrays for shadcn, ultracite, and next-devtools improves readability and maintains consistency with the llms.txt configuration at the top of the file.
Also applies to: 31-35, 39-43
44-46: LGTM! Sentry MCP server integration added correctly.The new Sentry entry follows the same pattern as the existing betterauth server (using the
urlfield) and correctly references the organization/project structure that aligns with the observability environment configuration introduced in this PR.src/lib/observability/cache.ts (2)
71-105: Cache read instrumentation correctly handles cache hit detection.The hit detection logic properly distinguishes between
undefined(cache miss) and falsy values likenull,0,"",false(cache hits). The explanatory comment on lines 89-90 makes the intent clear.
131-149: Well-designed preset configurations.The helper functions provide clean, reusable cache backend configurations with sensible defaults. The string identifiers for in-memory and Next.js caches are clear and consistent.
| export const instrumentCacheSet = async <T>( | ||
| options: CacheSetOptions, | ||
| setter: () => Promise<T> | T | ||
| ): Promise<T> => { | ||
| try { | ||
| const { startSpan } = require("@sentry/nextjs"); | ||
| const { primaryKey } = normalizeKey(options.key); | ||
|
|
||
| return await startSpan( | ||
| { | ||
| name: `cache.set ${primaryKey}`, | ||
| op: "cache.put", | ||
| attributes: { | ||
| ...baseCacheAttributes(options), | ||
| ...(options.itemSize && { "cache.item_size": options.itemSize }), | ||
| }, | ||
| }, | ||
| setter | ||
| ); | ||
| } catch { | ||
| // Fallback without instrumentation | ||
| return await setter(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Sentry JavaScript SDK version 10.32.1 startSpan API documentation
💡 Result:
Here’s the startSpan API for Sentry JavaScript SDK v10.32.1.
Signature
- startSpan(options: StartSpanOptions, callback: (span: Span) => T): T. [1][2]
StartSpanOptions (key fields)
- name: string (required)
- op?: string
- attributes?: Record<string, string|number|boolean|null|undefined>
- startTime?: number
- forceTransaction?: boolean
- parentSpan?: Span | null
- onlyIfParent?: boolean. [1][2]
Behavior
- Starts a new active span and runs the provided callback with the span.
- Spans created inside the callback become children of this span.
- The span is automatically ended when the callback returns (callback may be async). [1][2]
Short examples
- Synchronous:
Sentry.startSpan({ name: "my-span" }, (span) => { doWork(); }); - Async:
await Sentry.startSpan({ name: "my-span" }, async (span) => { await doAsync(); return result; }); [1][2]
Sources
- Sentry docs — Tracing API (startSpan & options). [1]
- jsDocs entry for @sentry/core / @sentry/browser (v10.32.1) showing startSpan signature and examples. [2]
References
[1] Sentry documentation — Tracing API / startSpan.
[2] jsDocs for @sentry/browser / @sentry/core v10.32.1.
Fix callback signature to match Sentry startSpan API requirement.
The startSpan API expects the callback to accept a span parameter with signature (span: Span) => T, but setter is defined as () => Promise<T> | T without parameters. While this works at runtime (JavaScript ignores extra arguments), it's semantically incorrect and not type-safe.
Wrap the setter to match the expected signature:
Suggested fix
return await startSpan(
{
name: `cache.set ${primaryKey}`,
op: "cache.put",
attributes: {
...baseCacheAttributes(options),
...(options.itemSize && { "cache.item_size": options.itemSize }),
},
},
(span) => setter()
);🤖 Prompt for AI Agents
In @src/lib/observability/cache.ts around lines 43 - 66, The startSpan callback
in instrumentCacheSet must accept a Span parameter but setter is typed as () =>
Promise<T> | T; wrap the existing setter when calling startSpan so the callback
matches the expected signature (e.g., pass (span) => setter()) while keeping the
existing attributes and catch fallback behavior; update the call site in
instrumentCacheSet that invokes startSpan to use the wrapped callback so
TypeScript correctly recognizes the (span: Span) => T signature.
| export const keys = () => | ||
| createEnv({ | ||
| server: { | ||
| SENTRY_ORG: z.string().optional(), | ||
| SENTRY_PROJECT: z.string().optional(), | ||
| SENTRY_RELEASE: z.string().optional(), | ||
| SENTRY_AUTH_TOKEN: z.string().optional(), | ||
| }, | ||
| client: { | ||
| NEXT_PUBLIC_SENTRY_DSN: z.url().optional(), | ||
| NEXT_PUBLIC_SENTRY_RELEASE: z.string().optional(), | ||
| }, | ||
| runtimeEnv: { | ||
| SENTRY_ORG: process.env.SENTRY_ORG, | ||
| SENTRY_PROJECT: process.env.SENTRY_PROJECT, | ||
| SENTRY_RELEASE: process.env.SENTRY_RELEASE, | ||
| SENTRY_AUTH_TOKEN: process.env.SENTRY_AUTH_TOKEN, | ||
| NEXT_PUBLIC_SENTRY_DSN: process.env.NEXT_PUBLIC_SENTRY_DSN, | ||
| NEXT_PUBLIC_SENTRY_RELEASE: process.env.NEXT_PUBLIC_SENTRY_RELEASE, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Validate environment once at module load, not on every function call.
The current pattern wraps createEnv() in a function, causing validation to run every time keys() is invoked. This violates the principle that environment variables should be validated once at startup. Since keys() is called from multiple modules (e.g., env.ts, auth/config.ts), this results in redundant validation overhead and potential inconsistency.
Based on learnings, environment validation must occur at startup in the backend.
♻️ Refactor to validate at module load time
-/**
- * Get validated observability environment variables
- * Uses t3-env for runtime validation and type safety
- * @returns Validated environment configuration for observability (Sentry)
- */
-export const keys = () =>
- createEnv({
+/**
+ * Validated observability environment variables
+ * Uses t3-env for runtime validation and type safety
+ * Validated once at module load time
+ */
+export const keys = createEnv({
server: {
SENTRY_ORG: z.string().optional(),
SENTRY_PROJECT: z.string().optional(),
SENTRY_RELEASE: z.string().optional(),
SENTRY_AUTH_TOKEN: z.string().optional(),
},
client: {
NEXT_PUBLIC_SENTRY_DSN: z.url().optional(),
NEXT_PUBLIC_SENTRY_RELEASE: z.string().optional(),
},
runtimeEnv: {
SENTRY_ORG: process.env.SENTRY_ORG,
SENTRY_PROJECT: process.env.SENTRY_PROJECT,
SENTRY_RELEASE: process.env.SENTRY_RELEASE,
SENTRY_AUTH_TOKEN: process.env.SENTRY_AUTH_TOKEN,
NEXT_PUBLIC_SENTRY_DSN: process.env.NEXT_PUBLIC_SENTRY_DSN,
NEXT_PUBLIC_SENTRY_RELEASE: process.env.NEXT_PUBLIC_SENTRY_RELEASE,
},
});Then update call sites to use keys directly instead of keys():
// Before: const env = keys()
// After: import { keys } from "./keys"
// // use keys.SENTRY_ORG, keys.NEXT_PUBLIC_SENTRY_DSN, etc.📝 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.
| export const keys = () => | |
| createEnv({ | |
| server: { | |
| SENTRY_ORG: z.string().optional(), | |
| SENTRY_PROJECT: z.string().optional(), | |
| SENTRY_RELEASE: z.string().optional(), | |
| SENTRY_AUTH_TOKEN: z.string().optional(), | |
| }, | |
| client: { | |
| NEXT_PUBLIC_SENTRY_DSN: z.url().optional(), | |
| NEXT_PUBLIC_SENTRY_RELEASE: z.string().optional(), | |
| }, | |
| runtimeEnv: { | |
| SENTRY_ORG: process.env.SENTRY_ORG, | |
| SENTRY_PROJECT: process.env.SENTRY_PROJECT, | |
| SENTRY_RELEASE: process.env.SENTRY_RELEASE, | |
| SENTRY_AUTH_TOKEN: process.env.SENTRY_AUTH_TOKEN, | |
| NEXT_PUBLIC_SENTRY_DSN: process.env.NEXT_PUBLIC_SENTRY_DSN, | |
| NEXT_PUBLIC_SENTRY_RELEASE: process.env.NEXT_PUBLIC_SENTRY_RELEASE, | |
| }, | |
| }); | |
| export const keys = createEnv({ | |
| server: { | |
| SENTRY_ORG: z.string().optional(), | |
| SENTRY_PROJECT: z.string().optional(), | |
| SENTRY_RELEASE: z.string().optional(), | |
| SENTRY_AUTH_TOKEN: z.string().optional(), | |
| }, | |
| client: { | |
| NEXT_PUBLIC_SENTRY_DSN: z.url().optional(), | |
| NEXT_PUBLIC_SENTRY_RELEASE: z.string().optional(), | |
| }, | |
| runtimeEnv: { | |
| SENTRY_ORG: process.env.SENTRY_ORG, | |
| SENTRY_PROJECT: process.env.SENTRY_PROJECT, | |
| SENTRY_RELEASE: process.env.SENTRY_RELEASE, | |
| SENTRY_AUTH_TOKEN: process.env.SENTRY_AUTH_TOKEN, | |
| NEXT_PUBLIC_SENTRY_DSN: process.env.NEXT_PUBLIC_SENTRY_DSN, | |
| NEXT_PUBLIC_SENTRY_RELEASE: process.env.NEXT_PUBLIC_SENTRY_RELEASE, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In @src/lib/observability/keys.ts around lines 9 - 29, Change keys from a
function that calls createEnv each time to a module-level validated object by
invoking createEnv at load time (export const keys = createEnv({...}) instead of
export const keys = () => createEnv({...})); update all call sites (e.g., places
that do keys()) to import { keys } and read properties directly
(keys.SENTRY_ORG, keys.NEXT_PUBLIC_SENTRY_DSN, etc.), leaving the existing
server/client/runtimeEnv shape intact so validation runs once at startup.
…paths - Added comments to clarify that the /monitoring route is excluded from the /app matcher, enhancing the understanding of route handling in the proxy configuration. - This change improves the documentation within the code, making it clearer for future developers how specific routes are managed.
…handling - Changed the tunnelRoute configuration to "/api/monitoring" to align with the manual route handler for Sentry requests. - Added comments to clarify the purpose of the tunnelRoute setting, enhancing documentation for future developers.
- Added a new route handler for Sentry events that forwards requests to Sentry's ingest endpoint, bypassing ad-blockers and CSP restrictions. - Implemented environment variable validation for Sentry DSN configuration, ensuring proper error handling for misconfigurations. - Included a health check endpoint to verify the service status of the Sentry tunnel.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/app/api/monitoring/route.ts:
- Around line 25-81: The POST handler currently proxies raw envelopes to Sentry
without validation, size limits, or rate limiting and uses console.error; update
the POST function to enforce rate limiting (per user/IP) before processing (use
token-bucket or Redis-backed limiter keyed by request.ip or auth user id),
validate the envelope text (check minimal Sentry envelope structure/headers and
reject malformed payloads), enforce a request size cap on request.text() to
reject overly large bodies, and replace console.error("Sentry tunnel error:",
error) with the project's structured logger (use the existing logging utility)
when logging failures; ensure error responses return appropriate status codes
and short diagnostic messages.
- Around line 36-48: The DSN parsing fails for self‑hosted Sentry because
projectId is taken as dsnUrl.pathname.split("/")[1] and the PATH component is
ignored when building the endpoint; update parsing to derive projectId via
dsnUrl.pathname.split("/").filter(Boolean).pop() (so it always picks the last
path segment), compute the optional basePath as the remaining pathname segments
(e.g., split/filter(Boolean).slice(0, -1).join("/")) and then include that
basePath when constructing the envelope endpoint (use orgDomain plus the
basePath if present, ensuring proper slashes) rather than always using
`https://${orgDomain}/api/${projectId}/envelope/`. Ensure references:
NEXT_PUBLIC_SENTRY_DSN, dsnUrl, projectId, orgDomain, and the envelope endpoint
construction are updated accordingly.
In @src/proxy.ts:
- Around line 30-48: In buildCSPHeader, remove the `'strict-dynamic'` token from
the style-src directive because it is not valid for styles; update the
construction of styleSrc (the variable named styleSrc) so it only includes
`'self'` and the nonce (`'nonce-${nonce}'`) and leave `'strict-dynamic'` only in
the script-src (scriptSrc remains unchanged); ensure the returned CSP header
uses the new styleSrc value.
🧹 Nitpick comments (1)
src/proxy.ts (1)
89-121: Verify httpOnly: false necessity and consider additional CSP directives.The implementation looks solid overall. Two considerations:
Cookie Security (Line 96):
httpOnly: falseallows client-side access to the nonce. Verify that client-side access is truly necessary—consider whether the nonce could be passed via data attributes or meta tags instead, which would allowhttpOnly: truefor better security.CSP Completeness (Optional): Consider adding recommended security directives:
object-src 'none',base-uri 'self',frame-ancestors 'self', andform-action 'self'for defense-in-depth.Can you confirm whether client-side JavaScript actually needs cookie access to the nonce, or could it retrieve the nonce from a meta tag or data attribute instead?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/api/monitoring/route.tssrc/lib/next-config/with-observability.tssrc/proxy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/next-config/with-observability.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*route*.{ts,tsx,js,jsx} : Clean, non-duplicated REST routes in backend code
Applied to files:
src/app/api/monitoring/route.ts
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js 16+ runtime diagnostics and live application state inspection, use `nextjs_index` to discover dev servers and `nextjs_call` with toolName parameters like 'get_errors', 'get_page_metadata', or 'get_logs'
Applied to files:
src/app/api/monitoring/route.ts
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js development error handling, retry MCP calls and check server availability for network issues, use Better Auth session validation for authentication errors, and fall back to general knowledge if MCP is unavailable
Applied to files:
src/proxy.ts
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For MCP response optimization in Next.js applications, cache responses when available, use single calls for related information instead of batch operations, and specify router types and use precise search terms in documentation queries
Applied to files:
src/proxy.ts
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For authenticated MCP tool handlers in Next.js applications, use the `withMcpAuth(auth, handler)` pattern with Better Auth to enable session-based access control
Applied to files:
src/proxy.ts
🧬 Code graph analysis (1)
src/app/api/monitoring/route.ts (1)
src/env.ts (1)
env(7-12)
⏰ 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). (1)
- GitHub Check: Seer Code Review
🔇 Additional comments (6)
src/proxy.ts (3)
19-24: LGTM! Cryptographically secure nonce generation.The implementation correctly uses
randomBytes(16)for 128 bits of entropy and base64 encoding, which is appropriate for CSP nonces.
50-84: LGTM! Well-structured middleware with consistent nonce handling.The proxy function correctly generates a nonce per request and consistently applies it to both redirect and authenticated responses. The optimistic authentication check aligns with Next.js middleware best practices.
123-142: LGTM! Matcher configuration is correct.The updated comments accurately reflect the exclusion of the
/monitoringSentry tunnel route, and the matcher regex correctly handles static files and Next.js internals.src/app/api/monitoring/route.ts (3)
1-19: LGTM! Well-documented route handler.The imports are appropriate and the documentation clearly explains the Sentry tunnel's purpose, including a reference to official documentation.
21-23: LGTM! Appropriate runtime configuration.The
force-dynamicandnodejsruntime settings are correct for a route that proxies requests to external services.
83-86: LGTM! Simple and effective health check.The GET endpoint provides a useful health check for monitoring the tunnel service availability.
| export async function POST(request: NextRequest) { | ||
| const env = keys(); | ||
|
|
||
| // Only enable tunneling if Sentry is configured | ||
| if (!env.NEXT_PUBLIC_SENTRY_DSN) { | ||
| return NextResponse.json( | ||
| { error: "Sentry DSN not configured" }, | ||
| { status: 503 } | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| // Parse the DSN to get the Sentry organization domain and project ID | ||
| const dsnUrl = new URL(env.NEXT_PUBLIC_SENTRY_DSN); | ||
| const publicKey = dsnUrl.username; | ||
| const projectId = dsnUrl.pathname.split("/")[1]; | ||
| const orgDomain = dsnUrl.hostname; | ||
|
|
||
| if (!(publicKey && projectId && orgDomain)) { | ||
| return NextResponse.json( | ||
| { error: "Invalid Sentry DSN format" }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| // Read the envelope from the request body | ||
| const envelope = await request.text(); | ||
|
|
||
| // Forward the envelope to Sentry's ingest endpoint | ||
| // Note: We remove the sentry_key from query params as it's in the DSN | ||
| const sentryUrl = `https://${orgDomain}/api/${projectId}/envelope/`; | ||
|
|
||
| const response = await fetch(sentryUrl, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/x-sentry-envelope", | ||
| "User-Agent": request.headers.get("user-agent") || "Sentry-Proxy", | ||
| }, | ||
| body: envelope, | ||
| }); | ||
|
|
||
| // Return the response from Sentry | ||
| const responseText = await response.text(); | ||
| return new NextResponse(responseText, { | ||
| status: response.status, | ||
| headers: { | ||
| "Content-Type": response.headers.get("content-type") || "text/plain", | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| console.error("Sentry tunnel error:", error); | ||
| return NextResponse.json( | ||
| { error: "Failed to tunnel request to Sentry" }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add rate limiting to prevent abuse of the Sentry tunnel.
This endpoint forwards arbitrary data to Sentry without validation or rate limiting. Since it's accessible to any authenticated user (via the proxy middleware), it could be abused to:
- Send fake events to Sentry
- Exhaust your Sentry quota
- Spam the Sentry ingest API
Consider adding:
- Rate limiting per user/IP
- Basic envelope validation to ensure it's well-formed Sentry data
- Request size limits
Additionally, Line 75 uses console.error instead of structured logging. Given this PR's focus on observability, consider using the project's logging utilities for consistency.
🤖 Prompt for AI Agents
In @src/app/api/monitoring/route.ts around lines 25 - 81, The POST handler
currently proxies raw envelopes to Sentry without validation, size limits, or
rate limiting and uses console.error; update the POST function to enforce rate
limiting (per user/IP) before processing (use token-bucket or Redis-backed
limiter keyed by request.ip or auth user id), validate the envelope text (check
minimal Sentry envelope structure/headers and reject malformed payloads),
enforce a request size cap on request.text() to reject overly large bodies, and
replace console.error("Sentry tunnel error:", error) with the project's
structured logger (use the existing logging utility) when logging failures;
ensure error responses return appropriate status codes and short diagnostic
messages.
| try { | ||
| // Parse the DSN to get the Sentry organization domain and project ID | ||
| const dsnUrl = new URL(env.NEXT_PUBLIC_SENTRY_DSN); | ||
| const publicKey = dsnUrl.username; | ||
| const projectId = dsnUrl.pathname.split("/")[1]; | ||
| const orgDomain = dsnUrl.hostname; | ||
|
|
||
| if (!(publicKey && projectId && orgDomain)) { | ||
| return NextResponse.json( | ||
| { error: "Invalid Sentry DSN format" }, | ||
| { status: 500 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Sentry DSN format specification and structure
💡 Result:
Sentry DSN — concise format and components
-
Template: {PROTOCOL}://{PUBLIC_KEY}:{SECRET_KEY}@{HOST}{PATH}/{PROJECT_ID}. [1][2]
-
Components:
- PROTOCOL — scheme (http or https). [1]
- PUBLIC_KEY — public API key (used by SDKs to identify the project). [1][2]
- SECRET_KEY — optional/legacy private key (may be omitted; clients should accept it but servers may ignore it). [1][2]
- HOST — ingest host (for sentry.io uses patterns like o{orgid}.ingest.sentry.io or o{orgid}.ingest.{region}.sentry.io; self‑hosted instances use your host). [1][5]
- PATH — optional base path for self‑hosted installs (often empty for sentry.io). [1]
- PROJECT_ID — project identifier (string). [1][2]
-
How the SDK constructs the upload endpoint:
- BASE_URI = {PROTOCOL}://{HOST}{PATH}
- Upload endpoints: {BASE_URI}/api/{PROJECT_ID}/{ENDPOINT}/ (common endpoints: /store/ for JSON events, /envelope/, /minidump/, /unreal/, /security/). [1][2]
-
Auth header (what servers expect on submissions):
- X-Sentry-Auth: Sentry sentry_version=7, sentry_client=, sentry_timestamp=, sentry_key=, sentry_secret=. [2]
References
- [1] Sentry SDK developer docs — DSN parsing and endpoint construction.
- [2] Sentry product docs — DSN usage, examples, and authentication header.
- [3] Sentry API — example DSN field in project-keys response (shows full DSN example).
(If you want, I can parse a DSN you provide into its components.)
🏁 Script executed:
# Check if there's any context about whether this is sentry.io-only or supports self-hosted
rg -i "sentry|dsn|self.*host" src/app/api/monitoring/ -A 3 -B 3Repository: allthingslinux/portal
Length of output: 5281
🏁 Script executed:
# Check environment variable documentation or comments
rg "NEXT_PUBLIC_SENTRY_DSN" . --type ts --type js -B 5 -A 5Repository: allthingslinux/portal
Length of output: 8255
🏁 Script executed:
# Extract just the pathname parsing and endpoint URL construction to see the full context
sed -n '36,65p' src/app/api/monitoring/route.tsRepository: allthingslinux/portal
Length of output: 1057
Fix DSN parsing to handle self-hosted Sentry instances with optional PATH component.
The current implementation fails for self-hosted Sentry deployments. According to Sentry's DSN specification ({PROTOCOL}://{PUBLIC_KEY}:{SECRET_KEY}@{HOST}{PATH}/{PROJECT_ID}), the PATH component is optional (empty for sentry.io, present in self-hosted installs like /sentry/).
Two critical issues:
-
Line 40:
dsnUrl.pathname.split("/")[1]assumes the project ID is always at index 1. When a PATH exists, this returns the path segment instead of the project ID.- For sentry.io (no PATH):
pathname="/projectid"→ correctly gets"projectid" - For self-hosted (PATH="/sentry/"):
pathname="/sentry/projectid"→ incorrectly gets"sentry" - Fix: Use
dsnUrl.pathname.split("/").filter(Boolean).pop()to extract the last segment as project ID
- For sentry.io (no PATH):
-
Line 57: The endpoint URL construction ignores the PATH component entirely. For self-hosted instances, the URL should include the optional path (e.g.,
https://host/sentry/api/projectid/envelope/), but the code always constructshttps://${orgDomain}/api/${projectId}/envelope/.- Fix: Extract
pathnameup to the last segment, preserve it in the endpoint URL
- Fix: Extract
🤖 Prompt for AI Agents
In @src/app/api/monitoring/route.ts around lines 36 - 48, The DSN parsing fails
for self‑hosted Sentry because projectId is taken as
dsnUrl.pathname.split("/")[1] and the PATH component is ignored when building
the endpoint; update parsing to derive projectId via
dsnUrl.pathname.split("/").filter(Boolean).pop() (so it always picks the last
path segment), compute the optional basePath as the remaining pathname segments
(e.g., split/filter(Boolean).slice(0, -1).join("/")) and then include that
basePath when constructing the envelope endpoint (use orgDomain plus the
basePath if present, ensuring proper slashes) rather than always using
`https://${orgDomain}/api/${projectId}/envelope/`. Ensure references:
NEXT_PUBLIC_SENTRY_DSN, dsnUrl, projectId, orgDomain, and the envelope endpoint
construction are updated accordingly.
| function buildCSPHeader(nonce: string, sentryOrgDomain?: string): string { | ||
| // Use strict-dynamic with nonce - allows scripts with nonce and scripts they load | ||
| const scriptSrc = `'self' 'nonce-${nonce}' 'strict-dynamic' https://js.sentry-cdn.com`; | ||
| const styleSrc = `'self' 'nonce-${nonce}' 'strict-dynamic'`; | ||
|
|
||
| // Connect-src for API calls and Sentry | ||
| const connectSrc = sentryOrgDomain | ||
| ? `'self' https://${sentryOrgDomain} https://js.sentry-cdn.com` | ||
| : `'self'`; | ||
|
|
||
| return [ | ||
| `default-src 'self'`, | ||
| `script-src ${scriptSrc}`, | ||
| `style-src ${styleSrc}`, | ||
| `img-src 'self' data: https:`, | ||
| `font-src 'self' data:`, | ||
| `connect-src ${connectSrc}`, | ||
| ].join("; "); | ||
| } |
There was a problem hiding this comment.
Remove 'strict-dynamic' from style-src - CSP spec violation.
Line 33 includes 'strict-dynamic' in the style-src directive, which is invalid per the CSP specification. The 'strict-dynamic' keyword only applies to script-src and will be ignored by browsers in style-src, potentially causing confusion during CSP policy review.
🔒 Proposed fix
function buildCSPHeader(nonce: string, sentryOrgDomain?: string): string {
// Use strict-dynamic with nonce - allows scripts with nonce and scripts they load
const scriptSrc = `'self' 'nonce-${nonce}' 'strict-dynamic' https://js.sentry-cdn.com`;
- const styleSrc = `'self' 'nonce-${nonce}' 'strict-dynamic'`;
+ const styleSrc = `'self' 'nonce-${nonce}'`;📝 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.
| function buildCSPHeader(nonce: string, sentryOrgDomain?: string): string { | |
| // Use strict-dynamic with nonce - allows scripts with nonce and scripts they load | |
| const scriptSrc = `'self' 'nonce-${nonce}' 'strict-dynamic' https://js.sentry-cdn.com`; | |
| const styleSrc = `'self' 'nonce-${nonce}' 'strict-dynamic'`; | |
| // Connect-src for API calls and Sentry | |
| const connectSrc = sentryOrgDomain | |
| ? `'self' https://${sentryOrgDomain} https://js.sentry-cdn.com` | |
| : `'self'`; | |
| return [ | |
| `default-src 'self'`, | |
| `script-src ${scriptSrc}`, | |
| `style-src ${styleSrc}`, | |
| `img-src 'self' data: https:`, | |
| `font-src 'self' data:`, | |
| `connect-src ${connectSrc}`, | |
| ].join("; "); | |
| } | |
| function buildCSPHeader(nonce: string, sentryOrgDomain?: string): string { | |
| // Use strict-dynamic with nonce - allows scripts with nonce and scripts they load | |
| const scriptSrc = `'self' 'nonce-${nonce}' 'strict-dynamic' https://js.sentry-cdn.com`; | |
| const styleSrc = `'self' 'nonce-${nonce}'`; | |
| // Connect-src for API calls and Sentry | |
| const connectSrc = sentryOrgDomain | |
| ? `'self' https://${sentryOrgDomain} https://js.sentry-cdn.com` | |
| : `'self'`; | |
| return [ | |
| `default-src 'self'`, | |
| `script-src ${scriptSrc}`, | |
| `style-src ${styleSrc}`, | |
| `img-src 'self' data: https:`, | |
| `font-src 'self' data:`, | |
| `connect-src ${connectSrc}`, | |
| ].join("; "); | |
| } |
🤖 Prompt for AI Agents
In @src/proxy.ts around lines 30 - 48, In buildCSPHeader, remove the
`'strict-dynamic'` token from the style-src directive because it is not valid
for styles; update the construction of styleSrc (the variable named styleSrc) so
it only includes `'self'` and the nonce (`'nonce-${nonce}'`) and leave
`'strict-dynamic'` only in the script-src (scriptSrc remains unchanged); ensure
the returned CSP header uses the new styleSrc value.
…ecution - Updated the DevTools component to conditionally render Script components only on the client side, preventing hydration mismatches during SSR. - Added comments to clarify the rationale behind the changes and the handling of nonces in relation to CSP.
…nt-side execution - Refactored the Sentry integration logic to separate client-only integrations into a dedicated function, improving code organization and readability. - Enhanced error handling for HTTP client and reporting observer integrations, ensuring they are only added in a browser context. - Consolidated the integration building process, maintaining the functionality while simplifying the overall structure.
…interfaces - Updated the fingerprinting module to replace deprecated Sentry types from @sentry/types with custom inline interface definitions for SentryEvent and SentryHint. - This change enhances type safety in event processors while maintaining compatibility with the current Sentry integration.
…prove error handling - Removed the deprecated body size calculation function to streamline the code. - Updated the span handling to use the imported Span type directly, enhancing type safety. - Simplified error status handling by directly using OpenTelemetry span status codes, improving clarity and maintainability.
…ty and simplified error handling - Introduced TypeScript interfaces for trace headers and improved type safety by using the imported Span type directly. - Streamlined error status handling in queue consumer instrumentation by utilizing OpenTelemetry span status codes, enhancing clarity and maintainability.
…ubleshooting functions - Implemented null checks for span.setAttribute in setLongAttribute and setUrlAttributes functions to prevent potential runtime errors. - This change enhances the robustness of the observability code by ensuring that attributes are only set when the span object is valid.
- Moved the type attribute for the script tag in the JsonLd component to ensure proper rendering of JSON-LD data. - This change enhances the component's compliance with structured data standards.
…ndpoints - Reformatted the error response structure in the GET and PATCH methods to improve readability and maintainability. - Ensured consistent response formatting when a user is not found, enhancing the clarity of API responses.
- Reformatted the error response structure in the GET and PATCH methods for user retrieval and updates to enhance readability. - Ensured consistent formatting of error messages when a user is not found, improving the clarity of API responses.
- Reformatted the error response structure in the GET and DELETE methods for session retrieval and deletion to enhance readability. - Ensured consistent formatting of error messages when a session is not found, improving the clarity of API responses.
- Reformatted the error response structure in the GET and DELETE methods for API key retrieval and deletion to improve readability. - Ensured consistent formatting of error messages when an API key is not found, enhancing the clarity of API responses.
- Consolidated the argument arrays for various commands in mcp.json to improve readability and maintainability. - Ensured consistent formatting across command definitions, enhancing clarity in the configuration file.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Introduced a new function to accurately determine if a URL is a Sentry endpoint by parsing the hostname, preventing false positives from paths or query strings. - Updated the logic to ignore outgoing requests to Sentry, utilizing the new detection method for improved accuracy. - Enhanced the handling of Sentry hostname by deriving it from the configured DSN, allowing for more precise matching.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/lib/observability/http.ts`:
- Around line 36-43: The URL parsing currently calls new URL(options.url, typeof
window !== "undefined" ? window.location.origin : undefined) which throws for
relative paths on the server (window undefined); wrap the URL construction in a
safe branch/try-catch: if typeof window === "undefined" and options.url
startsWith("/") (or new URL(...) throws), construct the URL with a dummy base
like 'http://localhost' (e.g., new URL(options.url, 'http://localhost')) so
parsedURL and subsequent span.setAttribute("server.address", parsedURL.hostname)
/ span.setAttribute("server.port", Number(parsedURL.port)) will not throw;
ensure you reference options.url, parsedURL, and span.setAttribute in the fix.
In `@src/lib/observability/queue.ts`:
- Around line 82-125: The outer try-catch in instrumentQueueConsumer wraps the
entire continueTrace call and will call consumer() in its catch, which can cause
double execution if consumer throws; remove that outer fallback and let errors
from consumer propagate (or alternatively, implement a guard flag that tracks
whether consumer has already been invoked and only call consumer in the fallback
when it has not been run). Specifically update instrumentQueueConsumer (the
continueTrace / startSpan block that currently wraps consumer) so the outer
catch does not re-execute consumer; either delete the catch fallback or add an
"executed" boolean around the consumer invocation to ensure consumer() is never
called twice.
- Around line 53-77: instrumentQueueProducer currently wraps the entire
startSpan call in a try/catch which can catch errors thrown by the producer and
then call producer({}) a second time; change it so the producer is invoked
exactly once and its errors are allowed to propagate: call getTraceData() inside
a narrow try/catch to safely obtain traceHeaders (falling back to {} on
failure), then call startSpan with those traceHeaders and return the producer
result without wrapping the startSpan/producer call in a broad catch; reference
instrumentQueueProducer, getTraceData, startSpan, and the producer callback to
locate and implement this change.
♻️ Duplicate comments (2)
src/app/api/admin/api-keys/[id]/route.ts (1)
49-49: Success response shapes remain inconsistent with error responses.Error paths now correctly return
{ ok: false, error: "..." }, but success responses use different patterns:
- Line 49:
{ apiKey: ... }(nookfield)- Line 75:
{ success: true }(different boolean field name)Consider adopting
{ ok: true, data: ... }for success responses to provide a uniform shape that clients can reliably check.Also applies to: 75-75
src/lib/observability/queue.ts (1)
9-33: Export the type definitions for consumer use.The interfaces
TraceHeaders,QueueMessage,QueueProducerOptions, andQueueConsumerOptionsshould be exported so consumers can use them for type-safe queue instrumentation. Note:TraceHeaderswas not mentioned in the previous review but should also be exported.📤 Proposed fix
// Shared type for trace headers -interface TraceHeaders { +export interface TraceHeaders { "sentry-trace"?: string; baggage?: string; } -interface QueueMessage { +export interface QueueMessage { id: string; body: unknown; timestamp: number; retryCount?: number; } -interface QueueProducerOptions { +export interface QueueProducerOptions { messageId: string; queueName: string; messageSize: number; } -interface QueueConsumerOptions { +export interface QueueConsumerOptions { messageId: string; queueName: string; messageSize: number; retryCount?: number; receiveLatency?: number; }
🧹 Nitpick comments (1)
src/components/dev-tools.tsx (1)
36-58: Correct approach to prevent hydration mismatches.The
isClient &&guard combined withafterInteractivestrategy is the right pattern here—beforeInteractivewould be contradictory since the condition already prevents SSR. The CSP/nonce documentation is helpful for future maintainers.Minor consistency notes (optional):
- Consider using explicit
https://instead of protocol-relative//URLs- The first script has
crossOrigin="anonymous"while the second doesn't—may want to align these for consistency
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.cursor/mcp.jsonsrc/app/api/admin/api-keys/[id]/route.tssrc/app/api/admin/sessions/[id]/route.tssrc/app/api/admin/users/[id]/route.tssrc/app/api/user/me/route.tssrc/app/error.tsxsrc/app/global-error.tsxsrc/components/dev-tools.tsxsrc/lib/observability/client.tssrc/lib/observability/fingerprinting.tssrc/lib/observability/http.tssrc/lib/observability/queue.tssrc/lib/observability/troubleshooting.tssrc/lib/seo/json-ld.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/lib/observability/troubleshooting.ts
- src/app/api/user/me/route.ts
- src/lib/observability/fingerprinting.ts
- .cursor/mcp.json
- src/app/api/admin/users/[id]/route.ts
- src/app/api/admin/sessions/[id]/route.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js 16+ runtime diagnostics and live application state inspection, use `nextjs_index` to discover dev servers and `nextjs_call` with toolName parameters like 'get_errors', 'get_page_metadata', or 'get_logs'
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js development error handling, retry MCP calls and check server availability for network issues, use Better Auth session validation for authentication errors, and fall back to general knowledge if MCP is unavailable
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Remove redundant try/catch blocks (red flag in nuclear mode)
Applied to files:
src/app/error.tsxsrc/app/global-error.tsx
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: For Next.js development error handling, retry MCP calls and check server availability for network issues, use Better Auth session validation for authentication errors, and fall back to general knowledge if MCP is unavailable
Applied to files:
src/app/error.tsxsrc/app/global-error.tsx
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Server Components by default; `use client` only when essential in frontend
Applied to files:
src/components/dev-tools.tsx
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : When using shadcn: composition > modification; no template copying
Applied to files:
src/components/dev-tools.tsx
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Maintain semantic HTML, reading order, contrast, and keyboard navigation in UI/UX work
Applied to files:
src/components/dev-tools.tsx
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx,css} : No hard-coded visual values in frontend; use tokens
Applied to files:
src/components/dev-tools.tsx
📚 Learning: 2025-12-18T18:18:18.374Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/mcp.mdc:0-0
Timestamp: 2025-12-18T18:18:18.374Z
Learning: When working with Next.js projects, ALWAYS call `init` first to establish documentation-first context, list available runtime tools, and set up proper development environment
Applied to files:
src/lib/observability/client.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/error*.{ts,tsx,js,jsx} : Errors must have a uniform shape in backend
Applied to files:
src/app/global-error.tsxsrc/app/api/admin/api-keys/[id]/route.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/api/**/*.{ts,tsx,js,jsx} : API, DB, auth contracts, or tokens require mandatory review
Applied to files:
src/lib/observability/queue.tssrc/app/api/admin/api-keys/[id]/route.ts
🧬 Code graph analysis (4)
src/lib/observability/client.ts (4)
src/lib/observability/keys.ts (1)
keys(9-29)src/lib/observability/troubleshooting.ts (1)
initializeTransactionSanitization(120-133)src/lib/observability/fingerprinting.ts (1)
initializeFingerprinting(179-209)src/lib/observability/sampling.ts (1)
portalSampler(72-107)
src/lib/observability/http.ts (1)
src/lib/observability/index.ts (2)
instrumentHttpRequest(25-25)httpClient(25-25)
src/lib/observability/queue.ts (2)
src/lib/observability/index.ts (5)
instrumentQueueProducer(43-43)instrumentQueueConsumer(42-42)continueTrace(63-63)createQueueMessage(41-41)calculateReceiveLatency(40-40)src/lib/observability/trace.ts (1)
continueTrace(23-39)
src/lib/seo/json-ld.tsx (1)
src/lib/security/nonce.ts (1)
getCSPNonce(8-16)
🪛 ast-grep (0.40.5)
src/lib/seo/json-ld.tsx
[warning] 22-22: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🔇 Additional comments (16)
src/app/api/admin/api-keys/[id]/route.ts (2)
36-41: Good improvement to error response structure.Adding
ok: falseto the 404 error response aligns with the structured error pattern used byhandleAPIError().
68-73: Good improvement to error response structure.Adding
ok: falseto the DELETE 404 error response maintains consistency with the GET handler and other admin endpoints.src/lib/seo/json-ld.tsx (3)
1-7: LGTM!Clean imports with proper typing from
schema-dts. Usingreadonlyon thecodeprop enforces immutability, which is a good practice for props.
9-15: Well-implemented security escaping for JSON-LD.The escape sequence order is correct (escaping
&first prevents double-escaping), and the three characters escaped (&,<,>) are sufficient to prevent script tag injection attacks. The Unicode escapes (\u003c, etc.) are valid JSON, so the parsed structured data will contain the original characters.
17-29: Static analysis concern is properly mitigated.The
dangerouslySetInnerHTMLwarning from static analysis is a false positive here. The XSS risk is adequately addressed because:
- The
codeprop is typed (WithContext<Thing>), not arbitrary user inputJSON.stringifyproduces valid JSON structureescapeJsonForHtmlprevents</script>injectionThe
biome-ignorecomment appropriately documents this intentional use. The async Server Component pattern is correct for accessing cookies in Next.js App Router.src/app/global-error.tsx (1)
40-48: LGTM! Error handling for Sentry capture is properly implemented.The try/catch block gracefully handles cases where
captureExceptionmight fail at runtime, with a fallback to console logging. This addresses the previous review feedback about guarding the Sentry call.src/app/error.tsx (1)
27-35: LGTM! Consistent error handling pattern with global-error.tsx.The Sentry capture is properly guarded with a try/catch block, ensuring the error boundary doesn't break if Sentry fails.
src/lib/observability/http.ts (2)
85-92: LGTM! Safe body size calculation.The
calculateBodySizehelper properly handles non-serializable bodies (circular references, BigInt, etc.) by returningundefinedinstead of throwing.
119-157: LGTM! Clean HTTP client API.The
httpClientprovides a consistent, well-documented interface for instrumented HTTP requests. The design correctly separates concerns—buildHttpOptionshandles option construction whileinstrumentHttpRequestowns the instrumentation behavior.src/lib/observability/client.ts (4)
58-63: LGTM! Clean initialization guard.Properly checks for DSN availability before proceeding with initialization, returning
undefinedearly when Sentry is not configured.
289-308: Sampling adapter is functional but has subtle behavior difference.The
inheritOrSampleWithimplementation makes the random sampling decision immediately (returning 1 or 0) rather than passing the rate to Sentry. While this works correctly, it means the sampling decision happens earlier in the pipeline than typical Sentry behavior.This is fine for most cases, but be aware that:
- Sentry's built-in sampling deduplication won't apply since decisions are already made
- Debug tools expecting a rate may show confusing 0/1 values
Consider if this behavior aligns with your intended sampling strategy. If you prefer Sentry to make the final decision:
inheritOrSampleWith: (fallbackRate: number) => { if (samplingContext.parentSampled !== undefined) { return samplingContext.parentSampled ? 1 : 0; } - return Math.random() < fallbackRate ? 1 : 0; + return fallbackRate; },
145-179: LGTM! Well-structured integration builder.The
buildIntegrationsfunction cleanly orchestrates the integration setup with proper separation of concerns. Placing replay first viaunshiftensures correct initialization order.
230-251: LGTM! Appropriate PII filtering in beforeSend.The hook correctly removes sensitive user data (email, IP) and filters HMR errors in production. The type narrowing for
hint.originalExceptionis thorough.src/lib/observability/queue.ts (3)
35-48: LGTM!Clean helper function using conditional spreading for optional attributes. The OpenTelemetry semantic convention attribute names are appropriate.
127-142: LGTM!The helper correctly creates a message with embedded trace context for distributed tracing propagation.
144-149: LGTM!Simple and correct latency calculation. Note that in distributed systems with clock skew between producer and consumer machines, this could occasionally return negative values - consider clamping to zero if that's undesirable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const parsedURL = new URL( | ||
| options.url, | ||
| typeof window !== "undefined" ? window.location.origin : undefined | ||
| ); | ||
| span.setAttribute("server.address", parsedURL.hostname); | ||
| if (parsedURL.port) { | ||
| span.setAttribute("server.port", Number(parsedURL.port)); | ||
| } |
There was a problem hiding this comment.
URL parsing fails for relative URLs on server-side.
When options.url is relative (e.g., /api/users) and this code runs on the server where window is undefined, new URL('/api/users', undefined) throws a TypeError: Invalid URL. This causes the entire span to fail and fall back to uninstrumented execution.
🐛 Proposed fix with safe URL parsing
try {
// Parse URL for server attributes
- const parsedURL = new URL(
- options.url,
- typeof window !== "undefined" ? window.location.origin : undefined
- );
- span.setAttribute("server.address", parsedURL.hostname);
- if (parsedURL.port) {
- span.setAttribute("server.port", Number(parsedURL.port));
+ try {
+ const base =
+ typeof window !== "undefined"
+ ? window.location.origin
+ : "http://localhost";
+ const parsedURL = new URL(options.url, base);
+ span.setAttribute("server.address", parsedURL.hostname);
+ if (parsedURL.port) {
+ span.setAttribute("server.port", Number(parsedURL.port));
+ }
+ } catch {
+ // URL parsing failed, skip server attributes
}🤖 Prompt for AI Agents
In `@src/lib/observability/http.ts` around lines 36 - 43, The URL parsing
currently calls new URL(options.url, typeof window !== "undefined" ?
window.location.origin : undefined) which throws for relative paths on the
server (window undefined); wrap the URL construction in a safe branch/try-catch:
if typeof window === "undefined" and options.url startsWith("/") (or new
URL(...) throws), construct the URL with a dummy base like 'http://localhost'
(e.g., new URL(options.url, 'http://localhost')) so parsedURL and subsequent
span.setAttribute("server.address", parsedURL.hostname) /
span.setAttribute("server.port", Number(parsedURL.port)) will not throw; ensure
you reference options.url, parsedURL, and span.setAttribute in the fix.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
… handling - Added a flag to track if the consumer function has been executed, allowing for more precise error handling. - Updated the catch block to only fallback to the consumer if it hasn't been executed, addressing cases where instrumentation fails before execution. - Enhanced clarity in error management by re-throwing the original error if the consumer was already executed.
Summary by Sourcery
Integrate Sentry-based observability, T3 env validation, and bundle analysis into the Next.js app while tightening API error handling and SEO utilities.
New Features:
Enhancements:
Build: