fix: replayOnError capture on iOS (#2306)

* fix: on-error only replay crashes the app * fix: on-error replay capture on iOS * chore: changelog
getsentry · Sep 25, 2024 · e112bbc · e112bbc
1 parent 48c3cf1
commit e112bbc
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,10 @@
   - Fixes ([#2103](https://github.com/getsentry/sentry-dart/issues/2103))
   - Fixes ([#2233](https://github.com/getsentry/sentry-dart/issues/2233))
 
+### Fixes
+
+- iOS replay integration when only `onErrorSampleRate` is specified ([#2306](https://github.com/getsentry/sentry-dart/pull/2306))
+
 ## 8.9.0
 
 ### Features

diff --git a/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m b/flutter/ios/Classes/SentryFlutterReplayScreenshotProvider.m
@@ -19,9 +19,12 @@ - (instancetype _Nonnull)initWithChannel:
 - (void)imageWithView:(UIView *_Nonnull)view
               options:(id<SentryRedactOptions> _Nonnull)options
            onComplete:(void (^_Nonnull)(UIImage *_Nonnull))onComplete {
+  // Replay ID may be null if session replay is disabled.
+  // Replay is still captured for on-error replays.
+  NSString *replayId = [PrivateSentrySDKOnly getReplayId];
   [self->channel
       invokeMethod:@"captureReplayScreenshot"
-         arguments:@{@"replayId" : [PrivateSentrySDKOnly getReplayId]}
+         arguments:@{@"replayId" : replayId ? replayId : [NSNull null]}
             result:^(id value) {
               if (value == nil) {
                 NSLog(@"SentryFlutterReplayScreenshotProvider received null "
@@ -33,6 +36,11 @@ - (void)imageWithView:(UIView *_Nonnull)view
                     (FlutterStandardTypedData *)value;
                 UIImage *image = [UIImage imageWithData:typedData.data];
                 onComplete(image);
+              } else if ([value isKindOfClass:[FlutterError class]]) {
+                FlutterError *error = (FlutterError *)value;
+                NSLog(@"SentryFlutterReplayScreenshotProvider received an "
+                      @"error: %@. Cannot capture a replay screenshot.",
+                      error.message);
               } else {
                 NSLog(@"SentryFlutterReplayScreenshotProvider received an "
                       @"unexpected result. "

diff --git a/flutter/lib/src/event_processor/replay_event_processor.dart b/flutter/lib/src/event_processor/replay_event_processor.dart
@@ -5,17 +5,23 @@ import 'package:sentry/sentry.dart';
 import '../native/sentry_native_binding.dart';
 
 class ReplayEventProcessor implements EventProcessor {
+  final Hub _hub;
   final SentryNativeBinding _binding;
 
-  ReplayEventProcessor(this._binding);
+  ReplayEventProcessor(this._hub, this._binding);
 
   @override
   Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
     if (event.eventId != SentryId.empty() &&
         event.exceptions?.isNotEmpty == true) {
       final isCrash =
           event.exceptions!.any((e) => e.mechanism?.handled == false);
-      await _binding.captureReplay(isCrash);
+      final replayId = await _binding.captureReplay(isCrash);
+      // If session replay is disabled, this is the first time we receive the ID.
+      _hub.configureScope((scope) {
+        // ignore: invalid_use_of_internal_member
+        scope.replayId = replayId;
+      });
     }
     return event;
   }

diff --git a/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart b/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart
@@ -27,16 +27,17 @@ class SentryNativeCocoa extends SentryNativeChannel {
         options.platformChecker.platform.isIOS) {
       // We only need the integration when error-replay capture is enabled.
       if ((options.experimental.replay.onErrorSampleRate ?? 0) > 0) {
-        options.addEventProcessor(ReplayEventProcessor(this));
+        options.addEventProcessor(ReplayEventProcessor(hub, this));
       }
 
       channel.setMethodCallHandler((call) async {
         switch (call.method) {
           case 'captureReplayScreenshot':
             _replayRecorder ??=
                 ScreenshotRecorder(ScreenshotRecorderConfig(), options);
-            final replayId =
-                SentryId.fromId(call.arguments['replayId'] as String);
+            final replayId = call.arguments['replayId'] == null
+                ? null
+                : SentryId.fromId(call.arguments['replayId'] as String);
             if (_replayId != replayId) {
               _replayId = replayId;
               hub.configureScope((s) {

diff --git a/flutter/lib/src/native/java/sentry_native_java.dart b/flutter/lib/src/native/java/sentry_native_java.dart
@@ -25,7 +25,7 @@ class SentryNativeJava extends SentryNativeChannel {
     if (options.experimental.replay.isEnabled) {
       // We only need the integration when error-replay capture is enabled.
       if ((options.experimental.replay.onErrorSampleRate ?? 0) > 0) {
-        options.addEventProcessor(ReplayEventProcessor(this));
+        options.addEventProcessor(ReplayEventProcessor(hub, this));
       }
 
       channel.setMethodCallHandler((call) async {

diff --git a/flutter/test/replay/replay_event_processor_test.dart b/flutter/test/replay/replay_event_processor_test.dart
@@ -0,0 +1,78 @@
+// ignore_for_file: invalid_use_of_internal_member
+
+import 'dart:async';
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:mockito/mockito.dart';
+import 'package:sentry_flutter/sentry_flutter.dart';
+import 'package:sentry_flutter/src/event_processor/replay_event_processor.dart';
+
+import '../mocks.dart';
+import '../mocks.mocks.dart';
+
+void main() {
+  late _Fixture fixture;
+  setUp(() {
+    fixture = _Fixture();
+  });
+
+  for (var isHandled in [true, false]) {
+    test(
+        'captures replay for ${isHandled ? 'handled' : 'unhandled'} exceptions',
+        () async {
+      final event = await fixture.apply(isHandled: isHandled);
+      bool isCrash = verify(fixture.binding.captureReplay(captureAny))
+          .captured
+          .single as bool;
+      expect(isCrash, !isHandled);
+      expect(event, isNotNull);
+    });
+
+    test(
+        'sets scope replay ID for ${isHandled ? 'handled' : 'unhandled'} exceptions',
+        () async {
+      expect(fixture.scope.replayId, isNull);
+      await fixture.apply(isHandled: isHandled);
+      expect(fixture.scope.replayId, SentryId.fromId('42'));
+    });
+  }
+
+  test('does not capture replay for non-errors', () async {
+    await fixture.apply(hasException: false);
+    verifyNever(fixture.binding.captureReplay(any));
+    expect(fixture.scope.replayId, isNull);
+  });
+}
+
+class _Fixture {
+  late final ReplayEventProcessor sut;
+  final MockHub hub = MockHub();
+  final MockSentryNativeBinding binding = MockSentryNativeBinding();
+  Scope scope = Scope(defaultTestOptions());
+
+  _Fixture() {
+    when(binding.captureReplay(captureAny))
+        .thenAnswer((_) async => SentryId.fromId('42'));
+    when(hub.configureScope(any)).thenAnswer((invocation) async {
+      final callback = invocation.positionalArguments.first as FutureOr<void>
+          Function(Scope);
+      await callback(scope);
+    });
+    sut = ReplayEventProcessor(hub, binding);
+  }
+  Future<SentryEvent?> apply(
+      {bool hasException = true, bool isHandled = false}) {
+    final event = SentryEvent(
+      eventId: SentryId.newId(),
+      exceptions: hasException
+          ? [
+              SentryException(
+                  type: 'type',
+                  value: 'value',
+                  mechanism: Mechanism(type: 'foo', handled: isHandled))
+            ]
+          : [],
+    );
+    return sut.apply(event, Hint());
+  }
+}
diff --git a/flutter/test/replay/replay_native_test.dart b/flutter/test/replay/replay_native_test.dart
@@ -4,7 +4,6 @@
 library flutter_test;
 
 import 'dart:async';
-import 'dart:typed_data';
 
 import 'package:file/file.dart';
 import 'package:file/memory.dart';
@@ -33,24 +32,24 @@ void main() {
       late MockHub hub;
       late FileSystem fs;
       late Directory replayDir;
-      late final Map<String, dynamic> replayConfig;
-
-      if (mockPlatform.isIOS) {
-        replayConfig = {
-          'replayId': '123',
-          'directory': 'dir',
-        };
-      } else if (mockPlatform.isAndroid) {
-        replayConfig = {
-          'replayId': '123',
-          'directory': 'dir',
-          'width': 800,
-          'height': 600,
-          'frameRate': 10,
-        };
-      }
+      late Map<String, dynamic> replayConfig;
 
       setUp(() {
+        if (mockPlatform.isIOS) {
+          replayConfig = {
+            'replayId': '123',
+            'directory': 'dir',
+          };
+        } else if (mockPlatform.isAndroid) {
+          replayConfig = {
+            'replayId': '123',
+            'directory': 'dir',
+            'width': 800,
+            'height': 600,
+            'frameRate': 10,
+          };
+        }
+
         hub = MockHub();
 
         fs = MemoryFileSystem.test();
@@ -233,8 +232,14 @@ void main() {
               await nextFrame();
 
               final imagaData = await native.invokeFromNative(
-                  'captureReplayScreenshot', replayConfig) as ByteData;
-              expect(imagaData.lengthInBytes, greaterThan(3000));
+                  'captureReplayScreenshot', replayConfig);
+              expect(imagaData?.lengthInBytes, greaterThan(3000));
+
+              // Happens if the session-replay rate is 0.
+              replayConfig['replayId'] = null;
+              final imagaData2 = await native.invokeFromNative(
+                  'captureReplayScreenshot', replayConfig);
+              expect(imagaData2?.lengthInBytes, greaterThan(3000));
             } else {
               fail('unsupported platform');
             }