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-2016] Fix Swift fatal error message reporting #948

Merged
merged 9 commits into from
Jan 5, 2021

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Dec 18, 2020

Goal

Improve the reliability of capturing Swift fatal error messages passed to the following functions:

  • assertionFailure()
  • fatalError()
  • preconditionFailure()

It also covers implicit messages emitted by the Swift standard library, e.g.

Unexpectedly found nil while unwrapping an Optional value

As a bonus, the same mechanism also captures error messages from other libraries such as libdispatch, e.g.

BUG IN CLIENT OF LIBDISPATCH: dispatch_sync called on queue already owned by current thread

Design

The implementation examines the binary image of the crashing thread's top stack frame for a message in its __crash_info section, and parses this to extract the error type and message.

The full, raw value of the __crash_info message is also made available in the event metadata, which may be useful when diagnosing certain types of crash.

Screenshot 2020-12-17 at 16 28 39

Changeset

  • Updated KSCrash to capture the __crash_info message and store it in the KSCrashReport.
  • Updated BugsnagError and BugsnagEvent to read the message stored in the KSCrashReport, parse it, and update the error & event payload.
  • Removed code for old mechanism (RegisterErrorData)
  • Disabled KSCrash's memory introspection option.

Testing

  • Manually verified with example app using iPhone Simulator and real devices, including 32-bit CPUs.
  • Added new unit test case to validate parsing of crash info message.
  • Updated E2E scenarios to validate that Swift error messages are reported correctly.
  • Added new E2E scenario for libdispatch crash.

Bugsnag/Payload/BugsnagError.m Outdated Show resolved Hide resolved
@sethfri
Copy link

sethfri commented Dec 21, 2020

This is so awesome! Thanks for your work on this @nickdowell. Since this covers force unwrapping optionals, do you expect this to also capture the Error thrown from a try!, described in #950?

@nickdowell
Copy link
Contributor Author

nickdowell commented Dec 21, 2020

@sethfri I am not sure about the errors thrown from a try! - will be investigating 🤓

@nickdowell
Copy link
Contributor Author

@sethfri quick update: a description of the Error is included in the message when try! causes a crash 😄 Example:

Screenshot 2020-12-21 at 11 59 53

Bugsnag/BugsnagCrashSentry.m Show resolved Hide resolved
Bugsnag/Payload/BugsnagError.m Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jan 5, 2021

Bugsnag.framework binary size

iOS arm64
Before 1,055,544 bytes
After 1,053,864 bytes
Diff -1680

Generated by 🚫 Danger

@kstenerud kstenerud self-requested a review January 5, 2021 15:20
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.

4 participants