Major Sentry feedback integration update #2361
Conversation
- Upgrade @sentry/cli from v2.38.0 to v2.57.0 - Replace custom Sentry loader with official CDN bundle approach - Switch to browser.sentry-cdn.com with proper integrity hash - Simplify initialization by removing complex queue management - Maintain backward compatibility with existing window.whenSentryReady/getSentry APIs - Keep all existing configuration (tracing, replay, environment settings) - Reduce complexity while improving security and maintainability The modernized setup follows current Sentry best practices and will be easier to maintain going forward while preserving all existing functionality.
- Add targeted filtering for high-volume error patterns identified in Sentry data - Filter RequireJS script loading errors (BUMBLEBEE-D: 13k events) - Filter Google Tag Manager __tcfapi errors (BUMBLEBEE-7QW/7QV: 6.8k events) - Filter syntax errors from invalid tokens (BUMBLEBEE-1TC: 172k events) - Filter jQuery/Bootstrap loading issues (~5k events combined) - Filter third-party script errors (GA, Pinterest, CDN) - Implement daily replay quotas (100/day prod, 1000/day dev) - Add aggressive payload size reduction (block media, limit network capture) - Enable logging and Core Web Vitals experimental spans - Expected ~99.9% reduction in replay usage while maintaining debug value Fixes: BUMBLEBEE-1TC, BUMBLEBEE-D, BUMBLEBEE-7QW, BUMBLEBEE-7QV, BUMBLEBEE-1R2
thostetler
changed the title
thostetler
force-pushed
the
feature/sentry-feedback-general
branch
from
467a806 to
502c36a
Compare
|
@shinyichen no real way to test this -- but just make sure things make sense. Basically this was an attempt to reduce the number of error replays based on the type of repetitive error messages we get. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors Sentry integration and adds feedback functionality by moving Sentry configuration from inline HTML to a separate JavaScript file, upgrading to a pinned Sentry version, and implementing a new feedback submission system.
- Moved Sentry initialization code from inline
src/index.htmltosrc/config/sentry.js - Added
_sendFeedbackToSentrymethod to capture feedback submissions in Sentry - Upgraded
@sentry/clifrom 2.38.0 to 2.57.0
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/js/widgets/navbar/widget.js | Added _sendFeedbackToSentry method to send form data to Sentry's feedback API alongside existing form submission |
| src/index.html | Removed inline Sentry initialization code and replaced CDN URL with pinned version, loading configuration from external file |
| src/config/sentry.js | New file containing all Sentry initialization logic including replay quota management, error filtering, and helper functions |
| package-lock.json | Updated @sentry/cli and related platform-specific packages to version 2.57.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| return; | ||
| } | ||
| setTimeout(() => waitForSentry(callback, errorCallback), 50); |
There was a problem hiding this comment.
The waitForSentry function implements recursive polling via setTimeout without a maximum retry limit or timeout. This could lead to infinite polling if window.Sentry never becomes available. Consider adding a maximum retry count or timeout mechanism to prevent indefinite execution.
| extra[name] = value; | ||
| } |
There was a problem hiding this comment.
When a field appears multiple times in the form, only the last occurrence is added to the extra object (line 460). This overwrites previous values instead of handling them as arrays like the fields object does. This inconsistency could lead to loss of data in the Sentry context. Consider applying the same array-handling logic to the extra object.
| extra[name] = value; | |
| } | |
| if (!Object.prototype.hasOwnProperty.call(extra, name)) { | |
| extra[name] = value; | |
| } else if (Array.isArray(extra[name])) { | |
| extra[name].push(value); | |
| } else { | |
| extra[name] = [extra[name], value]; | |
| } |
| const captureContext = { | ||
| tags: _.extend({}, tags), | ||
| extra: _.extend( | ||
| { | ||
| userAgent: navigator.userAgent, | ||
| }, | ||
| extra | ||
| ), | ||
| }; |
There was a problem hiding this comment.
The tags object is shallow-copied with _.extend({}, tags) on line 508, but this is unnecessary since tags is a newly created object in this function scope and is not reused elsewhere. This creates redundant overhead. Consider using tags directly or documenting why the copy is necessary.
| window.Sentry.init({ | ||
| dsn: 'https://46062cbe0aeb7a3b2bb4c3a9b8cd1ac7@o1060269.ingest.us.sentry.io/4507341192036352', | ||
| tracesSampleRate: window.ENV === 'development' ? 1.0 : 0.75, | ||
| debug: false, |
There was a problem hiding this comment.
The enableLogs: true configuration option is undocumented and may not be a standard Sentry option. If this is a custom or experimental feature, it should be documented with a comment explaining its purpose and behavior.
| debug: false, | |
| debug: false, | |
| // enableLogs: true is not a standard Sentry option. | |
| // If supported by your Sentry build, it enables verbose logging for debugging purposes. | |
| // Remove or update this option if not needed or unsupported. |
2 participants
Uh oh!
There was an error while loading. Please reload this page.