enh: refactor setExtra and removeExtra to FFI/JNI (#3314)

* Refactor setContext and removeContext to FFI/JNI

* Update scope

* Integration test

* Fix swift lint

* CHANGELOG

* Improve objc conversion

* Fix tests

* Remove native channel from coverage

* Refactor setTag and removeTag to FFI/JNI

* Refactor setExtra and removeExtra to FFI/JNI

* Update CHANGELOG

* Update CHANGELOG

* Fix analyze

* Null handling on Android side

* Unused import

* Unused import

* Fix test

* Fix test

* Update

* Update

Author: buenaflor

CHANGELOG.md

@@ -9,6 +9,7 @@
 
 ### Enhancements
 
+- Refactor `setExtra` and `removeExtra` to use FFI/JNI ([#3314](https://github.com/getsentry/sentry-dart/pull/3314))
 - Refactor `setTag` and `removeTag` to use FFI/JNI ([#3313](https://github.com/getsentry/sentry-dart/pull/3313))
 - Refactor `setContexts` and `removeContexts` to use FFI/JNI ([#3312](https://github.com/getsentry/sentry-dart/pull/3312))
 - Refactor `setUser` to use FFI/JNI ([#3295](https://github.com/getsentry/sentry-dart/pull/3295/))

packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt

@@ -61,8 +61,6 @@ class SentryFlutterPlugin :
     when (call.method) {
       "initNativeSdk" -> initNativeSdk(call, result)
       "closeNativeSdk" -> closeNativeSdk(result)
-      "setExtra" -> setExtra(call.argument("key"), call.argument("value"), result)
-      "removeExtra" -> removeExtra(call.argument("key"), result)
       "setReplayConfig" -> setReplayConfig(call, result)
       "captureReplay" -> captureReplay(result)
       else -> result.notImplemented()
@@ -137,33 +135,6 @@ class SentryFlutterPlugin :
     }
   }
 
-  private fun setExtra(
-    key: String?,
-    value: String?,
-    result: Result,
-  ) {
-    if (key == null || value == null) {
-      result.success("")
-      return
-    }
-    Sentry.setExtra(key, value)
-
-    result.success("")
-  }
-
-  private fun removeExtra(
-    key: String?,
-    result: Result,
-  ) {
-    if (key == null) {
-      result.success("")
-      return
-    }
-    Sentry.removeExtra(key)
-
-    result.success("")
-  }
-
   private fun closeNativeSdk(result: Result) {
     ScopesAdapter.getInstance().close()
 

packages/flutter/example/integration_test/integration_test.dart

@@ -1,6 +1,4 @@
-// ignore_for_file: avoid_print
-// ignore_for_file: invalid_use_of_internal_member
-// ignore_for_file: unused_local_variable
+// ignore_for_file: avoid_print, invalid_use_of_internal_member, unused_local_variable, deprecated_member_use
 
 import 'dart:async';
 import 'dart:convert';
@@ -130,9 +128,7 @@ void main() {
       await scope.addBreadcrumb(breadcrumb);
       await scope.clearBreadcrumbs();
 
-      // ignore: deprecated_member_use
       await scope.setExtra('extra-key', 'extra-value');
-      // ignore: deprecated_member_use
       await scope.removeExtra('extra-key');
 
       await scope.setTag('tag-key', 'tag-value');
@@ -823,6 +819,64 @@ void main() {
     }
   });
 
+  testWidgets('setExtra and removeExtra sync to native', (tester) async {
+    await restoreFlutterOnErrorAfter(() async {
+      await setupSentryAndApp(tester);
+    });
+
+    await Sentry.configureScope((scope) async {
+      scope.setExtra('key1', 'randomValue');
+      scope.setExtra('key2',
+          {'String': 'Value', 'Bool': true, 'Int': 123, 'Double': 12.3});
+      scope.setExtra('key3', true);
+      scope.setExtra('key4', 12);
+      scope.setExtra('key5', 12.3);
+    });
+
+    var contexts = await SentryFlutter.native?.loadContexts();
+
+    final extras = (Platform.isIOS || Platform.isMacOS)
+        ? contexts!['extra']
+        : contexts!['extras'];
+    expect(extras, isNotNull, reason: 'Extras are null');
+
+    if (Platform.isIOS || Platform.isMacOS) {
+      expect(extras['key1'], 'randomValue', reason: 'key1 mismatch');
+      expect(extras['key2'],
+          {'String': 'Value', 'Bool': 1, 'Int': 123, 'Double': 12.3},
+          reason: 'key2 mismatch');
+      // bool values are mapped to num values of 1 or 0 during objc conversion
+      expect(extras['key3'], 1, reason: 'key3 mismatch');
+      expect(extras['key4'], 12, reason: 'key4 mismatch');
+      expect(extras['key5'], 12.3, reason: 'key5 mismatch');
+    } else if (Platform.isAndroid) {
+      // Sentry Java's setExtra only allows String values so this is after normalization
+      expect(extras['key1'], 'randomValue', reason: 'key1 mismatch');
+      expect(
+          extras['key2'], '{String: Value, Bool: true, Int: 123, Double: 12.3}',
+          reason: 'key2 mismatch');
+      expect(extras['key3'], 'true', reason: 'key3 mismatch');
+      expect(extras['key4'], '12', reason: 'key4 mismatch');
+      expect(extras['key5'], '12.3', reason: 'key5 mismatch');
+    }
+
+    await Sentry.configureScope((scope) async {
+      scope.removeExtra('key1');
+      scope.removeExtra('key2');
+      scope.removeExtra('key3');
+      scope.removeExtra('key4');
+      scope.removeExtra('key5');
+    });
+
+    contexts = await SentryFlutter.native?.loadContexts();
+    final extraKey = (Platform.isIOS || Platform.isMacOS) ? 'extra' : 'extras';
+    if (Platform.isIOS || Platform.isMacOS) {
+      expect(contexts![extraKey], isNull, reason: 'Extra are not null');
+    } else {
+      expect(contexts![extraKey], {}, reason: 'Extra are not empty');
+    }
+  });
+
   group('e2e', () {
     var output = find.byKey(const Key('output'));
     late Fixture fixture;

packages/flutter/ffi-cocoa.yaml

@@ -42,6 +42,8 @@ objc-interfaces:
         - 'removeContextForKey:'
         - 'setTagValue:forKey:'
         - 'removeTagForKey:'
+        - 'setExtraValue:forKey:'
+        - 'removeExtraForKey:'
 preamble: |
   // ignore_for_file: type=lint, unused_element
 

packages/flutter/ios/sentry_flutter/Sources/sentry_flutter/SentryFlutterPlugin.swift

@@ -75,17 +75,6 @@ public class SentryFlutterPlugin: NSObject, FlutterPlugin {
         case "closeNativeSdk":
             closeNativeSdk(call, result: result)
 
-        case "setExtra":
-            let arguments = call.arguments as? [String: Any?]
-            let key = arguments?["key"] as? String
-            let value = arguments?["value"] as? Any
-            setExtra(key: key, value: value, result: result)
-
-        case "removeExtra":
-            let arguments = call.arguments as? [String: Any?]
-            let key = arguments?["key"] as? String
-            removeExtra(key: key, result: result)
-
         #if !os(tvOS) && !os(watchOS)
         case "discardProfiler":
             discardProfiler(call, result)
@@ -243,30 +232,6 @@ public class SentryFlutterPlugin: NSObject, FlutterPlugin {
         return !name.isEmpty
     }
 
-    private func setExtra(key: String?, value: Any?, result: @escaping FlutterResult) {
-      guard let key = key else {
-        result("")
-        return
-      }
-      SentrySDK.configureScope { scope in
-        scope.setExtra(value: value, key: key)
-
-        result("")
-      }
-    }
-
-    private func removeExtra(key: String?, result: @escaping FlutterResult) {
-      guard let key = key else {
-        result("")
-        return
-      }
-      SentrySDK.configureScope { scope in
-        scope.removeExtra(key: key)
-
-        result("")
-      }
-    }
-
     private func collectProfile(_ call: FlutterMethodCall, _ result: @escaping FlutterResult) {
         guard let arguments = call.arguments as? [String: Any],
               let traceId = arguments["traceId"] as? String else {

packages/flutter/lib/src/native/cocoa/binding.dart

@@ -1150,6 +1150,9 @@ interface class SentrySerializable extends objc.ObjCProtocolBase
 
 late final _sel_setTagValue_forKey_ = objc.registerName("setTagValue:forKey:");
 late final _sel_removeTagForKey_ = objc.registerName("removeTagForKey:");
+late final _sel_setExtraValue_forKey_ =
+    objc.registerName("setExtraValue:forKey:");
+late final _sel_removeExtraForKey_ = objc.registerName("removeExtraForKey:");
 late final _sel_clearBreadcrumbs = objc.registerName("clearBreadcrumbs");
 final _objc_msgSend_1pl9qdv = objc.msgSendPointer
     .cast<
@@ -1198,6 +1201,19 @@ class SentryScope extends objc.NSObject implements SentrySerializable {
         this.ref.pointer, _sel_removeTagForKey_, key.ref.pointer);
   }
 
+  /// Set global extra -> these will be sent with every event
+  void setExtraValue(objc.ObjCObjectBase? value,
+      {required objc.NSString forKey}) {
+    _objc_msgSend_pfv6jd(this.ref.pointer, _sel_setExtraValue_forKey_,
+        value?.ref.pointer ?? ffi.nullptr, forKey.ref.pointer);
+  }
+
+  /// Remove the extra for the specified key.
+  void removeExtraForKey(objc.NSString key) {
+    _objc_msgSend_xtuoz7(
+        this.ref.pointer, _sel_removeExtraForKey_, key.ref.pointer);
+  }
+
   /// Clears all breadcrumbs in the scope
   void clearBreadcrumbs() {
     _objc_msgSend_1pl9qdv(this.ref.pointer, _sel_clearBreadcrumbs);

packages/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart

@@ -284,6 +284,27 @@ class SentryNativeCocoa extends SentryNativeChannel {
           scope.removeTagForKey(key.toNSString());
         }));
       });
+
+  @override
+  void setExtra(String key, dynamic value) => tryCatchSync('setExtra', () {
+        if (value == null) return;
+
+        cocoa.SentrySDK.configureScope(
+            cocoa.ObjCBlock_ffiVoid_SentryScope.fromFunction(
+                (cocoa.SentryScope scope) {
+          scope.setExtraValue(_dartToNSObject(value as Object),
+              forKey: key.toNSString());
+        }));
+      });
+
+  @override
+  void removeExtra(String key) => tryCatchSync('removeExtra', () {
+        cocoa.SentrySDK.configureScope(
+            cocoa.ObjCBlock_ffiVoid_SentryScope.fromFunction(
+                (cocoa.SentryScope scope) {
+          scope.removeExtraForKey(key.toNSString());
+        }));
+      });
 }
 
 // The default conversion does not handle bool so we will add it ourselves

packages/flutter/lib/src/native/java/sentry_native_java.dart

@@ -8,6 +8,7 @@ import '../../../sentry_flutter.dart';
 import '../../replay/scheduled_recorder_config.dart';
 import '../native_app_start.dart';
 import '../sentry_native_channel.dart';
+import '../utils/data_normalizer.dart';
 import '../utils/utf8_json.dart';
 import 'android_envelope_sender.dart';
 import 'android_replay_recorder.dart';
@@ -336,6 +337,27 @@ class SentryNativeJava extends SentryNativeChannel {
           native.Sentry.removeTag(jKey);
         });
       });
+
+  @override
+  void setExtra(String key, dynamic value) => tryCatchSync('setExtra', () {
+        if (value == null) return;
+
+        using((arena) {
+          final jKey = key.toJString()..releasedBy(arena);
+          final jVal = normalize(value).toString().toJString()
+            ..releasedBy(arena);
+
+          native.Sentry.setExtra(jKey, jVal);
+        });
+      });
+
+  @override
+  void removeExtra(String key) => tryCatchSync('removeExtra', () {
+        using((arena) {
+          final jKey = key.toJString()..releasedBy(arena);
+          native.Sentry.removeExtra(jKey);
+        });
+      });
 }
 
 JObject? _dartToJObject(Object? value, Arena arena) => switch (value) {

packages/flutter/lib/src/native/sentry_native_channel.dart

@@ -12,7 +12,6 @@ import 'native_app_start.dart';
 import 'sentry_native_binding.dart';
 import 'sentry_native_invoker.dart';
 import 'sentry_safe_method_channel.dart';
-import 'utils/data_normalizer.dart';
 
 /// Provide typed methods to access native layer via MethodChannel.
 @internal
@@ -148,14 +147,14 @@ class SentryNativeChannel
   }
 
   @override
-  Future<void> setExtra(String key, dynamic value) => channel.invokeMethod(
-        'setExtra',
-        {'key': key, 'value': normalize(value)},
-      );
+  FutureOr<void> setExtra(String key, dynamic value) {
+    assert(false, 'setExtra should not be used through method channels.');
+  }
 
   @override
-  Future<void> removeExtra(String key) =>
-      channel.invokeMethod('removeExtra', {'key': key});
+  FutureOr<void> removeExtra(String key) {
+    assert(false, 'removeExtra should not be used through method channels.');
+  }
 
   @override
   FutureOr<void> setTag(String key, String value) {

packages/flutter/test/sentry_native_channel_test.dart

@@ -11,7 +11,6 @@ import 'package:sentry/src/platform/mock_platform.dart';
 import 'package:sentry_flutter/sentry_flutter.dart';
 import 'package:sentry_flutter/src/native/factory.dart';
 import 'package:sentry_flutter/src/native/sentry_native_binding.dart';
-import 'package:sentry_flutter/src/native/utils/data_normalizer.dart';
 import 'package:sentry_flutter/src/replay/replay_config.dart';
 
 import 'mocks.dart';
@@ -119,26 +118,29 @@ void main() {
         verifyZeroInteractions(channel);
       });
 
-      test('setExtra', () async {
+      test('setExtra', () {
         final value = {'object': Object()};
-        final normalizedValue = normalize(value);
-        when(channel.invokeMethod(
-                'setExtra', {'key': 'fixture-key', 'value': normalizedValue}))
-            .thenAnswer((_) => Future.value());
+        final matcher = _nativeUnavailableMatcher(
+          mockPlatform,
+          includeLookupSymbol: true,
+          includeFailedToLoadClassException: true,
+        );
 
-        await sut.setExtra('fixture-key', value);
+        expect(() => sut.setExtra('fixture-key', value), matcher);
 
-        verify(channel.invokeMethod(
-            'setExtra', {'key': 'fixture-key', 'value': normalizedValue}));
+        verifyZeroInteractions(channel);
       });
 
-      test('removeExtra', () async {
-        when(channel.invokeMethod('removeExtra', {'key': 'fixture-key'}))
-            .thenAnswer((_) => Future.value());
+      test('removeExtra', () {
+        final matcher = _nativeUnavailableMatcher(
+          mockPlatform,
+          includeLookupSymbol: true,
+          includeFailedToLoadClassException: true,
+        );
 
-        await sut.removeExtra('fixture-key');
+        expect(() => sut.removeExtra('fixture-key'), matcher);
 
-        verify(channel.invokeMethod('removeExtra', {'key': 'fixture-key'}));
+        verifyZeroInteractions(channel);
       });
 
       test('setTag', () async {