Skip to content

Commit

Permalink
fix: Use latest value when checking release stage (#412)
Browse files Browse the repository at this point in the history
Resolve inconsistencies between updated release stage values and
the value reported in crashes. Use the last value before a crash
as the release stage in the report.
  • Loading branch information
kattrali authored Sep 18, 2019
2 parents e1b5842 + ce91abb commit b5f8d8d
Show file tree
Hide file tree
Showing 10 changed files with 233 additions and 12 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ Changelog

* Ensure UIKit APIs are not called from background threads if
`Bugsnag.start()` is called in the background
* Fix bug in `notifyReleaseStages` where if the release stage of a build was
changed after `start()`, only the initial value was used to determine whether
to send a report
[#405](https://github.com/bugsnag/bugsnag-cocoa/issues/405)

## 5.22.5 (2019-08-14)

Expand Down
20 changes: 15 additions & 5 deletions Source/BugsnagCrashReport.m
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,22 @@
return error[BSGKeyReason] ?: @"";
}

id BSGLoadConfigValue(NSDictionary *report, NSString *valueName) {
NSString *keypath = [NSString stringWithFormat:@"user.config.%@", valueName];
NSString *fallbackKeypath = [NSString stringWithFormat:@"user.config.config.%@", valueName];

return [report valueForKeyPath:keypath]
?: [report valueForKeyPath:fallbackKeypath]; // some custom values are nested
}

NSString *BSGParseContext(NSDictionary *report, NSDictionary *metaData) {
id context = [report valueForKeyPath:@"user.overrides.context"];
if ([context isKindOfClass:[NSString class]])
return context;
context = metaData[BSGKeyContext];
if ([context isKindOfClass:[NSString class]])
return context;
context = [report valueForKeyPath:@"user.config.context"];
context = BSGLoadConfigValue(report, @"context");
if ([context isKindOfClass:[NSString class]])
return context;
return nil;
Expand All @@ -132,7 +140,7 @@

NSString *BSGParseReleaseStage(NSDictionary *report) {
return [report valueForKeyPath:@"user.overrides.releaseStage"]
?: [report valueForKeyPath:@"user.config.releaseStage"];
?: BSGLoadConfigValue(report, @"releaseStage");
}

BSGSeverity BSGParseSeverity(NSString *severity) {
Expand Down Expand Up @@ -253,8 +261,7 @@ - (instancetype)initWithKSReport:(NSDictionary *)report
}
} else {
FallbackReportData *fallback = [[FallbackReportData alloc] initWithMetadata:metadata];
_notifyReleaseStages =
[report valueForKeyPath:@"user.config.notifyReleaseStages"];
_notifyReleaseStages = BSGLoadConfigValue(report, @"notifyReleaseStages");
_releaseStage = BSGParseReleaseStage(report);
_incomplete = report.count == 0;
_threads = [report valueForKeyPath:@"crash.threads"];
Expand All @@ -276,7 +283,10 @@ - (instancetype)initWithKSReport:(NSDictionary *)report
_deviceState = BSGParseDeviceState(report);
_device = BSGParseDevice(report);
_app = BSGParseApp(report);
_appState = BSGParseAppState(report[BSGKeySystem], [report valueForKeyPath:@"user.config.appVersion"]);
_appState = BSGParseAppState(report[BSGKeySystem],
BSGLoadConfigValue(report, @"appVersion"),
_releaseStage, // Already loaded from config
BSGLoadConfigValue(report, @"codeBundleId"));
_groupingHash = BSGParseGroupingHash(report, _metaData);
_overrides = [report valueForKeyPath:@"user.overrides"];
_customException = BSGParseCustomException(report, [_errorClass copy],
Expand Down
5 changes: 4 additions & 1 deletion Source/BugsnagKSCrashSysInfoParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,8 @@

NSDictionary *_Nonnull BSGParseDevice(NSDictionary *_Nonnull report);
NSDictionary *_Nonnull BSGParseApp(NSDictionary *_Nonnull report);
NSDictionary *_Nonnull BSGParseAppState(NSDictionary *_Nonnull report, NSString *_Nullable preferredVersion);
NSDictionary *_Nonnull BSGParseAppState(NSDictionary *_Nonnull report,
NSString *_Nullable preferredVersion,
NSString *_Nullable releaseStage,
NSString *_Nullable codeBundleId);
NSDictionary *_Nonnull BSGParseDeviceState(NSDictionary *_Nonnull report);
6 changes: 3 additions & 3 deletions Source/BugsnagKSCrashSysInfoParser.m
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,17 @@
return appState;
}

NSDictionary *BSGParseAppState(NSDictionary *report, NSString *preferredVersion) {
NSDictionary *BSGParseAppState(NSDictionary *report, NSString *preferredVersion, NSString *releaseStage, NSString *codeBundleId) {
NSMutableDictionary *app = [NSMutableDictionary dictionary];

NSString *version = preferredVersion ?: report[@"CFBundleShortVersionString"];

BSGDictSetSafeObject(app, report[@"CFBundleVersion"], @"bundleVersion");
BSGDictSetSafeObject(app, [Bugsnag configuration].releaseStage,
BSGDictSetSafeObject(app, releaseStage,
BSGKeyReleaseStage);
BSGDictSetSafeObject(app, version, BSGKeyVersion);

BSGDictSetSafeObject(app, [Bugsnag configuration].codeBundleId, @"codeBundleId");
BSGDictSetSafeObject(app, codeBundleId, @"codeBundleId");

NSString *notifierType;
#if TARGET_OS_TV
Expand Down
5 changes: 4 additions & 1 deletion Source/BugsnagSessionTrackingPayload.m
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ - (NSMutableDictionary *)toJson {
BSGDictSetSafeObject(dict, [Bugsnag notifier].details, BSGKeyNotifier);

NSDictionary *systemInfo = [BSG_KSSystemInfo systemInfo];
BSGDictSetSafeObject(dict, BSGParseAppState(systemInfo, [Bugsnag configuration].appVersion), @"app");
BSGDictSetSafeObject(dict, BSGParseAppState(systemInfo,
[Bugsnag configuration].appVersion,
[Bugsnag configuration].releaseStage,
[Bugsnag configuration].codeBundleId), @"app");
BSGDictSetSafeObject(dict, BSGParseDeviceState(systemInfo), @"device");
return dict;
}
Expand Down
4 changes: 3 additions & 1 deletion Tests/BugsnagSinkTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ - (void)setUp {
BugsnagConfiguration *config = [BugsnagConfiguration new];
config.autoNotify = NO;
config.apiKey = @"apiKeyHere";
// This value should not appear in the assertions, as it is not equal to
// the release stage in the serialized report
config.releaseStage = @"MagicalTestingTime";

// set a dummy endpoint, avoid hitting production
Expand Down Expand Up @@ -298,7 +300,7 @@ - (void)testEventApp {
XCTAssertEqualObjects(app[@"version"], @"1.0");
XCTAssertEqualObjects(app[@"name"], @"CrashProbeiOS");
XCTAssertEqualObjects(app[@"bundleVersion"], @"1");
XCTAssertEqualObjects(app[@"releaseStage"], @"MagicalTestingTime");
XCTAssertEqualObjects(app[@"releaseStage"], @"production");
XCTAssertEqualObjects(app[@"dsymUUIDs"], @[@"D0A41830-4FD2-3B02-A23B-0741AD4C7F52"]);
XCTAssertEqualObjects(app[@"duration"], @4000);
XCTAssertEqualObjects(app[@"durationInForeground"], @2000);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
8A530CCC22FDDBF000F0C108 /* ManyConcurrentNotifyScenario.m in Sources */ = {isa = PBXBuildFile; fileRef = 8A530CCB22FDDBF000F0C108 /* ManyConcurrentNotifyScenario.m */; };
8A840FBA21AF5C450041DBFA /* SwiftAssertion.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8A840FB921AF5C450041DBFA /* SwiftAssertion.swift */; };
8A98400320FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.m in Sources */ = {isa = PBXBuildFile; fileRef = 8A98400220FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.m */; };
8AB1081923301FE600672818 /* ReleaseStageScenarios.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB1081823301FE600672818 /* ReleaseStageScenarios.swift */; };
8AB8866420404DD30003E444 /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB8866320404DD30003E444 /* AppDelegate.swift */; };
8AB8866620404DD30003E444 /* ViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB8866520404DD30003E444 /* ViewController.swift */; };
8AB8866920404DD30003E444 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 8AB8866720404DD30003E444 /* Main.storyboard */; };
Expand Down Expand Up @@ -89,6 +90,7 @@
8A840FB921AF5C450041DBFA /* SwiftAssertion.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwiftAssertion.swift; sourceTree = "<group>"; };
8A98400120FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AutoSessionCustomVersionScenario.h; sourceTree = "<group>"; };
8A98400220FD11BF0023ECD1 /* AutoSessionCustomVersionScenario.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = AutoSessionCustomVersionScenario.m; sourceTree = "<group>"; };
8AB1081823301FE600672818 /* ReleaseStageScenarios.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReleaseStageScenarios.swift; sourceTree = "<group>"; };
8AB8866020404DD30003E444 /* iOSTestApp.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = iOSTestApp.app; sourceTree = BUILT_PRODUCTS_DIR; };
8AB8866320404DD30003E444 /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppDelegate.swift; sourceTree = "<group>"; };
8AB8866520404DD30003E444 /* ViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewController.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -238,6 +240,7 @@
F42953DE2BB41023C0B07F41 /* scenarios */ = {
isa = PBXGroup;
children = (
8AB1081823301FE600672818 /* ReleaseStageScenarios.swift */,
8A56EE8022E22ED80066B9DC /* OOMWillTerminateScenario.h */,
8A56EE7F22E22ED80066B9DC /* OOMWillTerminateScenario.m */,
8A14F0F42282D4AE00337B05 /* ReportBackgroundOOMsEnabledScenario.h */,
Expand Down Expand Up @@ -455,10 +458,10 @@
F42955E0916B8851F074D9B3 /* UserEmailScenario.swift in Sources */,
F4295968571A4118D6A4606A /* UserEnabledScenario.swift in Sources */,
F4295A036B228AF608641699 /* UserDisabledScenario.swift in Sources */,
8A56EE8222E22ED80066B9DC /* OOMWillTerminateScenario.m in Sources */,
8A14F0F62282D4AE00337B05 /* ReportOOMsDisabledScenario.m in Sources */,
E7767F13221C21E30006648C /* ResumedSessionScenario.swift in Sources */,
8AEFC73120F8D1A000A78779 /* AutoSessionWithUserScenario.m in Sources */,
8AB1081923301FE600672818 /* ReleaseStageScenarios.swift in Sources */,
8AF6FD7A225E3FA00056EF9E /* ResumeSessionOOMScenario.m in Sources */,
F429565A951303E2C3136D0D /* UserIdScenario.swift in Sources */,
8AEEBBD020FC9E1D00C60763 /* AutoSessionMixedEventsScenario.m in Sources */,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import UIKit

class MagicError : NSError {}

class NotifyWhenReleaseStageInNotifyReleaseStages : Scenario {

override func startBugsnag() {
self.config.shouldAutoCaptureSessions = false;
self.config.releaseStage = "prod"
self.config.notifyReleaseStages = ["dev", "prod"]
super.startBugsnag()
}

override func run() {
Bugsnag.notifyError(MagicError(domain: "com.example",
code: 43,
userInfo: [NSLocalizedDescriptionKey: "incoming!"]))
}
}

class CrashWhenReleaseStageInNotifyReleaseStages : Scenario {

override func startBugsnag() {
self.config.shouldAutoCaptureSessions = false;
self.config.releaseStage = "prod"
self.config.notifyReleaseStages = ["dev", "prod"]
super.startBugsnag()
}

override func run() {
abort();
}
}

class CrashWhenReleaseStageInNotifyReleaseStagesChanges : Scenario {

override func startBugsnag() {
self.config.shouldAutoCaptureSessions = false;
if (self.eventMode == "noevent") {
// The event is evaluated whether to be sent
self.config.releaseStage = "test"
} else {
// A crash will occur
self.config.releaseStage = "prod"
}
self.config.notifyReleaseStages = ["dev", "prod"]
super.startBugsnag()
}

override func run() {
abort();
}
}

class CrashWhenReleaseStageNotInNotifyReleaseStagesChanges : Scenario {

override func startBugsnag() {
self.config.shouldAutoCaptureSessions = false;
if (self.eventMode == "noevent") {
// The event is evaluated whether to be sent
self.config.releaseStage = "prod"
} else {
// A crash will occur
self.config.releaseStage = "test"
}
self.config.notifyReleaseStages = ["dev", "prod"]
super.startBugsnag()
}

override func run() {
abort();
}
}

class NotifyWhenReleaseStageNotInNotifyReleaseStages : Scenario {

override func startBugsnag() {
self.config.shouldAutoCaptureSessions = false;
self.config.releaseStage = "dev"
self.config.notifyReleaseStages = ["prod"]
super.startBugsnag()
}

override func run() {
Bugsnag.notifyError(MagicError(domain: "com.example",
code: 43,
userInfo: [NSLocalizedDescriptionKey: "incoming!"]))
}
}

class CrashWhenReleaseStageNotInNotifyReleaseStages : Scenario {

override func startBugsnag() {
self.config.shouldAutoCaptureSessions = false;
self.config.releaseStage = "dev"
self.config.notifyReleaseStages = ["prod"]
super.startBugsnag()
}

override func run() {
abort();
}
}
51 changes: 51 additions & 0 deletions features/release_stages.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
Feature: Discarding reports based on release stage

Scenario: Crash when release stage is not present in "notify release stages"
When I crash the app using "CrashWhenReleaseStageNotInNotifyReleaseStages"
And I relaunch the app
And I wait for 10 seconds
Then I should receive 0 requests

Scenario: Crash when release stage is present in "notify release stages"
When I crash the app using "CrashWhenReleaseStageInNotifyReleaseStages"
And I relaunch the app
And I wait for a request
Then the request is valid for the error reporting API
And the exception "errorClass" equals "SIGABRT"
And the event "unhandled" is true
And the event "app.releaseStage" equals "prod"

Scenario: Crash when release stage is changed to not present in "notify release stages" before the event
If the current run has a different release stage than the crashing context,
the report should only be sent if the release stage was in "notify release stages"
at the time of the crash. Release stages can change for a single build of an app
if the app is used as a test harness or if the build can receive code updates,
such as JavaScript execution contexts.

When I crash the app using "CrashWhenReleaseStageNotInNotifyReleaseStagesChanges"
And I relaunch the app
And I wait for 10 seconds
Then I should receive 0 requests

Scenario: Crash when release stage is changed to be present in "notify release stages" before the event
When I crash the app using "CrashWhenReleaseStageInNotifyReleaseStagesChanges"
And I relaunch the app
And I wait for a request
Then the request is valid for the error reporting API
And the exception "errorClass" equals "SIGABRT"
And the event "unhandled" is true
And the event "app.releaseStage" equals "prod"

Scenario: Notify when release stage is not present in "notify release stages"
When I run "NotifyWhenReleaseStageNotInNotifyReleaseStages"
And I wait for 10 seconds
Then I should receive 0 requests

Scenario: Notify when release stage is present in "notify release stages"
When I run "NotifyWhenReleaseStageInNotifyReleaseStages"
And I wait for a request
Then the request is valid for the error reporting API
And the exception "errorClass" equals "iOSTestApp.MagicError"
And the exception "message" equals "incoming!"
And the event "unhandled" is false
And the event "app.releaseStage" equals "prod"
42 changes: 42 additions & 0 deletions iOS/BugsnagTests/BugsnagKSCrashSysInfoParserTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,46 @@ - (void)testDeviceFreeSpaceShouldBeNilWhenFailsToRetrieveIt {
XCTAssertNil(freeBytes, @"expect nil when fails to retrieve free space for the directory");
}

- (void)testParseAppInfo {
NSDictionary *rawInfo = @{
@"CFBundleShortVersionString":@"4.1.1",
@"CFBundleVersion":@"4.1.1.2362",
@"extra":@"foo",
};
NSDictionary *state = BSGParseAppState(rawInfo, nil, @"prod", nil);
XCTAssertEqual(state.count, 5);
XCTAssertEqualObjects(state[@"releaseStage"], @"prod");
XCTAssertEqualObjects(state[@"version"], @"4.1.1");
XCTAssertEqualObjects(state[@"bundleVersion"], @"4.1.1.2362");
XCTAssertEqual(state[@"codeBundleId"], [NSNull null]);
#if TARGET_OS_TV
XCTAssertEqualObjects(state[@"type"], @"tvOS");
#elif TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
XCTAssertEqualObjects(state[@"type"], @"iOS");
#elif TARGET_OS_MAC
XCTAssertEqualObjects(state[@"type"], @"macOS");
#endif
}

- (void)testParseAppInfoPreferredValues {
NSDictionary *rawInfo = @{
@"CFBundleShortVersionString":@"4.1.1",
@"CFBundleVersion":@"4.1.1.2362",
@"extra":@"foo",
};
NSDictionary *state = BSGParseAppState(rawInfo, @"2.0", @"prod", @"4.2.0-cbd");
XCTAssertEqual(state.count, 5);
XCTAssertEqualObjects(state[@"releaseStage"], @"prod");
XCTAssertEqualObjects(state[@"version"], @"2.0");
XCTAssertEqualObjects(state[@"bundleVersion"], @"4.1.1.2362");
XCTAssertEqualObjects(state[@"codeBundleId"], @"4.2.0-cbd");
#if TARGET_OS_TV
XCTAssertEqualObjects(state[@"type"], @"tvOS");
#elif TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
XCTAssertEqualObjects(state[@"type"], @"iOS");
#elif TARGET_OS_MAC
XCTAssertEqualObjects(state[@"type"], @"macOS");
#endif
}

@end

0 comments on commit b5f8d8d

Please sign in to comment.