Update axum and http #1431

davidpdrsn · 2023-11-27T21:55:20Z

Updates to axum 0.7 and http 1.0.

Note that this is a breaking change because http is a public dependency of async-graphql.

Some integrations, such as warp, can't be updated yet because they don't support http 1.0. Might also get issues with actix-web which appears to use http 0.2 🤔

Fixes #1430

Updates to axum 0.7 and http 1.0. Note that this is a breaking change because http is a public dependency of async-await. Some integrations, such as warp, can't be updated yet because they don't support http 1.0.

ruizdiazever · 2023-11-29T10:37:41Z

@davidpdrsn great contribution, thanks!

IvanUkhov · 2023-11-30T19:57:34Z

integrations/axum/src/extract.rs

    S: Send + Sync,
    R: IntoResponse + From<ParseRequestError>,
 {
    type Rejection = R;

-    async fn from_request(req: Request<B>, state: &S) -> Result<Self, Self::Rejection> {
+    async fn from_request(req: Request, _state: &S) -> Result<Self, Self::Rejection> {


Not caught by the existing tests, but it fails for me:

102 | if let (&Method::GET, uri) = (req.method(), req.uri()) { | --- has type `&axum::http::Request<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

A solution is to check the method without binding:

if req.method() == Method::GET { let uri = req.uri(); …

Not sure how this is possible that it does not compile under my usage, while it does when running the test available in this repository 🤔 Perhaps there are no tests for this integration?

IvanUkhov · 2023-12-01T09:27:26Z

Continuing with my comment above, if you initialize the examples submodule and go to examples/axum/starwars and try to compile, you will get something like this:

   Compiling async-graphql-axum v6.0.11 (/Users/ivanukhov/Downloads/async-graphql/integrations/axum)
error: future cannot be sent between threads safely
   --> /Users/ivanukhov/Downloads/async-graphql/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 `Sync` is not implemented for `(dyn HttpBody<Error = axum::Error, Data = bytes::Bytes> + std::marker::Send + 'static)`
note: future is not `Send` as this value is used across an await
   --> /Users/ivanukhov/Downloads/async-graphql/integrations/axum/src/extract.rs:128:18
    |
102 |         if let (&Method::GET, uri) = (req.method(), req.uri()) {
    |                                       --- has type `&axum::http::Request<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
   --> /Users/ivanukhov/Downloads/async-graphql/integrations/axum/src/extract.rs:102:39
    |
102 |         if let (&Method::GET, uri) = (req.method(), req.uri()) {
    |                                       ^^^^^^^^^^^^
    = note: required for the cast from `Pin<Box<[async block@/Users/ivanukhov/Downloads/async-graphql/integrations/axum/src/extract.rs:101:86: 132:6]>>` to `Pin<Box<(dyn futures_util::Future<Output = Result<GraphQLBatchRequest<R>, R>> + std::marker::Send + 'async_trait)>>`

error: could not compile `async-graphql-axum` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...

davidpdrsn · 2023-12-01T10:20:44Z

Thanks for noticing! Wasn't aware of the submodule.

But before I fix that I'd like to know if updating now is even feasible given that it requires bumping http to 1.0 which is a public dependency of async-graphql. If that breaks the actix-web integration then that's quite unfortunate.

bkonkle · 2023-12-08T01:28:29Z

I'm seeing the same thing as @IvanUkhov. His suggested fix in the extract.rs solves it for me.

I do see the actix-web, poem, and warp integrations breaking because of the http 1.0 change, though.

Actix is targeting their http upgrade for v5.0 with this ticket.

I couldn't find any Poem or Warp activity around updating http yet.

It works for Axum though, so in the meantime I'll point to my Github forks. 😁

sunli829 · 2023-12-10T00:49:59Z

@davidpdrsn use cargo +nightly fmt 🙂

sunli829 · 2023-12-10T01:51:04Z

waiting on multer release for http 1.0.

toxeus · 2023-12-14T08:48:32Z

@sunli829 multer for http 1.0 has been released: https://crates.io/crates/multer/3.0.0

IgnisDa · 2023-12-17T23:44:51Z

@sunli829 When do you plan to make a new release?

sunli829 · 2024-01-06T03:59:47Z

released in v7.0.0

Update axum and http

9dbad9d

Updates to axum 0.7 and http 1.0. Note that this is a breaking change because http is a public dependency of async-await. Some integrations, such as warp, can't be updated yet because they don't support http 1.0.

IvanUkhov reviewed Nov 30, 2023

View reviewed changes

IvanUkhov mentioned this pull request Dec 1, 2023

Resolve thread safety in GraphQLBatchRequest::from_request davidpdrsn/async-graphql#1

Closed

ziimakc mentioned this pull request Dec 2, 2023

Support axum 0.7 #1434

Closed

sunli829 added 4 commits December 10, 2023 09:22

add http02 feature

715ec14

rustfmt

2e7dca2

update integrations

65a810d

Update examples

62f1a65

update examples

a666be8

sunli829 merged commit 4218162 into async-graphql:master Dec 16, 2023
0 of 8 checks passed

aotimme mentioned this pull request Jan 9, 2024

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update axum and http #1431

Update axum and http #1431

davidpdrsn commented Nov 27, 2023 •

edited

ruizdiazever commented Nov 29, 2023

IvanUkhov Nov 30, 2023 •

edited

IvanUkhov commented Dec 1, 2023 •

edited

davidpdrsn commented Dec 1, 2023

bkonkle commented Dec 8, 2023

sunli829 commented Dec 10, 2023

sunli829 commented Dec 10, 2023

toxeus commented Dec 14, 2023

IgnisDa commented Dec 17, 2023

sunli829 commented Jan 6, 2024

Update axum and http #1431

Update axum and http #1431

Conversation

davidpdrsn commented Nov 27, 2023 • edited

ruizdiazever commented Nov 29, 2023

IvanUkhov Nov 30, 2023 • edited

Choose a reason for hiding this comment

IvanUkhov commented Dec 1, 2023 • edited

davidpdrsn commented Dec 1, 2023

bkonkle commented Dec 8, 2023

sunli829 commented Dec 10, 2023

sunli829 commented Dec 10, 2023

toxeus commented Dec 14, 2023

IgnisDa commented Dec 17, 2023

sunli829 commented Jan 6, 2024

davidpdrsn commented Nov 27, 2023 •

edited

IvanUkhov Nov 30, 2023 •

edited

IvanUkhov commented Dec 1, 2023 •

edited