(Beta version) : reference to client causes error with axum

[nicolaspernoud]

Hello,

When using oauth_client.exchange_code(AuthorizationCode::new(query.code.clone())) .request_async(&client) in an axum handler, I get the following error :

• the trait std::marker::Send is not implemented for dyn Future<Output = Result<StandardTokenResponse<EmptyExtraTokenFields, BasicTokenType>, ...>>
• future is not Send as it awaits another future which is not Send I use a custom client but it is implemented like the Reqwest one is. It would seem that taking a reference to the client (which is cheap to clone anyway and should be cloned and moved is the problem).

[ramosbugs]

Hey @nicolaspernoud,

Was this working in 4.x and isn't in the 5.x alpha, or is this net new code? Something similar came up in #255, but as far as I can tell this should have been an issue in 4.x too since reqwest's client returns a non-Send future: https://docs.rs/reqwest/latest/reqwest/struct.Client.html#method.execute

It would seem that taking a reference to the client (which is cheap to clone anyway and should be cloned and moved is the problem).

I'm not sure what you mean here. I think the issue is related to the AsyncHttpClient returning a non-Send future, not passing a reference vs. an owned client (not all of which may be cheap to clone).

Is there something about Axum that requires the async code to be Send?

[LorenzoLeonardo]

Are you usng #[async_trait]? You try putting ?Send like this -> #[async_trait(?Send)]

[ramosbugs]

I've been digging a bit more, and somehow the future returned by client.exchange_code(code).request_async(oauth2::reqwest::async_http_client) in 4.4.2 is in fact Send despite reqwest::Client::execute's return type not explicitly listing it in its trait bounds 🤔

I'm working on adding Send and Sync bounds to all of the futures. if that compiles, I'll go ahead and merge it, which seems worth it even though it will require all custom HTTP clients to also return Send + Sync futures.

[LorenzoLeonardo]

Are you usng #async_trait? You try putting #async_trait(?Send)

@ramosbugs so I can now use again the #[async_trait] the one without the ?Send?

[ramosbugs]

Alright, 6e583bd03203e42ef712fc90edb57cf5a389f9b7 should fix the issue. I think what happened is that impl Future and async fn are smart enough to infer whether the future is Send while dyn Future requires it to be specified explicitly. We can't always require Send because the reqwest WASM client is not Send, while in non-WASM builds it is Send (with hyper >= 0.14.14). The new interface should support both and return Send futures opportunistically.

Thank you both for helping to test the recent changes! If you wouldn't mind, please let me know if this fixes your issues. I'm still waiting for reqwest to cut a 0.12 release before cutting the next oauth2 release, which will hopefully be soon.

[ramosbugs]

@ramosbugs so I can now use again the #[async_trait] the one without the ?Send?

should be able to!

[nicolaspernoud]

I confirm that solved the problem ! Thanks a lot for solving this problem and for your amazing work on this crate !

[nicolaspernoud]

Hello, @ramosbugs just for information, do we know when the 5.0 stable is scheduled to be released ?

[ramosbugs]

Hello, @ramosbugs just for information, do we know when the 5.0 stable is scheduled to be released ?

Given the significant breaking changes that are part of this release, I'd like to wait at least another couple of months to see if additional issues like this one get filed. The latest alpha release should be safe to use in the meantime; it's just not guaranteed to be API-stable.

[nicolaspernoud]

Hello, thanks a lot for the insight, and duly noted for that the alpha is usable in the meantime. Thanks again for this very useful library.