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

Fix: Only call method channels on native platforms #1196

Merged
merged 9 commits into from
Jan 9, 2023
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Fix: Only call method channels on native platforms ([#1196](https://github.com/getsentry/sentry-dart/pull/1196))
marandaneto marked this conversation as resolved.
Show resolved Hide resolved

## 6.18.1

### Fixes
Expand Down
5 changes: 5 additions & 0 deletions flutter/ios/Classes/SentryFlutterPluginApple.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?]
Expand Down
8 changes: 5 additions & 3 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 =
Expand All @@ -104,7 +107,6 @@ mixin SentryFlutter {
if (options.platformChecker.platform.isAndroid) {
options
.addEventProcessor(AndroidPlatformExceptionEventProcessor(options));
options.addScopeObserver(NativeScopeObserver(SentryNative()));
}

_setSdk(options);
Expand Down
4 changes: 3 additions & 1 deletion flutter/lib/src/sentry_native.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class Fixture {
late final native = SentryNative();

Fixture() {
native.setNativeChannel(wrapper);
native.nativeChannel = wrapper;
native.reset();
when(hub.options).thenReturn(options);
}
Expand Down
26 changes: 17 additions & 9 deletions flutter/test/mocks.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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,
Expand Down
47 changes: 47 additions & 0 deletions flutter/test/sentry_flutter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<Integration> 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,
Expand All @@ -68,6 +75,9 @@ void main() {
hasFileSystemTransport: true,
);

testScopeObserver(
options: sentryFlutterOptions!, hasNativeScopeObserver: true);

testConfiguration(
integrations: integrations,
shouldHaveIntegrations: [
Expand All @@ -86,18 +96,22 @@ void main() {
beforeIntegration: WidgetsFlutterBindingIntegration,
afterIntegration: OnErrorIntegration);

expect(SentryNative().nativeChannel, isNotNull);

await Sentry.close();
}, testOn: 'vm');

test('iOS', () async {
List<Integration> 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,
Expand All @@ -109,6 +123,9 @@ void main() {
hasFileSystemTransport: true,
);

testScopeObserver(
options: sentryFlutterOptions!, hasNativeScopeObserver: true);

testConfiguration(
integrations: integrations,
shouldHaveIntegrations: [
Expand All @@ -125,18 +142,22 @@ void main() {
beforeIntegration: WidgetsFlutterBindingIntegration,
afterIntegration: OnErrorIntegration);

expect(SentryNative().nativeChannel, isNotNull);

await Sentry.close();
}, testOn: 'vm');

test('macOS', () async {
List<Integration> 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,
Expand All @@ -148,6 +169,9 @@ void main() {
hasFileSystemTransport: true,
);

testScopeObserver(
options: sentryFlutterOptions!, hasNativeScopeObserver: true);

testConfiguration(
integrations: integrations,
shouldHaveIntegrations: [
Expand All @@ -164,18 +188,22 @@ void main() {
beforeIntegration: WidgetsFlutterBindingIntegration,
afterIntegration: OnErrorIntegration);

expect(SentryNative().nativeChannel, isNotNull);

await Sentry.close();
}, testOn: 'vm');

test('Windows', () async {
List<Integration> 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,
Expand All @@ -187,6 +215,9 @@ void main() {
hasFileSystemTransport: false,
);

testScopeObserver(
options: sentryFlutterOptions!, hasNativeScopeObserver: false);

testConfiguration(
integrations: integrations,
shouldHaveIntegrations: [
Expand All @@ -205,18 +236,22 @@ void main() {
beforeIntegration: WidgetsFlutterBindingIntegration,
afterIntegration: OnErrorIntegration);

expect(SentryNative().nativeChannel, isNull);

await Sentry.close();
}, testOn: 'vm');

test('Linux', () async {
List<Integration> 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,
Expand All @@ -228,6 +263,9 @@ void main() {
hasFileSystemTransport: false,
);

testScopeObserver(
options: sentryFlutterOptions!, hasNativeScopeObserver: false);

testConfiguration(
integrations: integrations,
shouldHaveIntegrations: [
Expand All @@ -246,18 +284,22 @@ void main() {
beforeIntegration: WidgetsFlutterBindingIntegration,
afterIntegration: OnErrorIntegration);

expect(SentryNative().nativeChannel, isNull);

await Sentry.close();
}, testOn: 'vm');

test('Web', () async {
List<Integration> 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,
Expand All @@ -272,6 +314,9 @@ void main() {
hasFileSystemTransport: false,
);

testScopeObserver(
options: sentryFlutterOptions!, hasNativeScopeObserver: false);

testConfiguration(
integrations: integrations,
shouldHaveIntegrations: platformAgnosticIntegrations,
Expand All @@ -288,6 +333,8 @@ void main() {
beforeIntegration: RunZonedGuardedIntegration,
afterIntegration: WidgetsFlutterBindingIntegration);

expect(SentryNative().nativeChannel, isNull);

await Sentry.close();
});

Expand Down
13 changes: 13 additions & 0 deletions flutter/test/sentry_flutter_util.dart
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -13,6 +14,18 @@ void testTransport({
);
}

void testScopeObserver(
{required SentryFlutterOptions options,
required bool hasNativeScopeObserver}) {
try {
options.scopeObservers
.firstWhere((element) => element.runtimeType == NativeScopeObserver);
expect(true, hasNativeScopeObserver);
} catch (_) {
expect(false, hasNativeScopeObserver);
}
denrase marked this conversation as resolved.
Show resolved Hide resolved
}

void testConfiguration({
required Iterable<Integration> integrations,
required Iterable<Type> shouldHaveIntegrations,
Expand Down
2 changes: 1 addition & 1 deletion flutter/test/sentry_native_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class Fixture {

SentryNative getSut() {
final sut = SentryNative();
sut.setNativeChannel(channel);
sut.nativeChannel = channel;
return sut;
}
}
4 changes: 2 additions & 2 deletions flutter/test/sentry_navigator_observer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -75,7 +75,7 @@ void main() {
mockNativeChannel.nativeFrames = nativeFrames;

final mockNative = SentryNative();
mockNative.setNativeChannel(mockNativeChannel);
mockNative.nativeChannel = mockNativeChannel;

final sut = fixture.getSut(
hub: hub,
Expand Down