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

Add bad_request_handler equivalent for METHOD_NOT_ALLOWED #343

Closed
banool opened this issue Jul 26, 2022 · 5 comments
Closed

Add bad_request_handler equivalent for METHOD_NOT_ALLOWED #343

banool opened this issue Jul 26, 2022 · 5 comments
Labels
enhancement New feature or request Stale

Comments

@banool
Copy link
Contributor

banool commented Jul 26, 2022

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 banool added the enhancement New feature or request label Jul 26, 2022
@banool
Copy link
Contributor Author

banool commented Jul 26, 2022

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

@banool
Copy link
Contributor Author

banool commented Jul 27, 2022

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

banool commented Jul 27, 2022

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.

@github-actions
Copy link

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 github-actions bot added the Stale label Aug 27, 2022
@github-actions
Copy link

github-actions bot commented Sep 1, 2022

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

@github-actions github-actions bot closed this as completed Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale
Projects
None yet
Development

No branches or pull requests

1 participant