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

Bump to Cocoa library to 6.4.1 #1197

Merged
merged 1 commit into from
Dec 14, 2020
Merged

Bump to Cocoa library to 6.4.1 #1197

merged 1 commit into from
Dec 14, 2020

Conversation

kstenerud
Copy link
Contributor

@kstenerud kstenerud commented Dec 14, 2020

Changes:

  • Ran update.ios script to import cocoa
  • Modified BugsnagEventDeserializer.m to update for changed API

return [[BugsnagHandledState alloc] initWithSeverityReason:reason
severity:severity
unhandled:unhandled
unhandledOverridden:unhandledOverridden
Copy link
Contributor Author

@kstenerud kstenerud Dec 14, 2020

Choose a reason for hiding this comment

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

Changes to cocoa internal API required these fixes RN-side (BugsnagEventDeserializer).

@kstenerud kstenerud marked this pull request as ready for review December 14, 2020 13:45
Copy link
Contributor

@bengourley bengourley left a comment

Choose a reason for hiding this comment

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

Approving subject to satisfactory answers to my queries inline…

@@ -156,9 +157,11 @@ - (BugsnagHandledState *)deserializeHandledState:(NSDictionary *)payload {

BSGSeverity severity = BSGParseSeverity(payload[@"severity"]);
BOOL unhandled = [payload[@"unhandled"] boolValue];
BOOL unhandledOverridden = [severityReason[@"unhandledOverridden"] boolValue];
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the behaviour of this when

a. severityReason is nil?
b. severityReason.unhandledOverridden doesn't exist, or isn't a boolean?

Copy link
Contributor Author

@kstenerud kstenerud Dec 14, 2020

Choose a reason for hiding this comment

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

Messages to nil objects in ObjC return 0, which is converted to nil, and then the subcall (also to a nil object) is converted to boolean false in this case.
If unhandledOverridden is a string, number, boolean, or nil, it will return Objective-C's interpretation of that type as a boolean value. If it's any other type, it will throw an exception.

@bugsnagbot
Copy link
Collaborator

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 40.70 kB 12.54 kB
After 40.70 kB 12.54 kB
± No change No change

Generated by 🚫 dangerJS against ff106c0

Copy link
Contributor

@fractalwrench fractalwrench left a comment

Choose a reason for hiding this comment

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

These changes LGTM. There needs to be a separate PR that adapts the existing React Native E2E tests before we can release the unhandled override work.

@kstenerud kstenerud merged commit 0f803f9 into next Dec 14, 2020
@nickdowell nickdowell deleted the cocoa-6.4.1 branch March 11, 2021 10:36
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