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

Rejection, recover and request state/ext #134

Open
asaaki opened this issue Nov 16, 2018 · 7 comments
Open

Rejection, recover and request state/ext #134

asaaki opened this issue Nov 16, 2018 · 7 comments

Comments

@asaaki
Copy link

asaaki commented Nov 16, 2018

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
Copy link

bwalter commented Apr 19, 2020

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
Copy link
Author

asaaki commented Apr 19, 2020

@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
Copy link

bwalter commented Apr 20, 2020

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
Copy link
Author

asaaki commented Apr 20, 2020

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
Copy link
Author

asaaki commented Apr 20, 2020

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
Copy link

bwalter commented Apr 20, 2020

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
Copy link
Author

asaaki commented Apr 21, 2020

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

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