refactor: move all native calls to SentryNativeBinding (#2097)

* refactor: move all native calls to SentryNativeBinding [wip] * refactor tests * fix tests * cleanup * fix tests * fix tests * code coverage * fix tests post merge
getsentry · Jun 19, 2024 · beeb73d · beeb73d
1 parent c3b8a98
commit beeb73d
Show file tree

Hide file tree

Showing 40 changed files with 1,234 additions and 1,761 deletions.
diff --git a/flutter/lib/src/event_processor/native_app_start_event_processor.dart b/flutter/lib/src/event_processor/native_app_start_event_processor.dart
@@ -4,35 +4,30 @@ import 'dart:async';
 
 import '../../sentry_flutter.dart';
 import '../integrations/integrations.dart';
-import '../native/sentry_native.dart';
 
 // ignore: implementation_imports
 import 'package:sentry/src/sentry_tracer.dart';
 
 /// EventProcessor that enriches [SentryTransaction] objects with app start
 /// measurement.
 class NativeAppStartEventProcessor implements EventProcessor {
-  final SentryNative _native;
   final Hub _hub;
 
-  NativeAppStartEventProcessor(
-    this._native, {
-    Hub? hub,
-  }) : _hub = hub ?? HubAdapter();
+  NativeAppStartEventProcessor({Hub? hub}) : _hub = hub ?? HubAdapter();
 
   @override
   Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
     final options = _hub.options;
-    if (_native.didAddAppStartMeasurement ||
+    if (NativeAppStartIntegration.didAddAppStartMeasurement ||
         event is! SentryTransaction ||
         options is! SentryFlutterOptions) {
       return event;
     }
 
     final appStartInfo = await NativeAppStartIntegration.getAppStartInfo();
 
-    final appStartEnd = _native.appStartEnd;
     if (!options.autoAppStart) {
+      final appStartEnd = NativeAppStartIntegration.appStartEnd;
       if (appStartEnd != null) {
         appStartInfo?.end = appStartEnd;
       } else {
@@ -44,7 +39,7 @@ class NativeAppStartEventProcessor implements EventProcessor {
     final measurement = appStartInfo?.toMeasurement();
     if (measurement != null) {
       event.measurements[measurement.name] = measurement;
-      _native.didAddAppStartMeasurement = true;
+      NativeAppStartIntegration.didAddAppStartMeasurement = true;
     }
 
     if (appStartInfo != null) {

diff --git a/flutter/lib/src/file_system_transport.dart b/flutter/lib/src/file_system_transport.dart
@@ -5,20 +5,21 @@ import 'dart:typed_data';
 import 'package:flutter/services.dart';
 import 'package:sentry/sentry.dart';
 
+import 'native/sentry_native_binding.dart';
+
 class FileSystemTransport implements Transport {
-  FileSystemTransport(this._channel, this._options);
+  FileSystemTransport(this._native, this._options);
 
-  final MethodChannel _channel;
+  final SentryNativeBinding _native;
   final SentryOptions _options;
 
   @override
   Future<SentryId?> send(SentryEnvelope envelope) async {
     final envelopeData = <int>[];
     await envelope.envelopeStream(_options).forEach(envelopeData.addAll);
-    // https://flutter.dev/docs/development/platform-integration/platform-channels#codec
-    final args = [Uint8List.fromList(envelopeData)];
     try {
-      await _channel.invokeMethod('captureEnvelope', args);
+      // TODO avoid copy
+      await _native.captureEnvelope(Uint8List.fromList(envelopeData));
     } catch (exception, stackTrace) {
       _options.logger(
         SentryLevel.error,

diff --git a/flutter/lib/src/integrations/load_contexts_integration.dart b/flutter/lib/src/integrations/load_contexts_integration.dart
@@ -1,7 +1,7 @@
 import 'dart:async';
 
-import 'package:flutter/services.dart';
 import 'package:sentry/sentry.dart';
+import '../native/sentry_native_binding.dart';
 import '../sentry_flutter_options.dart';
 
 /// Load Device's Contexts from the iOS & Android SDKs.
@@ -15,32 +15,30 @@ import '../sentry_flutter_options.dart';
 ///
 /// This integration is only executed on iOS, macOS & Android Apps.
 class LoadContextsIntegration extends Integration<SentryFlutterOptions> {
-  final MethodChannel _channel;
+  final SentryNativeBinding _native;
 
-  LoadContextsIntegration(this._channel);
+  LoadContextsIntegration(this._native);
 
   @override
   void call(Hub hub, SentryFlutterOptions options) {
     options.addEventProcessor(
-      _LoadContextsIntegrationEventProcessor(_channel, options),
+      _LoadContextsIntegrationEventProcessor(_native, options),
     );
     options.sdk.addIntegration('loadContextsIntegration');
   }
 }
 
 class _LoadContextsIntegrationEventProcessor implements EventProcessor {
-  _LoadContextsIntegrationEventProcessor(this._channel, this._options);
+  _LoadContextsIntegrationEventProcessor(this._native, this._options);
 
-  final MethodChannel _channel;
+  final SentryNativeBinding _native;
   final SentryFlutterOptions _options;
 
   @override
   Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
+    // TODO don't copy everything (i.e. avoid unnecessary Map.from())
     try {
-      final loadContexts = await _channel.invokeMethod('loadContexts');
-
-      final infos =
-          Map<String, dynamic>.from(loadContexts is Map ? loadContexts : {});
+      final infos = await _native.loadContexts() ?? {};
       final contextsMap = infos['contexts'] as Map?;
       if (contextsMap != null && contextsMap.isNotEmpty) {
         final contexts = Contexts.fromJson(

diff --git a/flutter/lib/src/integrations/load_image_list_integration.dart b/flutter/lib/src/integrations/load_image_list_integration.dart
@@ -1,19 +1,19 @@
 import 'dart:async';
 
-import 'package:flutter/services.dart';
 import 'package:sentry/sentry.dart';
+import '../native/sentry_native_binding.dart';
 import '../sentry_flutter_options.dart';
 
 /// Loads the native debug image list for stack trace symbolication.
 class LoadImageListIntegration extends Integration<SentryFlutterOptions> {
-  final MethodChannel _channel;
+  final SentryNativeBinding _native;
 
-  LoadImageListIntegration(this._channel);
+  LoadImageListIntegration(this._native);
 
   @override
   void call(Hub hub, SentryFlutterOptions options) {
     options.addEventProcessor(
-      _LoadImageListIntegrationEventProcessor(_channel, options),
+      _LoadImageListIntegrationEventProcessor(_native),
     );
 
     options.sdk.addIntegration('loadImageListIntegration');
@@ -32,64 +32,34 @@ extension _NeedsSymbolication on SentryEvent {
     return frames.any((frame) => 'native' == frame?.platform);
   }
 
-  List<SentryStackFrame?>? _getStacktraceFrames() {
+  Iterable<SentryStackFrame?>? _getStacktraceFrames() {
     if (exceptions?.isNotEmpty == true) {
       return exceptions?.first.stackTrace?.frames;
     }
     if (threads?.isNotEmpty == true) {
       var stacktraces = threads?.map((e) => e.stacktrace);
       return stacktraces
           ?.where((element) => element != null)
-          .expand((element) => element!.frames)
-          .toList();
+          .expand((element) => element!.frames);
     }
     return null;
   }
 }
 
 class _LoadImageListIntegrationEventProcessor implements EventProcessor {
-  _LoadImageListIntegrationEventProcessor(this._channel, this._options);
+  _LoadImageListIntegrationEventProcessor(this._native);
 
-  final MethodChannel _channel;
-  final SentryFlutterOptions _options;
+  final SentryNativeBinding _native;
 
   @override
   Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
     if (event.needsSymbolication()) {
-      try {
-        // we call on every event because the loaded image list is cached
-        // and it could be changed on the Native side.
-        final loadImageList = await _channel.invokeMethod('loadImageList');
-        final imageList = List<Map<dynamic, dynamic>>.from(
-          loadImageList is List ? loadImageList : [],
-        );
-        return copyWithDebugImages(event, imageList);
-      } catch (exception, stackTrace) {
-        _options.logger(
-          SentryLevel.error,
-          'loadImageList failed',
-          exception: exception,
-          stackTrace: stackTrace,
-        );
+      final images = await _native.loadDebugImages();
+      if (images != null) {
+        return event.copyWith(debugMeta: DebugMeta(images: images));
       }
     }
 
     return event;
   }
-
-  static SentryEvent copyWithDebugImages(
-      SentryEvent event, List<Object?> imageList) {
-    if (imageList.isEmpty) {
-      return event;
-    }
-
-    final newDebugImages = <DebugImage>[];
-    for (final obj in imageList) {
-      final jsonMap = Map<String, dynamic>.from(obj as Map<dynamic, dynamic>);
-      final image = DebugImage.fromJson(jsonMap);
-      newDebugImages.add(image);
-    }
-
-    return event.copyWith(debugMeta: DebugMeta(images: newDebugImages));
-  }
 }
diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart
@@ -6,7 +6,7 @@ import 'package:meta/meta.dart';
 
 import '../../sentry_flutter.dart';
 import '../frame_callback_handler.dart';
-import '../native/sentry_native.dart';
+import '../native/sentry_native_binding.dart';
 import '../event_processor/native_app_start_event_processor.dart';
 
 /// Integration which handles communication with native frameworks in order to
@@ -16,10 +16,23 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
       {Hub? hub})
       : _hub = hub ?? HubAdapter();
 
-  final SentryNative _native;
+  final SentryNativeBinding _native;
   final FrameCallbackHandler _frameCallbackHandler;
   final Hub _hub;
 
+  /// This timestamp marks the end of app startup. Either set automatically when
+  /// [SentryFlutterOptions.autoAppStart] is true, or by calling
+  /// [SentryFlutter.setAppStartEnd]
+  @internal
+  static DateTime? appStartEnd;
+
+  /// Flag indicating if app start was already fetched.
+  static bool _didFetchAppStart = false;
+
+  /// Flag indicating if app start measurement was added to the first transaction.
+  @internal
+  static bool didAddAppStartMeasurement = false;
+
   /// Timeout duration to wait for the app start info to be fetched.
   static const _timeoutDuration = Duration(seconds: 30);
 
@@ -55,6 +68,14 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
   static void clearAppStartInfo() {
     _appStartInfo = null;
     _appStartCompleter = Completer<AppStartInfo?>();
+    didAddAppStartMeasurement = false;
+  }
+
+  /// Reset state
+  @visibleForTesting
+  static void reset() {
+    appStartEnd = null;
+    _didFetchAppStart = false;
   }
 
   @override
@@ -73,11 +94,12 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
       return;
     }
 
-    if (_native.didFetchAppStart) {
+    if (_didFetchAppStart) {
       return;
     }
 
     _frameCallbackHandler.addPostFrameCallback((timeStamp) async {
+      _didFetchAppStart = true;
       final nativeAppStart = await _native.fetchNativeAppStart();
       if (nativeAppStart == null) {
         setAppStartInfo(null);
@@ -94,14 +116,12 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
           nativeAppStart.appStartTime.toInt());
       final pluginRegistrationDateTime = DateTime.fromMillisecondsSinceEpoch(
           nativeAppStart.pluginRegistrationTime);
-      DateTime? appStartEndDateTime;
 
       if (options.autoAppStart) {
         // We only assign the current time if it's not already set - this is useful in tests
-        _native.appStartEnd ??= options.clock();
-        appStartEndDateTime = _native.appStartEnd;
+        appStartEnd ??= options.clock();
 
-        final duration = appStartEndDateTime?.difference(appStartDateTime);
+        final duration = appStartEnd?.difference(appStartDateTime);
 
         // We filter out app start more than 60s.
         // This could be due to many different reasons.
@@ -143,7 +163,7 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
       final appStartInfo = AppStartInfo(
           nativeAppStart.isColdStart ? AppStartType.cold : AppStartType.warm,
           start: appStartDateTime,
-          end: appStartEndDateTime,
+          end: appStartEnd,
           pluginRegistration: pluginRegistrationDateTime,
           sentrySetupStart: sentrySetupStartDateTime,
           nativeSpanTimes: nativeSpanTimes);
@@ -174,7 +194,7 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
       }
     });
 
-    options.addEventProcessor(NativeAppStartEventProcessor(_native, hub: hub));
+    options.addEventProcessor(NativeAppStartEventProcessor(hub: hub));
 
     options.sdk.addIntegration('nativeAppStartIntegration');
   }

diff --git a/flutter/lib/src/integrations/native_sdk_integration.dart b/flutter/lib/src/integrations/native_sdk_integration.dart
@@ -1,15 +1,15 @@
 import 'dart:async';
 
 import 'package:sentry/sentry.dart';
-import '../native/sentry_native.dart';
+import '../native/sentry_native_binding.dart';
 import '../sentry_flutter_options.dart';
 
 /// Enables Sentry's native SDKs (Android and iOS) with options.
 class NativeSdkIntegration implements Integration<SentryFlutterOptions> {
   NativeSdkIntegration(this._native);
 
   SentryFlutterOptions? _options;
-  final SentryNative _native;
+  final SentryNativeBinding _native;
 
   @override
   Future<void> call(Hub hub, SentryFlutterOptions options) async {

diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart
@@ -10,14 +10,14 @@ import 'binding.dart' as cocoa;
 class SentryNativeCocoa extends SentryNativeChannel {
   late final _lib = cocoa.SentryCocoa(DynamicLibrary.process());
 
-  SentryNativeCocoa(super.channel);
+  SentryNativeCocoa(super.options, super.channel);
 
   @override
-  int? startProfiler(SentryId traceId) {
-    final cSentryId = cocoa.SentryId1.alloc(_lib)
-      ..initWithUUIDString_(cocoa.NSString(_lib, traceId.toString()));
-    final startTime =
-        cocoa.PrivateSentrySDKOnly.startProfilerForTrace_(_lib, cSentryId);
-    return startTime;
-  }
+  int? startProfiler(SentryId traceId) => tryCatchSync('startProfiler', () {
+        final cSentryId = cocoa.SentryId1.alloc(_lib)
+          ..initWithUUIDString_(cocoa.NSString(_lib, traceId.toString()));
+        final startTime =
+            cocoa.PrivateSentrySDKOnly.startProfilerForTrace_(_lib, cSentryId);
+        return startTime;
+      });
 }
diff --git a/flutter/lib/src/native/factory_real.dart b/flutter/lib/src/native/factory_real.dart
@@ -6,12 +6,14 @@ import 'java/sentry_native_java.dart';
 import 'sentry_native_binding.dart';
 import 'sentry_native_channel.dart';
 
-SentryNativeBinding createBinding(PlatformChecker pc, MethodChannel channel) {
-  if (pc.platform.isIOS || pc.platform.isMacOS) {
-    return SentryNativeCocoa(channel);
-  } else if (pc.platform.isAndroid) {
-    return SentryNativeJava(channel);
+SentryNativeBinding createBinding(SentryFlutterOptions options,
+    {MethodChannel channel = const MethodChannel('sentry_flutter')}) {
+  final platform = options.platformChecker.platform;
+  if (platform.isIOS || platform.isMacOS) {
+    return SentryNativeCocoa(options, channel);
+  } else if (platform.isAndroid) {
+    return SentryNativeJava(options, channel);
   } else {
-    return SentryNativeChannel(channel);
+    return SentryNativeChannel(options, channel);
   }
 }
diff --git a/flutter/lib/src/native/factory_web.dart b/flutter/lib/src/native/factory_web.dart
@@ -4,6 +4,7 @@ import '../../sentry_flutter.dart';
 import 'sentry_native_binding.dart';
 
 // This isn't actually called, see SentryFlutter.init()
-SentryNativeBinding createBinding(PlatformChecker _, MethodChannel __) {
+SentryNativeBinding createBinding(SentryFlutterOptions options,
+    {MethodChannel channel = const MethodChannel('sentry_flutter')}) {
   throw UnsupportedError("Native binding is not supported on this platform.");
 }
diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart
@@ -6,5 +6,5 @@ import '../sentry_native_channel.dart';
 // generated JNI bindings. See https://github.com/getsentry/sentry-dart/issues/1444
 @internal
 class SentryNativeJava extends SentryNativeChannel {
-  SentryNativeJava(super.channel);
+  SentryNativeJava(super.options, super.channel);
 }