diff --git a/CHANGELOG.md b/CHANGELOG.md index c38e37bedd..2715fb3186 100644 --- a/CHANGELOG.md +++ b/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/)) diff --git a/packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt b/packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt index 522bd0e62f..4d47d974de 100644 --- a/packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt +++ b/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() diff --git a/packages/flutter/example/integration_test/integration_test.dart b/packages/flutter/example/integration_test/integration_test.dart index 104e966027..8515ec4a01 100644 --- a/packages/flutter/example/integration_test/integration_test.dart +++ b/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; diff --git a/packages/flutter/ffi-cocoa.yaml b/packages/flutter/ffi-cocoa.yaml index 0ce1d9751c..7d37479087 100644 --- a/packages/flutter/ffi-cocoa.yaml +++ b/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 diff --git a/packages/flutter/ios/sentry_flutter/Sources/sentry_flutter/SentryFlutterPlugin.swift b/packages/flutter/ios/sentry_flutter/Sources/sentry_flutter/SentryFlutterPlugin.swift index f9f856a69d..50012639d9 100644 --- a/packages/flutter/ios/sentry_flutter/Sources/sentry_flutter/SentryFlutterPlugin.swift +++ b/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 { diff --git a/packages/flutter/lib/src/native/cocoa/binding.dart b/packages/flutter/lib/src/native/cocoa/binding.dart index 8db80acafa..98b943ac70 100644 --- a/packages/flutter/lib/src/native/cocoa/binding.dart +++ b/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); diff --git a/packages/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/packages/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart index 5bfd0ef131..cdf0358389 100644 --- a/packages/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart +++ b/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 diff --git a/packages/flutter/lib/src/native/java/sentry_native_java.dart b/packages/flutter/lib/src/native/java/sentry_native_java.dart index ac5142a73b..64bb9dc6ec 100644 --- a/packages/flutter/lib/src/native/java/sentry_native_java.dart +++ b/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) { diff --git a/packages/flutter/lib/src/native/sentry_native_channel.dart b/packages/flutter/lib/src/native/sentry_native_channel.dart index 3c4754c07e..89bc6a18e4 100644 --- a/packages/flutter/lib/src/native/sentry_native_channel.dart +++ b/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 setExtra(String key, dynamic value) => channel.invokeMethod( - 'setExtra', - {'key': key, 'value': normalize(value)}, - ); + FutureOr setExtra(String key, dynamic value) { + assert(false, 'setExtra should not be used through method channels.'); + } @override - Future removeExtra(String key) => - channel.invokeMethod('removeExtra', {'key': key}); + FutureOr removeExtra(String key) { + assert(false, 'removeExtra should not be used through method channels.'); + } @override FutureOr setTag(String key, String value) { diff --git a/packages/flutter/test/sentry_native_channel_test.dart b/packages/flutter/test/sentry_native_channel_test.dart index d8f7f5e47a..d46b164edd 100644 --- a/packages/flutter/test/sentry_native_channel_test.dart +++ b/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 {