From 2966d88d828c6e2604b15b95f0d0d958acc2cc68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= 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> { 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> { } Future _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; }