TrueLayer / reqwest-middleware

Wrapper around reqwest to allow for client middleware chains.
Apache License 2.0
257 stars 78 forks source link

Can the `async_trait` macro be removed on the `Middleware` trait? #174

Closed avsaase closed 2 months ago

avsaase commented 2 months ago

Motivations

Rust 1.75 stabilized "return position impl Trait in traits" (RPITIT) which allows using async functions in traits. This removes the need for the async_trait proc-macro, both in the library and in the projects that use it, and thus removes a level of dynamic dispatch.

There is still a limitation that the library user cannot add their own bounds on the returned future but the rust project provided a macro to optionally add additional bounds like Send. The benefit is that this macro only needs to be used to by the library and not the library consumers.

Solution

Adopt async function in traits.

Alternatives

Additional context

https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html

eopb commented 2 months ago

Hi @avsaase

From the article you linked

Traits that use -> impl Trait and async fn are not object-safe, which means they lack support for dynamic dispatch. We plan to provide utilities that enable dynamic dispatch in an upcoming version of the trait-variant crate.

https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html#dynamic-dispatch

request-middleware relies heavily on dynamic dispatch internally. Unfortunately this means that RPITIT isn't going to be an option for us yet.

For now, I'm going to close this issue. If you have any other thoughts or feel it should be reopened, feel welcome to still leave any comments.