chore: Support for non-fully-blocking app hangs (#4286)

Add the logic for non-fully-blocking app hangs when the frame delay exceeds 99%. This is required for GH-3492.
getsentry · Aug 16, 2024 · fff4a70 · fff4a70
1 parent 2ca8e73
commit fff4a70
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 26 deletions.
diff --git a/Samples/iOS-Swift/iOS-Swift/Tools/TriggerAppHang.swift b/Samples/iOS-Swift/iOS-Swift/Tools/TriggerAppHang.swift
@@ -2,14 +2,17 @@ import Foundation
 import UIKit
 
 /// Triggers a non-fully blocking app hang by blocking the app for 0.5 seconds,
-/// then allowing it to draw a couple of frames, and blocking it again. While the app
+/// then allowing it to draw one or two frames, and blocking it again. While the app
 /// is not fully blocked because it renders a few frames, it still seems blocked to the
-/// user and should be considered an app hang.
+/// user and should be considered an app hang. We have to pick a low timer interval
+/// for the Thread.sleep on the background thread, because otherwise the app renders
+/// too many frames and is able to still handle user input, such as navigating to a
+/// different screen.
 func triggerNonFullyBlockingAppHang() {
 
     DispatchQueue.global().async {
         for _ in 0...10 {
-            Thread.sleep(forTimeInterval: 0.001)
+            Thread.sleep(forTimeInterval: 0.0001)
             DispatchQueue.main.sync {
                 Thread.sleep(forTimeInterval: 0.5)
             }

diff --git a/Sources/Sentry/SentryANRTrackerV2.m b/Sources/Sentry/SentryANRTrackerV2.m
@@ -173,7 +173,17 @@ - (void)detectANRs
             SENTRY_LOG_WARN(@"App Hang detected: fully-blocking.");
 
             reported = YES;
-            [self ANRDetected];
+            [self ANRDetected:SentryANRTypeFullyBlocking];
+        }
+
+        NSTimeInterval nonFullyBlockingFramesDelayThreshold = self.timeoutInterval * 0.99;
+        if (!isFullyBlocking
+            && framesDelayForTimeInterval.delayDuration > nonFullyBlockingFramesDelayThreshold) {
+
+            SENTRY_LOG_WARN(@"App Hang detected: non-fully-blocking.");
+
+            reported = YES;
+            [self ANRDetected:SentryANRTypeNonFullyBlocking];
         }
     }
 
@@ -183,15 +193,15 @@ - (void)detectANRs
     }
 }
 
-- (void)ANRDetected
+- (void)ANRDetected:(enum SentryANRType)type
 {
     NSArray *localListeners;
     @synchronized(self.listeners) {
         localListeners = [self.listeners allObjects];
     }
 
     for (id<SentryANRTrackerV2Delegate> target in localListeners) {
-        [target anrDetected];
+        [target anrDetectedWithType:type];
     }
 }
 

diff --git a/Sources/Sentry/SentryANRTrackingIntegrationV2.m b/Sources/Sentry/SentryANRTrackingIntegrationV2.m
@@ -76,7 +76,7 @@ - (void)dealloc
     [self uninstall];
 }
 
-- (void)anrDetected
+- (void)anrDetectedWithType:(enum SentryANRType)type
 {
     if (self.reportAppHangs == NO) {
         SENTRY_LOG_DEBUG(@"AppHangTracking paused. Ignoring reported app hang.")

diff --git a/Sources/Sentry/include/SentryANRTrackerV2.h b/Sources/Sentry/include/SentryANRTrackerV2.h
@@ -10,6 +10,16 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+/**
+ * This class detects ANRs with a dedicated watchdog thread. It periodically checks the frame delay.
+ * If the app cannot render a single frame and the frame delay is 100%, then it reports a
+ * fully-blocking app hang. If the frame delay exceeds 99%, then this class reports a
+ * non-fully-blocking app hang. We pick an extra high threshold of 99% because only then the app
+ * seems to be hanging. With a lower threshold, the logic would overreport. Even when the app hangs
+ * for 0.5 seconds and has a chance to render around five frames and then hangs again for 0.5
+ * seconds, it can still respond to user input to navigate to a different screen, for example. In
+ * that scenario, the frame delay is around 97%.
+ */
 @interface SentryANRTrackerV2 : NSObject
 SENTRY_NO_INIT
 

diff --git a/Sources/Swift/Integrations/ANR/SentryANRTrackerV2Delegate.swift b/Sources/Swift/Integrations/ANR/SentryANRTrackerV2Delegate.swift
@@ -2,6 +2,12 @@ import Foundation
 
 @objc
 protocol SentryANRTrackerV2Delegate {
-    func anrDetected()
+    func anrDetected(type: SentryANRType)
     func anrStopped()
 }
+
+@objc
+enum SentryANRType: Int {
+    case fullyBlocking
+    case nonFullyBlocking
+}
diff --git a/Tests/SentryTests/Integrations/ANR/SentryANRTrackerV2Tests.swift b/Tests/SentryTests/Integrations/ANR/SentryANRTrackerV2Tests.swift
@@ -85,17 +85,40 @@ class SentryANRTrackerV2Tests: XCTestCase {
     /// [||||------|--------]
     /// - means no frame rendered
     /// | means a rendered frame
-    func testNonFullyBlockingAppHang_NotReported() throws {
+    func testNonFullyBlockingAppHang_Reported() throws {
+        let (sut, _, displayLinkWrapper, _, _, _) = try getSut()
+        defer { sut.clear() }
+
+        let listener = SentryANRTrackerV2TestDelegate()
+
+        sut.addListener(listener)
+
+        triggerNonFullyBlockingAppHang(displayLinkWrapper)
+
+        wait(for: [listener.anrDetectedExpectation], timeout: waitTimeout)
+        XCTAssertEqual(listener.anrsDetected.last, .nonFullyBlocking)
+
+        renderNormalFramesToStopAppHang(displayLinkWrapper)
+
+        wait(for: [listener.anrStoppedExpectation], timeout: waitTimeout)
+    }
+
+    /// 3 frozen frames aren't enough for a non fully blocking app hang.
+    ///
+    /// [||||---|------|-----]
+    /// - means no frame rendered
+    /// | means a rendered frame
+    func testAlmostNonFullyBlockingAppHang_NoneReported() throws {
         let (sut, _, displayLinkWrapper, _, _, _) = try getSut()
         defer { sut.clear() }
 
         let listener = SentryANRTrackerV2TestDelegate(shouldANRBeDetected: false, shouldStoppedBeCalled: false)
 
         sut.addListener(listener)
 
-        displayLinkWrapper.frameWith(delay: 1.0)
-        displayLinkWrapper.normalFrame()
-        displayLinkWrapper.frameWith(delay: 0.91)
+        displayLinkWrapper.frameWith(delay: 0.7)
+        displayLinkWrapper.frameWith(delay: 0.7)
+        displayLinkWrapper.frameWith(delay: 0.7)
 
         wait(for: [listener.anrDetectedExpectation], timeout: waitTimeout)
 
@@ -141,14 +164,11 @@ class SentryANRTrackerV2Tests: XCTestCase {
 
         sut.addListener(listener)
 
-        dateProvider.advance(by: 2.1)
+        triggerFullyBlockingAppHang(dateProvider)
 
         wait(for: [listener.anrDetectedExpectation], timeout: waitTimeout)
 
-        displayLinkWrapper.normalFrame()
-        dateProvider.advance(by: 1.0)
-        displayLinkWrapper.normalFrame()
-        dateProvider.advance(by: 1.0)
+        triggerNonFullyBlockingAppHang(displayLinkWrapper)
 
         renderNormalFramesToStopAppHang(displayLinkWrapper)
 
@@ -425,12 +445,19 @@ class SentryANRTrackerV2Tests: XCTestCase {
             currentDate.advance(by: 0.01)
         }
     }
+
+    private func triggerNonFullyBlockingAppHang(_ displayLinkWrapper: TestDisplayLinkWrapper) {
+        displayLinkWrapper.frameWith(delay: 1.0)
+        displayLinkWrapper.frameWith(delay: 1.0)
+    }
+
 }
 
 class SentryANRTrackerV2TestDelegate: NSObject, SentryANRTrackerV2Delegate {
 
     let anrDetectedExpectation = XCTestExpectation(description: "Test Delegate ANR Detection")
     let anrStoppedExpectation  = XCTestExpectation(description: "Test Delegate ANR Stopped")
+    let anrsDetected = Invocations<Sentry.SentryANRType>()
 
     init(shouldANRBeDetected: Bool = true, shouldStoppedBeCalled: Bool = true) {
         if !shouldANRBeDetected {
@@ -449,7 +476,8 @@ class SentryANRTrackerV2TestDelegate: NSObject, SentryANRTrackerV2Delegate {
         anrStoppedExpectation.fulfill()
     }
 
-    func anrDetected() {
+    func anrDetected(type: Sentry.SentryANRType) {
+        anrsDetected.record(type)
         anrDetectedExpectation.fulfill()
     }
 }

diff --git a/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift b/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift
@@ -161,7 +161,7 @@ class SentryANRTrackingIntegrationTests: SentrySDKIntegrationTestsBase {
         setUpThreadInspector()
         SentryDependencyContainer.sharedInstance().application = BackgroundSentryUIApplication()
 
-        Dynamic(sut).anrDetected()
+        Dynamic(sut).anrsDetected()
 
         assertNoEventCaptured()
     }

diff --git a/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationV2Tests.swift b/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationV2Tests.swift
@@ -1,3 +1,5 @@
+@testable import _SentryPrivate
+@testable import Sentry
 import SentryTestUtils
 import XCTest
 
@@ -69,7 +71,7 @@ class SentryANRTrackingIntegrationV2Tests: SentrySDKIntegrationTestsBase {
         givenInitializedTracker()
         setUpThreadInspector()
 
-        Dynamic(sut).anrDetected()
+        Dynamic(sut).anrDetectedWithType(SentryANRType.fullyBlocking)
 
         try assertEventWithScopeCaptured { event, _, _ in
             XCTAssertNotNil(event)
@@ -107,7 +109,7 @@ class SentryANRTrackingIntegrationV2Tests: SentrySDKIntegrationTestsBase {
         setUpThreadInspector()
         sut.pauseAppHangTracking()
 
-        Dynamic(sut).anrDetected()
+        Dynamic(sut).anrDetectedWithType(SentryANRType.fullyBlocking)
 
         assertNoEventCaptured()
     }
@@ -118,7 +120,7 @@ class SentryANRTrackingIntegrationV2Tests: SentrySDKIntegrationTestsBase {
         sut.pauseAppHangTracking()
         sut.resumeAppHangTracking()
 
-        Dynamic(sut).anrDetected()
+        Dynamic(sut).anrDetectedWithType(SentryANRType.fullyBlocking)
 
         try assertEventWithScopeCaptured { event, _, _ in
             XCTAssertNotNil(event)
@@ -136,18 +138,18 @@ class SentryANRTrackingIntegrationV2Tests: SentrySDKIntegrationTestsBase {
 
         testConcurrentModifications(asyncWorkItems: 100, writeLoopCount: 10, writeWork: {_ in
             self.sut.pauseAppHangTracking()
-            Dynamic(self.sut).anrDetected()
+            Dynamic(self.sut).anrDetectedWithType(SentryANRType.fullyBlocking)
         }, readWork: {
             self.sut.resumeAppHangTracking()
-            Dynamic(self.sut).anrDetected()
+            Dynamic(self.sut).anrDetectedWithType(SentryANRType.fullyBlocking)
         })
     }
 
     func testANRDetected_ButNoThreads_EventNotCaptured() {
         givenInitializedTracker()
         setUpThreadInspector(addThreads: false)
 
-        Dynamic(sut).anrDetected()
+        Dynamic(sut).anrDetectedWithType(SentryANRType.fullyBlocking)
 
         assertNoEventCaptured()
     }
@@ -162,7 +164,7 @@ class SentryANRTrackingIntegrationV2Tests: SentrySDKIntegrationTestsBase {
         setUpThreadInspector()
         SentryDependencyContainer.sharedInstance().application = BackgroundSentryUIApplication()
 
-        Dynamic(sut).anrDetected()
+        Dynamic(sut).anrDetectedWithType(SentryANRType.fullyBlocking)
 
         assertNoEventCaptured()
     }