by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • 08:47

    dependabot-preview[bot] on cargo

    (compare)

  • 08:47
    dependabot-preview[bot] closed #457
  • 08:47
    dependabot-preview[bot] commented #457
  • 08:47
    dependabot-preview[bot] labeled #460
  • 08:47
    dependabot-preview[bot] opened #460
  • 08:47

    dependabot-preview[bot] on cargo

    Update tera requirement from 0.… (compare)

  • Aug 10 10:17
    codecov[bot] commented #459
  • Aug 10 09:58
    codecov[bot] commented #459
  • Aug 10 09:36
    dependabot-preview[bot] labeled #459
  • Aug 10 09:36
    dependabot-preview[bot] opened #459
  • Aug 10 09:36

    dependabot-preview[bot] on cargo

    Update tokio-tungstenite requir… (compare)

  • Jul 29 10:00
    codecov[bot] commented #458
  • Jul 29 09:34
    codecov[bot] commented #458
  • Jul 29 09:12

    dependabot-preview[bot] on cargo

    (compare)

  • Jul 29 09:12
    dependabot-preview[bot] closed #454
  • Jul 29 09:12
    dependabot-preview[bot] commented #454
  • Jul 29 09:12
    dependabot-preview[bot] labeled #458
  • Jul 29 09:12
    dependabot-preview[bot] opened #458
  • Jul 29 09:12

    dependabot-preview[bot] on cargo

    Update askama requirement from … (compare)

  • Jul 27 10:30
    codecov[bot] commented #456
Denis Lisov
@tanriol
I think these could be implemented with just a single function, choosing the only implementation that makes sense for the function passed as the argument... but that can be too much magic.
Denis Lisov
@tanriol
Waiting for feedback on #450: should I try to generalize it for FnOnce(&mut State) -> .../FnOnce(&State) -> ...? for returning IntoResponse instead of a Result? try handling sync and async in the same function? try making into_handler_error not needed?
technic93
@technic
I've added minor comment on github.
msrd0
@msrd0_gitlab
I believe that when we finally get rid of mapping Err(e) => Err((state, e)) at the end of every handler, we should also remove the obligatory into_handler_error() call
With #438 it should be possible to ask for an anyhow::Result<T> instead of Result<T, HandlerError>, that would allow for using ? directly
Denis Lisov
@tanriol
Asking for anyhow::Result<T>, IIUC, would not support custom status codes for errors, or would it?
msrd0
@msrd0_gitlab
doesn't HandlerError always produce a 500 response?
Denis Lisov
@tanriol
By default it does, but it has a with_status method
msrd0
@msrd0_gitlab
ok well we could use E: Into<HandlerError> but I don't know if rustc would be able to deduce the appropriate type in a closure handler
Denis Lisov
@tanriol
Most likely it will not - I've seen errors with the combination of E: Into<HandlerError> and all errors being returned via ?
msrd0
@msrd0_gitlab
ok well HandlerError implements the Error trait which doesn't make life easier but I guess we could implement a new struct as a wrapper that can implement From<E> for E: Error, basically as anyhow::Error does, but have a with_status method (and map_err_with_status or similar to make the use of ? convenient)
Denis Lisov
@tanriol
That's what I've been thinking about, will look in a day or so (and also wait for feedback from others?)
David Laban
@alsuren
I left some feedback for you during my lunchbreak
Denis Lisov
@tanriol
Thank you, will look into it.
David Laban
@alsuren
re .to(handler) vs .to_async(async_fn). I tried making them the same function but you can't. With .to(), the handler's return type is a struct (either Box<_> or Result<_>). You can put two blanket impls for IntoHandlerFuture (or whatever we called it) and they are guaranteed to not collide, because they're implemented over structs.
with asyncfn, the return type is impl Future<>. You can't put a blanket impl from impl Future<_> and Result<_>, because the upstream rustc maintauners could come along in std and impl Future<_> for Result<_>
so the compiler only lets you have one blanket impl if it's over a trait.
</lunch>
Denis Lisov
@tanriol
Unless you use some black magic...
Denis Lisov
@tanriol
What an error...
37 |         .boxed()
   |          ^^^^^ one type is more general than the other
   |
   = note: expected type `std::marker::Send`
              found type `std::marker::Send`
technic93
@technic
Lol
Denis Lisov
@tanriol
Ok, looks like I can answer one of my questions: an async handler taking &State is inherently problematic because we require the future to be Send and for this reference to be Send the State needs to be Sync.
Denis Lisov
@tanriol
The change to &mut in my PR was to allow taking parts of state, but it accidentally also lifted this restriction.
matrixbot
@matrixbot
technic93 Do you have any ideas about running async tests? I tried it out with actix framework and their TestServer, and it works. The idea is instead of running each request with block_on wrapper, the whole test function is wrapped inside runtime. Then I can do await in the tests.

technic93 * Do you have any ideas about running async tests? I tried it out with actix framework and their TestServer, and it works. The idea is instead of running each request with block_on wrapper, the whole test function is wrapped inside runtime. Then I can do await in the tests.

P.S. I also wanted to see how matrix bot works 🙂

Denis Lisov
@tanriol
Is there some specific reason that .to(...) requires Copy, not just Clone?
Denis Lisov
@tanriol
In other words, should I for #450 require Clone or Copy?
Denis Lisov
@tanriol
Also what's the status of #438?
msrd0
@msrd0_gitlab
I think closures are always Copy so I don't think it makes a difference if we take Clone or Copy
#438 is waiting for a review from @nyarly or @colinbankier but my experience with gotham shows that it might take several month before that happens
Denis Lisov
@tanriol
IIRC, closures are Copy if all captures are Copy; Clone if all captures are Clone but some aren't Copy; and is neither if any capture is neither.
Does #438 intentionally not touch HandlerError?
msrd0
@msrd0_gitlab
I'd say it makes more sense to use Arc instead of Cloneing non-mutable closures (if you really want to not use Copy)
My main goal with #438 was to remove the outdated failure crate and fix the NewMiddleware error types
any idea/suggestion on how I should change HandlerError?
Denis Lisov
@tanriol
I'm not sure whether this is a goal, but if we want to use ? in handlers returning (a future of) Result<_, HandlerError> we need the corresponding Into implementation...
msrd0
@msrd0_gitlab
I agree, it might make sense to replace IntoHandlerError with Into<HandlerError> (which requires HandlerError to stop implement Error itself)
I'll have a look at that tomorrow if its possible to incorporate that into #438
Denis Lisov
@tanriol
A stupid question. If we merge #450, we'd have both a to_async method for handlers taking State by value and to_async_borrowed one for by-unique-reference handlers. Do we want to have both or to change to_async into the borrowed version? If both, which should be the preferred one?
msrd0
@msrd0_gitlab
Different question: As soon as we have a borrowed version, what do we need the owned version for? Creating a new State is made impossible (for a good reason) by gotham so consuming the state is impossible anyways
Denis Lisov
@tanriol
And an extension of the questions above: would we want to refactor the other State-passing gotham APIs to reference-based?
msrd0
@msrd0_gitlab
I don't think I have use case for an owned-version of State-passing APIs so I'd be happy if all of them get refactored
Denis Lisov
@tanriol
@nyarly, @colinbankier, what are your opinions?
technic93
@technic
How about to_async and to_async_moved? I think the borrowed version is more suitable for every day/novice usage
David Laban
@alsuren
I would be in favour of to_async and to_async_moved, or just drop the moved variant (people can mem::replace() if they need the owned version) . I tried desparately hard to make a borrowing version of to_async when I was writing it, but I was just not clever enough. to_async is new in the 0.5.0-rc1 release, so it might be acceptable to break its api before 0.5.x.
David Laban
@alsuren
the other thing that is worth thinking about is whether it is possible to say .to_async(|state| async { Ok("seems fine") } (or however that's spelled).
I have a similar thread open with the Goose maintainers in tag1consulting/goose#94 , and they mentioned that their original async callback implementation only accepts function pointers (not closures) but it's looking likely that this helper-trait approach will allow closures as well. Would be good to include an example that uses a closure as a route hander, if it's possible.
2 replies
msrd0
@msrd0_gitlab
Does anybody know how I could predict the return type of something like map_err(|e| e.foobar()) on a future F? Rust isn't happy about future::MapErr<F, FnOnce(E) -> HandlerError>
Jonathan Dickinson
@jcdickinson
Hey! I'm creating an async/await JSON API, and I've got this boilerplate for every method. Seems incredibly excessive. Am I missing something?
    let body = body::to_bytes(Body::take_from(&mut state)).await;
    let request = match body {
        Ok(r) => match serde_json::from_slice::<models::AuthParametersRequest>(r.as_ref()) {
            Ok(r) => r,
            Err(e) => {
                return Err((
                    state,
                    e.into_handler_error().with_status(StatusCode::BAD_REQUEST),
                ))
            }
        },
        Err(e) => {
            return Err((
                state,
                e.into_handler_error().with_status(StatusCode::BAD_REQUEST),
            ))
        }
    };
Denis Lisov
@tanriol
@jcdickinson Don't remember whether the owning to_async supports ?, but the to_async_borrowing from #450 tries to.
Simão Mata
@simao
Hi. Is there anyway I could get the client's ssl certificate inside a midleware when using client side tls auth? This would only be possible with openssl, but I see the openssl create exposes SslStream which has a ssl method that returns SslRef with the information I need, but I cannot see a way to get a SslStream inside a midleware. Could I somehow add that ref to State ?