Allow handlers to return user-defined error types (#1180)

Currently, all endpoint handler functions must return a
`Result<T, HttpError>`, where `T` implements `HttpResponse`. This is
unfortunate, as it limits how error return values are represented in
the API. There isn't presently a mechanism for an endpoint to return
a structured error value of its own which is part of the OpenAPI spec
for that endpoint. This is discussed at length in issue #39.

This branch relaxes this requirement, and instead allows endpoint
handler functions to return `Result<T, E>`, where `E` is any type that
implements a new `HttpResponseError` trait. 

The `HttpResponseError` trait defines how to produce an error response
for an error value. This is implemented by `dropshot`'s `HttpError`
type, but it may also be implemented by user errors. Types implementing
this trait must implement `HttpResponseContent`, to determine how to
generate the response body and define its schema, and they must also
implement a method `HttpResponseError::status_code` to provide the
status code to use for the error response. This is somewhat different
from the existing `HttpCodedResponse` trait, which allows successful
responses to indicate at compile time that they will always have a
particular status code, such as 201 Created. Errors are a bit different:
we would like to be able to return any number of different error status
codes, but would still like to ensure that they represent *errors*, in
order to correctly generate an OpenAPI document where the error schemas
are returned only for error responses (see [this comment][1] for
details). As discussed [here][2], we ensure this by providing new
`ErrorStatusCode` and `ClientErrorStatusCode` types, which are newtypes
around `http::StatusCode` that only contain a 4xx or 5xx status (in the
case of `ErrorStatusCode`), or only contain a 4xx (in the case of
`ClientErrorStatusCode`). These types may be fallibly converted from an
`http::StatusCode` at runtime, but we also provide constants for
well-known 4xx and 5xx statuses, which can be used infallibly. The
`HttpResponseError::status_code` method returns an `ErrorStatusCode`
rather than a `http::StatusCode`, allowing us to ensure that error types
always have error statuses and generate a correct OpenAPI document.

Additionally, while adding `ErrorStatusCode`s, I've gone ahead and
changed the `dropshot::HttpError` type to also use it, and changed the
`HttpError::for_client_error` and `HttpError::for_status` constructors
to take a `ClientErrorStatusCode`. Although this is a breaking change,
it resolves a long-standing issue with these APIs: currently, they
assert that the provided status code is a 4xx internally, which is often
surprising to the user. Thus, this PR fixes #693.

Fixes #39
Fixes #693
Fixes #801

This branch is a second attempt at the change originally proposed in PR
#1164, so this closes #1164. This design is substantially simpler, and
*only* addresses the ability for handler functions to return
user-defined types. Other changes made in #1164, such as a way to
specify a global handler for dropshot-generated errors, and adding
headers to `HttpError` responses, can be addressed separately. For now,
all extractors and internal errors still produce `dropshot::HttpError`s.
A subsequent change will implement a mechanism for providing alternate
presentation for such errors (such as an HTML 404 page).

[1]:
    #39 (comment)
[2]:
    #39 (comment)

Author: hawkw

CHANGELOG.adoc

@@ -15,6 +15,87 @@
 
 https://github.com/oxidecomputer/dropshot/compare/v0.14.0\...HEAD[Full list of commits]
 
+=== Breaking changes
+
+
+* The `HttpError` type now contains a `dropshot::ErrorStatusCode` rather than an
+`http::StatusCode`. An `ErrorStatusCode` is a newtype around `http::StatusCode`
+that may only be constructed from 4xx or 5xx status codes.
++
+Code which uses `http::StatusCode` constants for well-known status codes can
+be updated to the new API by replacing `http::StatusCode::...` with
+`dropshot::ErrorStatusCode`. For example:
++
+```rust
+dropshot::HttpError {
+    status: http::StatusCode::NOT_FOUND,
+    // ...
+}
+```
++
+becomes:
++
+```rust
+dropshot::HttpError {
+    status: dropshot::ErrorStatusCode::NOT_FOUND,
+    // ...
+}
+```
++
+To represent extension status codes that lack well-known constants, use
+`ErrorStatusCode::from_u16` (or the corresponding `TryFrom` implementation).
+This is analogous to the similarly-named method on `http::StatusCode`, so this:
++
+```rust
+http::StatusCode::from_u16(420).expect("420 is a valid status code")
+```
++
+becomes this:
++
+```rust
+dropshot::ErrorStatusCode::from_u16(420).expect("420 is a valid 4xx status code")
+```
++
+Finally, note that `ErrorStatusCode` implements `TryFrom<http::StatusCode>`, so
+`StatusCode`s from external sources may be converted into `ErrorStatusCode`s as
+necessary.
+
+* The `HttpError::for_status` constructor, which required that the provided
+status code be a 4xx client error and panicked if it was not, has been removed.
+It has been replaced with a new `HttpError::for_client_error_with_status`
+constructor, which takes a `dropshot::ClientErrorStatusCode` type rather than a
+`http::StatusCode`. Ensuring that only client errors are passed to this
+constructor at the type level removes the often-surprising panic on non-4xx errors.
++
+`ClientErrorStatusCode` provides constants for each well known 4xx status code,
+similarly to `ErrorStatusCode`. Uses of `HttpError::for_status`
+that use a constant status code, like this:
++
+```rust
+HttpError::for_status(None, http::StatusCode::GONE)
+```
++
+becomes this:
++
+```rust
+HttpError::for_client_error_with_status(None, dropshot::ClientErrorStatusCode::GONE)
+```
++
+Additionally, `ErrorStatusCode` provides an `as_client_error` method that
+returns a `ClientErrorStatusCode` if the status code is a client error, or an
+error.
+
+=== Other notable changes
+
+* Endpoint handler functions may now return any error type that implements the
+new `dropshot::HttpResponseError` trait. Previously, they could only return
+`dropshot::HttpError`. This change permits endpoints to return user-defined
+error types, and generate OpenAPI response schemas for those types.
++
+For details on how to implement `HttpResponseError` for user-defined types, see
+the trait documentation, or
+https://github.com/oxidecomputer/dropshot/blob/main/dropshot/examples/custom-error.rs[`examples/custom-error.rs`].
+
 == 0.14.0 (released 2024-12-02)
 
 https://github.com/oxidecomputer/dropshot/compare/v0.13.0\...v0.14.0[Full list of commits]

dropshot/examples/custom-error.rs

@@ -0,0 +1,191 @@
+// Copyright 2024 Oxide Computer Company
+
+//! An example demonstrating how to return user-defined error types from
+//! endpoint handlers.
+
+use dropshot::endpoint;
+use dropshot::ApiDescription;
+use dropshot::ConfigLogging;
+use dropshot::ConfigLoggingLevel;
+use dropshot::ErrorStatusCode;
+use dropshot::HttpError;
+use dropshot::HttpResponseError;
+use dropshot::HttpResponseOk;
+use dropshot::Path;
+use dropshot::RequestContext;
+use dropshot::ServerBuilder;
+use schemars::JsonSchema;
+use serde::Deserialize;
+use serde::Serialize;
+
+// This is the custom error type returned by our API. A common use case for
+// custom error types is to return an `enum` type that represents
+// application-specific errors in a way that generated clients can interact with
+// programmatically, so the error in this example will be an enum type.
+//
+// In order to be returned from an endpoint handler, it must implement the
+// `HttpResponseError` trait, which requires implementations of:
+//
+// - `HttpResponseContent`, which determines how to produce a response body
+//   from the error type,
+// - `std::fmt::Display`, which determines how to produce a human-readable
+//    message for Dropshot to log when returning the error,
+// - `From<dropshot::HttpError>`, so that errors returned by request extractors
+//   and resposne body serialization can be converted to the user-defined error
+//   type.
+#[derive(Debug)]
+// Deriving `Serialize` and `JsonSchema` for our error type provides an
+// implementation of the `HttpResponseContent` trait, which is required to
+// implement `HttpResponseError`:
+#[derive(serde::Serialize, schemars::JsonSchema)]
+// `HttpResponseError` also requires a `std::fmt::Display` implementation,
+// which we'll generate using `thiserror`'s `Error` derive:
+#[derive(thiserror::Error)]
+enum ThingyError {
+    // First, define some application-specific error variants that represent
+    // structured error responses from our API:
+    /// No thingies are currently available to satisfy this request.
+    #[error("no thingies are currently available")]
+    NoThingies,
+
+    /// The requested thingy is invalid.
+    #[error("invalid thingy: {:?}", .name)]
+    InvalidThingy { name: String },
+
+    // Then, we'll define a variant that can be constructed from a
+    // `dropshot::HttpError`, so that errors returned by Dropshot can also be
+    // represented in the error schema for our API:
+    #[error("{internal_message}")]
+    Other {
+        message: String,
+        error_code: Option<String>,
+
+        // Skip serializing these fields, as they are used for the
+        // `fmt::Display` implementation and for determining the status
+        // code, respectively, rather than included in the response body:
+        #[serde(skip)]
+        internal_message: String,
+        #[serde(skip)]
+        status: ErrorStatusCode,
+    },
+}
+
+impl HttpResponseError for ThingyError {
+    // Note that this method returns a `dropshot::ErrorStatusCode`, rather than
+    // an `http::StatusCode`. This type is a refinement of `http::StatusCode`
+    // that can only be constructed from status codes in 4xx (client error) or
+    // 5xx (server error) ranges.
+    fn status_code(&self) -> dropshot::ErrorStatusCode {
+        match self {
+            ThingyError::NoThingies => {
+                // The `dropshot::ErrorStatusCode` type provides constants for
+                // all well-known 4xx and 5xx status codes, such as 503 Service
+                // Unavailable.
+                dropshot::ErrorStatusCode::SERVICE_UNAVAILABLE
+            }
+            ThingyError::InvalidThingy { .. } => {
+                // Alternatively, an `ErrorStatusCode` can be constructed from a
+                // u16, but the `ErrorStatusCode::from_u16` constructor
+                // validates that the status code is a 4xx or 5xx.
+                //
+                // This allows using extended status codes, while still
+                // validating that they are errors.
+                dropshot::ErrorStatusCode::from_u16(442)
+                    .expect("442 is a 4xx status code")
+            }
+            ThingyError::Other { status, .. } => *status,
+        }
+    }
+}
+
+impl From<HttpError> for ThingyError {
+    fn from(error: HttpError) -> Self {
+        ThingyError::Other {
+            message: error.external_message,
+            internal_message: error.internal_message,
+            status: error.status_code,
+            error_code: error.error_code,
+        }
+    }
+}
+
+/// Just some kind of thingy returned by the API. This doesn't actually matter.
+#[derive(Deserialize, Serialize, JsonSchema)]
+struct Thingy {
+    magic_number: u64,
+}
+
+#[derive(Deserialize, JsonSchema)]
+struct ThingyPathParams {
+    name: ThingyName,
+}
+
+// Using an enum as a path parameter allows the API to also return extractor
+// errors. Try sending a `GET` request for `/thingy/baz` or similar to see how
+// the extractor error is converted into our custom error representation.
+#[derive(Debug, Deserialize, Serialize, JsonSchema)]
+#[serde(rename_all = "lowercase")]
+enum ThingyName {
+    Foo,
+    Bar,
+}
+
+/// Fetch the thingy with the provided name.
+#[endpoint {
+    method = GET,
+    path = "/thingy/{name}",
+}]
+async fn get_thingy(
+    _rqctx: RequestContext<()>,
+    path_params: Path<ThingyPathParams>,
+) -> Result<HttpResponseOk<Thingy>, ThingyError> {
+    let ThingyPathParams { name } = path_params.into_inner();
+    Err(ThingyError::InvalidThingy { name: format!("{name:?}") })
+}
+
+#[endpoint {
+    method = GET,
+    path = "/nothing",
+}]
+async fn get_nothing(
+    _rqctx: RequestContext<()>,
+) -> Result<HttpResponseOk<Thingy>, ThingyError> {
+    Err(ThingyError::NoThingies)
+}
+
+/// Endpoints which return `Result<_, HttpError>` may be part of the same
+/// API as endpoints which return user-defined error types.
+#[endpoint {
+    method = GET,
+    path = "/something",
+}]
+async fn get_something(
+    _rqctx: RequestContext<()>,
+) -> Result<HttpResponseOk<Thingy>, dropshot::HttpError> {
+    Ok(HttpResponseOk(Thingy { magic_number: 42 }))
+}
+
+#[tokio::main]
+async fn main() -> Result<(), String> {
+    // See dropshot/examples/basic.rs for more details on most of these pieces.
+    let config_logging =
+        ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info };
+    let log = config_logging
+        .to_logger("example-custom-error")
+        .map_err(|error| format!("failed to create logger: {}", error))?;
+
+    let mut api = ApiDescription::new();
+    api.register(get_thingy).unwrap();
+    api.register(get_nothing).unwrap();
+    api.register(get_something).unwrap();
+
+    api.openapi("Custom Error Example", semver::Version::new(0, 0, 0))
+        .write(&mut std::io::stdout())
+        .map_err(|e| e.to_string())?;
+
+    let server = ServerBuilder::new(api, (), log)
+        .start()
+        .map_err(|error| format!("failed to create server: {}", error))?;
+
+    server.await
+}