TF-4343 Fix load Sentry from Sentry CDN#4348
TF-4343 Fix load Sentry from Sentry CDN#4348dab246 wants to merge 5 commits intofeatures/feb26_sprintfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughTwo script tags were added to the HTML head: 🚥 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 1
🧹 Nitpick comments (2)
core/lib/utils/sentry/sentry_web_interop_impl.dart (1)
25-25: Success message useslogWarning— should this belogInfoor lower?
logWarningfor a successful initialization will produce unnecessary noise in production logs/Sentry breadcrumbs. Warnings typically signal something unexpected. Based on learnings, the team ensures trace-level/warning logs are used selectively to avoid noise in production Sentry reports.Proposed fix
- logWarning('[SentryWebInterop] Sentry JS SDK initialized locally.'); + logInfo('[SentryWebInterop] Sentry JS SDK initialized locally.');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/lib/utils/sentry/sentry_web_interop_impl.dart` at line 25, The success message currently uses logWarning ('logWarning') which creates unnecessary noise; change that call to a lower-severity logger such as logInfo (or logDebug/trace if available) so successful initialization is recorded at info/trace level instead of warning—update the call in the Sentry web interop initialization (the spot calling logWarning('[SentryWebInterop] Sentry JS SDK initialized locally.')) to use logInfo (or the appropriate debug/trace helper) and keep the exact message text for context.web/index.html (1)
44-44: Synchronous loading is correct; consider SRI hash as optional improvement.The synchronous loading (no
defer/async) is appropriate since the Sentry global must be available before the Flutter app invokesSentry.init()via JS interop. The file exists and is committed as a 118 KB bundle.The
?v=1.0.0cache-busting parameter requires manual updates. As an optional enhancement, consider tying it to the Sentry SDK version or a build variable. Adding anintegrityattribute would be a good practice for self-hosted bundles, though no scripts in this codebase currently use SRI attributes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/index.html` at line 44, The script tag loading sentry-tracing.min.js is intentionally synchronous because Sentry must be available before the Flutter app calls Sentry.init(); keep the synchronous load but replace the hard-coded cache-bust query string (`?v=1.0.0`) with a build-time variable or the Sentry SDK version (e.g., inject BUILD_SENTRY_VERSION or SENTRY_SDK_VERSION into the HTML template) so it updates automatically, and optionally add a Subresource Integrity attribute (integrity and crossorigin) for the sentry-tracing.min.js script tag to harden the self-hosted bundle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/lib/utils/sentry/sentry_initializer.dart`:
- Around line 30-38: Remove the manual JavaScript SDK initialization by deleting
the call to initSentryWeb(...) in the web branch so you only initialize Sentry
via SentryFlutter.init(...); ensure you do not reintroduce any Sentry.init JS
calls elsewhere and keep autoInitializeNativeSdk = false as-is (it only affects
native SDKs), relying solely on SentryFlutter.init for web error/tracing
capture.
---
Nitpick comments:
In `@core/lib/utils/sentry/sentry_web_interop_impl.dart`:
- Line 25: The success message currently uses logWarning ('logWarning') which
creates unnecessary noise; change that call to a lower-severity logger such as
logInfo (or logDebug/trace if available) so successful initialization is
recorded at info/trace level instead of warning—update the call in the Sentry
web interop initialization (the spot calling logWarning('[SentryWebInterop]
Sentry JS SDK initialized locally.')) to use logInfo (or the appropriate
debug/trace helper) and keep the exact message text for context.
In `@web/index.html`:
- Line 44: The script tag loading sentry-tracing.min.js is intentionally
synchronous because Sentry must be available before the Flutter app calls
Sentry.init(); keep the synchronous load but replace the hard-coded cache-bust
query string (`?v=1.0.0`) with a build-time variable or the Sentry SDK version
(e.g., inject BUILD_SENTRY_VERSION or SENTRY_SDK_VERSION into the HTML template)
so it updates automatically, and optionally add a Subresource Integrity
attribute (integrity and crossorigin) for the sentry-tracing.min.js script tag
to harden the self-hosted bundle.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
web/js/sentry-tracing.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (5)
core/lib/utils/sentry/sentry_initializer.dartcore/lib/utils/sentry/sentry_web_interface.dartcore/lib/utils/sentry/sentry_web_interop.dartcore/lib/utils/sentry/sentry_web_interop_impl.dartweb/index.html
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4348. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ypass CDN blockers
…or to bypass CDN blockers
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/js/sentry-interceptor.js`:
- Around line 31-44: The custom Object.defineProperty for element.src must
delegate to the native HTMLScriptElement.prototype.src descriptor for
non-blocked URLs so behavior stays identical to the browser; replace direct
getAttribute/setAttribute usage with the native descriptor: fetch const
nativeSrcDesc = Object.getOwnPropertyDescriptor(HTMLScriptElement.prototype,
'src') and in your setter call nativeSrcDesc.set.call(this, val) when
shouldBlockSentryUrl(urlString) is false, and in your getter return
nativeSrcDesc.get.call(this) when not blocked; keep the blocked-path logic
(console.log + dispatch load event) intact but only short-circuit when
shouldBlockSentryUrl(urlString) is true.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/index.htmlweb/js/sentry-interceptor.js
🚧 Files skipped from review as they are similar to previous changes (1)
- web/index.html
…terceptor to bypass CDN blockers
dab246
force-pushed
the
features/feb26_sprint
branch
from
391f304 to
e8ad5f4
Compare
3 participants
Issue
#4343
Reproduce
reproduce.mov
Root Cause
By default, the
sentry_flutterSDK on Flutter Web automatically injects a<script>tag to load thebundle.tracing.min.jsfile from an external CDN (browser.sentry-cdn.com). Browsers and ad-blocking extensions often flag this as a third-party tracker and block the network request, resulting in theERR_BLOCKED_BY_CLIENTerror.Attempting to fix this by disabling
autoInitializeNativeSdkand manually callingSentry.initvia JavaScript Interop is not recommended for the Web platform. It violates Sentry's shared environment guidelines and leads to Duplicate Events because both the Dart SDK and JS SDK attach global error handlers simultaneously.Solution
Implement a DOM Hijacking (Interceptor) approach to serve the script locally without altering the native behavior of the
sentry_flutterDart SDK:bundle.tracing.min.jsfile, place it in theweb/js/directory, and load it statically in the<head>ofweb/index.htmlusing<script src="js/sentry-tracing.min.js?v=1.0.0"></script>. The?v=...query string ensures cache busting on future updates.sentry-interceptor.js): Create a lightweight JavaScript file and load it before the Flutter bootstrap script. This script overrides the browser's nativedocument.createElementmethod to act as a middleware.srcassignment. It blocks the external URL, prevents the network request, and artificially dispatches aloadevent. This tricks the Dart SDK into believing the CDN script loaded successfully, forcing it to seamlessly use the local Sentry instance we already provided.TrustedScriptURLobjects used by Flutter's strict CSP during bootstrap) before checking for the CDN URL. This ensures the interceptor does not crash the Flutter engine's initialization process.Resolved
Screen.Recording.2026-02-25.at.16.43.17.mov
Summary by CodeRabbit