Fix build error on production privy dependency #324
Conversation
WalkthroughThe default export in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
next.config.mjs (1)
1-1: Unused import should be removed or documented.The
withSentryConfigimport is no longer used since Sentry integration is commented out. Either remove this import to keep the code clean, or add a comment explaining it's retained for easy re-enablement.🔎 Apply this diff if Sentry re-enablement is not planned immediately:
-import { withSentryConfig } from "@sentry/nextjs"; +// import { withSentryConfig } from "@sentry/nextjs"; // Uncomment when re-enabling Sentry
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
next.config.mjs(1 hunks)
⏰ 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: build (20.x)
🔇 Additional comments (1)
next.config.mjs (1)
128-147: Review of Sentry configuration change is safe but verify client-side code exists.Commenting out
withSentryConfigdisables build-time Sentry instrumentation, which is fine for memory optimization. Since the Sentry SDK gracefully handles a missing or empty DSN without runtime errors, there's no risk of method calls failing. However, if you have Sentry client initialization code (sentry.client.config.ts, instrumentation files, or direct Sentry.init calls), verify those aren't relying on build-time Sentry configuration to function correctly. Consider whether you want to also disable or conditional-guard those files when Sentry is disabled.
| }, | ||
| }; | ||
|
|
||
| // Disable Sentry to save memory during builds - comment out this line and uncomment block below to re-enable |
There was a problem hiding this comment.
Misleading comment - disables Sentry at runtime, not just during builds.
The comment states this saves memory "during builds," but exporting nextConfig directly disables Sentry integration entirely, including at runtime in production. This affects error tracking and monitoring in your live application, not just the build process.
If the goal is specifically to reduce build memory usage, consider using the existing LOW_MEMORY_BUILD environment variable (referenced in line 140) to disable only sourcemaps during builds, while keeping Sentry active at runtime.
🤖 Prompt for AI Agents
In next.config.mjs around line 128, the comment incorrectly says Sentry is
disabled "during builds" while exporting nextConfig actually disables Sentry at
runtime too; update the comment to clearly state that exporting nextConfig
disables Sentry integration at runtime (production) and not only during builds,
and either revert to the previous Sentry-enabled export or change the logic to
gate only build-time behavior behind the LOW_MEMORY_BUILD env var (so
sourcemaps/build memory can be reduced) while keeping Sentry middleware/config
active at runtime.
| // Disable Sentry to save memory during builds - comment out this line and uncomment block below to re-enable | ||
| export default nextConfig; |
There was a problem hiding this comment.
Disabling Sentry removes critical production observability.
Exporting nextConfig directly completely disables Sentry error tracking and performance monitoring in production, not just during builds. This significantly degrades your ability to debug production issues and monitor application health.
Impact:
- No error tracking or crash reports
- No performance monitoring
- Loss of production observability
- Unable to proactively identify and debug issues
Alternative approaches to consider:
- Use the existing
LOW_MEMORY_BUILDenvironment variable with conditional logic rather than commenting out the entire integration - Disable only sourcemaps during builds (already controlled by
sourcemaps.disablein line 140) - Use environment-based conditional Sentry initialization (enable only in production, disable in development/CI)
- Address the root cause of the build memory issue mentioned in the PR title
💡 Example: Conditional Sentry configuration based on environment
-// Disable Sentry to save memory during builds - comment out this line and uncomment block below to re-enable
-export default nextConfig;
-
-/*
-export default withSentryConfig(nextConfig, {
+// Only enable Sentry in production or when explicitly requested
+const shouldEnableSentry = process.env.NODE_ENV === 'production' && process.env.DISABLE_SENTRY !== 'true';
+
+export default shouldEnableSentry ? withSentryConfig(nextConfig, {
org: "Paycrest",
project: "noblocks",
sentryUrl: process.env.SENTRY_URL,
authToken: process.env.SENTRY_AUTH_TOKEN,
release: "2.0.0",
silent: !process.env.CI,
sourcemaps: {
disable: process.env.LOW_MEMORY_BUILD === "true",
},
widenClientFileUpload: true,
tunnelRoute: "/monitoring",
disableLogger: true,
automaticVercelMonitors: true,
-});
-*/
+}) : nextConfig;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In next.config.mjs around lines 128-129, the current direct export of nextConfig
disables Sentry entirely; restore conditional Sentry integration so production
observability remains. Update the file to initialize/wrap nextConfig with the
Sentry plugin only when not running a low-memory build (use
process.env.LOW_MEMORY_BUILD !== 'true') and/or when NODE_ENV === 'production',
otherwise export nextConfig untouched; preserve existing sourcemaps.disable
logic and any CI/dev checks so sourcemaps or other settings remain controlled
separately. Ensure the conditional preserves build-time behavior (skip Sentry
plugin during low-memory CI builds) but enables full Sentry error and
performance instrumentation in production.
1 participant
Description
comment out sentry nextjs package to test something.
References
Testing
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.