refactor: move native SDK init and close to SentryNativeBinding (#2030)

* refactor: move native SDK init and close to SentryNativeBinding * fixup * rename test file * test files cleanup
getsentry · May 6, 2024 · c978477 · c978477
1 parent 6fedcab
commit c978477
Show file tree

Hide file tree

Showing 12 changed files with 362 additions and 329 deletions.
diff --git a/flutter/lib/src/integrations/native_sdk_integration.dart b/flutter/lib/src/integrations/native_sdk_integration.dart
@@ -1,61 +1,26 @@
 import 'dart:async';
 
-import 'package:flutter/services.dart';
 import 'package:sentry/sentry.dart';
+import '../native/sentry_native.dart';
 import '../sentry_flutter_options.dart';
 
 /// Enables Sentry's native SDKs (Android and iOS) with options.
 class NativeSdkIntegration implements Integration<SentryFlutterOptions> {
-  NativeSdkIntegration(this._channel);
+  NativeSdkIntegration(this._native);
 
-  final MethodChannel _channel;
   SentryFlutterOptions? _options;
+  final SentryNative _native;
 
   @override
   Future<void> call(Hub hub, SentryFlutterOptions options) async {
     _options = options;
+
     if (!options.autoInitializeNativeSdk) {
       return;
     }
-    try {
-      await _channel.invokeMethod('initNativeSdk', <String, dynamic>{
-        'dsn': options.dsn,
-        'debug': options.debug,
-        'environment': options.environment,
-        'release': options.release,
-        'enableAutoSessionTracking': options.enableAutoSessionTracking,
-        'enableNativeCrashHandling': options.enableNativeCrashHandling,
-        'attachStacktrace': options.attachStacktrace,
-        'attachThreads': options.attachThreads,
-        'autoSessionTrackingIntervalMillis':
-            options.autoSessionTrackingInterval.inMilliseconds,
-        'dist': options.dist,
-        'integrations': options.sdk.integrations,
-        'packages':
-            options.sdk.packages.map((e) => e.toJson()).toList(growable: false),
-        'diagnosticLevel': options.diagnosticLevel.name,
-        'maxBreadcrumbs': options.maxBreadcrumbs,
-        'anrEnabled': options.anrEnabled,
-        'anrTimeoutIntervalMillis': options.anrTimeoutInterval.inMilliseconds,
-        'enableAutoNativeBreadcrumbs': options.enableAutoNativeBreadcrumbs,
-        'maxCacheItems': options.maxCacheItems,
-        'sendDefaultPii': options.sendDefaultPii,
-        'enableWatchdogTerminationTracking':
-            options.enableWatchdogTerminationTracking,
-        'enableNdkScopeSync': options.enableNdkScopeSync,
-        'enableAutoPerformanceTracing': options.enableAutoPerformanceTracing,
-        'sendClientReports': options.sendClientReports,
-        'proguardUuid': options.proguardUuid,
-        'maxAttachmentSize': options.maxAttachmentSize,
-        'recordHttpBreadcrumbs': options.recordHttpBreadcrumbs,
-        'captureFailedRequests': options.captureFailedRequests,
-        'enableAppHangTracking': options.enableAppHangTracking,
-        'connectionTimeoutMillis': options.connectionTimeout.inMilliseconds,
-        'readTimeoutMillis': options.readTimeout.inMilliseconds,
-        'appHangTimeoutIntervalMillis':
-            options.appHangTimeoutInterval.inMilliseconds,
-      });
 
+    try {
+      await _native.init(options);
       options.sdk.addIntegration('nativeSdkIntegration');
     } catch (exception, stackTrace) {
       options.logger(
@@ -69,19 +34,17 @@ class NativeSdkIntegration implements Integration<SentryFlutterOptions> {
 
   @override
   Future<void> close() async {
-    final options = _options;
-    if (options != null && !options.autoInitializeNativeSdk) {
-      return;
-    }
-    try {
-      await _channel.invokeMethod('closeNativeSdk');
-    } catch (exception, stackTrace) {
-      _options?.logger(
-        SentryLevel.fatal,
-        'nativeSdkIntegration failed to be closed',
-        exception: exception,
-        stackTrace: stackTrace,
-      );
+    if (_options?.autoInitializeNativeSdk == true) {
+      try {
+        await _native.close();
+      } catch (exception, stackTrace) {
+        _options?.logger(
+          SentryLevel.fatal,
+          'nativeSdkIntegration failed to be closed',
+          exception: exception,
+          stackTrace: stackTrace,
+        );
+      }
     }
   }
 }
diff --git a/flutter/lib/src/native/factory_real.dart b/flutter/lib/src/native/factory_real.dart
@@ -2,12 +2,15 @@ import 'package:flutter/services.dart';
 
 import '../../sentry_flutter.dart';
 import 'cocoa/sentry_native_cocoa.dart';
+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);
   } else {
     return SentryNativeChannel(channel);
   }

diff --git a/flutter/lib/src/native/factory_web.dart b/flutter/lib/src/native/factory_web.dart
@@ -4,6 +4,6 @@ import '../../sentry_flutter.dart';
 import 'sentry_native_binding.dart';
 
 // This isn't actually called, see SentryFlutter.init()
-SentryNativeBinding createBinding(PlatformChecker pc, MethodChannel channel) {
+SentryNativeBinding createBinding(PlatformChecker _, MethodChannel __) {
   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
@@ -0,0 +1,10 @@
+import 'package:meta/meta.dart';
+
+import '../sentry_native_channel.dart';
+
+// Note: currently this doesn't do anything. Later, it shall be used with
+// generated JNI bindings. See https://github.com/getsentry/sentry-dart/issues/1444
+@internal
+class SentryNativeJava extends SentryNativeChannel {
+  SentryNativeJava(super.channel);
+}
diff --git a/flutter/lib/src/native/sentry_native.dart b/flutter/lib/src/native/sentry_native.dart
@@ -30,6 +30,11 @@ class SentryNative {
   /// Flag indicating if app start measurement was added to the first transaction.
   bool didAddAppStartMeasurement = false;
 
+  Future<void> init(SentryFlutterOptions options) async =>
+      _invoke("init", () => _binding.init(options));
+
+  Future<void> close() async => _invoke("close", _binding.close);
+
   /// Fetch [NativeAppStart] from native channels. Can only be called once.
   Future<NativeAppStart?> fetchNativeAppStart() async {
     _didFetchAppStart = true;

diff --git a/flutter/lib/src/native/sentry_native_binding.dart b/flutter/lib/src/native/sentry_native_binding.dart
@@ -9,6 +9,9 @@ import 'sentry_native.dart';
 @internal
 abstract class SentryNativeBinding {
   // TODO Move other native calls here.
+  Future<void> init(SentryFlutterOptions options);
+
+  Future<void> close();
 
   Future<NativeAppStart?> fetchNativeAppStart();
 

diff --git a/flutter/lib/src/native/sentry_native_channel.dart b/flutter/lib/src/native/sentry_native_channel.dart
@@ -17,6 +17,49 @@ class SentryNativeChannel implements SentryNativeBinding {
 
   // TODO Move other native calls here.
 
+  @override
+  Future<void> init(SentryFlutterOptions options) async =>
+      _channel.invokeMethod('initNativeSdk', <String, dynamic>{
+        'dsn': options.dsn,
+        'debug': options.debug,
+        'environment': options.environment,
+        'release': options.release,
+        'enableAutoSessionTracking': options.enableAutoSessionTracking,
+        'enableNativeCrashHandling': options.enableNativeCrashHandling,
+        'attachStacktrace': options.attachStacktrace,
+        'attachThreads': options.attachThreads,
+        'autoSessionTrackingIntervalMillis':
+            options.autoSessionTrackingInterval.inMilliseconds,
+        'dist': options.dist,
+        'integrations': options.sdk.integrations,
+        'packages':
+            options.sdk.packages.map((e) => e.toJson()).toList(growable: false),
+        'diagnosticLevel': options.diagnosticLevel.name,
+        'maxBreadcrumbs': options.maxBreadcrumbs,
+        'anrEnabled': options.anrEnabled,
+        'anrTimeoutIntervalMillis': options.anrTimeoutInterval.inMilliseconds,
+        'enableAutoNativeBreadcrumbs': options.enableAutoNativeBreadcrumbs,
+        'maxCacheItems': options.maxCacheItems,
+        'sendDefaultPii': options.sendDefaultPii,
+        'enableWatchdogTerminationTracking':
+            options.enableWatchdogTerminationTracking,
+        'enableNdkScopeSync': options.enableNdkScopeSync,
+        'enableAutoPerformanceTracing': options.enableAutoPerformanceTracing,
+        'sendClientReports': options.sendClientReports,
+        'proguardUuid': options.proguardUuid,
+        'maxAttachmentSize': options.maxAttachmentSize,
+        'recordHttpBreadcrumbs': options.recordHttpBreadcrumbs,
+        'captureFailedRequests': options.captureFailedRequests,
+        'enableAppHangTracking': options.enableAppHangTracking,
+        'connectionTimeoutMillis': options.connectionTimeout.inMilliseconds,
+        'readTimeoutMillis': options.readTimeout.inMilliseconds,
+        'appHangTimeoutIntervalMillis':
+            options.appHangTimeoutInterval.inMilliseconds,
+      });
+
+  @override
+  Future<void> close() async => _channel.invokeMethod('closeNativeSdk');
+
   @override
   Future<NativeAppStart?> fetchNativeAppStart() async {
     final json =

diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart
@@ -158,8 +158,8 @@ mixin SentryFlutter {
     // The ordering here matters, as we'd like to first start the native integration.
     // That allow us to send events to the network and then the Flutter integrations.
     // Flutter Web doesn't need that, only Android and iOS.
-    if (platformChecker.hasNativeIntegration) {
-      integrations.add(NativeSdkIntegration(channel));
+    if (_native != null) {
+      integrations.add(NativeSdkIntegration(_native!));
     }
 
     // Will enrich events with device context, native packages and integrations