seanmonstar / warp

A super-easy, composable, web server framework for warp speeds.
https://seanmonstar.com/post/176530511587/warp
MIT License
9.57k stars 719 forks source link

Rejection, recover and request state/ext #134

Open asaaki opened 5 years ago

asaaki commented 5 years ago

I struggle a bit with the rejection system/recover method. I want to build an API which can succeed and fail, and still use computed state like a request/correlation ID from the beginning of the filter chain:

// this tries to retrieve the ID from a header or if not present, generates a new ID
let request_id = warp::header::headers_cloned().map(utils::get_or_create_request_id);

let api = warp::get2()
  .and(warp::path("somepath"))
  .and(request_id)
  .map(my_handler_which_could_also_fail)
  // and maybe some more complex filtering, additional routes ...
  // all relying on the initially retrieved/generated request ID for response generation
  ;

let final_thing = api.recover(reject_as_json); // now the tricky part

warp::serve(final_thing).run(config::addr());

// ...

fn reject_as_json(err: Rejection) -> Result<impl Reply, Rejection> {
  // handle the rejection and make a nice JSON response from it
  // BUT: no access to the request ID from the filter chain :'-(
}

or_else and recover only receive a Rejection, and I cannot do anything to get something from the request or state to be reused here.

What am I missing? Is this possible? (I looked into filters::ext, but get returns a filter, which I have to plug into the chain instead of using it within a filter like recover. Again, am I missing something?)

I hope somebody had a similar problem and found a solution to it with warp. I really like this framework, but it still comes with certain limitations blocking me here and there. Retrieving state or the request after a rejection is such case.

Is there something planned for the future around this part?

bwalter commented 4 years ago

Just a comment here because I'm facing the same issue and would be also happy to get a solution. Another use case would be to log the errors together with their corresponding requests.

asaaki commented 4 years ago

@bwalter Hi, I'm not sure if this specific example is still valid, forgot about this open issue, it's now 1½ years old and warp has probably changed in this time, but I haven't reevaluated my issue since then.

I would need to rebuild my playground project to see if I could replicate it still today.

Or maybe you have some example code snippet showcasing the issue with a more current version of warp?

bwalter commented 4 years ago

Hi @asaaki, as far as I know your example is still valid (besides minor syntactic changes) with the current version of warp.

Would be nice if @seanmonstar would have some information/recommendation about it. I could also work on a PR if such a feature makes sense (e.g. breaking change to Filter::recover() or an additional variant with route info).

asaaki commented 4 years ago

Okay, I played around again (using the rejection example). If the basic requirement is only to set a request ID header conditionally (either it came with the request or we need to generate one), then the following code should be sufficient enough:

// snip
#[tokio::main]
async fn main() {
    // extract or generate; very crude but working code
    let request_id = warp::header::headers_cloned().map(|headers: HeaderMap| {
        let my_uuid = Uuid::new_v4();
        let pregen_r = format!("internal-{}", my_uuid);
        let pre_hv = HeaderValue::from_str(&pregen_r).unwrap();
        let r: String = headers
            .get("x-request-id")
            .unwrap_or_else(|| &pre_hv)
            .to_str()
            .unwrap_or_else(|_| &pregen_r)
            .into();
        r
    });

    // ... definition of math filter ...

   let routes = warp::get()
        .and(math)
        .recover(handle_rejection)
        .and(request_id) // here we add our request ID filter, AFTER the recover handler
        // interestingly we do get now 2 required arguments here: the reply + our request ID value
        .map(|reply, request_id: String| {
            // so we can at least easily attach our header
            warp::reply::with_header(reply, "x-request-id", request_id)
        });

    warp::serve(routes).run(([127, 0, 0, 1], 3030)).await;

};
//snip

This seems to work in all cases, all handled errors also do have a request ID header. (And this would probably solve 90% of cases for me, where a request or correlation ID only needs to be passed around …)

If the request ID should be part of the body data, we would need to use a different approach, the service wrapping as described in this comment should probably be feasible (the Wrap trait is sadly not public and so we cannot write our own custom wrappers).

asaaki commented 4 years ago

And here a more elaborate example for request state with warp: https://github.com/asaaki/warp-demo

To be honest it's of course a bit more overhead and "noise" in the code, but on the other hand sufficient enough for my use case.

bwalter commented 4 years ago

Thanks for the example! Tokio's task_local! is a great idea but a more natural way to get both the error and the request would be probably even nicer :) I will maybe try to find some time and create a PR...

asaaki commented 4 years ago

So I'll leave this issue open for now, simply for visibility. Looking forward to a proposal and will happily test it, too.