oxidecomputer/dropshot

`HttpError::for_status` asserts when passed a 500-level error

gjcolombo opened this issue · 6 comments

for_status takes any HTTP status code as an input, but it unconditionally passes the code to for_client_error, which asserts that the status is 400-level:

/// Generates an `HttpError` for the given HTTP `status_code` where the
/// internal and external messages for the error come from the standard label
/// for this status code (e.g., the message for status code 404 is "Not
/// Found").
pub fn for_status(
error_code: Option<String>,
status_code: http::StatusCode,
) -> Self {
// TODO-polish This should probably be our own message.
let message = status_code.canonical_reason().unwrap().to_string();
HttpError::for_client_error(error_code, status_code, message)
}

pub fn for_client_error(
error_code: Option<String>,
status_code: http::StatusCode,
message: String,
) -> Self {
assert!(status_code.is_client_error());
HttpError {
status_code,
error_code,
internal_message: message.clone(),
external_message: message,
}
}

I have some error handling code that's tripping this assert: it calls an external service, gets back a Progenitor result, checks to see if the result contains a status, and if so passes that status to HttpError::for_status to re-encode it as an HTTP error. If there's a more idiomatic way to do this specific conversion, that'd be great too, but it seems strange that for_status theoretically allows any status but will explode on a subset of them.