From 0b3ad44acf785faabea8783e59b4b09cfd9d68a4 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Fri, 5 Jan 2024 09:31:15 -0300 Subject: [PATCH 1/7] app hang screenshot in the main thread --- Sources/Sentry/SentryANRTracker.m | 10 ++++++ Sources/Sentry/SentryScreenshotIntegration.m | 12 ++++++- Sources/Sentry/SentryViewHierarchy.m | 33 ++++++++++--------- .../Sentry/SentryViewHierarchyIntegration.m | 15 +++++++-- Sources/Sentry/include/SentryANRTracker.h | 2 ++ Sources/Sentry/include/SentryViewHierarchy.h | 9 +++++ .../SentryScreenshotIntegrationTests.swift | 23 +++++++++++++ .../Screenshot/TestSentryScreenShot.swift | 7 +++- .../SentryViewHierarchyIntegrationTests.swift | 26 +++++++++++++++ .../SentryViewHierarchyTests.swift | 8 ++--- 10 files changed, 121 insertions(+), 24 deletions(-) diff --git a/Sources/Sentry/SentryANRTracker.m b/Sources/Sentry/SentryANRTracker.m index 01bc4a15bfe..dd16cb4cae2 100644 --- a/Sources/Sentry/SentryANRTracker.m +++ b/Sources/Sentry/SentryANRTracker.m @@ -9,6 +9,8 @@ NS_ASSUME_NONNULL_BEGIN +static BOOL _appIsHanging = NO; + typedef NS_ENUM(NSInteger, SentryANRTrackerState) { kSentryANRTrackerNotRunning = 1, kSentryANRTrackerRunning, @@ -138,6 +140,8 @@ - (void)detectANRs - (void)ANRDetected { + _appIsHanging = YES; + NSArray *localListeners; @synchronized(self.listeners) { localListeners = [self.listeners allObjects]; @@ -150,6 +154,7 @@ - (void)ANRDetected - (void)ANRStopped { + _appIsHanging = NO; NSArray *targets; @synchronized(self.listeners) { targets = [self.listeners allObjects]; @@ -205,6 +210,11 @@ - (void)stop } } ++ (BOOL)appIsHanging +{ + return _appIsHanging; +} + @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/SentryScreenshotIntegration.m b/Sources/Sentry/SentryScreenshotIntegration.m index f3ceecb897b..c7d8f65f685 100644 --- a/Sources/Sentry/SentryScreenshotIntegration.m +++ b/Sources/Sentry/SentryScreenshotIntegration.m @@ -6,6 +6,7 @@ # import "SentryCrashC.h" # import "SentryDependencyContainer.h" # import "SentryEvent+Private.h" +# import "SentryException.h" # import "SentryHub+Private.h" # import "SentrySDK+Private.h" @@ -63,7 +64,16 @@ - (void)uninstall return attachments; } - NSArray *screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshots]; + NSArray *screenshot; + + // If the event is an App hanging event, we cant take the + // screenshot in the main thread because it's blocked. + if (event.exceptions.count == 1 && + [event.exceptions.firstObject.type isEqualToString:@"App Hanging"]) { + screenshot = [SentryDependencyContainer.sharedInstance.screenshot takeScreenshots]; + } else { + screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshots]; + } NSMutableArray *result = [NSMutableArray arrayWithCapacity:attachments.count + screenshot.count]; diff --git a/Sources/Sentry/SentryViewHierarchy.m b/Sources/Sentry/SentryViewHierarchy.m index 0522c250190..024a2f9e5e2 100644 --- a/Sources/Sentry/SentryViewHierarchy.m +++ b/Sources/Sentry/SentryViewHierarchy.m @@ -5,6 +5,7 @@ # import "SentryCrashFileUtils.h" # import "SentryCrashJSONCodec.h" # import "SentryDependencyContainer.h" +# import "SentryDispatchQueueWrapper.h" # import "SentryLog.h" # import "SentryUIApplication.h" # import @@ -46,26 +47,28 @@ - (BOOL)saveViewHierarchy:(NSString *)filePath return result; } -- (NSData *)fetchViewHierarchy +- (NSData *)appViewHierarchy { - __block NSMutableData *result = [[NSMutableData alloc] init]; + __block NSData *result; - void (^save)(void) = ^{ - NSArray *windows = - [SentryDependencyContainer.sharedInstance.application windows]; + void (^fetchViewHierarchy)(void) = ^{ result = [self fetchViewHierarchy]; }; - if (![self processViewHierarchy:windows - addFunction:writeJSONDataToMemory - userData:(__bridge void *)(result)]) { + [[SentryDependencyContainer sharedInstance].dispatchQueueWrapper + dispatchSyncOnMainQueue:fetchViewHierarchy]; - result = nil; - } - }; + return result; +} + +- (NSData *)fetchViewHierarchy +{ + NSMutableData *result = [[NSMutableData alloc] init]; + NSArray *windows = [SentryDependencyContainer.sharedInstance.application windows]; + + if (![self processViewHierarchy:windows + addFunction:writeJSONDataToMemory + userData:(__bridge void *)(result)]) { - if ([NSThread isMainThread]) { - save(); - } else { - dispatch_sync(dispatch_get_main_queue(), save); + result = nil; } return result; diff --git a/Sources/Sentry/SentryViewHierarchyIntegration.m b/Sources/Sentry/SentryViewHierarchyIntegration.m index 4154460372e..d296e1754da 100644 --- a/Sources/Sentry/SentryViewHierarchyIntegration.m +++ b/Sources/Sentry/SentryViewHierarchyIntegration.m @@ -5,10 +5,10 @@ # import "SentryCrashC.h" # import "SentryDependencyContainer.h" # import "SentryEvent+Private.h" +# import "SentryException.h" # import "SentryHub+Private.h" # import "SentrySDK+Private.h" # import "SentryViewHierarchy.h" - # if SENTRY_HAS_METRIC_KIT # import "SentryMetricKitIntegration.h" # endif // SENTRY_HAS_METRIC_KIT @@ -71,8 +71,17 @@ - (void)uninstall NSMutableArray *result = [NSMutableArray arrayWithArray:attachments]; - NSData *viewHierarchy = - [SentryDependencyContainer.sharedInstance.viewHierarchy fetchViewHierarchy]; + NSData *viewHierarchy; + + // If the event is an App hanging event, we cant take the + // view hierarchy in the main thread because it's blocked. + if (event.exceptions.count == 1 && + [event.exceptions.firstObject.type isEqualToString:@"App Hanging"]) { + viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy fetchViewHierarchy]; + } else { + viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchy]; + } + SentryAttachment *attachment = [[SentryAttachment alloc] initWithData:viewHierarchy filename:@"view-hierarchy.json" diff --git a/Sources/Sentry/include/SentryANRTracker.h b/Sources/Sentry/include/SentryANRTracker.h index 4c1971732b0..81edb712873 100644 --- a/Sources/Sentry/include/SentryANRTracker.h +++ b/Sources/Sentry/include/SentryANRTracker.h @@ -22,6 +22,8 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryANRTracker : NSObject SENTRY_NO_INIT +@property (nonatomic, readonly, class) BOOL appIsHanging; + - (instancetype)initWithTimeoutInterval:(NSTimeInterval)timeoutInterval crashWrapper:(SentryCrashWrapper *)crashWrapper dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper diff --git a/Sources/Sentry/include/SentryViewHierarchy.h b/Sources/Sentry/include/SentryViewHierarchy.h index 5051d068053..0a717179423 100644 --- a/Sources/Sentry/include/SentryViewHierarchy.h +++ b/Sources/Sentry/include/SentryViewHierarchy.h @@ -6,6 +6,15 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryViewHierarchy : NSObject +/** + Get the view hierarchy in a json format. + Always runs in the main thread. + */ +- (nullable NSData *)appViewHierarchy; + +/** + Get the view hierarchy in a json format. + */ - (nullable NSData *)fetchViewHierarchy; /** diff --git a/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift b/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift index 375095979e8..2a3a5b2546c 100644 --- a/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift @@ -171,6 +171,29 @@ class SentryScreenshotIntegrationTests: XCTestCase { } + func test_backgroundForAppHangs() { + let sut = fixture.getSut() + let testVH = TestSentryScreenshot() + SentryDependencyContainer.sharedInstance().screenshot = testVH + + let event = Event() + event.exceptions = [Sentry.Exception(value: "test", type: "App Hanging")] + + let ex = expectation(description: "Attachment Added") + + testVH.processViewHierarchyCallback = { + ex.fulfill() + XCTAssertFalse(Thread.isMainThread) + } + + let dispatch = DispatchQueue(label: "background") + dispatch.async { + sut.processAttachments([], for: event) + } + + wait(for: [ex], timeout: 1) + } + } #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) diff --git a/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift b/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift index 9dce11b464a..037b27237ac 100644 --- a/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift +++ b/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift @@ -3,11 +3,16 @@ class TestSentryScreenshot: SentryScreenshot { var result: [Data]? + var processViewHierarchyCallback: (() -> Void)? override func appScreenshots() -> [Data]? { return result } - + + override func takeScreenshots() -> [Data] { + processViewHierarchyCallback?() + return result ?? [] + } } #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) diff --git a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift index e989dbc0bfc..80db4520010 100644 --- a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift @@ -123,6 +123,32 @@ class SentryViewHierarchyIntegrationTests: XCTestCase { XCTAssertEqual(newAttachmentList?.count, 1) XCTAssertEqual(newAttachmentList?.first, attachment) } + + func test_backgroundForAppHangs() { + let sut = fixture.getSut() + let testVH = TestSentryViewHierarchy() + SentryDependencyContainer.sharedInstance().viewHierarchy = testVH + + let event = Event() + event.exceptions = [Sentry.Exception(value: "test", type: "App Hanging")] + + var newAttachmentList: [Attachment]? + + let ex = expectation(description: "Attachment Added") + + testVH.processViewHierarchyCallback = { + ex.fulfill() + XCTAssertFalse(Thread.isMainThread) + } + + let dispatch = DispatchQueue(label: "background") + dispatch.async { + newAttachmentList = sut.processAttachments([], for: event) + } + + wait(for: [ex], timeout: 1) + XCTAssertEqual(newAttachmentList?.count, 1) + } } #endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) diff --git a/Tests/SentryTests/SentryViewHierarchyTests.swift b/Tests/SentryTests/SentryViewHierarchyTests.swift index 8dfef6bb3b6..7e339e2a445 100644 --- a/Tests/SentryTests/SentryViewHierarchyTests.swift +++ b/Tests/SentryTests/SentryViewHierarchyTests.swift @@ -169,7 +169,7 @@ class SentryViewHierarchyTests: XCTestCase { XCTAssertNil(result) } - func test_fetchFromBackgroundTest() { + func test_appViewHierarchyFromBackgroundTest() { let sut = TestSentryViewHierarchy() let window = UIWindow(frame: CGRect(x: 0, y: 0, width: 10, height: 10)) fixture.uiApplication.windows = [window] @@ -182,13 +182,13 @@ class SentryViewHierarchyTests: XCTestCase { let dispatch = DispatchQueue(label: "background") dispatch.async { - let _ = sut.fetch() + let _ = sut.appViewHierarchy() } wait(for: [ex], timeout: 1) } - func test_fetch_usesMainThread() { + func test_appViewHierarchy_usesMainThread() { let sut = TestSentryViewHierarchy() let window = UIWindow(frame: CGRect(x: 0, y: 0, width: 10, height: 10)) fixture.uiApplication.windows = [window] @@ -196,7 +196,7 @@ class SentryViewHierarchyTests: XCTestCase { let ex = expectation(description: "Running on background Thread") let dispatch = DispatchQueue(label: "background") dispatch.async { - let _ = sut.fetch() + let _ = sut.appViewHierarchy() ex.fulfill() } From 012d3e33a20c288718cea2acf700751a1a37343d Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Fri, 5 Jan 2024 09:39:02 -0300 Subject: [PATCH 2/7] Update CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec56be7126d..a001fab0257 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Don't use main thread for app hang screenshot and view hierarchy (#3547) + ## 8.18.0 ### Features From 94052b26263a2de5f53515342b86407de8c06fc2 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Mon, 29 Jan 2024 16:07:41 +0100 Subject: [PATCH 3/7] Refactoring --- Sources/Sentry/PrivateSentrySDKOnly.mm | 4 ++-- Sources/Sentry/SentryANRTracker.m | 10 -------- Sources/Sentry/SentryEvent.m | 4 ++++ Sources/Sentry/SentryScreenshot.m | 8 +++---- Sources/Sentry/SentryScreenshotIntegration.m | 7 +++--- Sources/Sentry/SentryViewHierarchy.m | 6 ++--- .../Sentry/SentryViewHierarchyIntegration.m | 7 +++--- Sources/Sentry/include/SentryANRTracker.h | 2 -- Sources/Sentry/include/SentryEvent+Private.h | 5 ++++ Sources/Sentry/include/SentryScreenshot.h | 4 ++-- Sources/Sentry/include/SentryViewHierarchy.h | 4 ++-- .../SentryScreenshotIntegrationTests.swift | 3 +-- .../Screenshot/TestSentryScreenShot.swift | 10 ++++---- .../TestSentryViewHierarchy.swift | 4 ++-- Tests/SentryTests/SentryScreenShotTests.swift | 23 ++++--------------- .../SentryViewHierarchyTests.swift | 18 +++++++-------- 16 files changed, 50 insertions(+), 69 deletions(-) diff --git a/Sources/Sentry/PrivateSentrySDKOnly.mm b/Sources/Sentry/PrivateSentrySDKOnly.mm index bff864d12b3..fb614e5c5b7 100644 --- a/Sources/Sentry/PrivateSentrySDKOnly.mm +++ b/Sources/Sentry/PrivateSentrySDKOnly.mm @@ -207,7 +207,7 @@ + (SentryScreenFrames *)currentScreenFrames + (NSArray *)captureScreenshots { #if SENTRY_HAS_UIKIT - return [SentryDependencyContainer.sharedInstance.screenshot takeScreenshots]; + return [SentryDependencyContainer.sharedInstance.screenshot appScreenshots]; #else SENTRY_LOG_DEBUG( @"PrivateSentrySDKOnly.captureScreenshots only works with UIKit enabled. Ensure you're " @@ -219,7 +219,7 @@ + (SentryScreenFrames *)currentScreenFrames + (NSData *)captureViewHierarchy { #if SENTRY_HAS_UIKIT - return [SentryDependencyContainer.sharedInstance.viewHierarchy fetchViewHierarchy]; + return [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchy]; #else SENTRY_LOG_DEBUG( @"PrivateSentrySDKOnly.captureViewHierarchy only works with UIKit enabled. Ensure you're " diff --git a/Sources/Sentry/SentryANRTracker.m b/Sources/Sentry/SentryANRTracker.m index dd16cb4cae2..01bc4a15bfe 100644 --- a/Sources/Sentry/SentryANRTracker.m +++ b/Sources/Sentry/SentryANRTracker.m @@ -9,8 +9,6 @@ NS_ASSUME_NONNULL_BEGIN -static BOOL _appIsHanging = NO; - typedef NS_ENUM(NSInteger, SentryANRTrackerState) { kSentryANRTrackerNotRunning = 1, kSentryANRTrackerRunning, @@ -140,8 +138,6 @@ - (void)detectANRs - (void)ANRDetected { - _appIsHanging = YES; - NSArray *localListeners; @synchronized(self.listeners) { localListeners = [self.listeners allObjects]; @@ -154,7 +150,6 @@ - (void)ANRDetected - (void)ANRStopped { - _appIsHanging = NO; NSArray *targets; @synchronized(self.listeners) { targets = [self.listeners allObjects]; @@ -210,11 +205,6 @@ - (void)stop } } -+ (BOOL)appIsHanging -{ - return _appIsHanging; -} - @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index de83a6fa1a6..d32313fd40d 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -201,6 +201,10 @@ - (BOOL)isMetricKitEvent #endif // SENTRY_HAS_METRIC_KIT +- (bool)isAppHangEvent { + return self.exceptions.count == 1 && [self.exceptions.firstObject.type isEqualToString:@"App Hanging"]; +} + @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/SentryScreenshot.m b/Sources/Sentry/SentryScreenshot.m index 6835f0cc89d..50017f073df 100644 --- a/Sources/Sentry/SentryScreenshot.m +++ b/Sources/Sentry/SentryScreenshot.m @@ -10,11 +10,11 @@ @implementation SentryScreenshot -- (NSArray *)appScreenshots +- (NSArray *)appScreenshotsFromMainThread { __block NSArray *result; - void (^takeScreenShot)(void) = ^{ result = [self takeScreenshots]; }; + void (^takeScreenShot)(void) = ^{ result = [self appScreenshots]; }; [[SentryDependencyContainer sharedInstance].dispatchQueueWrapper dispatchSyncOnMainQueue:takeScreenShot]; @@ -29,7 +29,7 @@ - (void)saveScreenShots:(NSString *)imagesDirectoryPath // We did it this way because we use this function to save screenshots // during signal handling, and if we dispatch it to the main thread, // that is probably blocked by the crash event, we freeze the application. - [[self takeScreenshots] enumerateObjectsUsingBlock:^(NSData *obj, NSUInteger idx, BOOL *stop) { + [[self appScreenshots] enumerateObjectsUsingBlock:^(NSData *obj, NSUInteger idx, BOOL *stop) { NSString *name = idx == 0 ? @"screenshot.png" : [NSString stringWithFormat:@"screenshot-%li.png", (unsigned long)idx + 1]; @@ -38,7 +38,7 @@ - (void)saveScreenShots:(NSString *)imagesDirectoryPath }]; } -- (NSArray *)takeScreenshots +- (NSArray *)appScreenshots { NSArray *windows = [SentryDependencyContainer.sharedInstance.application windows]; diff --git a/Sources/Sentry/SentryScreenshotIntegration.m b/Sources/Sentry/SentryScreenshotIntegration.m index c7d8f65f685..5c6b2ff9f90 100644 --- a/Sources/Sentry/SentryScreenshotIntegration.m +++ b/Sources/Sentry/SentryScreenshotIntegration.m @@ -68,11 +68,10 @@ - (void)uninstall // If the event is an App hanging event, we cant take the // screenshot in the main thread because it's blocked. - if (event.exceptions.count == 1 && - [event.exceptions.firstObject.type isEqualToString:@"App Hanging"]) { - screenshot = [SentryDependencyContainer.sharedInstance.screenshot takeScreenshots]; - } else { + if (event.isAppHangEvent) { screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshots]; + } else { + screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshotsFromMainThread]; } NSMutableArray *result = diff --git a/Sources/Sentry/SentryViewHierarchy.m b/Sources/Sentry/SentryViewHierarchy.m index 024a2f9e5e2..6e8a15f8077 100644 --- a/Sources/Sentry/SentryViewHierarchy.m +++ b/Sources/Sentry/SentryViewHierarchy.m @@ -47,11 +47,11 @@ - (BOOL)saveViewHierarchy:(NSString *)filePath return result; } -- (NSData *)appViewHierarchy +- (NSData *)appViewHierarchyFromMainThread { __block NSData *result; - void (^fetchViewHierarchy)(void) = ^{ result = [self fetchViewHierarchy]; }; + void (^fetchViewHierarchy)(void) = ^{ result = [self appViewHierarchy]; }; [[SentryDependencyContainer sharedInstance].dispatchQueueWrapper dispatchSyncOnMainQueue:fetchViewHierarchy]; @@ -59,7 +59,7 @@ - (NSData *)appViewHierarchy return result; } -- (NSData *)fetchViewHierarchy +- (NSData *)appViewHierarchy { NSMutableData *result = [[NSMutableData alloc] init]; NSArray *windows = [SentryDependencyContainer.sharedInstance.application windows]; diff --git a/Sources/Sentry/SentryViewHierarchyIntegration.m b/Sources/Sentry/SentryViewHierarchyIntegration.m index d296e1754da..7a85ac3092c 100644 --- a/Sources/Sentry/SentryViewHierarchyIntegration.m +++ b/Sources/Sentry/SentryViewHierarchyIntegration.m @@ -75,11 +75,10 @@ - (void)uninstall // If the event is an App hanging event, we cant take the // view hierarchy in the main thread because it's blocked. - if (event.exceptions.count == 1 && - [event.exceptions.firstObject.type isEqualToString:@"App Hanging"]) { - viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy fetchViewHierarchy]; - } else { + if (event.isAppHangEvent) { viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchy]; + } else { + viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchyFromMainThread]; } SentryAttachment *attachment = diff --git a/Sources/Sentry/include/SentryANRTracker.h b/Sources/Sentry/include/SentryANRTracker.h index 81edb712873..4c1971732b0 100644 --- a/Sources/Sentry/include/SentryANRTracker.h +++ b/Sources/Sentry/include/SentryANRTracker.h @@ -22,8 +22,6 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryANRTracker : NSObject SENTRY_NO_INIT -@property (nonatomic, readonly, class) BOOL appIsHanging; - - (instancetype)initWithTimeoutInterval:(NSTimeInterval)timeoutInterval crashWrapper:(SentryCrashWrapper *)crashWrapper dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper diff --git a/Sources/Sentry/include/SentryEvent+Private.h b/Sources/Sentry/include/SentryEvent+Private.h index c28e338b29e..b6057135ef7 100644 --- a/Sources/Sentry/include/SentryEvent+Private.h +++ b/Sources/Sentry/include/SentryEvent+Private.h @@ -11,6 +11,11 @@ SentryEvent () */ @property (nonatomic) BOOL isCrashEvent; +/** + * This indicates whether this event represents an app hang. + */ +@property (nonatomic, readonly) BOOL isAppHangEvent; + /** * We're storing serialized breadcrumbs to disk in JSON, and when we're reading them back (in * the case of OOM), we end up with the serialized breadcrumbs again. Instead of turning those diff --git a/Sources/Sentry/include/SentryScreenshot.h b/Sources/Sentry/include/SentryScreenshot.h index a5ce0a5b63d..90c24b827a2 100644 --- a/Sources/Sentry/include/SentryScreenshot.h +++ b/Sources/Sentry/include/SentryScreenshot.h @@ -10,7 +10,7 @@ NS_ASSUME_NONNULL_BEGIN * Get a screenshot of every open window in the app. * @return An array of @c NSData instances containing PNG images. */ -- (nullable NSArray *)appScreenshots; +- (NSArray *)appScreenshotsFromMainThread; /** * Save the current app screen shots in the given directory. @@ -20,7 +20,7 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)saveScreenShots:(NSString *)imagesDirectoryPath; -- (NSArray *)takeScreenshots; +- (NSArray *)appScreenshots; @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentryViewHierarchy.h b/Sources/Sentry/include/SentryViewHierarchy.h index 0a717179423..381a432d578 100644 --- a/Sources/Sentry/include/SentryViewHierarchy.h +++ b/Sources/Sentry/include/SentryViewHierarchy.h @@ -10,12 +10,12 @@ NS_ASSUME_NONNULL_BEGIN Get the view hierarchy in a json format. Always runs in the main thread. */ -- (nullable NSData *)appViewHierarchy; +- (nullable NSData *)appViewHierarchyFromMainThread; /** Get the view hierarchy in a json format. */ -- (nullable NSData *)fetchViewHierarchy; +- (nullable NSData *)appViewHierarchy; /** * Save the current app view hierarchy in the given file path. diff --git a/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift b/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift index 2a3a5b2546c..942561a9644 100644 --- a/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/Screenshot/SentryScreenshotIntegrationTests.swift @@ -186,8 +186,7 @@ class SentryScreenshotIntegrationTests: XCTestCase { XCTAssertFalse(Thread.isMainThread) } - let dispatch = DispatchQueue(label: "background") - dispatch.async { + DispatchQueue.global().async { sut.processAttachments([], for: event) } diff --git a/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift b/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift index 037b27237ac..bad0017cc3c 100644 --- a/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift +++ b/Tests/SentryTests/Integrations/Screenshot/TestSentryScreenShot.swift @@ -2,16 +2,16 @@ class TestSentryScreenshot: SentryScreenshot { - var result: [Data]? + var result: [Data] = [] var processViewHierarchyCallback: (() -> Void)? - override func appScreenshots() -> [Data]? { + override func appScreenshots() -> [Data] { + processViewHierarchyCallback?() return result } - override func takeScreenshots() -> [Data] { - processViewHierarchyCallback?() - return result ?? [] + override func appScreenshotsFromMainThread() -> [Data] { + return result } } diff --git a/Tests/SentryTests/Integrations/ViewHierarchy/TestSentryViewHierarchy.swift b/Tests/SentryTests/Integrations/ViewHierarchy/TestSentryViewHierarchy.swift index a49d965db92..2f60770f0a8 100644 --- a/Tests/SentryTests/Integrations/ViewHierarchy/TestSentryViewHierarchy.swift +++ b/Tests/SentryTests/Integrations/ViewHierarchy/TestSentryViewHierarchy.swift @@ -7,10 +7,10 @@ class TestSentryViewHierarchy: SentryViewHierarchy { var processViewHierarchyCallback: (() -> Void)? var saveFilePathUsed: String? - override func fetch() -> Data? { + override func appViewHierarchy() -> Data? { guard let result = self.result else { - return super.fetch() + return super.appViewHierarchy() } return result } diff --git a/Tests/SentryTests/SentryScreenShotTests.swift b/Tests/SentryTests/SentryScreenShotTests.swift index eaacf748623..4ad3f778bc1 100644 --- a/Tests/SentryTests/SentryScreenShotTests.swift +++ b/Tests/SentryTests/SentryScreenShotTests.swift @@ -39,7 +39,7 @@ class SentryScreenShotTests: XCTestCase { let expect = expectation(description: "Screenshot") let _ = queue.async { - self.fixture.sut.appScreenshots() + self.fixture.sut.appScreenshotsFromMainThread() expect.fulfill() } @@ -72,11 +72,7 @@ class SentryScreenShotTests: XCTestCase { let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 10, height: 10)) fixture.uiApplication.windows = [testWindow] - guard let data = self.fixture.sut.appScreenshots() else { - XCTFail("Could not make window screenshot") - return - } - + let data = self.fixture.sut.appScreenshots() let image = UIImage(data: data[0]) XCTAssertEqual(image?.size.width, 10) @@ -87,10 +83,7 @@ class SentryScreenShotTests: XCTestCase { let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 0, height: 0)) fixture.uiApplication.windows = [testWindow] - guard let data = self.fixture.sut.appScreenshots() else { - XCTFail("Could not make window screenshot") - return - } + let data = self.fixture.sut.appScreenshots() XCTAssertEqual(0, data.count, "No screenshot should be taken, cause the image has zero size.") } @@ -99,10 +92,7 @@ class SentryScreenShotTests: XCTestCase { let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 0, height: 1_000)) fixture.uiApplication.windows = [testWindow] - guard let data = self.fixture.sut.appScreenshots() else { - XCTFail("Could not make window screenshot") - return - } + let data = self.fixture.sut.appScreenshots() XCTAssertEqual(0, data.count, "No screenshot should be taken, cause the image has zero width.") } @@ -111,10 +101,7 @@ class SentryScreenShotTests: XCTestCase { let testWindow = TestWindow(frame: CGRect(x: 0, y: 0, width: 1_000, height: 0)) fixture.uiApplication.windows = [testWindow] - guard let data = self.fixture.sut.appScreenshots() else { - XCTFail("Could not make window screenshot") - return - } + let data = self.fixture.sut.appScreenshots() XCTAssertEqual(0, data.count, "No screenshot should be taken, cause the image has zero height.") } diff --git a/Tests/SentryTests/SentryViewHierarchyTests.swift b/Tests/SentryTests/SentryViewHierarchyTests.swift index 7e339e2a445..6b9f68df556 100644 --- a/Tests/SentryTests/SentryViewHierarchyTests.swift +++ b/Tests/SentryTests/SentryViewHierarchyTests.swift @@ -46,7 +46,7 @@ class SentryViewHierarchyTests: XCTestCase { fixture.uiApplication.windows = [firstWindow, secondWindow] - guard let descriptions = self.fixture.sut.fetch() else { + guard let descriptions = self.fixture.sut.appViewHierarchy() else { XCTFail("Could not serialize view hierarchy") return } @@ -62,7 +62,7 @@ class SentryViewHierarchyTests: XCTestCase { window.accessibilityIdentifier = "WindowId" fixture.uiApplication.windows = [window] - guard let data = self.fixture.sut.fetch() + guard let data = self.fixture.sut.appViewHierarchy() else { XCTFail("Could not serialize view hierarchy") return @@ -76,7 +76,7 @@ class SentryViewHierarchyTests: XCTestCase { fixture.uiApplication.windows = [window] - guard let data = self.fixture.sut.fetch() + guard let data = self.fixture.sut.appViewHierarchy() else { XCTFail("Could not serialize view hierarchy") return @@ -96,7 +96,7 @@ class SentryViewHierarchyTests: XCTestCase { fixture.uiApplication.windows = [firstWindow] - guard let descriptions = self.fixture.sut.fetch() + guard let descriptions = self.fixture.sut.appViewHierarchy() else { XCTFail("Could not serialize view hierarchy") return @@ -120,7 +120,7 @@ class SentryViewHierarchyTests: XCTestCase { fixture.uiApplication.windows = [firstWindow] - guard let descriptions = self.fixture.sut.fetch() + guard let descriptions = self.fixture.sut.appViewHierarchy() else { XCTFail("Could not serialize view hierarchy") return @@ -165,7 +165,7 @@ class SentryViewHierarchyTests: XCTestCase { window.accessibilityIdentifier = "WindowId" fixture.uiApplication.windows = [window] - let result = sut.fetch() + let result = sut.appViewHierarchy() XCTAssertNil(result) } @@ -182,7 +182,7 @@ class SentryViewHierarchyTests: XCTestCase { let dispatch = DispatchQueue(label: "background") dispatch.async { - let _ = sut.appViewHierarchy() + let _ = sut.appViewHierarchyFromMainThread() } wait(for: [ex], timeout: 1) @@ -196,12 +196,12 @@ class SentryViewHierarchyTests: XCTestCase { let ex = expectation(description: "Running on background Thread") let dispatch = DispatchQueue(label: "background") dispatch.async { - let _ = sut.appViewHierarchy() + let _ = sut.appViewHierarchyFromMainThread() ex.fulfill() } wait(for: [ex], timeout: 1) - XCTAssertTrue(fixture.uiApplication.calledOnMainThread, "fetchViewHierarchy is not using the main thread to get UI windows") + XCTAssertTrue(fixture.uiApplication.calledOnMainThread, "appViewHierarchy is not using the main thread to get UI windows") } class TestSentryUIApplication: SentryUIApplication { From c832a4451e99aa240fbf3422ef4463f6e9cc1dd1 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Mon, 29 Jan 2024 16:14:56 +0100 Subject: [PATCH 4/7] Nimble --- .../ViewHierarchy/SentryViewHierarchyIntegrationTests.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift index 80db4520010..a5836c82384 100644 --- a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift @@ -3,6 +3,7 @@ import Sentry import SentryTestUtils import XCTest +import Nimble class SentryViewHierarchyIntegrationTests: XCTestCase { @@ -147,7 +148,7 @@ class SentryViewHierarchyIntegrationTests: XCTestCase { } wait(for: [ex], timeout: 1) - XCTAssertEqual(newAttachmentList?.count, 1) + expect(newAttachmentList?.count) == 1 } } From 15d697de4a72ffcecadce6c32b21d82c19f6672c Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Mon, 29 Jan 2024 16:17:08 +0100 Subject: [PATCH 5/7] Update SentryViewHierarchyIntegrationTests.swift --- .../SentryViewHierarchyIntegrationTests.swift | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift index a5836c82384..a34d33e3549 100644 --- a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift @@ -132,23 +132,20 @@ class SentryViewHierarchyIntegrationTests: XCTestCase { let event = Event() event.exceptions = [Sentry.Exception(value: "test", type: "App Hanging")] - - var newAttachmentList: [Attachment]? let ex = expectation(description: "Attachment Added") testVH.processViewHierarchyCallback = { ex.fulfill() - XCTAssertFalse(Thread.isMainThread) + expect(Thread.isMainThread) == false } let dispatch = DispatchQueue(label: "background") dispatch.async { - newAttachmentList = sut.processAttachments([], for: event) + sut.processAttachments([], for: event) } wait(for: [ex], timeout: 1) - expect(newAttachmentList?.count) == 1 } } From 8e7b5fb715e25bed0d0f6940dde2b561476f56c4 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Thu, 1 Feb 2024 11:19:17 +0100 Subject: [PATCH 6/7] Tests --- Sources/Sentry/SentryANRTrackingIntegration.m | 2 +- Sources/Sentry/SentryEvent.m | 5 +++-- .../include/SentryANRTrackingIntegration.h | 2 ++ .../SentryANRTrackingIntegrationTests.swift | 22 ++++++++++++------- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Sources/Sentry/SentryANRTrackingIntegration.m b/Sources/Sentry/SentryANRTrackingIntegration.m index 516ca669433..149fdb79cc2 100644 --- a/Sources/Sentry/SentryANRTrackingIntegration.m +++ b/Sources/Sentry/SentryANRTrackingIntegration.m @@ -88,7 +88,7 @@ - (void)anrDetected (long)(self.options.appHangTimeoutInterval * 1000)]; SentryEvent *event = [[SentryEvent alloc] initWithLevel:kSentryLevelError]; SentryException *sentryException = [[SentryException alloc] initWithValue:message - type:@"App Hanging"]; + type:SentryANRExceptionType]; sentryException.mechanism = [[SentryMechanism alloc] initWithType:@"AppHang"]; sentryException.stacktrace = [threads[0] stacktrace]; diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index d32313fd40d..53977c4368e 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -16,6 +16,7 @@ #import "SentryStacktrace.h" #import "SentryThread.h" #import "SentryUser.h" +#import "SentryANRTrackingIntegration.h" #if SENTRY_HAS_METRIC_KIT # import "SentryMechanism.h" @@ -201,8 +202,8 @@ - (BOOL)isMetricKitEvent #endif // SENTRY_HAS_METRIC_KIT -- (bool)isAppHangEvent { - return self.exceptions.count == 1 && [self.exceptions.firstObject.type isEqualToString:@"App Hanging"]; +- (BOOL)isAppHangEvent { + return self.exceptions.count == 1 && [self.exceptions.firstObject.type isEqualToString:SentryANRExceptionType]; } @end diff --git a/Sources/Sentry/include/SentryANRTrackingIntegration.h b/Sources/Sentry/include/SentryANRTrackingIntegration.h index e6171709c1c..c40ba4eaabb 100644 --- a/Sources/Sentry/include/SentryANRTrackingIntegration.h +++ b/Sources/Sentry/include/SentryANRTrackingIntegration.h @@ -5,6 +5,8 @@ NS_ASSUME_NONNULL_BEGIN +static NSString *const SentryANRExceptionType = @"App Hanging"; + @interface SentryANRTrackingIntegration : SentryBaseIntegration diff --git a/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift b/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift index a8b7efdae8f..f7c254933f4 100644 --- a/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift @@ -1,5 +1,6 @@ import SentryTestUtils import XCTest +import Nimble class SentryANRTrackingIntegrationTests: SentrySDKIntegrationTestsBase { @@ -28,7 +29,7 @@ class SentryANRTrackingIntegrationTests: SentrySDKIntegrationTestsBase { } override func tearDown() { - sut.uninstall() + sut?.uninstall() clearTestState() super.tearDown() } @@ -77,13 +78,14 @@ class SentryANRTrackingIntegrationTests: SentrySDKIntegrationTestsBase { return } - XCTAssertEqual(ex.mechanism?.type, "AppHang") - XCTAssertEqual(ex.type, "App Hanging") - XCTAssertEqual(ex.value, "App hanging for at least 4500 ms.") - XCTAssertNotNil(ex.stacktrace) - XCTAssertEqual(ex.stacktrace?.frames.first?.function, "main") - XCTAssertTrue(ex.stacktrace?.snapshot?.boolValue ?? false) - XCTAssertTrue(event?.threads?[0].current?.boolValue ?? false) + expect(ex.mechanism?.type) == "AppHang" + expect(ex.type) == "App Hanging" + expect(ex.value) == "App hanging for at least 4500 ms." + expect(ex.stacktrace) != nil + expect(ex.stacktrace?.frames.first?.function) == "main" + expect(ex.stacktrace?.snapshot?.boolValue) == true + expect(event?.threads?[0].current?.boolValue) == true + expect(event?.isAppHangEvent) == true guard let threads = event?.threads else { XCTFail("ANR Exception not found") @@ -144,6 +146,10 @@ class SentryANRTrackingIntegrationTests: SentrySDKIntegrationTestsBase { XCTAssertEqual(1, listeners?.count ?? 2) } + func testEventIsNotANR() { + expect(Event().isAppHangEvent) == false + } + private func givenInitializedTracker(isBeingTraced: Bool = false) { givenSdkWithHub() self.crashWrapper.internalIsBeingTraced = isBeingTraced From 4b1f1d90d63c61d953d0fd3cfe41c407436eaac4 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 1 Feb 2024 10:20:33 +0000 Subject: [PATCH 7/7] Format code --- Sources/Sentry/SentryANRTrackingIntegration.m | 4 ++-- Sources/Sentry/SentryEvent.m | 8 +++++--- Sources/Sentry/SentryScreenshotIntegration.m | 3 ++- Sources/Sentry/SentryViewHierarchyIntegration.m | 3 ++- .../ANR/SentryANRTrackingIntegrationTests.swift | 2 +- .../SentryViewHierarchyIntegrationTests.swift | 2 +- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Sources/Sentry/SentryANRTrackingIntegration.m b/Sources/Sentry/SentryANRTrackingIntegration.m index 149fdb79cc2..3d681cba0e1 100644 --- a/Sources/Sentry/SentryANRTrackingIntegration.m +++ b/Sources/Sentry/SentryANRTrackingIntegration.m @@ -87,8 +87,8 @@ - (void)anrDetected NSString *message = [NSString stringWithFormat:@"App hanging for at least %li ms.", (long)(self.options.appHangTimeoutInterval * 1000)]; SentryEvent *event = [[SentryEvent alloc] initWithLevel:kSentryLevelError]; - SentryException *sentryException = [[SentryException alloc] initWithValue:message - type:SentryANRExceptionType]; + SentryException *sentryException = + [[SentryException alloc] initWithValue:message type:SentryANRExceptionType]; sentryException.mechanism = [[SentryMechanism alloc] initWithType:@"AppHang"]; sentryException.stacktrace = [threads[0] stacktrace]; diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index 53977c4368e..132617af4ee 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -1,5 +1,6 @@ #import "NSDate+SentryExtras.h" #import "NSDictionary+SentrySanitize.h" +#import "SentryANRTrackingIntegration.h" #import "SentryBreadcrumb.h" #import "SentryClient.h" #import "SentryCurrentDateProvider.h" @@ -16,7 +17,6 @@ #import "SentryStacktrace.h" #import "SentryThread.h" #import "SentryUser.h" -#import "SentryANRTrackingIntegration.h" #if SENTRY_HAS_METRIC_KIT # import "SentryMechanism.h" @@ -202,8 +202,10 @@ - (BOOL)isMetricKitEvent #endif // SENTRY_HAS_METRIC_KIT -- (BOOL)isAppHangEvent { - return self.exceptions.count == 1 && [self.exceptions.firstObject.type isEqualToString:SentryANRExceptionType]; +- (BOOL)isAppHangEvent +{ + return self.exceptions.count == 1 && + [self.exceptions.firstObject.type isEqualToString:SentryANRExceptionType]; } @end diff --git a/Sources/Sentry/SentryScreenshotIntegration.m b/Sources/Sentry/SentryScreenshotIntegration.m index 5c6b2ff9f90..5272eb3e5cc 100644 --- a/Sources/Sentry/SentryScreenshotIntegration.m +++ b/Sources/Sentry/SentryScreenshotIntegration.m @@ -71,7 +71,8 @@ - (void)uninstall if (event.isAppHangEvent) { screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshots]; } else { - screenshot = [SentryDependencyContainer.sharedInstance.screenshot appScreenshotsFromMainThread]; + screenshot = + [SentryDependencyContainer.sharedInstance.screenshot appScreenshotsFromMainThread]; } NSMutableArray *result = diff --git a/Sources/Sentry/SentryViewHierarchyIntegration.m b/Sources/Sentry/SentryViewHierarchyIntegration.m index 7a85ac3092c..14490d4f900 100644 --- a/Sources/Sentry/SentryViewHierarchyIntegration.m +++ b/Sources/Sentry/SentryViewHierarchyIntegration.m @@ -78,7 +78,8 @@ - (void)uninstall if (event.isAppHangEvent) { viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchy]; } else { - viewHierarchy = [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchyFromMainThread]; + viewHierarchy = + [SentryDependencyContainer.sharedInstance.viewHierarchy appViewHierarchyFromMainThread]; } SentryAttachment *attachment = diff --git a/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift b/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift index f7c254933f4..48c44156d17 100644 --- a/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/ANR/SentryANRTrackingIntegrationTests.swift @@ -1,6 +1,6 @@ +import Nimble import SentryTestUtils import XCTest -import Nimble class SentryANRTrackingIntegrationTests: SentrySDKIntegrationTestsBase { diff --git a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift index a34d33e3549..bb2e74eb466 100644 --- a/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/ViewHierarchy/SentryViewHierarchyIntegrationTests.swift @@ -1,9 +1,9 @@ #if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) +import Nimble import Sentry import SentryTestUtils import XCTest -import Nimble class SentryViewHierarchyIntegrationTests: XCTestCase {