-
Notifications
You must be signed in to change notification settings - Fork 7
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
Make Service async fn-compatible #176
Conversation
Generate changelog in
|
|
||
fn call(&self, req: R) -> Self::Future { | ||
fn call(&self, req: R) -> impl Future<Output = Result<T::Response, T::Error>> { |
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.
Why can't we do
async fn call(&self), req: R) -> Result<T::Response, T::Error>
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 could, but that would add an extra layer of indirection - it would end up making a wrapper future that looked something like
enum WrapperFuture<F> {
Init,
Awaiting(F),
}
|
||
return Poll::Ready(Err(error)); | ||
if log_body { |
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.
nit: I think we can avoid the mut
on error
let error = if log_body { error.with_unsafe_param(....) } else { error }
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.
That's true, but IMO it's a bit more confusing.
Before this PR
The Service trait required that the
call
method's future be nameable and not borrow anything fromself
. This required lots of Arc cloning and manual future implementations.After this PR
==COMMIT_MSG==
The
Service
trait now supportsasync fn call
.==COMMIT_MSG==
Using the new support added in Rust 1.75 (https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html), we can change
Service
's signature to allow thecall
method to be written as a normalasync fn
.This significantly simplifies the internal service layers. Manual future implementations are now reserved for low level operations that can't be expressed with async/await.
Possible downsides?
Due to limitations of the initial implementation in the compiler, we currently require that the future returned by
call
isSend
. In practice this isn't really a limitation since the future needs to beSend
to work in a multithreaded runtime.