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-7417] Add new APIs for removing Swift callbacks #1240

Merged
merged 2 commits into from
Nov 29, 2021

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Nov 26, 2021

Goal

Make it possible to remove Swift OnBreadcrumb, OnSendError and OnSession closures.

Design

When a Swift closure is passed to addOnBreadcrumb(block:) or removeOnBreadcrumb(block:) the compiler creates an NSBlock which "wraps" the closure and makes it callable from Objective-C. When Bugsnag is asked to remove a previously added callback, testing the objects for equality always fails and the original block cannot be identified in the array.

This PR makes it possible to remove closures by passing the object from addOnBreadcrumb(block:) to a new removeOnBreadcrumb(_:) function. For simplicity of implementation, addOnBreadcrumb(block:) returns the (copy of the) block that was added to the array.

The old removeOnBreadcrumb(block:) function has been marked as deprecated.

The new removeOnBreadcrumb(_:) function method accepts an AnyObject to cause a compile error if misused by passing it a closure.

Note: all this also applies to OnSendError and OnSession.

Testing

Existing unit tests amended to verify new APIs, new test cases added to verify passing invalid values to the remove: APIs.

@github-actions
Copy link

Infer: No issues found 🎉

OCLint: No issues found 🎉

Bugsnag.framework binary size increased by 18,120 bytes from 1,256,344 to 1,274,464

Generated by 🚫 Danger

@nickdowell nickdowell marked this pull request as ready for review November 26, 2021 11:22
Bugsnag/include/Bugsnag/Bugsnag.h Outdated Show resolved Hide resolved
Bugsnag/include/Bugsnag/Bugsnag.h Outdated Show resolved Hide resolved
Bugsnag/include/Bugsnag/BugsnagClient.h Outdated Show resolved Hide resolved
Bugsnag/include/Bugsnag/BugsnagClient.h Outdated Show resolved Hide resolved
Bugsnag/include/Bugsnag/BugsnagConfiguration.h Outdated Show resolved Hide resolved
Bugsnag/include/Bugsnag/BugsnagConfiguration.h Outdated Show resolved Hide resolved
Bugsnag/include/Bugsnag/BugsnagConfiguration.h Outdated Show resolved Hide resolved
@nickdowell nickdowell merged commit 6288392 into next Nov 29, 2021
@nickdowell nickdowell deleted the nickdowell/swift-callback-removal branch November 29, 2021 13:09
@nickdowell nickdowell mentioned this pull request Dec 1, 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