Skip to content

Commit

Permalink
fix(config): Use latest value when checking release stage
Browse files Browse the repository at this point in the history
A serialization bug in crash reports would erroneously allow or prevent
crash reports from being delivered when the value of release stage
or notify release stages were changed after start() was called, or in a
subsequent build of the same app.

Some values in config are serialized as nested values, while others are
at the "top" level of config when written to disk. This change checks
the expected position and fallback value for every config option.

Fixes #405
  • Loading branch information
kattrali committed Sep 17, 2019
1 parent e1b5842 commit 59d282d
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 6 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
17 changes: 12 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,7 @@ - (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"));
_groupingHash = BSGParseGroupingHash(report, _metaData);
_overrides = [report valueForKeyPath:@"user.overrides"];
_customException = BSGParseCustomException(report, [_errorClass copy],
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"

0 comments on commit 59d282d

Please sign in to comment.