fix: Edge case for swizzleClassNameExclude (#4405)

Skip creating transactions for UIViewControllers ignored for swizzling via the option swizzleClassNameExclude. Due to some edge cases with nib files, the SDK doesn't swizzle the loadView method of the UIViewController subclasses, but instead, it swizzles the UIViewController.loadView method directly. Although the SDK doesn't swizzle the classes specified in swizzleClassNameExclude, it created transactions. Now, this is fixed. Fixes GH-4386
getsentry · Oct 8, 2024 · 9cd19dd · 9cd19dd
1 parent 913fab7
commit 9cd19dd
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,11 @@
 
 - Remove the deprecated experimental Metrics API (#4406): [Learn more](https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Metrics-Beta-Coming-to-an-End)
 
+### Fixes
+
+- Edge case for swizzleClassNameExclude (#4405): Skip creating transactions for UIViewControllers ignored for swizzling
+via the option `swizzleClassNameExclude`.
+
 ## 8.38.0-beta.1
 
 ### Features

diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj
@@ -100,6 +100,7 @@
 		62872B5F2BA1B7F300A4FA7D /* NSLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62872B5E2BA1B7F300A4FA7D /* NSLock.swift */; };
 		62872B632BA1B86100A4FA7D /* NSLockTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62872B622BA1B86100A4FA7D /* NSLockTests.swift */; };
 		62885DA729E946B100554F38 /* TestConncurrentModifications.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62885DA629E946B100554F38 /* TestConncurrentModifications.swift */; };
+		629428802CB3BF69002C454C /* SwizzleClassNameExclude.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6294287F2CB3BF4E002C454C /* SwizzleClassNameExclude.swift */; };
 		6294774C2C6F255F00846CBC /* SentryANRTrackerV2Delegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6294774B2C6F255F00846CBC /* SentryANRTrackerV2Delegate.swift */; };
 		62950F1029E7FE0100A42624 /* SentryTransactionContextTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62950F0F29E7FE0100A42624 /* SentryTransactionContextTests.swift */; };
 		629690532AD3E060000185FA /* SentryReachabilitySwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 629690522AD3E060000185FA /* SentryReachabilitySwiftTests.swift */; };
@@ -1093,6 +1094,7 @@
 		62872B5E2BA1B7F300A4FA7D /* NSLock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSLock.swift; sourceTree = "<group>"; };
 		62872B622BA1B86100A4FA7D /* NSLockTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSLockTests.swift; sourceTree = "<group>"; };
 		62885DA629E946B100554F38 /* TestConncurrentModifications.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestConncurrentModifications.swift; sourceTree = "<group>"; };
+		6294287F2CB3BF4E002C454C /* SwizzleClassNameExclude.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SwizzleClassNameExclude.swift; sourceTree = "<group>"; };
 		6294774B2C6F255F00846CBC /* SentryANRTrackerV2Delegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryANRTrackerV2Delegate.swift; sourceTree = "<group>"; };
 		62950F0F29E7FE0100A42624 /* SentryTransactionContextTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryTransactionContextTests.swift; sourceTree = "<group>"; };
 		629690522AD3E060000185FA /* SentryReachabilitySwiftTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryReachabilitySwiftTests.swift; sourceTree = "<group>"; };
@@ -3759,6 +3761,7 @@
 		D8739CF72BECFF92007D2F66 /* Performance */ = {
 			isa = PBXGroup;
 			children = (
+				6294287F2CB3BF4E002C454C /* SwizzleClassNameExclude.swift */,
 				D8739CF82BECFFB5007D2F66 /* SentryTransactionNameSource.swift */,
 			);
 			path = Performance;
@@ -4660,6 +4663,7 @@
 				7B8ECBFC26498958005FE2EF /* SentryAppStateManager.m in Sources */,
 				7B2A70DD27D6083D008B0D15 /* SentryThreadWrapper.m in Sources */,
 				D8ACE3C72762187200F5A213 /* SentryNSDataSwizzling.m in Sources */,
+				629428802CB3BF69002C454C /* SwizzleClassNameExclude.swift in Sources */,
 				638DC9A11EBC6B6400A66E41 /* SentryRequestOperation.m in Sources */,
 				63AA767A1EB8D20500D153DE /* SentryLogC.m in Sources */,
 				6344DDBA1EC3115C00D9160D /* SentryCrashReportConverter.m in Sources */,

diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h
@@ -446,17 +446,17 @@ NS_SWIFT_NAME(Options)
 @property (nonatomic, assign) BOOL enableSwizzling;
 
 /**
- * An array of class names to ignore for swizzling.
+ * A set of class names to ignore for swizzling.
  *
  * @discussion The SDK checks if a class name of a class to swizzle contains a class name of this
  * array. For example, if you add MyUIViewController to this list, the SDK excludes the following
  * classes from swizzling: YourApp.MyUIViewController, YourApp.MyUIViewControllerA,
  * MyApp.MyUIViewController.
- * We can't use an @c NSArray<Class>  here because we use this as a workaround for which users have
+ * We can't use an @c NSSet<Class>  here because we use this as a workaround for which users have
  * to pass in class names that aren't available on specific iOS versions. By using @c
- * NSArray<NSString *>, users can specify unavailable class names.
+ * NSSet<NSString *>, users can specify unavailable class names.
  *
- * @note Default is an empty array.
+ * @note Default is an empty set.
  */
 @property (nonatomic, strong) NSSet<NSString *> *swizzleClassNameExcludes;
 

diff --git a/Sources/Sentry/SentrySubClassFinder.m b/Sources/Sentry/SentrySubClassFinder.m
@@ -2,6 +2,7 @@
 #import "SentryDispatchQueueWrapper.h"
 #import "SentryLog.h"
 #import "SentryObjCRuntimeWrapper.h"
+#import "SentrySwift.h"
 #import <objc/runtime.h>
 #import <string.h>
 
@@ -61,13 +62,9 @@ - (void)actOnSubclassesOfViewControllerInImage:(NSString *)imageName block:(void
         for (int i = 0; i < count; i++) {
             NSString *className = [NSString stringWithUTF8String:classes[i]];
 
-            BOOL shouldExcludeClassFromSwizzling = NO;
-            for (NSString *swizzleClassNameExclude in self.swizzleClassNameExcludes) {
-                if ([className containsString:swizzleClassNameExclude]) {
-                    shouldExcludeClassFromSwizzling = YES;
-                    break;
-                }
-            }
+            BOOL shouldExcludeClassFromSwizzling = [SentrySwizzleClassNameExclude
+                shouldExcludeClassWithClassName:className
+                       swizzleClassNameExcludes:self.swizzleClassNameExcludes];
 
             // It is vital to avoid calling NSClassFromString for the excluded classes because we
             // had crashes for specific classes when calling NSClassFromString, such as

diff --git a/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m b/Sources/Sentry/SentryUIViewControllerPerformanceTracker.m
@@ -68,6 +68,18 @@ - (void)viewControllerLoadView:(UIViewController *)controller
         return;
     }
 
+    SentryOptions *options = [SentrySDK options];
+
+    if ([SentrySwizzleClassNameExclude
+            shouldExcludeClassWithClassName:NSStringFromClass([controller class])
+                   swizzleClassNameExcludes:options.swizzleClassNameExcludes]) {
+        SENTRY_LOG_DEBUG(@"Won't track view controller because it's excluded with the option "
+                         @"swizzleClassNameExcludes: %@",
+            controller);
+        callbackToOrigin();
+        return;
+    }
+
     [self limitOverride:@"loadView"
                   target:controller
         callbackToOrigin:callbackToOrigin

diff --git a/Sources/Sentry/SentryUIViewControllerSwizzling.m b/Sources/Sentry/SentryUIViewControllerSwizzling.m
@@ -36,6 +36,7 @@ @interface UIApplication (SentryUIApplication) <SentryUIApplication>
 
 @interface SentryUIViewControllerSwizzling ()
 
+@property (nonatomic, strong) SentryOptions *options;
 @property (nonatomic, strong) SentryInAppLogic *inAppLogic;
 @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue;
 @property (nonatomic, strong) id<SentryObjCRuntimeWrapper> objcRuntimeWrapper;
@@ -56,6 +57,7 @@ - (instancetype)initWithOptions:(SentryOptions *)options
                binaryImageCache:(SentryBinaryImageCache *)binaryImageCache
 {
     if (self = [super init]) {
+        self.options = options;
         self.inAppLogic = [[SentryInAppLogic alloc] initWithInAppIncludes:options.inAppIncludes
                                                             inAppExcludes:options.inAppExcludes];
         self.dispatchQueue = dispatchQueue;
@@ -341,6 +343,15 @@ - (void)swizzleViewControllerSubClass:(Class)class
  */
 - (BOOL)shouldSwizzleViewController:(Class)class
 {
+    NSString *className = NSStringFromClass(class);
+
+    BOOL shouldExcludeClassFromSwizzling = [SentrySwizzleClassNameExclude
+        shouldExcludeClassWithClassName:className
+               swizzleClassNameExcludes:self.options.swizzleClassNameExcludes];
+    if (shouldExcludeClassFromSwizzling) {
+        return NO;
+    }
+
     return [self.inAppLogic isClassInApp:class];
 }
 

diff --git a/Sources/Swift/Integrations/Performance/SwizzleClassNameExclude.swift b/Sources/Swift/Integrations/Performance/SwizzleClassNameExclude.swift
@@ -0,0 +1,13 @@
+import Foundation
+
+@objcMembers
+class SentrySwizzleClassNameExclude: NSObject {
+    static func shouldExcludeClass(className: String, swizzleClassNameExcludes: Set<String>) -> Bool {
+        for exclude in swizzleClassNameExcludes {
+            if className.contains(exclude) {
+                return true
+            }
+        }
+        return false
+    }
+}
diff --git a/...grations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift b/...grations/Performance/UIViewController/SentryUIViewControllerPerformanceTrackerTests.swift
@@ -26,15 +26,7 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase {
 
     private class Fixture {
 
-        var options: Options {
-            let options = Options.noIntegrations()
-            let imageName = String(
-                cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!,
-                encoding: .utf8)! as NSString
-            options.add(inAppInclude: imageName.lastPathComponent)
-            options.debug = true
-            return options
-        }
+        var options: Options
 
         let viewController = TestViewController()
         let tracker = SentryPerformanceTracker.shared
@@ -50,6 +42,13 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase {
         }
 
         init() {
+            options = Options.noIntegrations()
+            let imageName = String(
+                cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!,
+                encoding: .utf8)! as NSString
+            options.add(inAppInclude: imageName.lastPathComponent)
+            options.debug = true
+
             framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, dispatchQueueWrapper: TestSentryDispatchQueueWrapper(),
                                                 notificationCenter: TestNSNotificationCenterWrapper(), keepDelayedFramesDuration: 0)
             SentryDependencyContainer.sharedInstance().framesTracker = framesTracker
@@ -500,7 +499,7 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase {
         wait(for: [callbackExpectation], timeout: 0)
     }
 
-    func testLoadView_withUIViewController() {
+    func testLoadView_withNonInAppUIViewController_DoesNotStartTransaction() {
         let sut = fixture.getSut()
         let viewController = UIViewController()
         let tracker = fixture.tracker
@@ -519,6 +518,26 @@ class SentryUIViewControllerPerformanceTrackerTests: XCTestCase {
         wait(for: [callbackExpectation], timeout: 0)
     }
 
+    func testLoadView_withIgnoreSwizzleUIViewController_DoesNotStartTransaction() {
+        fixture.options.swizzleClassNameExcludes = ["TestViewController"]
+        let sut = fixture.getSut()
+        let viewController = fixture.viewController
+        let tracker = fixture.tracker
+        var transactionSpan: Span!
+        let callbackExpectation = expectation(description: "Callback Expectation")
+
+        XCTAssertTrue(getStack(tracker).isEmpty)
+
+        sut.viewControllerLoadView(viewController) {
+            let spans = self.getStack(tracker)
+            transactionSpan = spans.first
+            callbackExpectation.fulfill()
+        }
+
+        XCTAssertNil(transactionSpan, "Expected to transaction.")
+        wait(for: [callbackExpectation], timeout: 0)
+    }
+
     func testSecondLoadView() throws {
         let sut = fixture.getSut()
         let viewController = fixture.viewController

diff --git a/...ests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift b/...ests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift
@@ -13,14 +13,13 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
         let subClassFinder: TestSubClassFinder
         let processInfoWrapper = SentryNSProcessInfoWrapper()
         let binaryImageCache: SentryBinaryImageCache
+        var options: Options
 
         init() {
             subClassFinder = TestSubClassFinder(dispatchQueue: dispatchQueue, objcRuntimeWrapper: objcRuntimeWrapper, swizzleClassNameExcludes: [])
             binaryImageCache = SentryDependencyContainer.sharedInstance().binaryImageCache
-        }
-
-        var options: Options {
-            let options = Options.noIntegrations()
+
+            options = Options.noIntegrations()
 
             let imageName = String(
                 cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!,
@@ -31,8 +30,6 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
                 cString: class_getImageName(ExternalUIViewController.self)!,
                 encoding: .utf8)! as NSString
             options.add(inAppInclude: externalImageName.lastPathComponent)
-
-            return options
         }
 
         var sut: SentryUIViewControllerSwizzling {
@@ -98,6 +95,18 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
         XCTAssertFalse(result)
     }
 
+    func testShouldNotSwizzle_UIViewControllerExcludedFromSwizzling() {
+        fixture.options.swizzleClassNameExcludes = ["TestViewController"]
+
+        XCTAssertFalse(fixture.sut.shouldSwizzleViewController(TestViewController.self))
+    }
+
+    func testShouldSwizzle_UIViewControllerNotExcludedFromSwizzling() {
+        fixture.options.swizzleClassNameExcludes = ["TestViewController1"]
+
+        XCTAssertTrue(fixture.sut.shouldSwizzleViewController(TestViewController.self))
+    }
+
     func testUIViewController_loadView_noTransactionBoundToScope() {
         fixture.sut.start()
         let controller = UIViewController()