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.

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

[PLAT-6632] Fix KSCrash state storage for apps with no CFBundleName #1103

Merged
merged 1 commit into from
Jun 2, 2021

Conversation

nickdowell
Copy link
Contributor

Goal

Some apps (e.g. macOS command line tools and background daemons) may not have an Info.plist or CFBundleName, and this was causing the KSCrash state persistence to fail, resulting in failing to detect that the last run ended with a crash.

Changeset

When constructing the stateFilePath, the notifier now falls back to the process's name if it the app has no bundle name.

The bundle name is also no longer included in the KSCrashReport file names.

Testing

Reproduced the reported issue using the macOS stress test app and verified the fix.

E2E tests ensure that crash reporting continues to work as expected.

@github-actions
Copy link

Infer: No issues found 🎉

OCLint: No issues found 🎉

Bugsnag.framework binary size decreased by 448 bytes from 1,126,712 to 1,126,264 🎉

Generated by 🚫 Danger

@kattrali
Copy link
Contributor

kattrali commented Jun 2, 2021

What does this change mean for people who upgrade and have existing data in the "{bundle name}" directory?

Copy link
Contributor

@kstenerud kstenerud left a comment

Choose a reason for hiding this comment

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

Will this handle migration from the old filenames to the new ones?

@nickdowell
Copy link
Contributor Author

The KSCrash state file path remains KSCrashReports/${bundleName}-CrashState.json for apps with a bundle name.

Apps without a bundle name were never able to save a state file due to the invalid generated path, so there's nothing to migrate. They will now use KSCrashReports/${processName}-CrashState.json

The other change contained here is to strip the bundle name from the name of each KSCrashReport file. This does not require migration because BSGEventUploader will pick up any json file that does not end with -CrashState.json.

@kstenerud kstenerud self-requested a review June 2, 2021 12:50
@nickdowell nickdowell merged commit 91e54ec into next Jun 2, 2021
@nickdowell nickdowell deleted the nickdowell/fix-state-file-path branch June 2, 2021 13:51
@nickdowell nickdowell mentioned this pull request Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants