Feature/upgrade sentry packages #2303
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request primarily involve updates to the Sentry integration within the Next.js application, including modifications to the Webpack configuration in Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
pages/_app.tsx (2)
Line range hint
19-19: Remove unused import.The
RewriteFramesimport is no longer used after switching toSentry.rewriteFramesIntegration.-import { RewriteFrames } from '@sentry/integrations';
101-101: Remove commented code.The commented
denyUrlsline provides no value and should be removed.- // denyUrls: [], //commented out as this option is currently not utilized
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.json
📒 Files selected for processing (3)
next.config.js(2 hunks)pages/_app.tsx(2 hunks)src/features/user/Settings/DeleteProfile/DeleteProfileForm.tsx(1 hunks)
🔇 Additional comments (5)
src/features/user/Settings/DeleteProfile/DeleteProfileForm.tsx (1)
75-82: Verify Sentry configuration
Since this button is part of Sentry package upgrades, ensure Sentry is properly configured to capture these errors.
✅ Verification successful
Sentry is properly configured to capture errors
The verification shows that Sentry is correctly set up:
- Sentry is initialized in
pages/_app.tsxwith production environment check - Error handling is properly configured in
pages/_error.jswith multiplecaptureExceptioncalls - The test error button will work as expected since unhandled errors are automatically captured by Sentry's default configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Sentry initialization and configuration
echo "Checking Sentry configuration..."
rg -A 5 "Sentry.init"
echo "Checking for other Sentry error captures..."
rg -A 5 "captureException|captureMessage"
Length of output: 1566
next.config.js (2)
6-6: LGTM: Import statement correctly updated for newer Sentry SDK
The change from default import to named import aligns with the updated Sentry Webpack plugin API.
96-105: Verify source map configuration after parameter removal
The core Sentry configuration looks good, but several source map related parameters have been removed as deprecated. While this aligns with the SDK upgrade, we should ensure:
- Source maps are still being generated and uploaded correctly
- Stack traces in Sentry still show correct file paths
Let's verify the source map generation:
Consider adding documentation about:
- The new Sentry source map configuration
- How to verify source maps are working in the Sentry dashboard
- Any required Sentry project settings for source map support
✅ Verification successful
Source maps are properly configured despite parameter removal
The verification shows that source maps are correctly configured:
- Source map generation is explicitly enabled via
productionBrowserSourceMaps: truein next.config.js - The Sentry Webpack Plugin (@sentry/webpack-plugin) is updated to version 2.22.6, which handles source map upload automatically without the need for the deprecated parameters
- The removed parameters (include, ignore, stripPrefix, urlPrefix) were from an older version and are no longer required as the new version handles source map processing with sensible defaults
The core configuration using environment variables (org, project, authToken) combined with Next.js's source map generation is sufficient for proper source map functionality in Sentry.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if source maps are still being generated in the build output
# Test 1: Verify source map configuration in next.config.js
echo "Checking source map configuration..."
rg "productionBrowserSourceMaps" -A 2
# Test 2: Look for any Sentry source map documentation
echo "Checking for Sentry source map documentation..."
fd -t f "README|CONTRIBUTING|CHANGELOG" -x rg -l "sentry.*source.*map"
Length of output: 429
Script:
#!/bin/bash
# Let's check for Sentry configuration and source map related settings more thoroughly
# Check for any Sentry webpack plugin imports and configurations
echo "Checking Sentry webpack plugin usage..."
rg "@sentry/webpack-plugin" -A 5
# Check for source map related configurations in webpack or next config
echo "Checking source map configurations..."
rg "sourceMaps|sourceMap" -i
# Look for any Sentry documentation or configuration files
echo "Checking Sentry configuration files..."
fd -t f "sentry.properties|sentry.config" -x cat {}
# Check for any recent changes or documentation about Sentry
echo "Checking git history for Sentry changes..."
git log -p --grep="sentry" --since="3 months ago"
Length of output: 4158
pages/_app.tsx (2)
Line range hint 1-300: Component structure maintains proper separation of concerns.
The Sentry integration changes are well-isolated and don't impact the component's core functionality. The provider hierarchy, authentication flow, and layout handling remain intact.
67-74: Verify Sentry configuration in production.
The Sentry frame rewriting configuration looks correct. However, since this is a significant change to error tracking, please ensure:
- Error events are properly captured and symbolicated in production
- Source maps are correctly uploaded and applied
- Stack traces show correct file paths in the Sentry dashboard
✅ Verification successful
Let me gather more information about the Sentry configuration to provide a comprehensive verification.
Sentry configuration appears properly set up for production
The verification shows that the Sentry setup is correctly configured:
- Source maps are enabled via
productionBrowserSourceMaps: true - Webpack is properly configured to use
@sentry/browserfor client-side bundles - Sentry Webpack plugin is configured to upload source maps during build with:
- Correct organization and project settings
- Authentication token handling
- Source map upload capability
The frame rewriting configuration in _app.tsx will work correctly with this setup to provide accurate stack traces in the Sentry dashboard.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Sentry source map configuration in next.config.js
# Test: Check if Sentry Webpack plugin is properly configured
rg -A 10 "SentryWebpackPlugin" next.config.js
# Test: Verify source map generation settings
rg "productionBrowserSourceMaps" next.config.js
Length of output: 130
Script:
#!/bin/bash
# Check for Sentry webpack plugin configuration and source map upload settings
rg -A 20 "webpack:" next.config.js
# Look for any Sentry related configuration in next.config.js
rg -i "sentry" next.config.js
# Check if there are any sentry cli configuration files
fd -g ".sentryclirc"
fd -g "sentry.properties"
Length of output: 3037
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| throw new Error("Sentry Frontend Error"); | ||
| }} | ||
| > | ||
| Throw error | ||
| </button> |
There was a problem hiding this comment.
Consider environment-specific rendering of the error test button
The error test button should only be available in development environments to prevent accidental triggers in production.
Consider this implementation:
- <button
- type="button"
- onClick={() => {
- throw new Error("Sentry Frontend Error");
- }}
- >
- Throw error
- </button>
+ {process.env.NODE_ENV === 'development' && (
+ <Button
+ variant="contained"
+ color="warning"
+ onClick={() => {
+ try {
+ throw new Error("Sentry Frontend Error");
+ } catch (error) {
+ // Ensure error is caught by Sentry
+ if (typeof window !== 'undefined' && window.Sentry) {
+ window.Sentry.captureException(error);
+ }
+ throw error;
+ }
+ }}
+ >
+ Test Sentry Error Tracking
+ </Button>
+ )}This change:
- Restricts the button to development environment
- Uses MUI Button component for consistent styling
- Adds explicit Sentry error capture
- Improves button label clarity
📝 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.
| <button | |
| type="button" | |
| onClick={() => { | |
| throw new Error("Sentry Frontend Error"); | |
| }} | |
| > | |
| Throw error | |
| </button> | |
| {process.env.NODE_ENV === 'development' && ( | |
| <Button | |
| variant="contained" | |
| color="warning" | |
| onClick={() => { | |
| try { | |
| throw new Error("Sentry Frontend Error"); | |
| } catch (error) { | |
| // Ensure error is caught by Sentry | |
| if (typeof window !== 'undefined' && window.Sentry) { | |
| window.Sentry.captureException(error); | |
| } | |
| throw error; | |
| } | |
| }} | |
| > | |
| Test Sentry Error Tracking | |
| </Button> | |
| )} |
norbertschuler
force-pushed
the
feature/upgrade_sentry_packages
branch
from
fa66617
to
b223439
Compare
norbertschuler
force-pushed
the
feature/upgrade_sentry_packages
branch
from
b223439
to
5378b23
Compare
Fixes #
Changes in this pull request: