warp: Unreasonably hard to recover internal errors from Rejection.

Just to add my 2c, I wanted to be able to return errors from a JSON based API in a standard JSON format of my own choosing to the user. I had to look through the warp source to see what errors to match on, and then wrote a macro to make my life a little easier doing this.

Its pretty horrible, and it means that if new errors are exposed in warp, I’ll not be handling them until I notice. A generic way to obtain the status code so that I can handle them as I wish would make things cleaner and more future proof.

For interest, here’s my current error handling code to turn errors into my standard shape:

use warp::reject::*;
use http_error::HttpError;
use http::StatusCode;
use super::HttpErrorWrapper;

macro_rules! to_http_error {
    ($($ty:path => $code:path,)+) => (
        /// Attempt to convert a warp rejection into an HttpError.
        pub fn rejection_to_http_error(rejection: warp::reject::Rejection) -> HttpError {
            // Handle our HttpErrorWrapper types, and the "not found" outcome:
            if rejection.is_not_found() {
                return HttpError::path_not_found()
            } else if let Some(err) = rejection.find::<HttpErrorWrapper>() {
                return err.0.clone()
            }
            // Handle the warp types that we have access to:
            $(
                if let Some(err) = rejection.find::<$ty>() {
                    return HttpError {
                        code: $code.as_u16(),
                        internal_message: err.to_string(),
                        external_message: err.to_string(),
                        value: None
                    }
                }
            )+
            // We don't know what the error is, so handle it generically:
            return HttpError {
                code: 500,
                internal_message: format!("{:?}", rejection),
                external_message: HttpError::SERVER_ERROR.to_owned(),
                value: None
            }
        }
    );
}

to_http_error! (
    MethodNotAllowed => StatusCode::METHOD_NOT_ALLOWED,
    InvalidHeader => StatusCode::BAD_REQUEST,
    MissingHeader => StatusCode::BAD_REQUEST,
    MissingCookie => StatusCode::BAD_REQUEST,
    InvalidQuery => StatusCode::BAD_REQUEST,
    warp::body::BodyDeserializeError => StatusCode::BAD_REQUEST,
    LengthRequired => StatusCode::LENGTH_REQUIRED,
    PayloadTooLarge => StatusCode::PAYLOAD_TOO_LARGE,
    UnsupportedMediaType => StatusCode::UNSUPPORTED_MEDIA_TYPE,
    warp::cors::CorsForbidden => StatusCode::FORBIDDEN,
    warp::ext::MissingExtension => StatusCode::INTERNAL_SERVER_ERROR,
);

I’ve attempted to create a simple HTTP basic auth filter but ran into this issue. I can implement a filter that checks against the Authorization header and returns one of two custom rejections (i.e. “missing authorization header” and “incorrect auth”). A recovery function is then used to return a proper response for those custom rejections.

However, the issue here then forces me to return a different (500?) response for the known (built-in) rejections since I have no access to the reply machinery for Rejection.

I thought perhaps to try using a wrapping filter instead, but the implementation of those seem entirely private.

Even if we fix this issue so that we can get at a response representation of Rejection, it still means that the transformation of a rejection to a proper response would still take place in a recovery function, which is something that makes using a filter that checks auth a bit unergonomic, as omitting the recovery function would get back, by default, 500 responses for the custom rejections.

I might be way off base, though, and perhaps there’s a simple way to make an auth filter that plays nice with the built-in filters, but I’m currently at a loss as how to move forward.

Would having a function available that converts the Rejection into its default http::Response be something you’re looking for? And then you can modify any headers and the body?

Just offering that I have the same issue where I basically just want to marshal the rejection to a specific JSON structure without having to catch and handle all “standard” errors (after handling my custom ones). I think being able to access status and to_string would be sufficient for my needs.

@seanmonstar any progress on this? Not being able to extract the status codes from warp’s ‘known’ errors means you would have to either manually find all 16 known types to return the correct code or just blanket return a 500.

https://github.com/seanmonstar/warp/blob/fbbbc5b8377914463e4411ef6fc48bfd97d15a6d/examples/rejections.rs#L61 The above will return incorrect codes as the rejection is not passed along even thought the comment states it is.

I think conceptually that works? I guess either a http::Response or StatusCode are suitable for handing back as the response, but I’m probably biased here in that I really only need the status code so I can get the canonical reason.

Just to sort of rubber duck it out here, the difference would be something like…

async fn handle_rejection(err: Rejection) -> Result<impl Reply, Infallible> {
    let mut code = None;
    let mut message = None;

    if let Some(DivideByZero) = err.find() {
        code = Some(StatusCode::BAD_REQUEST);
        message = Some("DIVIDE_BY_ZERO");
    }
    // more if conditions here
    
    let mut response = err.into_response();
    if let Some(sc) = code {
        *response.status_mut() = code;
    }
    if let Some(m) = message {
        *response.body_mut() = m.into();
    }
    
    Ok(response)
}

vs

async fn handle_rejection(err: Rejection) -> Result<impl Reply, Infallible> {
    let code;
    let message;

    if let Some(DivideByZero) = err.find() {
        code = StatusCode::BAD_REQUEST;
        message = "DIVIDE_BY_ZERO";
    } else {
        code = err.status();
        message = code.canonical_reason();
    }
    // more if conditions here
    
    Ok(warp::reply::with_status(message, code))
}

This is, of course, ignoring the customized structure/encoding, but I think it captures the spirit of what I’m proposing here. To my eyes, having to mutate/extract from a pre-baked response feels like extra work, although admittedly I suppose it’s just exposing the allocation/construction of the response that would still otherwise happen?

But you wish to tweak the response somehow beyond the default? So just returning Err(rejection) isn’t enough?