Add onSendBlock methods

[robinmacharg]

Goal

Add missing add/remove/clearOnSendBlock() methods to Bugsnag and BugsnagConfiguration. These are used in this notifier in lieu of equivalent onError methods in others due to the tight requirements of code run in error handlers (e.g. no ObjC, async etc).

Design

Straightforward exposure of access to the BugsnagConfiguration onSendBlocks array.

Changeset

• Bugsnag, BugsnagConfiguration and associated tests.

• Some small non-functional rearrangement refactoring of headers has also been done.

Tests

Unit tests. Not tested prior to start being called.

Review

Outstanding Questions

• 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

[fractalwrench]

Left a few minor comments inline. The main question I had is whether this functionality is defined anywhere in the notifier spec? I had a quick browse and couldn't find anything, so if it's not defined it'd be good to fully document what the behaviour of OnSend is before considering merging.

[robinmacharg]

is this functionality defined anywhere in the notifier spec?

No, it's a divergence. Due to the limitations of the environment during a crash (no async, or ObjC etc) we can't implement onError like the other notifiers. onSend is the best we can do. There is a separate task to document the divergence.

[fractalwrench]

There is a separate task to document the divergence.

It would be preferable if the spec was up-to-date before we consider merging this PR.

Without a written specification that has been agreed upon, we run the risk of having to rework this interface or even scrap it entirely in favour of another solution. Documenting and scrutinising the requirements down beforehand reduces this risk.

[tomlongridge]

Without a written specification that has been agreed upon, we run the risk of having to rework this interface or even scrap it entirely in favour of another solution. Documenting and scrutinising the requirements down beforehand reduces this risk.

I've raised an issue, but I'd seen this more of a rename to get it more like the spec until such time as we have resolved whether the concept of OnSend is a Cocoa deviation or something we'll introduce as a broader concept.

I'm happy to merge this once we have the add and remove methods (with no rawEventData parameter) but I'll kick off discussion on the issue to make sure it's covered before we release.

[tomlongridge]

Now raised separately in the Spec to discuss naming, but I'm happy to merge this and raise additional work once that is resolved rather than hold off on this.