diff --git a/CHANGELOG.md b/CHANGELOG.md index a46c367af..6f6a1810e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -188,6 +188,9 @@ Bugsnag Notifiers on other platforms. `locale`) that were missing from the OOM reports. [#444](https://github.com/bugsnag/bugsnag-cocoa/pull/444) +* Increased the detail in handled event breadcrumbs + [#493](https://github.com/bugsnag/bugsnag-cocoa/pull/493) + ## 5.23.0 (2019-12-10) This release removes support for reporting 'partial' or 'minimal' crash reports diff --git a/Source/BugsnagBreadcrumb.m b/Source/BugsnagBreadcrumb.m index bac159a58..4c343947d 100644 --- a/Source/BugsnagBreadcrumb.m +++ b/Source/BugsnagBreadcrumb.m @@ -95,10 +95,12 @@ - (NSDictionary *)objectValue { metadata[[key copy]] = [_metadata[key] copy]; } return @{ - BSGKeyMessage : [_message copy], - BSGKeyTimestamp : timestamp, - BSGKeyType : BSGBreadcrumbTypeValue(_type), - BSGKeyMetadata : metadata + // Note: The Bugsnag Error Reporting API specifies that the breadcrumb "message" + // field should be delivered in as a "name" field. This comment notes that variance. + BSGKeyName : [_message copy], + BSGKeyTimestamp : timestamp, + BSGKeyType : BSGBreadcrumbTypeValue(_type), + BSGKeyMetadata : metadata }; } return nil; diff --git a/Source/BugsnagClient.m b/Source/BugsnagClient.m index dcf693c15..fa889ceba 100644 --- a/Source/BugsnagClient.m +++ b/Source/BugsnagClient.m @@ -275,6 +275,7 @@ @interface BugsnagConfiguration () @interface BugsnagEvent () @property(readonly, copy, nonnull) NSDictionary *overrides; @property(readwrite) NSUInteger depth; +@property(readonly, nonnull) BugsnagHandledState *handledState; @end @interface BugsnagMetadata () @@ -636,6 +637,8 @@ - (void)setupConnectivityListener { }]; } +// MARK: - Notify + - (void)notifyError:(NSError *)error block:(void (^)(BugsnagEvent *))block { BugsnagHandledState *state = @@ -744,16 +747,18 @@ - (void)notify:(NSException *)exception [self.sessionTracker handleHandledErrorEvent]; } - BugsnagEvent *report = [[BugsnagEvent alloc] + BugsnagEvent *event = [[BugsnagEvent alloc] initWithErrorName:exceptionName errorMessage:message configuration:self.configuration metadata:[self.configuration.metadata toDictionary] handledState:handledState session:self.sessionTracker.runningSession]; + if (block) { - block(report); + block(event); } + // We discard 5 stack frames (including this one) by default, // and sum that with the number specified by report.depth: // @@ -764,28 +769,36 @@ - (void)notify:(NSException *)exception // 3 -[BugsnagCrashSentry reportUserException:reason:] // 4 -[BugsnagClient notify:message:block:] - int depth = (int)(BSGNotifierStackFrameCount + report.depth); + int depth = (int)(BSGNotifierStackFrameCount + event.depth); - NSString *reportName = - report.errorClass ?: NSStringFromClass([NSException class]); - NSString *reportMessage = report.errorMessage ?: @""; + NSString *eventErrorClass = event.errorClass ?: NSStringFromClass([NSException class]); + NSString *eventMessage = event.errorMessage ?: @""; - [self.crashSentry reportUserException:reportName - reason:reportMessage + [self.crashSentry reportUserException:eventErrorClass + reason:eventMessage originalException:exception handledState:[handledState toJson] appState:[self.state toDictionary] - callbackOverrides:report.overrides - metadata:[report.metadata copy] + callbackOverrides:event.overrides + metadata:[event.metadata copy] config:[self.configuration.config toDictionary] discardDepth:depth]; - + + // A basic set of event metadata + NSMutableDictionary *metadata = [@{ + BSGKeyErrorClass : eventErrorClass, + BSGKeyUnhandled : [[event handledState] unhandled] ? @YES : @NO, + BSGKeySeverity : BSGFormatSeverity(event.severity) + } mutableCopy]; + + // Only include the eventMessage if it contains something + if (eventMessage && [eventMessage length] > 0) { + [metadata setValue:eventMessage forKey:BSGKeyName]; + } + [self addAutoBreadcrumbOfType:BSGBreadcrumbTypeError - withMessage:reportName - andMetadata:@{ - BSGKeyMessage : reportMessage, - BSGKeySeverity : BSGFormatSeverity(report.severity) - }]; + withMessage:eventErrorClass + andMetadata:metadata]; [self flushPendingReports]; } diff --git a/Source/BugsnagEvent.m b/Source/BugsnagEvent.m index 758257c56..0c246a4fc 100644 --- a/Source/BugsnagEvent.m +++ b/Source/BugsnagEvent.m @@ -627,7 +627,6 @@ - (void)setOverrideProperty:(NSString *)key value:(id)value { } _overrides = metadata; } - } - (NSDictionary *)toJson { diff --git a/Source/BugsnagHandledState.m b/Source/BugsnagHandledState.m index 8a07eb517..bc5eb8a22 100644 --- a/Source/BugsnagHandledState.m +++ b/Source/BugsnagHandledState.m @@ -19,10 +19,10 @@ BSGSeverity BSGParseSeverity(NSString *severity) { NSString *BSGFormatSeverity(BSGSeverity severity) { switch (severity) { - case BSGSeverityInfo: - return BSGKeyInfo; case BSGSeverityError: return BSGKeyError; + case BSGSeverityInfo: + return BSGKeyInfo; case BSGSeverityWarning: return BSGKeyWarning; } diff --git a/Tests/BugsnagBreadcrumbsTest.m b/Tests/BugsnagBreadcrumbsTest.m index 0ffe722d3..f80145d21 100644 --- a/Tests/BugsnagBreadcrumbsTest.m +++ b/Tests/BugsnagBreadcrumbsTest.m @@ -126,9 +126,9 @@ - (void)testArrayValue { XCTAssertTrue([[formatter dateFromString:item[@"timestamp"]] isKindOfClass:[NSDate class]]); } - XCTAssertEqualObjects(value[0][@"message"], @"Launch app"); - XCTAssertEqualObjects(value[1][@"message"], @"Tap button"); - XCTAssertEqualObjects(value[2][@"message"], @"Close tutorial"); + XCTAssertEqualObjects(value[0][@"name"], @"Launch app"); + XCTAssertEqualObjects(value[1][@"name"], @"Tap button"); + XCTAssertEqualObjects(value[2][@"name"], @"Close tutorial"); } - (void)testStateType { @@ -141,7 +141,7 @@ - (void)testStateType { awaitBreadcrumbSync(self.crumbs); NSArray *value = [crumbs arrayValue]; XCTAssertEqualObjects(value[0][@"metaData"][@"direction"], @"right"); - XCTAssertEqualObjects(value[0][@"message"], @"Rotated Menu"); + XCTAssertEqualObjects(value[0][@"name"], @"Rotated Menu"); XCTAssertEqualObjects(value[0][@"type"], @"state"); } @@ -150,13 +150,13 @@ - (void)testPersistentCrumbManual { NSArray *value = [NSJSONSerialization JSONObjectWithData:crumbs options:0 error:nil]; XCTAssertEqual(value.count, 3); XCTAssertEqualObjects(value[0][@"type"], @"manual"); - XCTAssertEqualObjects(value[0][@"message"], @"Launch app"); + XCTAssertEqualObjects(value[0][@"name"], @"Launch app"); XCTAssertNotNil(value[0][@"timestamp"]); XCTAssertEqualObjects(value[1][@"type"], @"manual"); - XCTAssertEqualObjects(value[1][@"message"], @"Tap button"); + XCTAssertEqualObjects(value[1][@"name"], @"Tap button"); XCTAssertNotNil(value[1][@"timestamp"]); XCTAssertEqualObjects(value[2][@"type"], @"manual"); - XCTAssertEqualObjects(value[2][@"message"], @"Close tutorial"); + XCTAssertEqualObjects(value[2][@"name"], @"Close tutorial"); XCTAssertNotNil(value[2][@"timestamp"]); } @@ -170,7 +170,7 @@ - (void)testPersistentCrumbCustom { NSArray *value = [NSJSONSerialization JSONObjectWithData:crumbs options:0 error:nil]; XCTAssertEqual(value.count, 4); XCTAssertEqualObjects(value[3][@"type"], @"state"); - XCTAssertEqualObjects(value[3][@"message"], @"Initiate sequence"); + XCTAssertEqualObjects(value[3][@"name"], @"Initiate sequence"); XCTAssertEqualObjects(value[3][@"metaData"][@"captain"], @"Bob"); XCTAssertNotNil(value[3][@"timestamp"]); } @@ -231,7 +231,7 @@ - (void)testAlwaysAllowManual { NSArray *value = [self.crumbs arrayValue]; XCTAssertEqual(1, value.count); XCTAssertEqualObjects(value[0][@"type"], @"manual"); - XCTAssertEqualObjects(value[0][@"message"], @"this is a test"); + XCTAssertEqualObjects(value[0][@"name"], @"this is a test"); } /** @@ -338,7 +338,7 @@ - (void)testCallbackFreeConstructors2 { XCTAssertEqual(Bugsnag.client.configuration.breadcrumbs.count, 8); XCTAssertEqualObjects(bc0[@"type"], @"state"); - XCTAssertEqualObjects(bc0[@"message"], @"Bugsnag loaded"); + XCTAssertEqualObjects(bc0[@"name"], @"Bugsnag loaded"); XCTAssertEqual([bc0[@"metaData"] count], 0); XCTAssertEqual([bc1[@"metaData"] count], 1); @@ -350,25 +350,25 @@ - (void)testCallbackFreeConstructors2 { XCTAssertEqual([bc4[@"metaData"] count], 2); XCTAssertEqual([bc6[@"metaData"] count], 2); - XCTAssertEqualObjects(bc1[@"message"], @"manual message"); + XCTAssertEqualObjects(bc1[@"name"], @"manual message"); XCTAssertEqualObjects(bc1[@"type"], @"manual"); - XCTAssertEqualObjects(bc2[@"message"], @"log message"); + XCTAssertEqualObjects(bc2[@"name"], @"log message"); XCTAssertEqualObjects(bc2[@"type"], @"log"); - XCTAssertEqualObjects(bc3[@"message"], @"navigation message"); + XCTAssertEqualObjects(bc3[@"name"], @"navigation message"); XCTAssertEqualObjects(bc3[@"type"], @"navigation"); - XCTAssertEqualObjects(bc4[@"message"], @"process message"); + XCTAssertEqualObjects(bc4[@"name"], @"process message"); XCTAssertEqualObjects(bc4[@"type"], @"process"); - XCTAssertEqualObjects(bc5[@"message"], @"request message"); + XCTAssertEqualObjects(bc5[@"name"], @"request message"); XCTAssertEqualObjects(bc5[@"type"], @"request"); - XCTAssertEqualObjects(bc6[@"message"], @"state message"); + XCTAssertEqualObjects(bc6[@"name"], @"state message"); XCTAssertEqualObjects(bc6[@"type"], @"state"); - XCTAssertEqualObjects(bc7[@"message"], @"user message"); + XCTAssertEqualObjects(bc7[@"name"], @"user message"); XCTAssertEqualObjects(bc7[@"type"], @"user"); } @@ -387,8 +387,8 @@ - (void)testCallbackFreeConstructors3 { NSDictionary *bc1 = [Bugsnag.client.configuration.breadcrumbs arrayValue][1]; NSDictionary *bc2 = [Bugsnag.client.configuration.breadcrumbs arrayValue][2]; - XCTAssertEqualObjects(bc1[@"message"], @"message1"); - XCTAssertEqualObjects(bc2[@"message"], @"message2"); + XCTAssertEqualObjects(bc1[@"name"], @"message1"); + XCTAssertEqualObjects(bc2[@"name"], @"message2"); XCTAssertEqual([bc1[@"metaData"] count], 0); XCTAssertEqual([bc2[@"metaData"] count], 0); diff --git a/Tests/BugsnagClientTests.m b/Tests/BugsnagClientTests.m new file mode 100644 index 000000000..1de6ada9f --- /dev/null +++ b/Tests/BugsnagClientTests.m @@ -0,0 +1,91 @@ +// +// BugsnagClientTests.m +// Tests +// +// Created by Robin Macharg on 18/03/2020. +// Copyright © 2020 Bugsnag. All rights reserved. +// + +#import "Bugsnag.h" +#import "BugsnagBreadcrumbs.h" +#import "BugsnagClient.h" +#import "BugsnagTestConstants.h" +#import "BugsnagKeys.h" +#import + +@interface BugsnagClientTests : XCTestCase +@end + +@interface Bugsnag () ++ (BugsnagConfiguration *)configuration; ++ (BugsnagClient *)client; +@end + +@interface BugsnagClient () +- (void)orientationChanged:(NSNotification *)notif; +@end + +@interface BugsnagBreadcrumb () +- (NSDictionary *)objectValue; +@end + +@interface BugsnagConfiguration () +@property(readonly, strong, nullable) BugsnagBreadcrumbs *breadcrumbs; +@end + +NSString *BSGFormatSeverity(BSGSeverity severity); + +@implementation BugsnagClientTests + +/** + * A boilerplate helper method to setup Bugsnag + */ +-(void)setUpBugsnagWillCallNotify:(bool)willNotify { + BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + if (willNotify) { + [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + } + [Bugsnag startBugsnagWithConfiguration:configuration]; +} + +/** + * Handled events leave a breadcrumb when notify() is called. Test that values are inserted + * correctly. + */ +- (void)testAutomaticNotifyBreadcrumbData { + + [self setUpBugsnagWillCallNotify:false]; + + NSException *ex = [[NSException alloc] initWithName:@"myName" reason:@"myReason1" userInfo:nil]; + + __block NSString *eventErrorClass; + __block NSString *eventErrorMessage; + __block BOOL eventUnhandled; + __block NSString *eventSeverity; + + // Check that the event is passed the apiKey + [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) { + XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1); + + // Grab the values that end up in the event for later comparison + eventErrorClass = [event errorClass]; + eventErrorMessage = [event errorMessage]; + eventUnhandled = [event valueForKeyPath:@"handledState.unhandled"] ? YES : NO; + eventSeverity = BSGFormatSeverity([event severity]); + }]; + + // Check that we can change it + [Bugsnag notify:ex]; + + NSDictionary *breadcrumb = [[[[Bugsnag client] configuration] breadcrumbs][1] objectValue]; + NSDictionary *metadata = [breadcrumb valueForKey:@"metaData"]; + + XCTAssertEqualObjects([breadcrumb valueForKey:@"type"], @"error"); + XCTAssertEqualObjects([breadcrumb valueForKey:@"name"], eventErrorClass); + XCTAssertEqualObjects([metadata valueForKey:@"errorClass"], eventErrorClass); + XCTAssertEqualObjects([metadata valueForKey:@"name"], eventErrorMessage); + XCTAssertEqual((bool)[metadata valueForKey:@"unhandled"], eventUnhandled); + XCTAssertEqualObjects([metadata valueForKey:@"severity"], eventSeverity); +} + +@end diff --git a/Tests/BugsnagSinkTests.m b/Tests/BugsnagSinkTests.m index d7b69634a..d8138cbb4 100644 --- a/Tests/BugsnagSinkTests.m +++ b/Tests/BugsnagSinkTests.m @@ -138,7 +138,7 @@ - (void)testEventBreadcrumbs { [self.processedData[@"events"] firstObject][@"breadcrumbs"]; XCTAssertEqual(2, breadcrumbs.count); for (int i = 0; i < breadcrumbs.count; i++) { - XCTAssertEqualObjects(expected[i][@"message"], breadcrumbs[i][@"message"]); + XCTAssertEqualObjects(expected[i][@"name"], breadcrumbs[i][@"message"]); XCTAssertEqualObjects(expected[i][@"type"], breadcrumbs[i][@"type"]); XCTAssertEqualObjects(expected[i][@"timestamp"], breadcrumbs[i][@"timestamp"]); XCTAssertEqualObjects(expected[i][@"metadata"], breadcrumbs[i][@"metadata"]); diff --git a/features/steps/ios_steps.rb b/features/steps/ios_steps.rb index 153705f12..f1af9b1d0 100644 --- a/features/steps/ios_steps.rb +++ b/features/steps/ios_steps.rb @@ -102,7 +102,7 @@ crumbs = read_key_path(find_request(0)[:body], "events.0.breadcrumbs") assert_not_equal(0, crumbs.length, "There are no breadcrumbs on this event") match = crumbs.detect do |crumb| - crumb["message"] == string && crumb["type"] == type + crumb["name"] == string && crumb["type"] == type end assert_not_nil(match, "No crumb matches the provided message and type") end @@ -111,7 +111,7 @@ crumbs = read_key_path(find_request(0)[:body], "events.0.breadcrumbs") assert_not_equal(0, crumbs.length, "There are no breadcrumbs on this event") match = crumbs.detect do |crumb| - crumb["message"] == string + crumb["name"] == string end assert_not_nil(match, "No crumb matches the provided message") end diff --git a/iOS/Bugsnag.xcodeproj/project.pbxproj b/iOS/Bugsnag.xcodeproj/project.pbxproj index 8fa7049b5..90260e644 100644 --- a/iOS/Bugsnag.xcodeproj/project.pbxproj +++ b/iOS/Bugsnag.xcodeproj/project.pbxproj @@ -18,6 +18,7 @@ 0061D84924067AF80041C068 /* BSG_SSKeychain.m in Sources */ = {isa = PBXBuildFile; fileRef = 0061D84524067AF70041C068 /* BSG_SSKeychain.m */; }; 0061D84A24067AF80041C068 /* BSG_SSKeychain.m in Sources */ = {isa = PBXBuildFile; fileRef = 0061D84524067AF70041C068 /* BSG_SSKeychain.m */; }; 0061D84B24067AF80041C068 /* BSG_SSKeychain.h in Headers */ = {isa = PBXBuildFile; fileRef = 0061D84624067AF80041C068 /* BSG_SSKeychain.h */; }; + 0089B70324221EDE00D5A7F2 /* BugsnagClientTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 0089B70224221EDE00D5A7F2 /* BugsnagClientTests.m */; }; 009131BC23F46279000810D9 /* BugsnagMetadataTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 009131BB23F46279000810D9 /* BugsnagMetadataTests.m */; }; 009131BE23F5884E000810D9 /* BugsnagBaseUnitTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 009131BD23F5884E000810D9 /* BugsnagBaseUnitTest.m */; }; 00D7ACAD23E9C63000FBE4A7 /* BugsnagTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 00D7ACAC23E9C63000FBE4A7 /* BugsnagTests.m */; }; @@ -435,6 +436,7 @@ 0061D84424067AF70041C068 /* LICENSE */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = LICENSE; sourceTree = ""; }; 0061D84524067AF70041C068 /* BSG_SSKeychain.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = BSG_SSKeychain.m; sourceTree = ""; }; 0061D84624067AF80041C068 /* BSG_SSKeychain.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BSG_SSKeychain.h; sourceTree = ""; }; + 0089B70224221EDE00D5A7F2 /* BugsnagClientTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = BugsnagClientTests.m; path = ../../Tests/BugsnagClientTests.m; sourceTree = ""; }; 009131BB23F46279000810D9 /* BugsnagMetadataTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = BugsnagMetadataTests.m; path = ../../Tests/BugsnagMetadataTests.m; sourceTree = ""; }; 009131BD23F5884E000810D9 /* BugsnagBaseUnitTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = BugsnagBaseUnitTest.m; path = ../../Tests/BugsnagBaseUnitTest.m; sourceTree = ""; }; 009131BF23F58930000810D9 /* BugsnagBaseUnitTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = BugsnagBaseUnitTest.h; path = ../../Tests/BugsnagBaseUnitTest.h; sourceTree = ""; }; @@ -786,9 +788,14 @@ 8A2C8F261C6BBD2300846019 /* Tests */ = { isa = PBXGroup; children = ( + 000DF29323DB4B4900A883CE /* TestConstants.m */, + 00F9393723FC4F63008C7073 /* BugsnagTestsDummyClass.h */, + 00F9393823FC4F64008C7073 /* BugsnagTestsDummyClass.m */, + 00D7ACAC23E9C63000FBE4A7 /* BugsnagTests.m */, 009131BD23F5884E000810D9 /* BugsnagBaseUnitTest.m */, 009131BF23F58930000810D9 /* BugsnagBaseUnitTest.h */, 8A2C8F8B1C6BBFDD00846019 /* BugsnagBreadcrumbsTest.m */, + 0089B70224221EDE00D5A7F2 /* BugsnagClientTests.m */, 4B3B193422CA7B0900475354 /* BugsnagCollectionsBSGDictSetSafeObjectTest.m */, 4B775FCE22CBDEB4004839C5 /* BugsnagCollectionsBSGDictInsertIfNotNilTest.m */, 4BE6C42522CAD61A0056305D /* BugsnagCollectionsBSGDictMergeTest.m */, @@ -798,6 +805,7 @@ E77316E11F73B46600A14F06 /* BugsnagHandledStateTest.m */, F429554A50F3ABE60537F70E /* BugsnagKSCrashSysInfoParserTest.m */, 009131BB23F46279000810D9 /* BugsnagMetadataTests.m */, + E72AE1FF241A57B100ED8972 /* BugsnagPluginTest.m */, E78C1EFB1FCC759B00B976D3 /* BugsnagSessionTest.m */, E78C1EF01FCC2F1700B976D3 /* BugsnagSessionTrackerTest.m */, E78C1EF21FCC615400B976D3 /* BugsnagSessionTrackingPayloadTest.m */, @@ -1328,6 +1336,8 @@ E733A76F1FD709B7003EAA29 /* KSCrashReportStore_Tests.m in Sources */, E70EE0781FD7039E00FA745C /* RFC3339DateTool_Tests.m in Sources */, 8A530CC422FDD51F00F0C108 /* KSCrashIdentifierTests.m in Sources */, + 000DF29423DB4B4900A883CE /* TestConstants.m in Sources */, + 0089B70324221EDE00D5A7F2 /* BugsnagClientTests.m in Sources */, 8A4E733F1DC13281001F7CC8 /* BugsnagConfigurationTests.m in Sources */, E784D25A1FD70C25004B01E1 /* KSJSONCodec_Tests.m in Sources */, 009131BC23F46279000810D9 /* BugsnagMetadataTests.m in Sources */,