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

[PLAT-13280] More robust crosstalk API, and also a permissive proxy class #357

Merged
merged 1 commit into from
Dec 10, 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
8 changes: 4 additions & 4 deletions BugsnagPerformance.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
09D59E1B2BDFE0D900199E1B /* NetworkHeaderInjector.mm in Sources */ = {isa = PBXBuildFile; fileRef = 09D59E192BDFE0D900199E1B /* NetworkHeaderInjector.mm */; };
09D807F52B9756B000D01DF5 /* BugsnagPerformanceSwiftUI.docc in Sources */ = {isa = PBXBuildFile; fileRef = 09D807F42B9756B000D01DF5 /* BugsnagPerformanceSwiftUI.docc */; };
09D807F62B9756B000D01DF5 /* BugsnagPerformanceSwiftUI.h in Headers */ = {isa = PBXBuildFile; fileRef = 09D807F32B9756B000D01DF5 /* BugsnagPerformanceSwiftUI.h */; settings = {ATTRIBUTES = (Public, ); }; };
09E313042BF363020081F219 /* CrossTalkTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 09E313032BF363020081F219 /* CrossTalkTests.m */; };
09E313042BF363020081F219 /* CrossTalkTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 09E313032BF363020081F219 /* CrossTalkTests.mm */; };
09F23A8C2CE351ED00F0D769 /* BugsnagSwiftTools.h in Headers */ = {isa = PBXBuildFile; fileRef = 09F23A8A2CE351ED00F0D769 /* BugsnagSwiftTools.h */; };
09F23A8D2CE351ED00F0D769 /* BugsnagSwiftTools.m in Sources */ = {isa = PBXBuildFile; fileRef = 09F23A8B2CE351ED00F0D769 /* BugsnagSwiftTools.m */; };
09F23A8F2CE3521D00F0D769 /* BugsnagSwiftToolsImpl.swift in Sources */ = {isa = PBXBuildFile; fileRef = 09F23A8E2CE3521D00F0D769 /* BugsnagSwiftToolsImpl.swift */; };
Expand Down Expand Up @@ -310,7 +310,7 @@
09D807F32B9756B000D01DF5 /* BugsnagPerformanceSwiftUI.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BugsnagPerformanceSwiftUI.h; sourceTree = "<group>"; };
09D807F42B9756B000D01DF5 /* BugsnagPerformanceSwiftUI.docc */ = {isa = PBXFileReference; lastKnownFileType = folder.documentationcatalog; path = BugsnagPerformanceSwiftUI.docc; sourceTree = "<group>"; };
09DC62282C6A2EF6000AA8E1 /* BugsnagPerformanceSpanContext+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "BugsnagPerformanceSpanContext+Private.h"; sourceTree = "<group>"; };
09E313032BF363020081F219 /* CrossTalkTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CrossTalkTests.m; sourceTree = "<group>"; };
09E313032BF363020081F219 /* CrossTalkTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CrossTalkTests.mm; sourceTree = "<group>"; };
09F23A8A2CE351ED00F0D769 /* BugsnagSwiftTools.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BugsnagSwiftTools.h; sourceTree = "<group>"; };
09F23A8B2CE351ED00F0D769 /* BugsnagSwiftTools.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = BugsnagSwiftTools.m; sourceTree = "<group>"; };
09F23A8E2CE3521D00F0D769 /* BugsnagSwiftToolsImpl.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BugsnagSwiftToolsImpl.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -662,7 +662,7 @@
CB0AD75C29641B59002A3FB6 /* BatchTests.mm */,
CB0AD76B296578D9002A3FB6 /* BugsnagPerformanceConfigurationTests.mm */,
0122C25F29019C05002D243C /* BugsnagPerformanceTests.mm */,
09E313032BF363020081F219 /* CrossTalkTests.m */,
09E313032BF363020081F219 /* CrossTalkTests.mm */,
CBEC51C8296ED98F009C0CE3 /* FileBasedTest.h */,
CBEC51C9296ED98F009C0CE3 /* FileBasedTest.m */,
CBEC51D82976D54B009C0CE3 /* FilesystemTests.m */,
Expand Down Expand Up @@ -1172,7 +1172,7 @@
CBEC51D92976D54B009C0CE3 /* FilesystemTests.m in Sources */,
CB0AD75D29641B59002A3FB6 /* BatchTests.mm in Sources */,
CB68FABC2A3C4208005B2CDB /* PersistentDeviceIDTest.mm in Sources */,
09E313042BF363020081F219 /* CrossTalkTests.m in Sources */,
09E313042BF363020081F219 /* CrossTalkTests.mm in Sources */,
CB747D21299E5458003CA1B4 /* TimeTests.mm in Sources */,
0122C27129019CEF002D243C /* OtlpTraceEncodingTests.mm in Sources */,
09509B752ADFE9A900A358EC /* TracerTests.mm in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,61 @@
// Copyright © 2024 Bugsnag. All rights reserved.
//

// Bugsnag CrossTalk API
//
// CrossTalk is an Objective-C layer for sharing private APIs between Bugsnag libraries.
// It allows client libraries to call internal functions of this one without the usual
// worries of breaking downstream clients whenever internal code changes.
//
// This code should be duplicated and used as a template for any Bugsnag Objective-C
// library that wants to expose its API to other Bugsnag libraries.
//
// NOTE: Your CrossTalk class name MUST be unique or else it will clash with another
// Bugsnag library's CrossTalk class name.
//
// See CrossTalkTests.mm for an example of how to use CrossTalk from a client library.
// It contains a full example for how to set up a client library to call this one.

#import <Foundation/Foundation.h>
#import "SpanStackingHandler.h"
#import <memory>

NS_ASSUME_NONNULL_BEGIN

@interface BugsnagPerformanceCrossTalkAPI : NSObject
namespace bugsnag {
class SpanStackingHandler;
}

#pragma mark Mandatory Methods
@interface BugsnagPerformanceCrossTalkAPI : NSObject

+ (instancetype) sharedInstance;

#pragma mark Configuration and Internal Functions
/**
* Use the configure method to pass any information this CrossTalk API requires to function.
*/
+ (void)configureWithSpanStackingHandler:(std::shared_ptr<bugsnag::SpanStackingHandler>) handler;

@end

/**
* A very permissive proxy that won't crash if a method or property doesn't exist.
*
* When returning instances of Bugsnag classes, wrap them in this proxy so that
* they don't crash when that class's API changes.
*
* WARNING: Returning internal classes is effectively creating a contract between Bugsnag libraries!
* Be VERY conservative about any internal class you expose, because its interfaces will effectively
* be "published", and changing a method's signature could break client libraries that use it.
*
* Adding/removing methods/properties is fine, but changing signatures WILL break things.
*
* Some ways to protect against breakage due to changed method signatures:
* - Convert to maps and arrays instead
* - Create custom classes designed specifically for library interop
* - Create versioned wrapper methods in the classes and access those instead (doStuffV1, doStuffV2, etc)
*/
@interface BugsnagPerformanceCrossTalkProxiedObject : NSProxy

@property(nonatomic) std::shared_ptr<SpanStackingHandler> spanStackingHandler;
+ (instancetype) proxied:(id _Nullable)delegate;

@end

Expand Down
197 changes: 183 additions & 14 deletions Sources/BugsnagPerformance/Private/BugsnagPerformanceCrossTalkAPI.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,43 @@
//

#import "BugsnagPerformanceCrossTalkAPI.h"
#import "SpanStackingHandler.h"
#import "Utils.h"
#import <objc/runtime.h>

using namespace bugsnag;


@interface BugsnagPerformanceCrossTalkAPI ()

// Declare the things your API class needs here

@property(nonatomic) std::shared_ptr<SpanStackingHandler> spanStackingHandler;

@end


@implementation BugsnagPerformanceCrossTalkAPI

/**
* You'll call your configure method during start up.
*/
+ (void)configureWithSpanStackingHandler:(std::shared_ptr<SpanStackingHandler>) handler {
BugsnagPerformanceCrossTalkAPI.sharedInstance.spanStackingHandler = handler;
}

#pragma mark Exposed API

// Implement internal functions you want to expose to a CrossTalk client library here.
// NOTE: ALWAYS ALWAYS ALWAYS check the mapping of every single API with a unit test!!!

/**
* For unit tests only.
*/
- (NSString *)returnStringTestV1 {
return @"test";
}

/**
* Return the current trace and span IDs as strings in a 2-entry array, or return nil if no current span exists.
*
Expand All @@ -36,30 +67,53 @@ - (NSArray * _Nullable)getCurrentTraceAndSpanIdV1 {

#pragma mark Internal Functionality

static NSString *BSGUserInfoKeyMapped = @"mapped";
static NSString *BSGUserInfoValueMappedYes = @"YES";
static NSString *BSGUserInfoValueMappedNo = @"NO";
static NSString *BSGUserInfoKeyIsSafeToCall = @"isSafeToCall";
static NSString *BSGUserInfoKeyWillNOOP = @"willNOOP";

static bool classImplementsSelector(Class cls, SEL selector) {
bool selectorExists = false;
unsigned int methodCount = 0;
Method *methods = class_copyMethodList(cls, &methodCount);
for (unsigned int i = 0; i < methodCount; i++) {
if (method_getName(methods[i]) == selector) {
selectorExists = true;
break;
}
}
free(methods);
return selectorExists;
}

/**
* Map a named API to a method with the specified selector.
* If an error occurs, the user info dictionary of the error will contain a field "mapped".
* If "mapped" is "YES", then the selector has been mapped to a null implementation (does nothing, returns nil).
* If "mapped" is "NO", then no mapping has occurred, and the method doesn't exist (alling it will result in no such selector).
*
* If an error occurs, the user info dictionary will contain the following NSNumber (boolean) fields:
* - "isSafeToCall": If @(YES), this method is safe to call (it has an implementation). Otherwise, calling it WILL throw a selector-not-found exception.
* - "willNOOP": If @(YES), calling the mapped method will no-op.
*
* Common scenarios:
* - The host library isn't linked in: isSafeToCall = YES, willNOOP = YES
* - apiName doesn't exist: isSafeToCall = YES, willNOOP = YES
* - toSelector already exists: isSafeToCall = YES, willNOOP = NO
* - Tried to map the same thing twice: isSafeToCall = YES, willNOOP = NO
* - Selector signature clash: isSafeToCall = NO, willNOOP = NO
*/
+ (NSError *)mapAPINamed:(NSString * _Nonnull)apiName toSelector:(SEL)toSelector {
NSError *err = nil;
// By default, we map to a "do nothing" implementation in case we don't find a real one.
SEL fromSelector = @selector(internal_doNothing);

// Note: ALWAYS ALWAYS ALWAYS check every single API mapping with a unit test!!!
if ([apiName isEqualToString:@"getCurrentTraceAndSpanIdV1"]) {
fromSelector = @selector(getCurrentTraceAndSpanIdV1);
// apiName should map to an existing method in this API
SEL apiSelector = NSSelectorFromString(apiName);
if (classImplementsSelector(self.class, apiSelector)) {
fromSelector = apiSelector;
} else {
err = [NSError errorWithDomain:@"com.bugsnag.BugsnagCocoaPerformance"
code:0
userInfo:@{
NSLocalizedDescriptionKey:[NSString stringWithFormat:@"No such API: %@", apiName],
BSGUserInfoKeyMapped:BSGUserInfoValueMappedYes
BSGUserInfoKeyIsSafeToCall:@YES,
BSGUserInfoKeyWillNOOP:@YES
}];
}

Expand All @@ -73,7 +127,8 @@ + (NSError *)mapAPINamed:(NSString * _Nonnull)apiName toSelector:(SEL)toSelector
apiName,
NSStringFromSelector(fromSelector),
self.class],
BSGUserInfoKeyMapped:BSGUserInfoValueMappedNo
BSGUserInfoKeyIsSafeToCall:@NO,
BSGUserInfoKeyWillNOOP:@NO
}];
}

Expand All @@ -87,7 +142,8 @@ + (NSError *)mapAPINamed:(NSString * _Nonnull)apiName toSelector:(SEL)toSelector
apiName,
NSStringFromSelector(fromSelector),
self.class],
BSGUserInfoKeyMapped:BSGUserInfoValueMappedNo
BSGUserInfoKeyIsSafeToCall:@NO,
BSGUserInfoKeyWillNOOP:@NO
}];
}

Expand All @@ -101,7 +157,23 @@ + (NSError *)mapAPINamed:(NSString * _Nonnull)apiName toSelector:(SEL)toSelector
apiName,
NSStringFromSelector(fromSelector),
self.class],
BSGUserInfoKeyMapped:BSGUserInfoValueMappedNo
BSGUserInfoKeyIsSafeToCall:@NO,
BSGUserInfoKeyWillNOOP:@NO
}];
}

// Don't add a method that already exists
if (classImplementsSelector(self.class, toSelector)) {
return [NSError errorWithDomain:@"com.bugsnag.BugsnagCocoaPerformance"
code:0
userInfo:@{
NSLocalizedDescriptionKey:[NSString stringWithFormat:
@"class_addMethod (while mapping api %@): Instance method %@ already exists in class %@",
apiName,
NSStringFromSelector(fromSelector),
self.class],
BSGUserInfoKeyIsSafeToCall:@YES,
BSGUserInfoKeyWillNOOP:@NO
}];
}

Expand All @@ -114,7 +186,8 @@ + (NSError *)mapAPINamed:(NSString * _Nonnull)apiName toSelector:(SEL)toSelector
apiName,
NSStringFromSelector(fromSelector),
self.class],
BSGUserInfoKeyMapped:BSGUserInfoValueMappedNo
BSGUserInfoKeyIsSafeToCall:@NO,
BSGUserInfoKeyWillNOOP:@NO
}];
}

Expand All @@ -133,3 +206,99 @@ + (instancetype) sharedInstance {
}

@end


#pragma mark BugsnagPerformanceCrossTalkProxiedObject

@interface BugsnagPerformanceCrossTalkProxiedObject ()

@property(nonatomic,strong) id delegate;

@end

@implementation BugsnagPerformanceCrossTalkProxiedObject

+ (instancetype) proxied:(id _Nullable)delegate {
BugsnagPerformanceCrossTalkProxiedObject *proxy = [BugsnagPerformanceCrossTalkProxiedObject alloc];
proxy.delegate = delegate;
return proxy;
}

// Allow faster access to ivars in these special cases
#pragma clang diagnostic ignored "-Wdirect-ivar-access"

- (void)forwardInvocation:(NSInvocation *)anInvocation {
if ([_delegate respondsToSelector:anInvocation.selector]) {
[anInvocation setTarget:_delegate];
[anInvocation invoke];
}
}

-(NSMethodSignature*)methodSignatureForSelector:(SEL)aSelector {
NSMethodSignature *sig = [_delegate methodSignatureForSelector:aSelector];
if (sig) {
return sig;
}

BSGLogWarning(@"CrossTalk: Tried to invoke unimplemented selector [%@] on proxied object %@",
NSStringFromSelector(aSelector), [_delegate debugDescription]);

// Return a no-arg signature that's guaranteed to exist
return [NSObject instanceMethodSignatureForSelector:@selector(init)];
}

#pragma mark NSObject protocol (BugsnagPerformanceCrossTalkProxiedObject)

- (Class)class {
return [_delegate class];
}

- (Class)superclass {
return [_delegate superclass];
}

- (BOOL)isKindOfClass:(Class)aClass {
return [_delegate isKindOfClass:aClass];
}

- (BOOL)isMemberOfClass:(Class)aClass {
return [_delegate isMemberOfClass:aClass];
}

- (BOOL)respondsToSelector:(SEL)aSelector {
// Be truthful about this
return [_delegate respondsToSelector:aSelector];
}

- (BOOL)conformsToProtocol:(Protocol *)aProtocol {
// Be truthful about this
return [_delegate conformsToProtocol:aProtocol];
}

- (BOOL)isEqual:(id)object {
return [_delegate isEqual:object];
}

- (NSUInteger)hash {
return [_delegate hash];
}

- (BOOL)isProxy {
return YES;
}

- (NSString *)description {
if (_delegate) {
return [_delegate description];
}
return super.description;
}

- (NSString *)debugDescription {
if (_delegate) {
return [_delegate debugDescription];
}
return super.debugDescription;
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@
[worker_ earlySetup];
[frameMetricsCollector_ earlySetup];

BugsnagPerformanceCrossTalkAPI.sharedInstance.spanStackingHandler = spanStackingHandler_;
[BugsnagPerformanceCrossTalkAPI configureWithSpanStackingHandler:spanStackingHandler_];
}

void BugsnagPerformanceImpl::configure(BugsnagPerformanceConfiguration *config) noexcept {
Expand Down
Loading
Loading