From 3141b759100f3b5498d45f88624174d80c0b320c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Mon, 9 Jan 2023 13:17:13 +0000 Subject: [PATCH] Fix: Only call method channels on native platforms (#1196) --- CHANGELOG.md | 4 ++ .../Classes/SentryFlutterPluginApple.swift | 5 ++ flutter/lib/src/sentry_flutter.dart | 8 +-- flutter/lib/src/sentry_native.dart | 4 +- .../native_app_start_integration_test.dart | 2 +- flutter/test/mocks.mocks.dart | 26 ++++++---- flutter/test/sentry_flutter_test.dart | 50 +++++++++++++++++++ flutter/test/sentry_flutter_util.dart | 14 ++++++ flutter/test/sentry_native_test.dart | 2 +- .../test/sentry_navigator_observer_test.dart | 4 +- 10 files changed, 102 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d170748d..f2a92e399 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Fix: Only call method channels on native platforms ([#1196](https://github.com/getsentry/sentry-dart/pull/1196)) + ### Dependencies - Bump Android SDK from v6.9.2 to v6.11.0 ([#1194](https://github.com/getsentry/sentry-dart/pull/1194), [#1209](https://github.com/getsentry/sentry-dart/pull/1209)) diff --git a/flutter/ios/Classes/SentryFlutterPluginApple.swift b/flutter/ios/Classes/SentryFlutterPluginApple.swift index 0e558ec51..52749c989 100644 --- a/flutter/ios/Classes/SentryFlutterPluginApple.swift +++ b/flutter/ios/Classes/SentryFlutterPluginApple.swift @@ -86,6 +86,11 @@ public class SentryFlutterPluginApple: NSObject, FlutterPlugin { let value = arguments?["value"] as? Any setContexts(key: key, value: value, result: result) + case "removeContexts": + let arguments = call.arguments as? [String: Any?] + let key = arguments?["key"] as? String + removeContexts(key: key, result: result) + case "setUser": let arguments = call.arguments as? [String: Any?] let user = arguments?["user"] as? [String: Any?] diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index 4bb94160c..944616be8 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -47,8 +47,10 @@ mixin SentryFlutter { } final nativeChannel = SentryNativeChannel(channel, flutterOptions); - final native = SentryNative(); - native.setNativeChannel(nativeChannel); + if (flutterOptions.platformChecker.hasNativeIntegration) { + final native = SentryNative(); + native.nativeChannel = nativeChannel; + } final platformDispatcher = PlatformDispatcher.instance; final wrapper = PlatformDispatcherWrapper(platformDispatcher); @@ -95,6 +97,7 @@ mixin SentryFlutter { // Not all platforms have a native integration. if (options.platformChecker.hasNativeIntegration) { options.transport = FileSystemTransport(channel, options); + options.addScopeObserver(NativeScopeObserver(SentryNative())); } var flutterEventProcessor = @@ -104,7 +107,6 @@ mixin SentryFlutter { if (options.platformChecker.platform.isAndroid) { options .addEventProcessor(AndroidPlatformExceptionEventProcessor(options)); - options.addScopeObserver(NativeScopeObserver(SentryNative())); } _setSdk(options); diff --git a/flutter/lib/src/sentry_native.dart b/flutter/lib/src/sentry_native.dart index 15c48381f..3727294f7 100644 --- a/flutter/lib/src/sentry_native.dart +++ b/flutter/lib/src/sentry_native.dart @@ -19,8 +19,10 @@ class SentryNative { return _instance; } + SentryNativeChannel? get nativeChannel => _instance._nativeChannel; + /// Provide [nativeChannel] for native communication. - void setNativeChannel(SentryNativeChannel nativeChannel) { + set nativeChannel(SentryNativeChannel? nativeChannel) { _instance._nativeChannel = nativeChannel; } diff --git a/flutter/test/integrations/native_app_start_integration_test.dart b/flutter/test/integrations/native_app_start_integration_test.dart index a880f78f2..3f0d3480c 100644 --- a/flutter/test/integrations/native_app_start_integration_test.dart +++ b/flutter/test/integrations/native_app_start_integration_test.dart @@ -104,7 +104,7 @@ class Fixture { late final native = SentryNative(); Fixture() { - native.setNativeChannel(wrapper); + native.nativeChannel = wrapper; native.reset(); when(hub.options).thenReturn(options); } diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index 24f616182..58eac0b82 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -391,6 +391,14 @@ class MockSentryTracer extends _i1.Mock implements _i8.SentryTracer { ), returnValueForMissingStub: null, ); + @override + void scheduleFinish() => super.noSuchMethod( + Invocation.method( + #scheduleFinish, + [], + ), + returnValueForMissingStub: null, + ); } /// A class which mocks [MethodChannel]. @@ -496,20 +504,20 @@ class MockSentryNative extends _i1.Mock implements _i10.SentryNative { returnValueForMissingStub: null, ); @override - bool get didFetchAppStart => (super.noSuchMethod( - Invocation.getter(#didFetchAppStart), - returnValue: false, - ) as bool); - @override - void setNativeChannel(_i11.SentryNativeChannel? nativeChannel) => + set nativeChannel(_i11.SentryNativeChannel? nativeChannel) => super.noSuchMethod( - Invocation.method( - #setNativeChannel, - [nativeChannel], + Invocation.setter( + #nativeChannel, + nativeChannel, ), returnValueForMissingStub: null, ); @override + bool get didFetchAppStart => (super.noSuchMethod( + Invocation.getter(#didFetchAppStart), + returnValue: false, + ) as bool); + @override _i6.Future<_i11.NativeAppStart?> fetchNativeAppStart() => (super.noSuchMethod( Invocation.method( #fetchNativeAppStart, diff --git a/flutter/test/sentry_flutter_test.dart b/flutter/test/sentry_flutter_test.dart index 4f3996bce..a037c39bc 100644 --- a/flutter/test/sentry_flutter_test.dart +++ b/flutter/test/sentry_flutter_test.dart @@ -6,6 +6,7 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/integrations/integrations.dart'; import 'package:sentry_flutter/src/integrations/screenshot_integration.dart'; import 'package:sentry_flutter/src/renderer/renderer.dart'; +import 'package:sentry_flutter/src/sentry_native.dart'; import 'package:sentry_flutter/src/version.dart'; import 'mocks.dart'; import 'mocks.mocks.dart'; @@ -46,17 +47,23 @@ void main() { group('Test platform integrations', () { setUp(() async { await Sentry.close(); + final sentryNative = SentryNative(); + sentryNative.nativeChannel = null; + sentryNative.reset(); }); test('Android', () async { List integrations = []; Transport transport = MockTransport(); + SentryFlutterOptions? sentryFlutterOptions; + await SentryFlutter.init( (options) async { options.dsn = fakeDsn; integrations = options.integrations; transport = options.transport; + sentryFlutterOptions = options; }, appRunner: appRunner, packageLoader: loadTestPackage, @@ -68,6 +75,9 @@ void main() { hasFileSystemTransport: true, ); + testScopeObserver( + options: sentryFlutterOptions!, expectedHasNativeScopeObserver: true); + testConfiguration( integrations: integrations, shouldHaveIntegrations: [ @@ -86,18 +96,22 @@ void main() { beforeIntegration: WidgetsFlutterBindingIntegration, afterIntegration: OnErrorIntegration); + expect(SentryNative().nativeChannel, isNotNull); + await Sentry.close(); }, testOn: 'vm'); test('iOS', () async { List integrations = []; Transport transport = MockTransport(); + SentryFlutterOptions? sentryFlutterOptions; await SentryFlutter.init( (options) async { options.dsn = fakeDsn; integrations = options.integrations; transport = options.transport; + sentryFlutterOptions = options; }, appRunner: appRunner, packageLoader: loadTestPackage, @@ -109,6 +123,9 @@ void main() { hasFileSystemTransport: true, ); + testScopeObserver( + options: sentryFlutterOptions!, expectedHasNativeScopeObserver: true); + testConfiguration( integrations: integrations, shouldHaveIntegrations: [ @@ -125,18 +142,22 @@ void main() { beforeIntegration: WidgetsFlutterBindingIntegration, afterIntegration: OnErrorIntegration); + expect(SentryNative().nativeChannel, isNotNull); + await Sentry.close(); }, testOn: 'vm'); test('macOS', () async { List integrations = []; Transport transport = MockTransport(); + SentryFlutterOptions? sentryFlutterOptions; await SentryFlutter.init( (options) async { options.dsn = fakeDsn; integrations = options.integrations; transport = options.transport; + sentryFlutterOptions = options; }, appRunner: appRunner, packageLoader: loadTestPackage, @@ -148,6 +169,9 @@ void main() { hasFileSystemTransport: true, ); + testScopeObserver( + options: sentryFlutterOptions!, expectedHasNativeScopeObserver: true); + testConfiguration( integrations: integrations, shouldHaveIntegrations: [ @@ -164,18 +188,22 @@ void main() { beforeIntegration: WidgetsFlutterBindingIntegration, afterIntegration: OnErrorIntegration); + expect(SentryNative().nativeChannel, isNotNull); + await Sentry.close(); }, testOn: 'vm'); test('Windows', () async { List integrations = []; Transport transport = MockTransport(); + SentryFlutterOptions? sentryFlutterOptions; await SentryFlutter.init( (options) async { options.dsn = fakeDsn; integrations = options.integrations; transport = options.transport; + sentryFlutterOptions = options; }, appRunner: appRunner, packageLoader: loadTestPackage, @@ -187,6 +215,10 @@ void main() { hasFileSystemTransport: false, ); + testScopeObserver( + options: sentryFlutterOptions!, + expectedHasNativeScopeObserver: false); + testConfiguration( integrations: integrations, shouldHaveIntegrations: [ @@ -205,18 +237,22 @@ void main() { beforeIntegration: WidgetsFlutterBindingIntegration, afterIntegration: OnErrorIntegration); + expect(SentryNative().nativeChannel, isNull); + await Sentry.close(); }, testOn: 'vm'); test('Linux', () async { List integrations = []; Transport transport = MockTransport(); + SentryFlutterOptions? sentryFlutterOptions; await SentryFlutter.init( (options) async { options.dsn = fakeDsn; integrations = options.integrations; transport = options.transport; + sentryFlutterOptions = options; }, appRunner: appRunner, packageLoader: loadTestPackage, @@ -228,6 +264,10 @@ void main() { hasFileSystemTransport: false, ); + testScopeObserver( + options: sentryFlutterOptions!, + expectedHasNativeScopeObserver: false); + testConfiguration( integrations: integrations, shouldHaveIntegrations: [ @@ -246,18 +286,22 @@ void main() { beforeIntegration: WidgetsFlutterBindingIntegration, afterIntegration: OnErrorIntegration); + expect(SentryNative().nativeChannel, isNull); + await Sentry.close(); }, testOn: 'vm'); test('Web', () async { List integrations = []; Transport transport = MockTransport(); + SentryFlutterOptions? sentryFlutterOptions; await SentryFlutter.init( (options) async { options.dsn = fakeDsn; integrations = options.integrations; transport = options.transport; + sentryFlutterOptions = options; }, appRunner: appRunner, packageLoader: loadTestPackage, @@ -272,6 +316,10 @@ void main() { hasFileSystemTransport: false, ); + testScopeObserver( + options: sentryFlutterOptions!, + expectedHasNativeScopeObserver: false); + testConfiguration( integrations: integrations, shouldHaveIntegrations: platformAgnosticIntegrations, @@ -288,6 +336,8 @@ void main() { beforeIntegration: RunZonedGuardedIntegration, afterIntegration: WidgetsFlutterBindingIntegration); + expect(SentryNative().nativeChannel, isNull); + await Sentry.close(); }); diff --git a/flutter/test/sentry_flutter_util.dart b/flutter/test/sentry_flutter_util.dart index 792bbf511..dcff13c05 100644 --- a/flutter/test/sentry_flutter_util.dart +++ b/flutter/test/sentry_flutter_util.dart @@ -1,6 +1,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/file_system_transport.dart'; +import 'package:sentry_flutter/src/native_scope_observer.dart'; void testTransport({ required Transport transport, @@ -13,6 +14,19 @@ void testTransport({ ); } +void testScopeObserver( + {required SentryFlutterOptions options, + required bool expectedHasNativeScopeObserver}) { + var actualHasNativeScopeObserver = false; + for (final scopeObserver in options.scopeObservers) { + if (scopeObserver.runtimeType == NativeScopeObserver) { + actualHasNativeScopeObserver = true; + break; + } + } + expect(actualHasNativeScopeObserver, expectedHasNativeScopeObserver); +} + void testConfiguration({ required Iterable integrations, required Iterable shouldHaveIntegrations, diff --git a/flutter/test/sentry_native_test.dart b/flutter/test/sentry_native_test.dart index f3ff2b313..4d38a4512 100644 --- a/flutter/test/sentry_native_test.dart +++ b/flutter/test/sentry_native_test.dart @@ -136,7 +136,7 @@ class Fixture { SentryNative getSut() { final sut = SentryNative(); - sut.setNativeChannel(channel); + sut.nativeChannel = channel; return sut; } } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index fb221f243..d3871b439 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -48,7 +48,7 @@ void main() { final mockHub = _MockHub(); final native = SentryNative(); final mockNativeChannel = MockNativeChannel(); - native.setNativeChannel(mockNativeChannel); + native.nativeChannel = mockNativeChannel; final tracer = getMockSentryTracer(); _whenAnyStart(mockHub, tracer); @@ -75,7 +75,7 @@ void main() { mockNativeChannel.nativeFrames = nativeFrames; final mockNative = SentryNative(); - mockNative.setNativeChannel(mockNativeChannel); + mockNative.nativeChannel = mockNativeChannel; final sut = fixture.getSut( hub: hub,