Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable ScreenshotIntegration, WidgetsBindingIntegration and SentryWidget in multi-view apps #2366

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
1b36c56
add multiview helper to make the sentry widget, multiview aware.
martinhaintz Oct 21, 2024
f6239fb
switch to implicit view approach
martinhaintz Oct 21, 2024
36a84aa
add changelog
martinhaintz Oct 21, 2024
540d1c0
make it flutter 3.0.0 compatible
martinhaintz Oct 21, 2024
1dab88e
fix exception
martinhaintz Oct 21, 2024
179091d
fix exception
martinhaintz Oct 21, 2024
3e8ab97
remove unused platform code
martinhaintz Oct 22, 2024
4f8cb52
automatically disable WidgetsBindingIntegration() for multiview
martinhaintz Oct 22, 2024
218975a
add debug message, if not available
martinhaintz Oct 22, 2024
0dbce52
fix `WidgetsBindingIntegration` call
martinhaintz Oct 22, 2024
9e610e4
fix, screenshotIntegration call
martinhaintz Oct 22, 2024
cae9153
Merge branch 'main' into feat/multiview-aware
martinhaintz Oct 22, 2024
481b982
Update CHANGELOG.md
martinhaintz Oct 28, 2024
55c4790
Move MultiViewCheck for WidgetsBindingIntegration inside object
martinhaintz Oct 28, 2024
f0e82e4
Move MultiViewCheck for ScreenshotIntegration inside object
martinhaintz Oct 28, 2024
d52c4fe
move platform dispatcher wrapper in a separate file
martinhaintz Oct 28, 2024
addcc64
Merge branch 'main' into feat/multiview-aware
martinhaintz Oct 28, 2024
11fc266
Merge branch 'main' into feat/multiview-aware
denrase Jan 28, 2025
c92c8de
Merge branch 'main' into feat/multiview-aware
denrase Feb 3, 2025
be2b2de
move multiview check to init
denrase Feb 3, 2025
df169ab
move back to integrations/widget and add tests
denrase Feb 4, 2025
0e36b03
update cl entry
denrase Feb 4, 2025
e037114
fix analyzer issues
denrase Feb 4, 2025
b228520
move wrapper to own file to fix analyzer warnign
denrase Feb 4, 2025
1d57db2
use onerror cb
denrase Feb 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

### Dependencies

- Bump Cocoa SDK from v8.44.0-beta.1 to v8.44.0 ([#2649](https://github.com/getsentry/sentry-dart/pull/2649))
Expand Down
49 changes: 2 additions & 47 deletions flutter/lib/src/integrations/on_error_integration.dart
Original file line number Diff line number Diff line change
@@ -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);

Expand Down Expand Up @@ -110,45 +107,3 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> {
}
}
}

/// 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;
}
}
8 changes: 8 additions & 0 deletions flutter/lib/src/integrations/screenshot_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ class ScreenshotIntegration implements Integration<SentryFlutterOptions> {

@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ class WidgetsBindingIntegration implements Integration<SentryFlutterOptions> {

@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,
Expand Down
11 changes: 7 additions & 4 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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].
Expand All @@ -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);
}
Expand Down
3 changes: 3 additions & 0 deletions flutter/lib/src/sentry_flutter_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,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');

Expand Down
37 changes: 30 additions & 7 deletions flutter/lib/src/sentry_widget.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -21,11 +35,20 @@ class _SentryWidgetState extends State<SentryWidget> {
@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,
);
}
}
}
70 changes: 70 additions & 0 deletions flutter/lib/src/utils/platform_dispatcher_wrapper.dart
Original file line number Diff line number Diff line change
@@ -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 {

Check warning on line 22 in flutter/lib/src/utils/platform_dispatcher_wrapper.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/utils/platform_dispatcher_wrapper.dart#L22

Added line #L22 was not covered by tests
// 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(

Check warning on line 27 in flutter/lib/src/utils/platform_dispatcher_wrapper.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/utils/platform_dispatcher_wrapper.dart#L27

Added line #L27 was not covered by tests
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) {

Check warning on line 33 in flutter/lib/src/utils/platform_dispatcher_wrapper.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/utils/platform_dispatcher_wrapper.dart#L33

Added line #L33 was not covered by tests
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 {

Check warning on line 52 in flutter/lib/src/utils/platform_dispatcher_wrapper.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/utils/platform_dispatcher_wrapper.dart#L52

Added line #L52 was not covered by tests
// 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(

Check warning on line 57 in flutter/lib/src/utils/platform_dispatcher_wrapper.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/utils/platform_dispatcher_wrapper.dart#L57

Added line #L57 was not covered by tests
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) {

Check warning on line 63 in flutter/lib/src/utils/platform_dispatcher_wrapper.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/utils/platform_dispatcher_wrapper.dart#L63

Added line #L63 was not covered by tests
rethrow;
}
return false;
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
1 change: 1 addition & 0 deletions flutter/test/integrations/on_error_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
25 changes: 21 additions & 4 deletions flutter/test/integrations/screenshot_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,34 @@ 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', () {
final integration = fixture.getSut(attachScreenshot: false);

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', () {
Expand Down
43 changes: 43 additions & 0 deletions flutter/test/integrations/widgets_binding_integration_test.dart
Original file line number Diff line number Diff line change
@@ -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();
}
Loading
Loading