From 2966d88d828c6e2604b15b95f0d0d958acc2cc68 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= <denrase@gmail.com>
Date: Wed, 14 Feb 2024 09:31:51 +0000
Subject: [PATCH] Fix iOS "Arithmetic Overflow" (#1874)

* guard against arithmetic overflow

* Fix issue where transaction was finished multiple times

* add changelog entry

* update test expectations

* test that didPop does not call finsh transaction multiple times
---
 CHANGELOG.md                                  |  1 +
 .../Classes/SentryFlutterPluginApple.swift    |  7 +++---
 .../navigation/sentry_navigator_observer.dart | 10 ++++++--
 .../test/sentry_navigator_observer_test.dart  | 25 ++++++++++++++++---
 4 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 56777b2fde..415db64a2e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 
 - Remove Flutter dependency from Drift integration ([#1867](https://github.com/getsentry/sentry-dart/pull/1867))
 - Remove dead code, cold start bool is now always present ([#1861](https://github.com/getsentry/sentry-dart/pull/1861))
+- Fix iOS "Arithmetic Overflow" ([#1874](https://github.com/getsentry/sentry-dart/pull/1874))
 
 ### Dependencies
 
diff --git a/flutter/ios/Classes/SentryFlutterPluginApple.swift b/flutter/ios/Classes/SentryFlutterPluginApple.swift
index be5e301791..ec6eb7cdb7 100644
--- a/flutter/ios/Classes/SentryFlutterPluginApple.swift
+++ b/flutter/ios/Classes/SentryFlutterPluginApple.swift
@@ -430,10 +430,9 @@ public class SentryFlutterPluginApple: NSObject, FlutterPlugin {
       }
 
       let currentFrames = PrivateSentrySDKOnly.currentScreenFrames
-
-      let total = currentFrames.total - totalFrames
-      let frozen = currentFrames.frozen - frozenFrames
-      let slow = currentFrames.slow - slowFrames
+      let total = max(Int(currentFrames.total) - Int(totalFrames), 0)
+      let frozen = max(Int(currentFrames.frozen) - Int(frozenFrames), 0)
+      let slow = max(Int(currentFrames.slow) - Int(slowFrames), 0)
 
       if total <= 0 && frozen <= 0 && slow <= 0 {
         result(nil)
diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart
index 6893d47aca..b24f4f0c7b 100644
--- a/flutter/lib/src/navigation/sentry_navigator_observer.dart
+++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart
@@ -207,6 +207,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
       autoFinishAfter: _autoFinishAfter,
       trimEnd: true,
       onFinish: (transaction) async {
+        _transaction = null;
         final nativeFrames = await _native
             ?.endNativeFramesCollection(transaction.context.traceId);
         if (nativeFrames != null) {
@@ -241,8 +242,13 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
   }
 
   Future<void> _finishTransaction() async {
-    _transaction?.status ??= SpanStatus.ok();
-    await _transaction?.finish();
+    final transaction = _transaction;
+    _transaction = null;
+    if (transaction == null || transaction.finished) {
+      return;
+    }
+    transaction.status ??= SpanStatus.ok();
+    await transaction.finish();
   }
 }
 
diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart
index c49588ab85..aca9646c0b 100644
--- a/flutter/test/sentry_navigator_observer_test.dart
+++ b/flutter/test/sentry_navigator_observer_test.dart
@@ -261,7 +261,7 @@ void main() {
       final secondRoute = route(RouteSettings(name: 'Second Route'));
 
       final hub = _MockHub();
-      final span = getMockSentryTracer();
+      final span = getMockSentryTracer(finished: false);
       when(span.context).thenReturn(SentrySpanContext(operation: 'op'));
       when(span.status).thenReturn(null);
       _whenAnyStart(hub, span);
@@ -279,7 +279,7 @@ void main() {
       final currentRoute = route(RouteSettings(name: 'Current Route'));
 
       final hub = _MockHub();
-      final span = getMockSentryTracer();
+      final span = getMockSentryTracer(finished: false);
       when(span.context).thenReturn(SentrySpanContext(operation: 'op'));
       when(span.status).thenReturn(null);
       _whenAnyStart(hub, span);
@@ -293,6 +293,24 @@ void main() {
       verify(span.finish());
     });
 
+    test('multiple didPop only finish transaction once', () {
+      final currentRoute = route(RouteSettings(name: 'Current Route'));
+
+      final hub = _MockHub();
+      final span = getMockSentryTracer(finished: false);
+      when(span.context).thenReturn(SentrySpanContext(operation: 'op'));
+      when(span.status).thenReturn(null);
+      _whenAnyStart(hub, span);
+
+      final sut = fixture.getSut(hub: hub);
+
+      sut.didPush(currentRoute, null);
+      sut.didPop(currentRoute, null);
+      sut.didPop(currentRoute, null);
+
+      verify(span.finish()).called(1);
+    });
+
     test('didPop re-starts previous', () {
       final previousRoute = route(RouteSettings(name: 'Previous Route'));
       final currentRoute = route(RouteSettings(name: 'Current Route'));
@@ -833,9 +851,10 @@ class _MockHub extends MockHub {
   }
 }
 
-ISentrySpan getMockSentryTracer({String? name}) {
+ISentrySpan getMockSentryTracer({String? name, bool? finished}) {
   final tracer = MockSentryTracer();
   when(tracer.name).thenReturn(name ?? 'name');
+  when(tracer.finished).thenReturn(finished ?? true);
   return tracer;
 }