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.

Sign up for GitHub

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

fix(oom): Report short version as app.version #349

Merged
merged 2 commits into from
May 21, 2019
Merged

fix(oom): Report short version as app.version #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67055f2
fix(oom): Report short version as app.version
kattrali May 21, 2019
cfeb982
build: Unset release version/date
kattrali May 21, 2019
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
9 changes: 7 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
Changelog
=========

## 5.22.1 (2019-05-20)
## TBD

* Fix handled stacktrace generation
* Report correct app version in out-of-memory reports. Previously the bundle
version was reported as the version number rather than the short version
string.
[#349](https://github.com/bugsnag/bugsnag-cocoa/pull/349)

* Fix missing stacktraces in reports generated from `notify()`
[#348](https://github.com/bugsnag/bugsnag-cocoa/pull/348)

## 5.22.0 (2019-05-09)
Expand Down
10 changes: 8 additions & 2 deletions Source/BSGOutOfMemoryWatchdog.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ - (BOOL)computeDidOOMLastLaunchWithConfig:(BugsnagConfiguration *)config {
NSDictionary *lastBootInfo = [self readSentinelFile];
if (lastBootInfo != nil) {
self.lastBootCachedFileInfo = lastBootInfo;
NSString *lastBootBundleVersion =
[lastBootInfo valueForKeyPath:@"app.bundleVersion"];
NSString *lastBootAppVersion =
[lastBootInfo valueForKeyPath:@"app.version"];
NSString *lastBootOSVersion =
Expand All @@ -154,9 +156,12 @@ - (BOOL)computeDidOOMLastLaunchWithConfig:(BugsnagConfiguration *)config {
[[lastBootInfo valueForKeyPath:@"app.inForeground"] boolValue];
NSString *osVersion = [BSG_KSSystemInfo osBuildVersion];
NSDictionary *appInfo = [[NSBundle mainBundle] infoDictionary];
NSString *bundleVersion =
[appInfo valueForKey:@BSG_KSSystemField_BundleVersion];
NSString *appVersion =
[appInfo valueForKey:(__bridge NSString *)kCFBundleVersionKey];
[appInfo valueForKey:@BSG_KSSystemField_BundleShortVersion];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is being used to detect if the app has the same version and hence report the OOM, should this be on app version and bundle version rather than just app version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question. The safest thing is to check both, so I'll make it do that.

BOOL sameVersions = [lastBootOSVersion isEqualToString:osVersion] &&
[lastBootBundleVersion isEqualToString:bundleVersion] &&
[lastBootAppVersion isEqualToString:appVersion];
BOOL shouldReport = config.reportOOMs && (config.reportBackgroundOOMs || lastBootInForeground);
[self deleteSentinelFile];
Expand Down Expand Up @@ -214,7 +219,8 @@ - (NSMutableDictionary *)generateCacheInfoWithConfig:(BugsnagConfiguration *)con
app[@"id"] = systemInfo[@BSG_KSSystemField_BundleID] ?: @"";
app[@"name"] = systemInfo[@BSG_KSSystemField_BundleName] ?: @"";
app[@"releaseStage"] = config.releaseStage;
app[@"version"] = systemInfo[@BSG_KSSystemField_BundleVersion] ?: @"";
app[@"version"] = systemInfo[@BSG_KSSystemField_BundleShortVersion] ?: @"";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to add the bundle version in as well? Think it would something like
app[@"bundleVersion"] = systemInfo[@BSG_KSSystemField_BundleVersion] ?: @"";

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - having both makes sense 👍

app[@"bundleVersion"] = systemInfo[@BSG_KSSystemField_BundleVersion] ?: @"";
app[@"inForeground"] = @YES;
#if TARGET_OS_TV
app[@"type"] = @"tvOS";
Expand Down
1 change: 1 addition & 0 deletions Tests/BugsnagCrashReportTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ - (void)testNoReportMetaData {
- (void)testAppVersion {
NSDictionary *dictionary = [self.report toJson];
XCTAssertEqualObjects(@"1.0", dictionary[@"app"][@"version"]);
XCTAssertEqualObjects(@"1", dictionary[@"app"][@"bundleVersion"]);
}

- (void)testAppVersionOverride {
Expand Down
4 changes: 4 additions & 0 deletions features/oom.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Feature: Reporting out of memory events
And the event "severity" equals "error"
And the event "severityReason.type" equals "outOfMemory"
And the event "app.releaseStage" equals "beta"
And the event "app.version" equals "1.0.3"
And the event "app.bundleVersion" equals "5"
And the event breadcrumbs contain "Crumb left before crash"

Scenario: The OS kills the application in the background
Expand All @@ -39,6 +41,8 @@ Feature: Reporting out of memory events
And the event "severity" equals "error"
And the event "severityReason.type" equals "outOfMemory"
And the event "app.releaseStage" equals "beta"
And the event "app.version" equals "1.0.3"
And the event "app.bundleVersion" equals "5"
And the event breadcrumbs contain "Crumb left before crash"

Scenario: The OS kills the application after a session is sent
Expand Down