From ea071abc3b4b5839d4c911547c2c389ad9809fda Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Fri, 14 Feb 2025 12:01:26 +0100 Subject: [PATCH] [v9] Enable JS SDK native integration by default (#2688) * load js by default * fix test * update CHANGELOG * maybe fix integration test * remove enableSentryJs * fix test --- CHANGELOG.md | 1 + dart/lib/src/sentry_client.dart | 2 +- .../integration_test/web_sdk_test.dart | 14 ++--- flutter/example/lib/main.dart | 1 - .../src/integrations/web_sdk_integration.dart | 6 +-- flutter/lib/src/sentry_flutter.dart | 8 +-- flutter/lib/src/sentry_flutter_options.dart | 7 --- .../web_sdk_integration_test.dart | 51 ------------------- flutter/test/sentry_flutter_test.dart | 9 ++-- 9 files changed, 17 insertions(+), 82 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dffd7754c..3611d5c01a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Remove deprecated `beforeScreenshot` ([#2662](https://github.com/getsentry/sentry-dart/pull/2662)) - Remove deprecated loggers ([#2685](https://github.com/getsentry/sentry-dart/pull/2685)) - Remove user segment ([#2687](https://github.com/getsentry/sentry-dart/pull/2687)) +- Enable JS SDK native integration by default ([#2688](https://github.com/getsentry/sentry-dart/pull/2688)) - Remove `enableTracing` ([#2695](https://github.com/getsentry/sentry-dart/pull/2695)) - Remove `options.autoAppStart` and `setAppStartEnd` ([#2680](https://github.com/getsentry/sentry-dart/pull/2680)) - Add hint for transactions ([#2675](https://github.com/getsentry/sentry-dart/pull/2675)) diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart index d9b4a66440..89a3709487 100644 --- a/dart/lib/src/sentry_client.dart +++ b/dart/lib/src/sentry_client.dart @@ -63,7 +63,7 @@ class SentryClient { options, options.transport, ); - // TODO: Web might change soon to use the JS SDK so we can remove it here later on + // TODO: Use spotlight integration directly through JS SDK, then we can remove isWeb check final enableFlutterSpotlight = (options.spotlight.enabled && (options.platformChecker.isWeb || options.platformChecker.platform.isLinux || diff --git a/flutter/example/integration_test/web_sdk_test.dart b/flutter/example/integration_test/web_sdk_test.dart index 5adf45c03a..b2643d1b5e 100644 --- a/flutter/example/integration_test/web_sdk_test.dart +++ b/flutter/example/integration_test/web_sdk_test.dart @@ -11,7 +11,6 @@ import 'dart:js_interop_unsafe'; import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/web/javascript_transport.dart'; import 'package:sentry_flutter_example/main.dart' as app; import 'utils.dart'; @@ -48,11 +47,14 @@ void main() { group('Web SDK Integration', () { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + tearDown(() async { + await Sentry.close(); + }); + group('enabled', () { testWidgets('Sentry JS SDK initialized', (tester) async { await restoreFlutterOnErrorAfter(() async { await SentryFlutter.init((options) { - options.enableSentryJs = true; options.dsn = fakeDsn; }, appRunner: () async { await tester.pumpWidget(const app.MyApp()); @@ -72,25 +74,20 @@ void main() { }); testWidgets('sends the correct envelope', (tester) async { - SentryFlutterOptions? configuredOptions; SentryEvent? dartEvent; await restoreFlutterOnErrorAfter(() async { await SentryFlutter.init((options) { - options.enableSentryJs = true; options.dsn = fakeDsn; options.beforeSend = (event, hint) { dartEvent = event; return event; }; - configuredOptions = options; }, appRunner: () async { await tester.pumpWidget(const app.MyApp()); }); }); - expect(configuredOptions!.transport, isA()); - final client = _getClient()!; final completer = Completer>(); @@ -131,7 +128,6 @@ void main() { await restoreFlutterOnErrorAfter(() async { await SentryFlutter.init((options) { - options.enableSentryJs = true; options.dsn = fakeDsn; options.attachScreenshot = true; @@ -165,8 +161,8 @@ void main() { testWidgets('Sentry JS SDK is not initialized', (tester) async { await restoreFlutterOnErrorAfter(() async { await SentryFlutter.init((options) { - options.enableSentryJs = false; options.dsn = fakeDsn; + options.autoInitializeNativeSdk = false; }, appRunner: () async { await tester.pumpWidget(const app.MyApp()); }); diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 87977f7113..a9496c1068 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -81,7 +81,6 @@ Future setupSentry( options.debug = kDebugMode; options.spotlight = Spotlight(enabled: true); options.enableTimeToFullDisplayTracing = true; - options.enableSentryJs = true; options.maxRequestBodySize = MaxRequestBodySize.always; options.maxResponseBodySize = MaxResponseBodySize.always; diff --git a/flutter/lib/src/integrations/web_sdk_integration.dart b/flutter/lib/src/integrations/web_sdk_integration.dart index 80bdb0ebd4..05d0d2e658 100644 --- a/flutter/lib/src/integrations/web_sdk_integration.dart +++ b/flutter/lib/src/integrations/web_sdk_integration.dart @@ -5,7 +5,6 @@ import 'package:sentry/sentry.dart'; import '../native/sentry_native_binding.dart'; import '../sentry_flutter_options.dart'; -import '../web/javascript_transport.dart'; import '../web/script_loader/sentry_script_loader.dart'; import '../web/sentry_js_bundle.dart'; @@ -27,7 +26,7 @@ class WebSdkIntegration implements Integration { @override FutureOr call(Hub hub, SentryFlutterOptions options) async { - if (!options.enableSentryJs || !options.autoInitializeNativeSdk) { + if (!options.autoInitializeNativeSdk) { return; } @@ -39,9 +38,6 @@ class WebSdkIntegration implements Integration { : productionScripts; await _scriptLoader.loadWebSdk(scripts); await _web.init(hub); - if (_web.supportsCaptureEnvelope) { - options.transport = JavascriptTransport(_web, options); - } options.sdk.addIntegration(name); } catch (exception, stackTrace) { options.logger( diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index 0d1d6dbbb6..2bf9de35c4 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -29,6 +29,7 @@ import 'replay/integration.dart'; import 'utils/platform_dispatcher_wrapper.dart'; import 'version.dart'; import 'view_hierarchy/view_hierarchy_integration.dart'; +import 'web/javascript_transport.dart'; /// Configuration options callback typedef FlutterOptionsConfiguration = FutureOr Function( @@ -125,10 +126,9 @@ mixin SentryFlutter { // Not all platforms have a native integration. if (_native != null) { if (_native!.supportsCaptureEnvelope) { - // Sentry's native web integration is only enabled when enableSentryJs=true. - // Transport configuration happens in web_integration because the configuration - // options aren't available until after the options callback executes. - if (!options.platformChecker.isWeb) { + if (options.platformChecker.isWeb) { + options.transport = JavascriptTransport(_native!, options); + } else { options.transport = FileSystemTransport(_native!, options); } } diff --git a/flutter/lib/src/sentry_flutter_options.dart b/flutter/lib/src/sentry_flutter_options.dart index 3fc6598219..537ba45b4d 100644 --- a/flutter/lib/src/sentry_flutter_options.dart +++ b/flutter/lib/src/sentry_flutter_options.dart @@ -284,13 +284,6 @@ class SentryFlutterOptions extends SentryOptions { /// you must use `SentryWidgetsFlutterBinding.ensureInitialized()` instead. bool enableFramesTracking = true; - /// Controls initialization of the Sentry Javascript SDK on web platforms. - /// When enabled and [autoInitializeNativeSdk] is true, loads and initializes - /// the JS SDK in the document head. - /// - /// Defaults to `false` - bool enableSentryJs = false; - /// By using this, you are disabling native [Breadcrumb] tracking and instead /// you are just tracking [Breadcrumb]s which result from events available /// in the current Flutter environment. diff --git a/flutter/test/integrations/web_sdk_integration_test.dart b/flutter/test/integrations/web_sdk_integration_test.dart index 9521e4c34d..70a18ea43a 100644 --- a/flutter/test/integrations/web_sdk_integration_test.dart +++ b/flutter/test/integrations/web_sdk_integration_test.dart @@ -3,9 +3,7 @@ library; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; -import 'package:sentry/src/transport/noop_transport.dart'; import 'package:sentry_flutter/src/integrations/web_sdk_integration.dart'; -import 'package:sentry_flutter/src/web/javascript_transport.dart'; import 'package:sentry_flutter/src/web/script_loader/sentry_script_loader.dart'; import '../mocks.dart'; @@ -27,7 +25,6 @@ void main() { group('enabled', () { setUp(() { - fixture.options.enableSentryJs = true; fixture.options.autoInitializeNativeSdk = true; }); @@ -52,17 +49,9 @@ void main() { _TestScenario( 'with autoInitializeNativeSdk=false', () { - fixture.options.enableSentryJs = true; fixture.options.autoInitializeNativeSdk = false; }, ), - _TestScenario( - 'with enableSentryJs=false', - () { - fixture.options.enableSentryJs = false; - fixture.options.autoInitializeNativeSdk = true; - }, - ), ]; for (final scenario in disabledScenarios) { @@ -84,46 +73,6 @@ void main() { } }); - group('transport configuration', () { - test('integration disabled: does not use javascript transport', () async { - fixture.options.enableSentryJs = false; - fixture.options.autoInitializeNativeSdk = false; - - expect(fixture.options.transport, isA()); - - await sut.call(fixture.hub, fixture.options); - - expect(fixture.options.transport, isA()); - }); - - test( - 'integration enabled and supportsCaptureEnvelope is false: does not use javascript transport', - () async { - fixture.options.enableSentryJs = true; - fixture.options.autoInitializeNativeSdk = true; - when(fixture.web.supportsCaptureEnvelope).thenReturn(false); - - expect(fixture.options.transport, isA()); - - await sut.call(fixture.hub, fixture.options); - - expect(fixture.options.transport, isA()); - }); - - test( - 'integration enabled and supportsCaptureEnvelope is true: uses javascript transport', - () async { - fixture.options.enableSentryJs = true; - fixture.options.autoInitializeNativeSdk = true; - - expect(fixture.options.transport, isA()); - - await sut.call(fixture.hub, fixture.options); - - expect(fixture.options.transport, isA()); - }); - }); - test('closes resources', () async { await sut.close(); diff --git a/flutter/test/sentry_flutter_test.dart b/flutter/test/sentry_flutter_test.dart index d6fd3d97b4..103e240b70 100644 --- a/flutter/test/sentry_flutter_test.dart +++ b/flutter/test/sentry_flutter_test.dart @@ -17,6 +17,7 @@ import 'package:sentry_flutter/src/renderer/renderer.dart'; import 'package:sentry_flutter/src/replay/integration.dart'; import 'package:sentry_flutter/src/version.dart'; import 'package:sentry_flutter/src/view_hierarchy/view_hierarchy_integration.dart'; +import 'package:sentry_flutter/src/web/javascript_transport.dart'; import 'mocks.dart'; import 'mocks.mocks.dart'; @@ -336,7 +337,7 @@ void main() { options: sentryFlutterOptions, ); - expect(transport, isNot(isA())); + expect(transport, isA()); testScopeObserver( options: sentryFlutterOptions, @@ -409,7 +410,7 @@ void main() { options: sentryFlutterOptions, ); - expect(transport, isNot(isA())); + expect(transport, isA()); testConfiguration( integrations: integrations, @@ -452,7 +453,7 @@ void main() { options: sentryFlutterOptions, ); - expect(transport, isNot(isA())); + expect(transport, isA()); testConfiguration( integrations: integrations, @@ -495,7 +496,7 @@ void main() { options: sentryFlutterOptions, ); - expect(transport, isNot(isA())); + expect(transport, isA()); testConfiguration( integrations: integrations,