Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Swizzling RootUIViewController if ignored #4407

Merged
merged 2 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

- Fix the versioning to support app release with Beta versions (#4368)
- Linking ongoing trace to crash event (#4393)
- Edge case for swizzleClassNameExclude (#4405): Skip creating transactions for UIViewControllers ignored for swizzling
via the option `swizzleClassNameExclude`.
- Swizzling RootUIViewController if ignored by `swizzleClassNameExclude` (#4407)

## 8.37.0-beta.1

Expand Down
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,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 */; };
Expand Down Expand Up @@ -1113,6 +1114,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>"; };
Expand Down Expand Up @@ -3838,6 +3840,7 @@
D8739CF72BECFF92007D2F66 /* Performance */ = {
isa = PBXGroup;
children = (
6294287F2CB3BF4E002C454C /* SwizzleClassNameExclude.swift */,
D8739CF82BECFFB5007D2F66 /* SentryTransactionNameSource.swift */,
);
path = Performance;
Expand Down Expand Up @@ -4744,6 +4747,7 @@
7B2A70DD27D6083D008B0D15 /* SentryThreadWrapper.m in Sources */,
62E146D02BAAE47600ED34FD /* LocalMetricsAggregator.swift 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 */,
Expand Down
8 changes: 4 additions & 4 deletions Sources/Sentry/Public/SentryOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
11 changes: 4 additions & 7 deletions Sources/Sentry/SentrySubClassFinder.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#import "SentryDispatchQueueWrapper.h"
#import "SentryLog.h"
#import "SentryObjCRuntimeWrapper.h"
#import "SentrySwift.h"
#import <objc/runtime.h>
#import <string.h>

Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions Sources/Sentry/SentryUIViewControllerPerformanceTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions Sources/Sentry/SentryUIViewControllerSwizzling.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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];
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)!,
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
Loading