Async borrowing (continuation of #449)

[tanriol]

It seems to work as expected.

Todo:

• Try to avoid mapping every error with into_handler_error before ?

Feedback needed:

• Could this allow for generic return types like R: IntoResponse? Yes.
• Could this allow for handlers receiving either &State or &mut State? Not really.
• Could this allow for returning the reponse without Ok-wrapping in the infallible case? Not really.

And "do we want that" for every question above :-)

[codecov]

Codecov Report

Merging #450 into master will increase coverage by 0.38%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master     #450      +/-   ##
==========================================
+ Coverage   84.10%   84.48%   +0.38%     
==========================================
  Files         110      110              
  Lines        5473     5556      +83     
==========================================
+ Hits         4603     4694      +91     
+ Misses        870      862       -8     

Impacted Files	Coverage Δ
gotham/src/handler/mod.rs	79.16% <ø> (ø)
gotham/src/router/builder/single.rs	91.17% <92.30%> (+0.70%)	⬆️
...s/handlers/simple_async_handlers_await/src/main.rs	87.71% <100.00%> (-0.22%)	⬇️
gotham/src/lib.rs	82.75% <0.00%> (ø)
examples/hello_world_tls/src/main.rs	40.74% <0.00%> (ø)
gotham/src/router/mod.rs	89.58% <0.00%> (+0.19%)	⬆️
gotham/src/middleware/session/backend/memory.rs	89.89% <0.00%> (+3.03%)	⬆️
gotham/src/router/non_match.rs	92.50% <0.00%> (+6.58%)	⬆️
gotham/src/router/route/matcher/and.rs	100.00% <0.00%> (+28.57%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2275a4...f853599. Read the comment docs.

[tanriol]

I've tried making this work for an async function of &State. The problem is that for the future to be Send we'd need &State to be Send, which, according to the standard rules for shared references, requires State to be Sync. The choice of &mut State works around this because unique references don't share this requirement.

[tanriol]

Looks like this would not work without Result in the infallible case because of ambiguous error type. The problem here is that there's the following impl

impl<T, E> IntoResponse for Result<T, E> where
    T: IntoResponse,
    E: IntoResponse, 
{ ... }

which means that returning impl IntoResponse and passing the errors with ? will result in Rust being unable to infer the E type from the ErrorTypeN: Into<E> and E: IntoResponse bounds.

[technic]

As far as I understand rust uses From trait for converting between types when ? operator is used (or Into).
So if the want to go via intermediate type which implements IntoResponse there is no way for rust to deduce this type automatically.

[tanriol]

Pushed an updated version. I haven't looked into IntoHandlerError because @msrd0 said that may go in #438.

The main question, IMHO, is whether we want to keep both to_async and to_async_borrowed or just one of them?

[tanriol]

@technic

As far as I understand rust uses From trait for converting between types when ? operator is used (or Into).
So if the want to go via intermediate type which implements IntoResponse there is no way for rust to deduce this type automatically.

Not exactly. Rust can handle inference like "there's an Ok being returned, so this is a Result, and the only impl for a Result is an impl for Result<_, HandlerError>, so the error type is known to be a HandlerError". If the impl above were an impl<T: IntoResponse> IntoResponse for Result<T, HandlerError>, it would compile successfully.

I'd also like to say that the mentioned impl looks wrong in my opinion. If I returned an Err("some error description"), I'd expect it to have an error status code by default, but according to the implementation that's not the case.

[msrd0]

What is the status of this pull request? #438 has finally been merged, so this PR should no longer be blocked on it.

[tanriol]

Will rebase in a couple days (maybe faster if the rebase is easy). IIRC, there's the problem that two different variants allow either borrowing (and passing an async function) in async or passing a closure (but no borrowing), not both.

[alsuren]

I will admit that I've stepped away from both of these projects, but I occasionally get notifications from both projects, so I'm leaving this here in case it provides inspiration for anyone who comes along.

The people over at the goose project seem to have made some headway with closures. Last I saw, they ask the user to wrap the closure in an Arc and Box::pin() the resulting future themselves (the Arc is goose-specific, because of how the containing struct derives Clone. Gotham could probably use Box there too. See https://github.com/tag1consulting/goose/pull/120/files#diff-1cd6d8660ef657deb6ee2a73b4d592d9R1922-R1926 https://github.com/tag1consulting/goose/pull/120/files#diff-8484c6c853111ca2278cc1dc616df376R189-R195 and https://github.com/tag1consulting/goose/pull/126/files) but there might have been some ergonomics improvements since then that I've not been keeping on top of (I'm not sure whether this approach went anywhere, for example: tag1consulting/goose#94 (comment) . I suspect not).

I suspect that supporting closures is more interesting for goose than it is for gotham. If the ergonomics of some theoretical .to_borrowing_closure() are a bit less nice than the owning .to() and .to_async() variants then maybe that's fine. It might be an argument for keeping the old variants around though, until rustc is capable of letting you express the appropriate types and lifetimes for your function arguments directly.

[technic]

Great job! thanks!