poem icon indicating copy to clipboard operation
poem copied to clipboard

Add `bad_request_handler` equivalent for `METHOD_NOT_ALLOWED`

Open banool opened this issue 2 years ago • 3 comments

I have an OpenAPI based server that says that it only ever returns JSON formatted errors. However, the framework doesn't do this by default. For most errors that originate from the framework, you can use bad_request_handler to convert the error, but this doesn't work for METHOD_NOT_ALLOWED. It would be nice if we had an option for this. Beyond that, perhaps just a method that is more general than bad_request_handler that works for every request the framework returns.

banool avatar Jul 26 '22 19:07 banool

For example NOT_FOUND, I'd love to be able to control that response.

banool avatar Jul 26 '22 19:07 banool

So instead of using bad_request_handler I'm now using catch_all_error on the endpoint:

// The way I'm determining which errors are framework errors is very janky, as
// is the way I'm building the response. See:
// - https://github.com/poem-web/poem/issues/343
// - https://github.com/poem-web/poem/pull/349

/// In the OpenAPI spec for this API, we say that every response we return will
/// be a JSON representation of AptosError. For our own code, this is exactly
/// what we do. The problem is the Poem framework does not conform to this
/// format, it can return errors in a different format. The purpose of this
/// function is to catch those errors and convert them to the correct format.
pub async fn convert_error(error: poem::Error) -> impl poem::IntoResponse {
    // This is a bit of a hack but errors we return have no source, whereas
    // those returned by the framework do. As such, if we cannot downcast the
    // error we know it's one of ours and we just return it directly.
    let error_string = error.to_string();
    let is_framework_error = error_string != "response";
    if is_framework_error {
        // Build the response.
        let mut response = error.into_response();
        // Replace the body with the response.
        response.set_body(build_error_response(error_string).take_body());
        response
    } else {
        error.into_response()
    }
}

fn build_error_response(error_string: String) -> Response {
    Json(AptosError::new(error_string)).into_response()
}

Unfortunately I don't have a great way to figure out which errors are framework errors now vs my own, do you have any tips? I'm currently exploiting the fact that my own errors don't have a source field set (I assume because they come from Error::from_response, which sets source to None, but this feels very janky. I wonder if there is a better way.

Perhaps something like catch_all_framework_errors that only triggers on errors returned by the framework?

The way I'm building the response is obviously also very janky, but I don't have a great suggestion there yet.

banool avatar Jul 27 '22 18:07 banool

Alternatively it'd be great if there was some way to identify my own error and then know that everything else is an API error. Unfortunately by this point my error is just an opaque Poem error.

banool avatar Jul 27 '22 19:07 banool

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Aug 27 '22 03:08 github-actions[bot]

This issue was closed because it has been stalled for 5 days with no activity.

github-actions[bot] avatar Sep 01 '22 03:09 github-actions[bot]