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

Document that/why returning a non-Result when handler is async is invalid #96

Open
WasabiFan opened this issue Aug 13, 2021 · 2 comments

Comments

@WasabiFan
Copy link

TL;DR: It seems like async route handlers must return a Result. The futures example returns a Result<impl rweb::Reply, Infallible>, suggesting this is known and intentional. Why is this, and can it be documented somewhere?


I see in various examples that a route handler can return an existential type impl Reply. One such example is sse_chat.rs. It compiles fine as-is.

If I make one such handler async, as follows:

diff --git a/examples/sse_chat.rs b/examples/sse_chat.rs
index 6dc28d9..d7c1424 100644
--- a/examples/sse_chat.rs
+++ b/examples/sse_chat.rs
@@ -54,7 +54,7 @@ fn recv_chat(#[data] users: Users) -> impl Reply {
 }
 
 #[get("/")]
-fn index() -> impl Reply {
+async fn index() -> impl Reply {
     rweb::http::Response::builder()
         .header("content-type", "text/html; charset=utf-8")
         .body(INDEX_HTML)

I get two errors:

Click to expand
error[E0271]: type mismatch resolving `<impl futures::Future as futures::Future>::Output == Result<_, _>`
  --> examples/sse_chat.rs:56:1
   |
56 | #[get("/")]
   | ^^^^^^^^^^^ expected opaque type, found enum `Result`
57 | async fn index() -> impl Reply {
   |                     ---------- the expected opaque type
   |
   = note: expected opaque type `impl Reply`
                     found enum `Result<_, _>`
   = note: required because of the requirements on the impl of `TryFuture` for `impl futures::Future`
   = note: this error originates in the attribute macro `get` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0271]: type mismatch resolving `<impl futures::Future as futures::Future>::Output == Result<_, _>`
  --> examples/sse_chat.rs:56:1
   |
56 | #[get("/")]
   | ^^^^^^^^^^^ expected enum `Result`, found opaque type
57 | async fn index() -> impl Reply {
   |                     ---------- the found opaque type
   |
   = note:     expected enum `Result<_, _>`
           found opaque type `impl Reply`
   = note: required because of the requirements on the impl of `TryFuture` for `impl futures::Future`

In short, there is now an extra bound applied somewhere that the returned value must be a Result with certain properties.

I've run both versions through cargo-expand, and see the following diff when adding async:

--- non_async.rs        2021-08-12 16:50:31.818319127 -0700
+++ with_async.rs       2021-08-12 16:50:41.918437102 -0700
@@ -78,16 +78,17 @@
         .map(recv_chat)
 }
 fn index(
-) -> impl rweb::Filter<Extract = (impl Reply,), Error = rweb::warp::Rejection> + rweb::rt::Clone {
+) -> impl rweb::Filter<Extract = (impl rweb::Reply,), Error = rweb::warp::Rejection> + rweb::rt::Clone
+{
     use rweb::Filter;
-    fn index() -> impl Reply {
+    async fn index() -> impl Reply {
         rweb::http::Response::builder()
             .header("content-type", "text/html; charset=utf-8")
             .body(INDEX_HTML)
     }
     rweb::filters::method::get()
         .and(rweb::filters::path::end())
-        .map(index)
+        .and_then(index)
 }
 fn main() {
     tokio::runtime::Builder::new_multi_thread()

Indeed, and_then requires a Future returning a Result, from a glance at the docs.

I haven't dug deeply into this. However, I can confirm that the "futures" example returns a Result with Infallible error, so I'm inclined to believe that this is fully intentional/understood.

What's the reason behind this? I guess map doesn't support async filters, and_then it the only alternative, and the route macros don't currently have support for introducing the Result wrapper? Is there somewhere this could be documented?

I figure this issue can stand in as documentation in the short term, which is why I'm opening it with context.

@WasabiFan
Copy link
Author

I suppose I also should make the last part an explicit suggestion -- there are a few other improvements that could potentially be made here, with varying amounts of effort:

  • Explicitly error when the return type of an async handler isn't a Result
  • Support implicitly wrapping the return type in a Result<_, Infallible> and later unwrap()ing it if needed

@kdy1
Copy link
Owner

kdy1 commented Aug 13, 2021

This is necessary to make rweb compatible with stable rustc.
I don't remember the exact reason.

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

No branches or pull requests

2 participants