-
Notifications
You must be signed in to change notification settings - Fork 3
Add cancellation support to the Swift wrapper #894
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
Conversation
b3cda73
to
9aa05cf
Compare
4b3c1a7
to
55eb06e
Compare
When exporting both, the native binding code would be so large that they can't be loaded by kotlin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very impressive – in my testing, it worked perfectly.
I wouldn't mind @oguzkocer taking a look as well, as it touches a lot of non-Swift bits.
Is this something that would be upstream-able? It seems like it generalizes nicely to anything that's async with Rust?
I will leave this PR open and wait for Oguz's review. The solution leans heavily on the native side: the cancellation happens in the native wrapper. The Rust side does not do much about cancellation, other than passing the |
01bcb91
to
5ccc5df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be accidentally checked in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ops, removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazytonyli Thank you for working on this and the extra iteration. On the Rust side, I think it hits the right balance in providing the necessary tools without being overly intrusive. Let's land this as is and maybe re-visit it if our needs change or if we gain any more context to find a slightly different solution.
wp_api/src/cancellation.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably move this to the request
module and/or maybe rename it, but doesn't seem super important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. The filename no longer matches the code.
Relates to #824.
This PR adds cancellation support to the Swift wrapper. The same API (for example,
api.posts.create(...)
), which was not cancellable, can now be cancelled.To verify this change, you can checkout the first commit (Add CancellationTests), run
make xcframework-only-macos
, then run theCancellationTests
from Xcode. The test should fail. Checkout the last commit, repeat the same steps, the same tests should pass.Goal
Use this Swift code as an example:
When you cancel the task that is creating a new post and the POST HTTP request is still going, we expect the HTTP request to be cancelled, and the following
viewPostDetails
call should not be called.Putting this scenario into the mobile app's context, you can think of it as cancelling the publishing of a new post, replying to a comment, uploading an image, etc. When user taps a cancel button to cancel those pending jobs, we should try our best to abort the underlying HTTP request to prevent the content reaching the server. Obviously, we can't 100% guarantee that, because the request may have already reached the server when user taps the cancel button, and we can't do much from client side about that.
Difficulties
There are several difficulties involved in supporting the native Swift task cancellation, when the task calls Rust's async functions underneath. Because both Swift's Task and Rust's Future API are involved, and they obviously work quite differently.
As I mentioned in #824, uniffi-rs does not have built-in cancellation support. We'd need to implement the mechanism ourselves.
Approach
First, Rust API needs to expose a
cancellation_token: CancellationToken
. This PR modified the derive macro to add the parameter to all endpoint functions.The generated code now looks like this:
This change does not break the endpoint APIs. Their function signature stays the same, and they cannot be cancelled as before. The Rust code acts as a facilitator, which accepts a
CancellationToken
instance and passes it to the native implementation ofRequestExecutor
, so that the request executor can cancel HTTP requests when cancelled.The
RequestExecutor
API is changed to support that. Both existing functions now accept acancellation_token: CancellationToken?
parameter. The native implementation can useCancellationToken.register_handler
function to get a callback whenCancellationToken
gets cancelled. The implementation here is isolated within the nativeRequestExecutor
, which makes the actual HTTP request cancellation relatively straightforward.Finally, in order to provide a cancellable version of
list_with_view_context
, and keep the source code backward compatible, I have introduced a feature to prevent thelist_with_view_context
from being exported as a uniffi function. The native wrapper can then generate thelist_with_view_context
function according to the cancellable version:list_with_view_context_cancellation
. It looks like this in Swift:Limitations
The cancellation support is opt-in. As in, the Rust functions need to accept a
CancellationToken
parameter, and make sure to pass the instance down to its call chain.The macro change should cover the majority of the HTTP requests sent to
RequestExecutor
. But there is still code that does not support cancellation. For example, the API discovery, which we can look into later if needed.