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

axum example fails to buid on axum 0.6 #104

Closed
Tracked by #106
dlight opened this issue Jan 7, 2023 · 7 comments
Closed
Tracked by #106

axum example fails to buid on axum 0.6 #104

dlight opened this issue Jan 7, 2023 · 7 comments

Comments

@dlight
Copy link

dlight commented Jan 7, 2023

It errors because it uses axum::RequestParts from axum 0.5 (which doesn't exist on 0.6) and also because it doesn't account for the FromRequest change (it added a new parameter). The release notes say why those traits were reworked

@dlight
Copy link
Author

dlight commented Jan 7, 2023

But I'm not sure if this should be fixed in this crate, or in the httpz crate, or both.

@oscartbeaumont
Copy link
Member

oscartbeaumont commented Jan 8, 2023

I have started work on this but it's extremely changing to find a good solution for it. This will primarily need to be fixed in httpz with a few changes to the Axum extractor system in rspc (which was already due for a refactor).

The challenge it that with the new Axum update you must pass ’S‘ (your Axum state type) to each extractor. This is annoying because httpz has no concept of your Axum state type within the handler function because the handler is defined before you chose the httpz Axum integration.

I still haven't thought it through but I may just drop official support for Axum extractors but leave in a non-typesafe escape hatch for people who really need them. This will loose us the Axum extractors ecosystem but I also am planning to support other web servers very soon and having web server specific extractors makes for a confusing DX.

@dlight
Copy link
Author

dlight commented Jan 8, 2023

Oh.. I'm still learning axum, so this is over my head. Do you mean that the new axum API is different enough that you can't write, with httpz, code that is generic between axum and other frameworks? Then maybe this point could be raised within the axum community, and maybe axum 0.7 becomes a little more tractable

But I think that instead of dropping axum support it's okay to make a few releases supporting axum 0.5, it's still useful.

@oscartbeaumont oscartbeaumont mentioned this issue Jan 8, 2023
51 tasks
@oscartbeaumont
Copy link
Member

I am definitely not dropping support for Axum (it is my webserver of choice) but I am highly considering dropping support for Axum extractors. I have been contemplating removing this feature for quite some time (it's mentioned in a previous release's notes) but what has held me back is being able to fallback on the Axum ecosystem for features I haven't got an official solution for. For example, the official example of using rspc with cookies recommended tower-cookies which uses the Axum extractor API. I will need to make my own official cookie API for httpz (the groundwork already in place so this should be easy). Removing this feature was probably inevitable as I support more than just Axum, but the upstream changes by Axum in the 0.6 update are just fast-forwarding that change.

I don't think Axum have done anything wrong with the changes in 0.6. It makes Axum way better but it just makes it really hard for httpz to work with it in a way that is not prone to user error.

In practical terms, I would drop support for this.

use axum::extract::Path;
use tower_cookies::{Cookie, CookieManagerLayer, Cookies};

 let app = axum::Router::new()
      .route(
          "/rspc/:id",
          router
              .clone()
              // Both `Path` and `Cookies` are Axum extractors and support will be dropped.
               .endpoint(|path: Path<String>, cookies: Cookies| { 
                    println!("Client requested operation '{}'", *path);
                    Ctx { cookies }
                })
              .axum(),
      );

and instead, support this (although API is subject to change because I haven't built it yet):

use rspc::httpz::{Request, CookieJar};

 let app = axum::Router::new()
      .route(
          "/rspc/:id",
          router
              .clone()
               // Using `httpz` alternatives to the Axum extractors above
               .endpoint(|req: Request, cookies: CookieJar| {
                    println!("Client requested operation '{}'", req.uri().path());
                    Ctx { cookies }
                })
              .axum(),
      );

@IgnisDa
Copy link

IgnisDa commented Jan 14, 2023

@oscartbeaumont Any updates on this? Is there any way to get cookies into the Ctx using the current API.

use axum::extract::Path;
use tower_cookies::{Cookie, CookieManagerLayer, Cookies};

 let app = axum::Router::new()
      .route(
          "/rspc/:id",
          router
              .clone()
               .endpoint(|path: Path<String>, cookies: Cookies| { 
                    println!("Client requested operation '{}'", *path);
                    Ctx { cookies }
                })
              .axum(),
      );

This does not work.

@oscartbeaumont
Copy link
Member

It looks like tower-cookies upgraded to Axum 0.6 so the latest version isn't compatible. @VarunPotti said he had luck with versions rspc v0.1.1, tower-cookies v0.7.0, axum v0.5.16.

@IgnisDa
Copy link

IgnisDa commented Jan 14, 2023

@oscartbeaumont Thank you, that works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants