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

Address axum integration compilation error with non-Sync body #1460

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

aotimme
Copy link
Contributor

@aotimme aotimme commented Jan 9, 2024

This fixes an error I'm seeing when compiling a project that uses async-graphql-axum with axum 0.7. The issue and fix was brought up in #1431 (review) but not implemented.

Without this change, we get the following when running cargo clippy --package async-graphql-axum:

    Checking async-graphql-axum v7.0.0 (/home/alden/async-graphql/integrations/axum)
error: future cannot be sent between threads safely
   --> integrations/axum/src/extract.rs:101:86
    |
101 |       async fn from_request(req: Request, _state: &S) -> Result<Self, Self::Rejection> {
    |  ______________________________________________________________________________________^
102 | |         if let (&Method::GET, uri) = (req.method(), req.uri()) {
103 | |             let res = async_graphql::http::parse_query_string(uri.query().unwrap_or_default())
104 | |                 .map_err(|err| {
...   |
131 | |         }
132 | |     }
    | |_____^ future created by async block is not `Send`
    |
    = help: the trait `std::marker::Sync` is not implemented for `(dyn axum::body::HttpBody<Data = bytes::Bytes, Error = axum::Error> + std::marker::Send + 'static)`
note: future is not `Send` as this value is used across an await
   --> integrations/axum/src/extract.rs:128:18
    |
102 |         if let (&Method::GET, uri) = (req.method(), req.uri()) {
    |                                       --- has type `&axum::http::Request<axum::body::Body>` which is not `Send`
...
128 |                 .await?,
    |                  ^^^^^ await occurs here, with `req` maybe used later
...
132 |     }
    |     - `req` is later dropped here
help: consider moving this into a `let` binding to create a shorter lived borrow
   --> integrations/axum/src/extract.rs:102:39
    |
102 |         if let (&Method::GET, uri) = (req.method(), req.uri()) {
    |                                       ^^^^^^^^^^^^
    = note: required for the cast from `std::pin::Pin<std::boxed::Box<[async block@integrations/axum/src/extract.rs:101:86: 132:6]>>` to `std::pin::Pin<std::boxed::Box<(dyn futures_util::Future<Output = std::result::Result<extract::GraphQLBatchRequest<R>, R>> + std::marker::Send + 'async_trait)>>`

error: could not compile `async-graphql-axum` (lib) due to previous error

After the change, this is no longer a problem.

Separately, I'm not entirely sure why it is not being flagged in CI.

@aotimme aotimme changed the title Address axum integration compilation error with non-Send body Address axum integration compilation error with non-Sync body Jan 10, 2024
@Miaxos Miaxos self-requested a review January 16, 2024 10:08
@aotimme
Copy link
Contributor Author

aotimme commented Jan 19, 2024

Hey @Miaxos any chance this could get merged and published in a 7.0.1 version?

@sunli829 sunli829 merged commit 9a8b804 into async-graphql:master Jan 21, 2024
7 checks passed
@sunli829
Copy link
Collaborator

released in 7.0.1

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.

None yet

3 participants