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

(feat) Implement configurable enabledErrorTypes #477

Conversation

robinmacharg
Copy link
Contributor

Goal

To allow users to configure the types of event that are reported.

Design

This is implemented as a bitfield on BugsnagConfiguration. BugsnagSentry translates this to equivalent KSCrash values. The OOM value is examined in BugsnagNotifier. The bitfield mapping has been broken out to make it testable.

Changeset

As above, BugsnagConfiguration, BugsnagSentry and BugsnagNotifier.

Tests

This is difficult to test at unit-level. What can be has been. E2E tests better test the behaviour and this is where the effort has been concentrated.

Review

Outstanding Questions

  • E2E Testing is extensive but I have lingering doubts over whether it's comprehensive. There's ambiguous behaviour around e.g. Mach errors being converted to Signals.
  • This pull request is ready for:
    • Initial review of the intended approach, not yet feature complete
    • Structural review of the classes, functions, and properties modified
    • Final review
  • The correct target branch has been selected (master for fixes, next for
    features)
  • Consistency across platforms for structures or concepts added or modified
  • Consistency between the changeset and the goal stated above
  • Internal consistency with the rest of the library - is there any overlap between existing interfaces and any which have been added?
  • Usage friction - is the proposed change in usage cumbersome or complicated?
  • Performance and complexity - are there any cases of unexpected O(n^3) when iterating, recursing, flat mapping, etc?
  • Concurrency concerns - if components are accessed asynchronously, what issues will arise
  • Thoroughness of added tests and any missing edge cases
  • Idiomatic use of the language

@robinmacharg robinmacharg force-pushed the robinmacharg/Move-Configuration.reportOOMs-into-Configuration.enabledErrorTypes-option branch from 43fc0a6 to b16b9c8 Compare February 28, 2020 14:22
@robinmacharg robinmacharg marked this pull request as ready for review February 28, 2020 14:49
@robinmacharg
Copy link
Contributor Author

Note to self: I need to check e2e on Mac vs iOS.

Source/BugsnagConfiguration.m Outdated Show resolved Hide resolved
Source/BugsnagNotifier.m Outdated Show resolved Hide resolved
UPGRADING.md Show resolved Hide resolved
Source/BugsnagConfiguration.m Outdated Show resolved Hide resolved
…ration.enabledErrorTypes-option' of github.com:bugsnag/bugsnag-cocoa into robinmacharg/Move-Configuration.reportOOMs-into-Configuration.enabledErrorTypes-option
@robinmacharg robinmacharg force-pushed the robinmacharg/Move-Configuration.reportOOMs-into-Configuration.enabledErrorTypes-option branch from b16585c to d5838c0 Compare March 2, 2020 11:23
@robinmacharg robinmacharg force-pushed the robinmacharg/Move-Configuration.reportOOMs-into-Configuration.enabledErrorTypes-option branch from d5838c0 to 9d1cf03 Compare March 2, 2020 13:48
@robinmacharg robinmacharg merged commit c7ae943 into spec-compliance Mar 5, 2020
@fractalwrench fractalwrench deleted the robinmacharg/Move-Configuration.reportOOMs-into-Configuration.enabledErrorTypes-option branch May 13, 2020 13:57
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.

2 participants