diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f851ec02e..b4139463b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Disable `ScreenshotIntegration`, `WidgetsBindingIntegration` and `SentryWidget` in multi-view apps #2366 ([#2366](https://github.com/getsentry/sentry-dart/pull/2366)) + ### Deprecations - Deprecate `autoAppStart` and `setAppStartEnd` ([#2681](https://github.com/getsentry/sentry-dart/pull/2681)) diff --git a/flutter/example/integration_test/web_sdk_test.dart b/flutter/example/integration_test/web_sdk_test.dart index f33bd59721..5adf45c03a 100644 --- a/flutter/example/integration_test/web_sdk_test.dart +++ b/flutter/example/integration_test/web_sdk_test.dart @@ -124,6 +124,41 @@ void main() { // sent in the JS layer expect(jsEventJson, equals(dartEventJson)); }); + + testWidgets('includes single-view supporting integrations', + (tester) async { + SentryFlutterOptions? confOptions; + + await restoreFlutterOnErrorAfter(() async { + await SentryFlutter.init((options) { + options.enableSentryJs = true; + options.dsn = fakeDsn; + options.attachScreenshot = true; + + confOptions = options; + }, appRunner: () async { + await tester.pumpWidget( + SentryWidget(child: const app.MyApp()), + ); + }); + }); + expect( + confOptions?.sdk.integrations.contains("screenshotIntegration"), + isTrue, + ); + expect( + confOptions?.sdk.integrations.contains("widgetsBindingIntegration"), + isTrue, + ); + expect( + find.byType(SentryScreenshotWidget), + findsOneWidget, + ); + expect( + find.byType(SentryUserInteractionWidget), + findsOneWidget, + ); + }); }); group('disabled', () { diff --git a/flutter/lib/src/integrations/on_error_integration.dart b/flutter/lib/src/integrations/on_error_integration.dart index f01d1a03ee..67c3b1100b 100644 --- a/flutter/lib/src/integrations/on_error_integration.dart +++ b/flutter/lib/src/integrations/on_error_integration.dart @@ -1,11 +1,8 @@ -import 'dart:ui'; - -import 'package:flutter/widgets.dart'; import 'package:sentry/sentry.dart'; -import '../sentry_flutter_options.dart'; - // ignore: implementation_imports import 'package:sentry/src/utils/stacktrace_utils.dart'; +import '../sentry_flutter_options.dart'; +import '../utils/platform_dispatcher_wrapper.dart'; typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace); @@ -110,45 +107,3 @@ class OnErrorIntegration implements Integration { } } } - -/// This class wraps the `this as dynamic` hack in a type-safe manner. -/// It helps to introduce code, which uses newer features from Flutter -/// without breaking Sentry on older versions of Flutter. -// Should not become part of public API. -@visibleForTesting -class PlatformDispatcherWrapper { - PlatformDispatcherWrapper(this._dispatcher); - - final PlatformDispatcher? _dispatcher; - - /// Should not be accessed if [isOnErrorSupported] == false - ErrorCallback? get onError => - (_dispatcher as dynamic)?.onError as ErrorCallback?; - - /// Should not be accessed if [isOnErrorSupported] == false - set onError(ErrorCallback? callback) { - (_dispatcher as dynamic)?.onError = callback; - } - - bool isOnErrorSupported(SentryFlutterOptions options) { - try { - onError; - } on NoSuchMethodError { - // This error is expected on pre 3.1 Flutter version - return false; - } catch (exception, stacktrace) { - // This error is neither expected on pre 3.1 nor on >= 3.1 Flutter versions - options.logger( - SentryLevel.debug, - 'An unexpected exception was thrown, please create an issue at https://github.com/getsentry/sentry-dart/issues', - exception: exception, - stackTrace: stacktrace, - ); - if (options.automatedTestMode) { - rethrow; - } - return false; - } - return true; - } -} diff --git a/flutter/lib/src/integrations/screenshot_integration.dart b/flutter/lib/src/integrations/screenshot_integration.dart index 10cf60228a..00782ea4aa 100644 --- a/flutter/lib/src/integrations/screenshot_integration.dart +++ b/flutter/lib/src/integrations/screenshot_integration.dart @@ -10,6 +10,14 @@ class ScreenshotIntegration implements Integration { @override void call(Hub hub, SentryFlutterOptions options) { + if (options.isMultiViewApp) { + // ignore: invalid_use_of_internal_member + options.logger( + SentryLevel.debug, + '`ScreenshotIntegration` is not available in multi-view applications.', + ); + return; + } if (options.attachScreenshot) { _options = options; final screenshotEventProcessor = ScreenshotEventProcessor(options); diff --git a/flutter/lib/src/integrations/widgets_binding_integration.dart b/flutter/lib/src/integrations/widgets_binding_integration.dart index 8ebab7e7af..64a4fd3717 100644 --- a/flutter/lib/src/integrations/widgets_binding_integration.dart +++ b/flutter/lib/src/integrations/widgets_binding_integration.dart @@ -12,6 +12,14 @@ class WidgetsBindingIntegration implements Integration { @override void call(Hub hub, SentryFlutterOptions options) { + if (options.isMultiViewApp) { + // ignore: invalid_use_of_internal_member + options.logger( + SentryLevel.debug, + '`WidgetsBindingIntegration` is not available in multi-view applications.', + ); + return; + } _options = options; final observer = SentryWidgetsBindingObserver( hub: hub, diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index 844f88098d..1d2a29548a 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -26,6 +26,7 @@ import 'native/sentry_native_binding.dart'; import 'profiling.dart'; import 'renderer/renderer.dart'; import 'replay/integration.dart'; +import 'utils/platform_dispatcher_wrapper.dart'; import 'version.dart'; import 'view_hierarchy/view_hierarchy_integration.dart'; @@ -66,8 +67,8 @@ mixin SentryFlutter { _native = createBinding(options); } - final platformDispatcher = PlatformDispatcher.instance; - final wrapper = PlatformDispatcherWrapper(platformDispatcher); + final wrapper = PlatformDispatcherWrapper(PlatformDispatcher.instance); + options.isMultiViewApp = wrapper.isMultiViewEnabled(options); // Flutter Web doesn't capture [Future] errors if using [PlatformDispatcher.onError] and not // the [runZonedGuarded]. @@ -85,8 +86,10 @@ mixin SentryFlutter { // first step is to install the native integration and set default values, // so we are able to capture future errors. - final defaultIntegrations = - _createDefaultIntegrations(options, isOnErrorSupported); + final defaultIntegrations = _createDefaultIntegrations( + options, + isOnErrorSupported, + ); for (final defaultIntegration in defaultIntegrations) { options.addIntegration(defaultIntegration); } diff --git a/flutter/lib/src/sentry_flutter_options.dart b/flutter/lib/src/sentry_flutter_options.dart index d01b775dc2..f8d5ba06b9 100644 --- a/flutter/lib/src/sentry_flutter_options.dart +++ b/flutter/lib/src/sentry_flutter_options.dart @@ -238,6 +238,9 @@ class SentryFlutterOptions extends SentryOptions { @meta.internal late RendererWrapper rendererWrapper = RendererWrapper(); + @meta.internal + bool isMultiViewApp = false; + @meta.internal late MethodChannel methodChannel = const MethodChannel('sentry_flutter'); diff --git a/flutter/lib/src/sentry_widget.dart b/flutter/lib/src/sentry_widget.dart index 40709a5465..027d351d50 100644 --- a/flutter/lib/src/sentry_widget.dart +++ b/flutter/lib/src/sentry_widget.dart @@ -10,8 +10,22 @@ final sentryWidgetGlobalKey = GlobalKey(debugLabel: 'sentry_widget'); /// as [SentryScreenshotWidget] and [SentryUserInteractionWidget]. class SentryWidget extends StatefulWidget { final Widget child; + late final Hub _hub; - const SentryWidget({super.key, required this.child}); + SentryWidget({ + super.key, + required this.child, + @internal Hub? hub, + }) { + _hub = hub ?? HubAdapter(); + } + + SentryFlutterOptions? get _options => + // ignore: invalid_use_of_internal_member + _hub.options is SentryFlutterOptions + // ignore: invalid_use_of_internal_member + ? _hub.options as SentryFlutterOptions? + : null; @override _SentryWidgetState createState() => _SentryWidgetState(); @@ -21,11 +35,20 @@ class _SentryWidgetState extends State { @override Widget build(BuildContext context) { Widget content = widget.child; - content = SentryScreenshotWidget(child: content); - content = SentryUserInteractionWidget(child: content); - return Container( - key: sentryWidgetGlobalKey, - child: content, - ); + if (widget._options?.isMultiViewApp ?? false) { + // ignore: invalid_use_of_internal_member + Sentry.currentHub.options.logger( + SentryLevel.debug, + '`SentryWidget` is not available in multi-view apps.', + ); + return content; + } else { + content = SentryScreenshotWidget(child: content); + content = SentryUserInteractionWidget(child: content); + return Container( + key: sentryWidgetGlobalKey, + child: content, + ); + } } } diff --git a/flutter/lib/src/utils/platform_dispatcher_wrapper.dart b/flutter/lib/src/utils/platform_dispatcher_wrapper.dart new file mode 100644 index 0000000000..38933d6269 --- /dev/null +++ b/flutter/lib/src/utils/platform_dispatcher_wrapper.dart @@ -0,0 +1,70 @@ +import 'dart:ui'; + +import 'package:meta/meta.dart'; +import 'package:sentry/sentry.dart'; +import '../integrations/on_error_integration.dart'; +import '../sentry_flutter_options.dart'; + +/// This class wraps the `this as dynamic` hack in a type-safe manner. +/// It helps to introduce code, which uses newer features from Flutter +/// without breaking Sentry on older versions of Flutter. +/// +/// Should not become part of public API. +@internal +class PlatformDispatcherWrapper { + PlatformDispatcherWrapper(this._dispatcher); + + final PlatformDispatcher? _dispatcher; + + bool isMultiViewEnabled(SentryFlutterOptions options) { + try { + return ((_dispatcher as dynamic)?.implicitView as FlutterView?) == null; + } on NoSuchMethodError { + // This error is expected on pre 3.10.0 Flutter version + return false; + } catch (exception, stacktrace) { + // This error is neither expected on pre 3.10.0 nor on >= 3.10.0 Flutter versions + options.logger( + SentryLevel.debug, + 'An unexpected exception was thrown, please create an issue at https://github.com/getsentry/sentry-dart/issues', + exception: exception, + stackTrace: stacktrace, + ); + if (options.automatedTestMode) { + rethrow; + } + return false; + } + } + + /// Should not be accessed if [isOnErrorSupported] == false + ErrorCallback? get onError => + (_dispatcher as dynamic)?.onError as ErrorCallback?; + + /// Should not be accessed if [isOnErrorSupported] == false + set onError(ErrorCallback? callback) { + (_dispatcher as dynamic)?.onError = callback; + } + + bool isOnErrorSupported(SentryFlutterOptions options) { + try { + onError; + } on NoSuchMethodError { + // This error is expected on pre 3.1 Flutter version + return false; + } catch (exception, stacktrace) { + // This error is neither expected on pre 3.1 nor on >= 3.1 Flutter versions + options.logger( + SentryLevel.debug, + 'An unexpected exception was thrown, please create an issue at https://github.com/getsentry/sentry-dart/issues', + exception: exception, + stackTrace: stacktrace, + ); + if (options.automatedTestMode) { + rethrow; + } + return false; + } + return true; + } +} diff --git a/flutter/test/integrations/not_initialized_widgets_binding_on_error_integration_test.dart b/flutter/test/integrations/not_initialized_widgets_binding_on_error_integration_test.dart index 6df7df0d0f..789c21af79 100644 --- a/flutter/test/integrations/not_initialized_widgets_binding_on_error_integration_test.dart +++ b/flutter/test/integrations/not_initialized_widgets_binding_on_error_integration_test.dart @@ -2,6 +2,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry_flutter/src/integrations/on_error_integration.dart'; +import 'package:sentry_flutter/src/utils/platform_dispatcher_wrapper.dart'; import '../mocks.dart'; import '../mocks.mocks.dart'; diff --git a/flutter/test/integrations/on_error_integration_test.dart b/flutter/test/integrations/on_error_integration_test.dart index 8d90905fde..8bcb4cc67c 100644 --- a/flutter/test/integrations/on_error_integration_test.dart +++ b/flutter/test/integrations/on_error_integration_test.dart @@ -2,6 +2,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry_flutter/src/integrations/on_error_integration.dart'; +import 'package:sentry_flutter/src/utils/platform_dispatcher_wrapper.dart'; import '../mocks.dart'; import '../mocks.mocks.dart'; diff --git a/flutter/test/integrations/screenshot_integration_test.dart b/flutter/test/integrations/screenshot_integration_test.dart index cedf8bca43..b45b8d9d80 100644 --- a/flutter/test/integrations/screenshot_integration_test.dart +++ b/flutter/test/integrations/screenshot_integration_test.dart @@ -41,8 +41,10 @@ void main() { integration(fixture.hub, fixture.options); - expect(fixture.options.sdk.integrations.contains('screenshotIntegration'), - true); + expect( + fixture.options.sdk.integrations.contains('screenshotIntegration'), + true, + ); }); test('screenshotIntegration does not add integration to the sdk list', () { @@ -50,8 +52,23 @@ void main() { integration(fixture.hub, fixture.options); - expect(fixture.options.sdk.integrations.contains('screenshotIntegration'), - false); + expect( + fixture.options.sdk.integrations.contains('screenshotIntegration'), + false, + ); + }); + + test( + 'screenshotIntegration does not add integration to the sdk list for multiview app', + () { + final integration = fixture.getSut(); + fixture.options.isMultiViewApp = true; + integration(fixture.hub, fixture.options); + + expect( + fixture.options.sdk.integrations.contains('screenshotIntegration'), + false, + ); }); test('screenshotIntegration close resets processor', () { diff --git a/flutter/test/integrations/widgets_binding_integration_test.dart b/flutter/test/integrations/widgets_binding_integration_test.dart new file mode 100644 index 0000000000..4a395738d8 --- /dev/null +++ b/flutter/test/integrations/widgets_binding_integration_test.dart @@ -0,0 +1,43 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/src/integrations/widgets_binding_integration.dart'; + +import '../mocks.dart'; +import '../mocks.mocks.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + group('WidgetsBindingIntegration', () { + test('adds integration', () { + final integration = WidgetsBindingIntegration(); + integration(fixture.hub, fixture.options); + + expect( + fixture.options.sdk.integrations.contains('widgetsBindingIntegration'), + true, + ); + }); + + test('does not add integration if multi-view app', () { + final integration = WidgetsBindingIntegration(); + fixture.options.isMultiViewApp = true; + integration(fixture.hub, fixture.options); + + expect( + fixture.options.sdk.integrations.contains('widgetsBindingIntegration'), + false, + ); + }); + }); +} + +class Fixture { + final hub = MockHub(); + final options = defaultTestOptions(); +} diff --git a/flutter/test/sentry_widget_test.dart b/flutter/test/sentry_widget_test.dart index a64fded42f..12314b60f5 100644 --- a/flutter/test/sentry_widget_test.dart +++ b/flutter/test/sentry_widget_test.dart @@ -1,7 +1,11 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; +import 'mocks.dart'; +import 'mocks.mocks.dart'; + void main() { group('SentryWidget', () { const testChild = Text('Test Child'); @@ -33,5 +37,27 @@ void main() { expect(find.byType(SentryScreenshotWidget), findsOneWidget); expect(find.byWidget(testChild), findsOneWidget); }); + + testWidgets( + 'SentryWidget does not add SentryUserInteractionWidget or SentryScreenshotWidget when multiview-app', + (WidgetTester tester) async { + final options = defaultTestOptions(); + options.isMultiViewApp = true; + final hub = MockHub(); + when(hub.options).thenReturn(options); + + await tester.pumpWidget( + MaterialApp( + home: SentryWidget( + hub: hub, + child: testChild, + ), + ), + ); + + expect(find.byType(SentryUserInteractionWidget), findsNothing); + expect(find.byType(SentryScreenshotWidget), findsNothing); + expect(find.byWidget(testChild), findsOneWidget); + }); }); }