From 4ac28abd5c34d4fc006e9c169d863b95d2f799ef Mon Sep 17 00:00:00 2001 From: Nick Dowell Date: Wed, 21 Apr 2021 10:00:45 +0100 Subject: [PATCH 1/7] Fix thread sending behaviour --- Bugsnag/Client/BugsnagClient.m | 13 +++++-------- Bugsnag/Payload/BugsnagError+Private.h | 2 +- Bugsnag/Payload/BugsnagError.m | 8 ++------ Bugsnag/Payload/BugsnagEvent.m | 15 +++++++++------ CHANGELOG.md | 7 +++++++ Tests/BugsnagErrorTest.m | 6 +++--- features/barebone_tests.feature | 9 ++++----- features/threads.feature | 20 +++++++------------- 8 files changed, 38 insertions(+), 42 deletions(-) diff --git a/Bugsnag/Client/BugsnagClient.m b/Bugsnag/Client/BugsnagClient.m index ed4be8145..aa9e793b2 100644 --- a/Bugsnag/Client/BugsnagClient.m +++ b/Bugsnag/Client/BugsnagClient.m @@ -66,6 +66,7 @@ #import "BugsnagSession+Private.h" #import "BugsnagSessionTracker+Private.h" #import "BugsnagSessionTrackingApiClient.h" +#import "BugsnagStackframe+Private.h" #import "BugsnagStateEvent.h" #import "BugsnagSystemState.h" #import "BugsnagThread+Private.h" @@ -870,18 +871,14 @@ - (void)notify:(NSException *)exception NSArray *callStack = exception.callStackReturnAddresses; if (!callStack.count) { + // If the NSException was not raised by the Objective-C runtime, it will be missing a call stack. + // Use the current call stack instead. callStack = BSGArraySubarrayFromIndex(NSThread.callStackReturnAddresses, depth); } BOOL recordAllThreads = self.configuration.sendThreads == BSGThreadSendPolicyAlways; - NSArray *threads = [BugsnagThread allThreads:recordAllThreads callStackReturnAddresses:callStack]; + NSArray *threads = recordAllThreads ? [BugsnagThread allThreads:YES callStackReturnAddresses:callStack] : @[]; - NSArray *stacktrace = nil; - for (BugsnagThread *thread in threads) { - if (thread.errorReportingThread) { - stacktrace = thread.stacktrace; - break; - } - } + NSArray *stacktrace = [BugsnagStackframe stackframesWithCallStackReturnAddresses:callStack]; BugsnagError *error = [[BugsnagError alloc] initWithErrorClass:exception.name ?: NSStringFromClass([exception class]) errorMessage:exception.reason ?: @"" diff --git a/Bugsnag/Payload/BugsnagError+Private.h b/Bugsnag/Payload/BugsnagError+Private.h index ec65da698..a9ec9ebf5 100644 --- a/Bugsnag/Payload/BugsnagError+Private.h +++ b/Bugsnag/Payload/BugsnagError+Private.h @@ -14,7 +14,7 @@ NS_ASSUME_NONNULL_BEGIN @interface BugsnagError () -- (instancetype)initWithEvent:(NSDictionary *)event errorReportingThread:(nullable BugsnagThread *)thread; +- (instancetype)initWithKSCrashReport:(NSDictionary *)event stacktrace:(NSArray *)stacktrace; - (instancetype)initWithErrorClass:(NSString *)errorClass errorMessage:(NSString *)errorMessage diff --git a/Bugsnag/Payload/BugsnagError.m b/Bugsnag/Payload/BugsnagError.m index 1f302a0c9..4b7858e54 100644 --- a/Bugsnag/Payload/BugsnagError.m +++ b/Bugsnag/Payload/BugsnagError.m @@ -83,11 +83,7 @@ @implementation BugsnagError @dynamic type; -- (instancetype)initWithErrorReportingThread:(BugsnagThread *)thread { - return [self initWithEvent:@{} errorReportingThread:thread]; -} - -- (instancetype)initWithEvent:(NSDictionary *)event errorReportingThread:(BugsnagThread *)thread { +- (instancetype)initWithKSCrashReport:(NSDictionary *)event stacktrace:(NSArray *)stacktrace { if (self = [super init]) { NSDictionary *error = [event valueForKeyPath:@"crash.error"]; NSString *errorType = error[BSGKeyType]; @@ -96,7 +92,7 @@ - (instancetype)initWithEvent:(NSDictionary *)event errorReportingThread:(Bugsna _typeString = BSGSerializeErrorType(BSGErrorTypeCocoa); if (![[event valueForKeyPath:@"user.state.didOOM"] boolValue]) { - _stacktrace = thread.stacktrace; + _stacktrace = stacktrace; } } return self; diff --git a/Bugsnag/Payload/BugsnagEvent.m b/Bugsnag/Payload/BugsnagEvent.m index 05aab39ee..6af0e0250 100644 --- a/Bugsnag/Payload/BugsnagEvent.m +++ b/Bugsnag/Payload/BugsnagEvent.m @@ -310,12 +310,9 @@ - (instancetype)initWithKSCrashData:(NSDictionary *)event { // generate threads/error info NSArray *binaryImages = event[@"binary_images"]; NSArray *threadDict = [event valueForKeyPath:@"crash.threads"]; - NSMutableArray *threads = [BugsnagThread threadsFromArray:threadDict - binaryImages:binaryImages - depth:depth - errorType:errorType]; + NSArray *threads = [BugsnagThread threadsFromArray:threadDict binaryImages:binaryImages depth:depth errorType:errorType]; - BugsnagThread *errorReportingThread; + BugsnagThread *errorReportingThread = nil; for (BugsnagThread *thread in threads) { if (thread.errorReportingThread) { errorReportingThread = thread; @@ -323,7 +320,13 @@ - (instancetype)initWithKSCrashData:(NSDictionary *)event { } } - NSArray *errors = @[[[BugsnagError alloc] initWithEvent:event errorReportingThread:errorReportingThread]]; + NSArray *errors = @[[[BugsnagError alloc] initWithKSCrashReport:event stacktrace:errorReportingThread.stacktrace ?: @[]]]; + + // KSCrash captures only the offending thread when sendThreads = BSGThreadSendPolicyNever. + // The BugsnagEvent should not contain threads in this case, only the stacktrace. + if (threads.count == 1) { + threads = @[]; + } if (errorReportingThread.crashInfoMessage) { [errors[0] updateWithCrashInfoMessage:(NSString * _Nonnull)errorReportingThread.crashInfoMessage]; diff --git a/CHANGELOG.md b/CHANGELOG.md index 6054134c6..682f3289e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## TBD + +### Bug fixes + +* `event.threads` will now be empty, rather than containing a single thread, if `sendThreads` dictates that threads should not be sent. + [#1077](https://github.com/bugsnag/bugsnag-cocoa/pull/1077) + ## 6.9.0 (2021-04-21) ### Enhancements diff --git a/Tests/BugsnagErrorTest.m b/Tests/BugsnagErrorTest.m index 996c37fbe..ba18fde06 100644 --- a/Tests/BugsnagErrorTest.m +++ b/Tests/BugsnagErrorTest.m @@ -65,7 +65,7 @@ - (void)setUp { - (void)testErrorLoad { BugsnagThread *thread = [self findErrorReportingThread:self.event]; - BugsnagError *error = [[BugsnagError alloc] initWithEvent:self.event errorReportingThread:thread]; + BugsnagError *error = [[BugsnagError alloc] initWithKSCrashReport:self.event stacktrace:thread.stacktrace]; XCTAssertEqualObjects(@"Foo Exception", error.errorClass); XCTAssertEqualObjects(@"Foo overload", error.errorMessage); XCTAssertEqual(BSGErrorTypeCocoa, error.type); @@ -79,7 +79,7 @@ - (void)testErrorLoad { - (void)testToDictionary { BugsnagThread *thread = [self findErrorReportingThread:self.event]; - BugsnagError *error = [[BugsnagError alloc] initWithEvent:self.event errorReportingThread:thread]; + BugsnagError *error = [[BugsnagError alloc] initWithKSCrashReport:self.event stacktrace:thread.stacktrace]; NSDictionary *dict = [error toDictionary]; XCTAssertEqualObjects(@"Foo Exception", dict[@"errorClass"]); XCTAssertEqualObjects(@"Foo overload", dict[@"message"]); @@ -141,7 +141,7 @@ - (void)testErrorMessageParse { - (void)testStacktraceOverride { BugsnagThread *thread = [self findErrorReportingThread:self.event]; - BugsnagError *error = [[BugsnagError alloc] initWithEvent:self.event errorReportingThread:thread]; + BugsnagError *error = [[BugsnagError alloc] initWithKSCrashReport:self.event stacktrace:thread.stacktrace]; XCTAssertNotNil(error.stacktrace); XCTAssertEqual(1, error.stacktrace.count); error.stacktrace = @[]; diff --git a/features/barebone_tests.feature b/features/barebone_tests.feature index 4ccc8e704..8ffbb0498 100644 --- a/features/barebone_tests.feature +++ b/features/barebone_tests.feature @@ -61,10 +61,6 @@ Feature: Barebone tests And the event "severity" equals "warning" And the event "severityReason.type" equals "handledException" And the event "severityReason.unhandledOverridden" is true - And the event "threads.0.errorReportingThread" is true - And the event "threads.0.id" equals "0" - And the event "threads.0.name" equals "com.apple.main-thread" - And the event "threads.0.stacktrace.0.method" matches "BareboneTestHandledScenario" And the event "unhandled" is true And the event "user.email" equals "foobar@example.com" And the event "user.id" equals "foobar" @@ -79,7 +75,7 @@ Feature: Barebone tests And the error payload field "events.0.device.freeMemory" is an integer And the error payload field "events.0.device.model" matches the test device model And the error payload field "events.0.device.totalMemory" is an integer - And the error payload field "events.0.threads" is an array with 1 elements + And the error payload field "events.0.threads" is an array with 0 elements And the "method" of stack frame 0 matches "BareboneTestHandledScenario" And I discard the oldest error @@ -158,6 +154,8 @@ Feature: Barebone tests And the error payload field "events.0.device.freeMemory" is an integer And the error payload field "events.0.device.model" matches the test device model And the error payload field "events.0.device.totalMemory" is an integer + And the error payload field "events.0.threads" is a non-empty array + And the error payload field "events.0.threads.1" is not null And the "method" of stack frame 0 matches "(assertionFailure|)" @skip_macos @@ -246,3 +244,4 @@ Feature: Barebone tests And the error payload field "events.0.device.freeMemory" is null And the error payload field "events.0.device.model" matches the test device model And the error payload field "events.0.device.totalMemory" is an integer + And the error payload field "events.0.threads" is an array with 0 elements diff --git a/features/threads.feature b/features/threads.feature index 9f43e622f..75dd8415b 100644 --- a/features/threads.feature +++ b/features/threads.feature @@ -1,4 +1,4 @@ -Feature: Handled Errors and Exceptions +Feature: Threads Background: Given I clear all persistent data @@ -10,6 +10,8 @@ Feature: Handled Errors and Exceptions And the event "unhandled" is false And the error payload field "events" is an array with 1 elements And the exception "message" equals "HandledErrorThreadSendAlwaysScenario" + And the error payload field "events.0.threads" is a non-empty array + And the error payload field "events.0.threads.1" is not null And the thread information is valid for the event Scenario: Threads are captured for unhandled errors by default @@ -20,6 +22,8 @@ Feature: Handled Errors and Exceptions And the event "unhandled" is true And the error payload field "events" is an array with 1 elements And the exception "message" equals "UnhandledErrorThreadSendAlwaysScenario" + And the error payload field "events.0.threads" is a non-empty array + And the error payload field "events.0.threads.1" is not null And the thread information is valid for the event Scenario: Threads are not captured for handled errors when sendThreads is set to unhandled_only @@ -29,12 +33,7 @@ Feature: Handled Errors and Exceptions And the event "unhandled" is false And the error payload field "events" is an array with 1 elements And the exception "errorClass" equals "HandledErrorThreadSendUnhandledOnlyScenario" - And the error payload field "events.0.threads" is an array with 1 elements - And the error payload field "events.0.threads.0.errorReportingThread" is true - And the error payload field "events.0.threads.0.id" is not null - And the error payload field "events.0.threads.0.name" equals "com.apple.main-thread" - And the error payload field "events.0.threads.0.type" equals "cocoa" - And the thread information is valid for the event + And the error payload field "events.0.threads" is an array with 0 elements Scenario: Threads are not captured for unhandled errors when sendThreads is set to never When I run "UnhandledErrorThreadSendNeverScenario" and relaunch the app @@ -44,9 +43,4 @@ Feature: Handled Errors and Exceptions And the event "unhandled" is true And the error payload field "events" is an array with 1 elements And the exception "message" equals "UnhandledErrorThreadSendNeverScenario" - And the error payload field "events.0.threads" is an array with 1 elements - And the error payload field "events.0.threads.0.errorReportingThread" is true - And the error payload field "events.0.threads.0.id" is not null - And the error payload field "events.0.threads.0.name" is null - And the error payload field "events.0.threads.0.type" equals "cocoa" - And the thread information is valid for the event + And the error payload field "events.0.threads" is an array with 0 elements From f7a398dc0bea816684cc0b79f3187849c41f67bc Mon Sep 17 00:00:00 2001 From: Nick Dowell Date: Wed, 21 Apr 2021 13:14:03 +0100 Subject: [PATCH 2/7] Remove app_hangs.feature from barebones E2E runs --- .buildkite/pipeline.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 0748d6207..514fb103a 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -138,7 +138,6 @@ steps: docker-compose#v3.3.0: run: cocoa-maze-runner command: - - "features/app_hangs.feature" - "features/barebone_tests.feature" - "--app=/app/build/iOSTestApp.ipa" - "--farm=bs" @@ -165,7 +164,6 @@ steps: docker-compose#v3.3.0: run: cocoa-maze-runner command: - - "features/app_hangs.feature" - "features/barebone_tests.feature" - "--app=/app/build/iOSTestApp.ipa" - "--farm=bs" @@ -192,7 +190,6 @@ steps: commands: - bundle install - bundle exec maze-runner - features/app_hangs.feature features/barebone_tests.feature --farm=local --os=macos From af342cfaf933f243e5a504a8c4c7c144ddde1847 Mon Sep 17 00:00:00 2001 From: Nick Dowell Date: Wed, 21 Apr 2021 14:09:57 +0100 Subject: [PATCH 3/7] Fix app info in OOM events --- Bugsnag/Client/BugsnagClient+OutOfMemory.m | 7 ++++++ Bugsnag/Payload/BugsnagApp+Private.h | 2 ++ Bugsnag/Payload/BugsnagApp.m | 23 +++++++++++++++---- Bugsnag/Payload/BugsnagAppWithState+Private.h | 1 + CHANGELOG.md | 3 +++ features/barebone_tests.feature | 9 ++++---- .../fixtures/shared/scenarios/OOMScenario.m | 4 ++++ 7 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Bugsnag/Client/BugsnagClient+OutOfMemory.m b/Bugsnag/Client/BugsnagClient+OutOfMemory.m index 806a9a6db..7d9a4846e 100644 --- a/Bugsnag/Client/BugsnagClient+OutOfMemory.m +++ b/Bugsnag/Client/BugsnagClient+OutOfMemory.m @@ -11,6 +11,7 @@ #import "BugsnagAppWithState+Private.h" #import "BugsnagBreadcrumbs.h" #import "BugsnagClient+Private.h" +#import "BugsnagConfiguration+Private.h" #import "BugsnagDeviceWithState+Private.h" #import "BugsnagError+Private.h" #import "BugsnagEvent+Private.h" @@ -27,6 +28,12 @@ - (BugsnagEvent *)generateOutOfMemoryEvent { app.dsymUuid = appDict[BSGKeyMachoUUID]; app.isLaunching = [self.stateMetadataFromLastLaunch[BSGKeyApp][BSGKeyIsLaunching] boolValue]; + if (self.configMetadataFromLastLaunch) { + [app setValuesFromConfiguration: + [[BugsnagConfiguration alloc] initWithDictionaryRepresentation: + (NSDictionary * _Nonnull)self.configMetadataFromLastLaunch]]; + } + NSDictionary *deviceDict = self.systemState.lastLaunchState[SYSTEMSTATE_KEY_DEVICE]; BugsnagDeviceWithState *device = [BugsnagDeviceWithState deviceFromJson:deviceDict]; device.manufacturer = @"Apple"; diff --git a/Bugsnag/Payload/BugsnagApp+Private.h b/Bugsnag/Payload/BugsnagApp+Private.h index 808f9e8f4..d9aafc599 100644 --- a/Bugsnag/Payload/BugsnagApp+Private.h +++ b/Bugsnag/Payload/BugsnagApp+Private.h @@ -20,6 +20,8 @@ NS_ASSUME_NONNULL_BEGIN + (void)populateFields:(BugsnagApp *)app dictionary:(NSDictionary *)event config:(BugsnagConfiguration *)config codeBundleId:(NSString *)codeBundleId; +- (void)setValuesFromConfiguration:(BugsnagConfiguration *)configuration; + - (NSDictionary *)toDict; @end diff --git a/Bugsnag/Payload/BugsnagApp.m b/Bugsnag/Payload/BugsnagApp.m index 3a1c916ab..f28be16d2 100644 --- a/Bugsnag/Payload/BugsnagApp.m +++ b/Bugsnag/Payload/BugsnagApp.m @@ -61,12 +61,27 @@ + (void)populateFields:(BugsnagApp *)app { NSDictionary *system = event[BSGKeySystem]; app.id = system[@"CFBundleIdentifier"]; - app.bundleVersion = config.bundleVersion ?: system[@"CFBundleVersion"]; + app.bundleVersion = system[@"CFBundleVersion"]; app.dsymUuid = system[@"app_uuid"]; - app.version = config.appVersion ?: system[@"CFBundleShortVersionString"]; - app.releaseStage = config.releaseStage; + app.version = system[@"CFBundleShortVersionString"]; app.codeBundleId = [event valueForKeyPath:@"user.state.app.codeBundleId"] ?: codeBundleId; - app.type = config.appType; + [app setValuesFromConfiguration:config]; +} + +- (void)setValuesFromConfiguration:(BugsnagConfiguration *)configuration +{ + if (configuration.appType) { + self.type = configuration.appType; + } + if (configuration.appVersion) { + self.version = configuration.appVersion; + } + if (configuration.bundleVersion) { + self.bundleVersion = configuration.bundleVersion; + } + if (configuration.releaseStage) { + self.releaseStage = configuration.releaseStage; + } } - (NSDictionary *)toDict diff --git a/Bugsnag/Payload/BugsnagAppWithState+Private.h b/Bugsnag/Payload/BugsnagAppWithState+Private.h index 55d8d5356..b53fafd9d 100644 --- a/Bugsnag/Payload/BugsnagAppWithState+Private.h +++ b/Bugsnag/Payload/BugsnagAppWithState+Private.h @@ -7,6 +7,7 @@ // #import "BugsnagAppWithState.h" +#import "BugsnagApp+Private.h" @class BugsnagConfiguration; diff --git a/CHANGELOG.md b/CHANGELOG.md index 682f3289e..d338f61f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ Changelog ### Bug fixes +* Fix `app` properties in OOMs for apps that override `appType`, `appVersion`, `bundleVersion` or `releaseStage` in their config. + [#1078](https://github.com/bugsnag/bugsnag-cocoa/pull/1078) + * `event.threads` will now be empty, rather than containing a single thread, if `sendThreads` dictates that threads should not be sent. [#1077](https://github.com/bugsnag/bugsnag-cocoa/pull/1077) diff --git a/features/barebone_tests.feature b/features/barebone_tests.feature index 8ffbb0498..5bb8c2f0f 100644 --- a/features/barebone_tests.feature +++ b/features/barebone_tests.feature @@ -178,17 +178,16 @@ Feature: Barebone tests And I wait to receive an error Then the error is an OOM event - And the event "app.bundleVersion" is not null + And the event "app.bundleVersion" equals "321.123" And the event "app.dsymUUIDs" is not null And the event "app.id" equals the platform-dependent string: | ios | com.bugsnag.iOSTestApp | | macos | com.bugsnag.macOSTestApp | And the event "app.inForeground" is true And the event "app.isLaunching" is true - And the event "app.type" equals the platform-dependent string: - | ios | iOS | - | macos | macOS | - And the event "app.version" is not null + And the event "app.releaseStage" equals "staging" + And the event "app.type" equals "vanilla" + And the event "app.version" equals "3.2.1" And the event "breadcrumbs.0.name" equals "Bugsnag loaded" And the event "breadcrumbs.1.name" equals "Memory Warning" And the event "device.id" is not null diff --git a/features/fixtures/shared/scenarios/OOMScenario.m b/features/fixtures/shared/scenarios/OOMScenario.m index 2ad8394a3..5dc39734f 100644 --- a/features/fixtures/shared/scenarios/OOMScenario.m +++ b/features/fixtures/shared/scenarios/OOMScenario.m @@ -24,6 +24,10 @@ - (void)startBugsnag { self.config.autoTrackSessions = YES; self.config.enabledErrorTypes.ooms = YES; self.config.launchDurationMillis = 0; // Ensure isLaunching will be true for the OOM, no matter how long it takes to occur. + self.config.appType = @"vanilla"; + self.config.appVersion = @"3.2.1"; + self.config.bundleVersion = @"321.123"; + self.config.releaseStage = @"staging"; [self.config addMetadata:@{@"bar": @"foo"} toSection:@"custom"]; [self.config setUser:@"foobar" withEmail:@"foobar@example.com" andName:@"Foo Bar"]; [self.config addOnSendErrorBlock:^BOOL(BugsnagEvent *event) { From 8915f25845a6e46b712f3fe16c27d335caeca6ea Mon Sep 17 00:00:00 2001 From: Nick Dowell Date: Thu, 22 Apr 2021 13:13:37 +0100 Subject: [PATCH 4/7] Fix missing context for crash, OOM, and app hang errors --- Bugsnag/Client/BugsnagClient+AppHangs.m | 2 ++ Bugsnag/Client/BugsnagClient+OutOfMemory.m | 2 ++ Bugsnag/Client/BugsnagClient+Private.h | 26 +++++++++++++++++++ Bugsnag/Client/BugsnagClient.m | 11 ++++---- Bugsnag/Helpers/BugsnagCollections.h | 3 +++ Bugsnag/Helpers/BugsnagCollections.m | 4 +++ Bugsnag/Helpers/BugsnagKeys.h | 1 + Bugsnag/Helpers/BugsnagKeys.m | 1 + Bugsnag/Payload/BugsnagEvent.m | 5 ++-- .../include/Bugsnag/BugsnagConfiguration.h | 2 +- CHANGELOG.md | 3 +++ features/app_hangs.feature | 2 ++ features/barebone_tests.feature | 2 ++ .../shared/scenarios/AppHangScenarios.swift | 1 + .../scenarios/BareboneTestScenarios.swift | 1 + .../fixtures/shared/scenarios/OOMScenario.m | 1 + 16 files changed, 59 insertions(+), 8 deletions(-) diff --git a/Bugsnag/Client/BugsnagClient+AppHangs.m b/Bugsnag/Client/BugsnagClient+AppHangs.m index 3f36a7ce2..47ee52737 100644 --- a/Bugsnag/Client/BugsnagClient+AppHangs.m +++ b/Bugsnag/Client/BugsnagClient+AppHangs.m @@ -60,6 +60,8 @@ - (void)appHangDetectedWithThreads:(nonnull NSArray *)threads { threads:threads session:self.sessionTracker.runningSession]; + self.appHangEvent.context = self.context; + NSError *writeError = nil; NSDictionary *json = [self.appHangEvent toJsonWithRedactedKeys:self.configuration.redactedKeys]; if (![BSGJSONSerialization writeJSONObject:json toFile:BSGFileLocations.current.appHangEvent options:0 error:&writeError]) { diff --git a/Bugsnag/Client/BugsnagClient+OutOfMemory.m b/Bugsnag/Client/BugsnagClient+OutOfMemory.m index 7d9a4846e..75ab93a2c 100644 --- a/Bugsnag/Client/BugsnagClient+OutOfMemory.m +++ b/Bugsnag/Client/BugsnagClient+OutOfMemory.m @@ -66,6 +66,8 @@ - (BugsnagEvent *)generateOutOfMemoryEvent { threads:@[] session:session]; + event.context = self.stateMetadataFromLastLaunch[BSGKeyClient][BSGKeyContext]; + return event; } diff --git a/Bugsnag/Client/BugsnagClient+Private.h b/Bugsnag/Client/BugsnagClient+Private.h index ca5f9b713..63e9d1630 100644 --- a/Bugsnag/Client/BugsnagClient+Private.h +++ b/Bugsnag/Client/BugsnagClient+Private.h @@ -72,6 +72,32 @@ NS_ASSUME_NONNULL_BEGIN @property (readonly, nonatomic) BOOL started; +/// State related metadata +/// +/// Upon change this is automatically persisted to disk, making it available when contructing OOM payloads. +/// Is it also added to KSCrashReports under `user.state` by `BSSerializeDataCrashHandler()`. +/// +/// Example contents: +/// +/// { +/// "app": { +/// "codeBundleId": "com.example.app", +/// "isLaunching": true +/// }, +/// "client": { +/// "context": "MyViewController", +/// }, +/// "deviceState": { +/// "batteryLevel": 0.5, +/// "charging": false, +/// "lowMemoryWarning": "2021-01-01T15:29:02.170Z", +/// "orientation": "portrait" +/// }, +/// "user": { +/// "id": "abc123", +/// "name": "bob" +/// } +/// } @property (strong, nonatomic) BugsnagMetadata *state; @property (strong, nonatomic) NSMutableArray *stateEventBlocks; diff --git a/Bugsnag/Client/BugsnagClient.m b/Bugsnag/Client/BugsnagClient.m index aa9e793b2..98102d376 100644 --- a/Bugsnag/Client/BugsnagClient.m +++ b/Bugsnag/Client/BugsnagClient.m @@ -99,9 +99,6 @@ // Contains notifier state, under "deviceState" and crash-specific // information under "crash". char *statePath; - // Contains properties in the Bugsnag payload overridden by the user before - // it was sent - char *userOverridesJSON; // User onCrash handler void (*onCrash)(const BSG_KSCrashReportWriter *writer); } bsg_g_bugsnag_data; @@ -248,7 +245,10 @@ - (instancetype)initWithConfiguration:(BugsnagConfiguration *)configuration { if ((self = [super init])) { // Take a shallow copy of the configuration _configuration = [configuration copy]; - _state = [[BugsnagMetadata alloc] initWithDictionary:@{BSGKeyApp: @{BSGKeyIsLaunching: @YES}}]; + _state = [[BugsnagMetadata alloc] initWithDictionary:@{ + BSGKeyApp: @{BSGKeyIsLaunching: @YES}, + BSGKeyClient: BSGDictionaryWithKeyAndObject(BSGKeyContext, _configuration.context) + }]; self.notifier = [BugsnagNotifier new]; self.systemState = [[BugsnagSystemState alloc] initWithConfiguration:configuration]; @@ -763,11 +763,12 @@ - (void)removeOnBreadcrumbBlock:(BugsnagOnBreadcrumbBlock _Nonnull)block { } // ============================================================================= -// MARK: - Other methods +// MARK: - Context // ============================================================================= - (void)setContext:(nullable NSString *)context { self.configuration.context = context; + [self.state addMetadata:context withKey:BSGKeyContext toSection:BSGKeyClient]; [self notifyObservers:[[BugsnagStateEvent alloc] initWithName:kStateEventContext data:context]]; } diff --git a/Bugsnag/Helpers/BugsnagCollections.h b/Bugsnag/Helpers/BugsnagCollections.h index b60e7e76a..d5638e4ed 100644 --- a/Bugsnag/Helpers/BugsnagCollections.h +++ b/Bugsnag/Helpers/BugsnagCollections.h @@ -39,6 +39,9 @@ NSArray * BSGArraySubarrayFromIndex(NSArray *array, NSUInteger index); // MARK: - NSDictionary +/// Returns a dictionary containing the key and object, or an empty dictionary if the object is nil. +NSDictionary * BSGDictionaryWithKeyAndObject(NSString *key, id _Nullable object); + /** * Merge values from source dictionary with destination * diff --git a/Bugsnag/Helpers/BugsnagCollections.m b/Bugsnag/Helpers/BugsnagCollections.m index 4fbebcd24..99bc60485 100644 --- a/Bugsnag/Helpers/BugsnagCollections.m +++ b/Bugsnag/Helpers/BugsnagCollections.m @@ -56,6 +56,10 @@ void BSGArrayAddIfNonnull(NSMutableArray *array, id _Nullable object) { // MARK: - NSDictionary +NSDictionary * BSGDictionaryWithKeyAndObject(NSString *key, id _Nullable object) { + return object ? @{key: (id _Nonnull)object} : @{}; +} + NSDictionary *BSGDictMerge(NSDictionary *source, NSDictionary *destination) { if ([destination count] == 0) { return source; diff --git a/Bugsnag/Helpers/BugsnagKeys.h b/Bugsnag/Helpers/BugsnagKeys.h index 1b3ac99c6..e7e6b4f9e 100644 --- a/Bugsnag/Helpers/BugsnagKeys.h +++ b/Bugsnag/Helpers/BugsnagKeys.h @@ -24,6 +24,7 @@ extern NSString *const BSGKeyBatteryLevel; extern NSString *const BSGKeyBreadcrumbs; extern NSString *const BSGKeyBundleVersion; extern NSString *const BSGKeyCharging; +extern NSString *const BSGKeyClient; extern NSString *const BSGKeyCodeBundleId; extern NSString *const BSGKeyConfig; extern NSString *const BSGKeyContext; diff --git a/Bugsnag/Helpers/BugsnagKeys.m b/Bugsnag/Helpers/BugsnagKeys.m index 2bbf2c6ac..f74ec3319 100644 --- a/Bugsnag/Helpers/BugsnagKeys.m +++ b/Bugsnag/Helpers/BugsnagKeys.m @@ -20,6 +20,7 @@ NSString *const BSGKeyBreadcrumbs = @"breadcrumbs"; NSString *const BSGKeyBundleVersion = @"bundleVersion"; NSString *const BSGKeyCharging = @"charging"; +NSString *const BSGKeyClient = @"client"; NSString *const BSGKeyCodeBundleId = @"codeBundleId"; NSString *const BSGKeyConfig = @"config"; NSString *const BSGKeyContext = @"context"; diff --git a/Bugsnag/Payload/BugsnagEvent.m b/Bugsnag/Payload/BugsnagEvent.m index 6af0e0250..9076d5599 100644 --- a/Bugsnag/Payload/BugsnagEvent.m +++ b/Bugsnag/Payload/BugsnagEvent.m @@ -246,7 +246,7 @@ - (instancetype)initWithKSReport:(NSDictionary *)event { } else if ([event valueForKeyPath:@"user.event"] != nil) { return [self initWithUserData:event]; } else { - return [self initWithKSCrashData:event]; + return [self initWithKSCrashReport:event]; } } @@ -259,7 +259,7 @@ - (instancetype)initWithKSReport:(NSDictionary *)event { * * @return a BugsnagEvent containing the parsed information */ -- (instancetype)initWithKSCrashData:(NSDictionary *)event { +- (instancetype)initWithKSCrashReport:(NSDictionary *)event { NSMutableDictionary *error = [[event valueForKeyPath:@"crash.error"] mutableCopy]; NSString *errorType = error[BSGKeyType]; @@ -377,6 +377,7 @@ - (instancetype)initWithKSCrashData:(NSDictionary *)event { obj.enabledReleaseStages = BSGLoadConfigValue(event, BSGKeyEnabledReleaseStages); obj.releaseStage = BSGParseReleaseStage(event); obj.deviceAppHash = deviceAppHash; + obj.context = [event valueForKeyPath:@"user.state.client.context"]; obj.customException = BSGParseCustomException(event, [errors[0].errorClass copy], [errors[0].errorMessage copy]); obj.error = error; obj.depth = depth; diff --git a/Bugsnag/include/Bugsnag/BugsnagConfiguration.h b/Bugsnag/include/Bugsnag/BugsnagConfiguration.h index d81bf2e58..f2bb2bef0 100644 --- a/Bugsnag/include/Bugsnag/BugsnagConfiguration.h +++ b/Bugsnag/include/Bugsnag/BugsnagConfiguration.h @@ -175,7 +175,7 @@ typedef BOOL (^BugsnagOnSessionBlock)(BugsnagSession *_Nonnull session); /** * A general summary of what was occuring in the application */ -@property (copy, nullable, nonatomic) NSString *context; +@property (copy, nullable, atomic) NSString *context; /** * The version of the application diff --git a/CHANGELOG.md b/CHANGELOG.md index d338f61f9..ce7a0bbde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ Changelog ### Bug fixes +* Fix missing `context` for crash, OOM, and app hang errors. + [#1079](https://github.com/bugsnag/bugsnag-cocoa/pull/1079) + * Fix `app` properties in OOMs for apps that override `appType`, `appVersion`, `bundleVersion` or `releaseStage` in their config. [#1078](https://github.com/bugsnag/bugsnag-cocoa/pull/1078) diff --git a/features/app_hangs.feature b/features/app_hangs.feature index a1f91bfaa..b146b5661 100644 --- a/features/app_hangs.feature +++ b/features/app_hangs.feature @@ -28,6 +28,8 @@ Feature: App hangs And the event "session.events.handled" equals 1 And the event "session.events.unhandled" equals 0 + And the event "context" equals "App Hang Scenario" + # # Checks copied from app_and_device_attributes.feature # diff --git a/features/barebone_tests.feature b/features/barebone_tests.feature index 5bb8c2f0f..7ef6042dd 100644 --- a/features/barebone_tests.feature +++ b/features/barebone_tests.feature @@ -116,6 +116,7 @@ Feature: Barebone tests And the event "app.version" equals "12.3" And the event "breadcrumbs.0.name" equals "Bugsnag loaded" And the event "breadcrumbs.1.name" is null + And the event "context" equals "Something" And the event "device.freeMemory" is less than the event "device.totalMemory" And the event "device.id" is not null And the event "device.jailbroken" is false @@ -190,6 +191,7 @@ Feature: Barebone tests And the event "app.version" equals "3.2.1" And the event "breadcrumbs.0.name" equals "Bugsnag loaded" And the event "breadcrumbs.1.name" equals "Memory Warning" + And the event "context" equals "OOM Scenario" And the event "device.id" is not null And the event "device.jailbroken" is false And the event "device.locale" is not null diff --git a/features/fixtures/shared/scenarios/AppHangScenarios.swift b/features/fixtures/shared/scenarios/AppHangScenarios.swift index 29e5bd410..c753ff189 100644 --- a/features/fixtures/shared/scenarios/AppHangScenarios.swift +++ b/features/fixtures/shared/scenarios/AppHangScenarios.swift @@ -14,6 +14,7 @@ class AppHangScenario: Scenario { } override func run() { + Bugsnag.setContext("App Hang Scenario") let timeInterval = TimeInterval(eventMode!)! NSLog("Simulating an app hang of \(timeInterval) seconds...") Thread.sleep(forTimeInterval: timeInterval) diff --git a/features/fixtures/shared/scenarios/BareboneTestScenarios.swift b/features/fixtures/shared/scenarios/BareboneTestScenarios.swift index 008726dd1..aaab20c21 100644 --- a/features/fixtures/shared/scenarios/BareboneTestScenarios.swift +++ b/features/fixtures/shared/scenarios/BareboneTestScenarios.swift @@ -116,6 +116,7 @@ class BareboneTestUnhandledErrorScenario: Scenario { } override func run() { + Bugsnag.setContext("Something") Bugsnag.setUser("barfoo", withEmail: "barfoo@example.com", andName: "Bar Foo") // Triggers "Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value: ..." diff --git a/features/fixtures/shared/scenarios/OOMScenario.m b/features/fixtures/shared/scenarios/OOMScenario.m index 5dc39734f..83c378e42 100644 --- a/features/fixtures/shared/scenarios/OOMScenario.m +++ b/features/fixtures/shared/scenarios/OOMScenario.m @@ -44,6 +44,7 @@ - (void)startBugsnag { } - (void)run { + [Bugsnag setContext:@"OOM Scenario"]; [NSNotificationCenter.defaultCenter addObserverForName:UIApplicationDidReceiveMemoryWarningNotification object:nil queue:nil usingBlock:^(NSNotification *note) { NSLog(@"*** Received memory warning"); From 57cf2beb07e6c69a86915e847c7647af541bd07a Mon Sep 17 00:00:00 2001 From: Nick Dowell Date: Mon, 26 Apr 2021 08:23:00 +0100 Subject: [PATCH 5/7] Fix -Wstrict-selector-match --- Bugsnag/Storage/BSGStorageMigratorV0V1.m | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/Bugsnag/Storage/BSGStorageMigratorV0V1.m b/Bugsnag/Storage/BSGStorageMigratorV0V1.m index 4df21e80a..561fe5848 100644 --- a/Bugsnag/Storage/BSGStorageMigratorV0V1.m +++ b/Bugsnag/Storage/BSGStorageMigratorV0V1.m @@ -11,26 +11,12 @@ #import "BSGFileLocations.h" #import "BugsnagLogger.h" -static NSString *getCachesDir() { - NSArray *dirs = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES); - if ([dirs count] == 0) { - bsg_log_err(@"Could not locate cache directory path."); - return nil; - } - - if ([dirs[0] length] == 0) { - bsg_log_err(@"Cache directory path is empty!"); - return nil; - } - return dirs[0]; -} - @implementation BSGStorageMigratorV0V1 + (BOOL) migrate { NSString *bundleName = [[NSBundle mainBundle] infoDictionary][@"CFBundleName"]; - NSString *cachesDir = getCachesDir(); - if(cachesDir == nil) { + NSString *cachesDir = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES).firstObject; + if (!cachesDir.length) { bsg_log_err(@"Could not migrate v0 data to v1."); return false; } From 5bdba58b26bd49fe775f28607f78e40aa722a092 Mon Sep 17 00:00:00 2001 From: Nick Dowell Date: Mon, 26 Apr 2021 08:35:15 +0100 Subject: [PATCH 6/7] Fix deadlock in bsg_recordException() --- .../Recording/Sentry/BSG_KSCrashSentry_NSException.m | 6 +++--- CHANGELOG.md | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m index b1aee2f83..79a746d4e 100644 --- a/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m +++ b/Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m @@ -116,9 +116,6 @@ void bsg_recordException(NSException *exception) { bsg_lastHandledException = exception; BSG_KSLOG_DEBUG(@"Writing exception info into a new report"); - BSG_KSLOG_DEBUG(@"Suspending all threads."); - bsg_kscrashsentry_suspendThreads(); - BSG_KSLOG_DEBUG(@"Filling out context."); NSArray *addresses = [exception callStackReturnAddresses]; NSUInteger numFrames = [addresses count]; @@ -137,6 +134,9 @@ void bsg_recordException(NSException *exception) { bsg_g_context->stackTrace = callstack; bsg_g_context->stackTraceLength = callstack ? (int)numFrames : 0; + BSG_KSLOG_DEBUG(@"Suspending all threads."); + bsg_kscrashsentry_suspendThreads(); + BSG_KSLOG_DEBUG(@"Calling main crash handler."); bsg_g_context->onCrash(crashContext()); diff --git a/CHANGELOG.md b/CHANGELOG.md index ce7a0bbde..e8c9390ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ Changelog ### Bug fixes +* Fix a possible deadlock when writing crash reports for uncaught Objective-C exceptions. + [#1082](https://github.com/bugsnag/bugsnag-cocoa/pull/1082) + * Fix missing `context` for crash, OOM, and app hang errors. [#1079](https://github.com/bugsnag/bugsnag-cocoa/pull/1079) From 5899b740b21825280deac59b8cdf7ba76e33c661 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Wed, 28 Apr 2021 10:03:09 +0200 Subject: [PATCH 7/7] Release v6.9.1 --- .jazzy.yaml | 4 ++-- Bugsnag.podspec.json | 4 ++-- Bugsnag/Payload/BugsnagNotifier.m | 2 +- CHANGELOG.md | 2 +- Framework/Info.plist | 2 +- Tests/Info.plist | 2 +- VERSION | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.jazzy.yaml b/.jazzy.yaml index 89562bfc2..6e6c31efb 100644 --- a/.jazzy.yaml +++ b/.jazzy.yaml @@ -2,11 +2,11 @@ author_url: "https://www.bugsnag.com" author: "Bugsnag Inc" clean: false # avoid deleting docs/.git framework_root: "Bugsnag" -github_file_prefix: "https://github.com/bugsnag/bugsnag-cocoa/tree/v6.9.0/Bugsnag" +github_file_prefix: "https://github.com/bugsnag/bugsnag-cocoa/tree/v6.9.1/Bugsnag" github_url: "https://github.com/bugsnag/bugsnag-cocoa" hide_documentation_coverage: true module: "Bugsnag" -module_version: "6.9.0" +module_version: "6.9.1" objc: true output: "docs" readme: "README.md" diff --git a/Bugsnag.podspec.json b/Bugsnag.podspec.json index 31fc8c4c4..f93f11910 100644 --- a/Bugsnag.podspec.json +++ b/Bugsnag.podspec.json @@ -1,6 +1,6 @@ { "name": "Bugsnag", - "version": "6.9.0", + "version": "6.9.1", "summary": "The Bugsnag crash reporting framework for Apple platforms.", "homepage": "https://bugsnag.com", "license": "MIT", @@ -9,7 +9,7 @@ }, "source": { "git": "https://github.com/bugsnag/bugsnag-cocoa.git", - "tag": "v6.9.0" + "tag": "v6.9.1" }, "frameworks": [ "Foundation", diff --git a/Bugsnag/Payload/BugsnagNotifier.m b/Bugsnag/Payload/BugsnagNotifier.m index 5c12248cd..0a3f8ac0e 100644 --- a/Bugsnag/Payload/BugsnagNotifier.m +++ b/Bugsnag/Payload/BugsnagNotifier.m @@ -23,7 +23,7 @@ - (instancetype)init { #else self.name = @"Bugsnag Objective-C"; #endif - self.version = @"6.9.0"; + self.version = @"6.9.1"; self.url = @"https://github.com/bugsnag/bugsnag-cocoa"; self.dependencies = [NSMutableArray new]; } diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c9390ce..1ddd2517c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ Changelog ========= -## TBD +## 6.9.1 (2021-04-28) ### Bug fixes diff --git a/Framework/Info.plist b/Framework/Info.plist index 84a1eee48..05db8be35 100644 --- a/Framework/Info.plist +++ b/Framework/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType FMWK CFBundleShortVersionString - 6.9.0 + 6.9.1 CFBundleVersion 1 diff --git a/Tests/Info.plist b/Tests/Info.plist index 9a8dda271..a4ceaaecf 100644 --- a/Tests/Info.plist +++ b/Tests/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType $(PRODUCT_BUNDLE_PACKAGE_TYPE) CFBundleShortVersionString - 6.9.0 + 6.9.1 CFBundleVersion 1 diff --git a/VERSION b/VERSION index 97f578152..dc3829f5e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.9.0 +6.9.1