Error responses in OpenAPI output (#286)

Author: ahl

CHANGELOG.adoc

@@ -26,6 +26,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.6.0\...HEAD[Full list of co
 * https://github.com/oxidecomputer/dropshot/pull/252[#252] Endpoints specified with the `#[endpoint ..]` attribute macro now use the first line of a doc comment as the OpenAPI `summary` and subsequent lines as the `description`. Previously all lines were used as the `description`.
 * https://github.com/oxidecomputer/dropshot/pull/260[#260] Pulls in a newer serde that changes error messages around parsing NonZeroU32.
 * https://github.com/oxidecomputer/dropshot/pull/283[#283] Add support for response headers with the `HttpResponseHeaders` type. Headers may either be defined by a struct type parameter (in which case they appear in the OpenAPI output) or *ad-hoc* added via `HttpResponseHeaders::headers_mut()`.
+* https://github.com/oxidecomputer/dropshot/pull/286[#286] OpenAPI output includes descriptions of 4xx and 5xx error responses.
 
 == 0.6.0 (released 2021-11-18)
 

dropshot/examples/pagination-multiple-sorts.rs

@@ -186,7 +186,7 @@ enum ProjectSort {
  * selector back, you find the object having the next value after the one stored
  * in the token and start returning results from there.
  */
-#[derive(Deserialize, JsonSchema, Serialize)]
+#[derive(Deserialize, Serialize)]
 #[serde(rename_all = "kebab-case")]
 enum ProjectScanPageSelector {
     Name(PaginationOrder, String),

dropshot/src/api_description.rs

@@ -14,6 +14,7 @@ use crate::server::ServerContext;
 use crate::type_util::type_is_scalar;
 use crate::type_util::type_is_string_enum;
 use crate::Extractor;
+use crate::HttpErrorResponseBody;
 use crate::CONTENT_TYPE_JSON;
 use crate::CONTENT_TYPE_OCTET_STREAM;
 
@@ -668,8 +669,12 @@ impl<Context: ServerContext> ApiDescription<Context> {
 
                 match &endpoint.response.success {
                     None => {
-                        operation.responses.default =
-                            Some(openapiv3::ReferenceOr::Item(response))
+                        // Without knowing the specific status code we use the
+                        // 2xx range.
+                        operation.responses.responses.insert(
+                            openapiv3::StatusCode::Range(2),
+                            openapiv3::ReferenceOr::Item(response),
+                        );
                     }
                     Some(code) => {
                         operation.responses.responses.insert(
@@ -678,9 +683,23 @@ impl<Context: ServerContext> ApiDescription<Context> {
                         );
                     }
                 }
+
+                // 4xx and 5xx responses all use the same error information
+                let err_ref = openapiv3::ReferenceOr::ref_(
+                    "#/components/responses/Error",
+                );
+                operation
+                    .responses
+                    .responses
+                    .insert(openapiv3::StatusCode::Range(4), err_ref.clone());
+                operation
+                    .responses
+                    .responses
+                    .insert(openapiv3::StatusCode::Range(5), err_ref);
             } else {
                 // If no schema was specified, the response is hand-rolled. In
-                // this case we'll fall back to the default response type.
+                // this case we'll fall back to the default response type which
+                // we assume to be inclusive of errors.
                 operation.responses.default =
                     Some(openapiv3::ReferenceOr::Item(openapiv3::Response {
                         // TODO: perhaps we should require even free-form
@@ -695,11 +714,35 @@ impl<Context: ServerContext> ApiDescription<Context> {
             method_ref.replace(operation);
         }
 
-        // Add the schemas for which we generated references.
-        let schemas = &mut openapi
+        let components = &mut openapi
             .components
-            .get_or_insert_with(openapiv3::Components::default)
-            .schemas;
+            .get_or_insert_with(openapiv3::Components::default);
+
+        // All endpoints share an error response
+        let responses = &mut components.responses;
+        let mut content = indexmap::IndexMap::new();
+        content.insert(
+            CONTENT_TYPE_JSON.to_string(),
+            openapiv3::MediaType {
+                schema: Some(j2oas_schema(
+                    None,
+                    &generator.subschema_for::<HttpErrorResponseBody>(),
+                )),
+                ..Default::default()
+            },
+        );
+
+        responses.insert(
+            "Error".to_string(),
+            openapiv3::ReferenceOr::Item(openapiv3::Response {
+                description: "Error".to_string(),
+                content: content,
+                ..Default::default()
+            }),
+        );
+
+        // Add the schemas for which we generated references.
+        let schemas = &mut components.schemas;
 
         let root_schema = generator.into_root_schema_for::<()>();
         root_schema.definitions.iter().for_each(|(key, schema)| {

dropshot/src/error.rs

@@ -45,6 +45,7 @@
  */
 
 use hyper::Error as HyperError;
+use schemars::JsonSchema;
 use serde::Deserialize;
 use serde::Serialize;
 use std::error::Error;
@@ -107,9 +108,19 @@ pub struct HttpError {
  * deserialize an HTTP response corresponding to an error in order to access the
  * error code, message, etc.
  */
-#[derive(Debug, Deserialize, Serialize)]
+/*
+ * TODO: does this need to be pub if it's going to be expressed in the OpenAPI
+ * output?
+ */
+#[derive(Debug, Deserialize, Serialize, JsonSchema)]
+#[schemars(rename = "Error")]
+#[schemars(description = "Error information from a response.")]
 pub struct HttpErrorResponseBody {
     pub request_id: String,
+    // The combination of default and required removes "nullable" from the
+    // OpenAPI-flavored JSON Schema output.
+    #[schemars(default, required)]
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub error_code: Option<String>,
     pub message: String,
 }
@@ -298,3 +309,30 @@ impl Error for HttpError {
         None
     }
 }
+
+#[cfg(test)]
+mod test {
+    use crate::HttpErrorResponseBody;
+
+    #[test]
+    fn test_serialize_error_response_body() {
+        let err = HttpErrorResponseBody {
+            request_id: "123".to_string(),
+            error_code: None,
+            message: "oy!".to_string(),
+        };
+        let out = serde_json::to_string(&err).unwrap();
+        assert_eq!(out, r#"{"request_id":"123","message":"oy!"}"#);
+
+        let err = HttpErrorResponseBody {
+            request_id: "123".to_string(),
+            error_code: Some("err".to_string()),
+            message: "oy!".to_string(),
+        };
+        let out = serde_json::to_string(&err).unwrap();
+        assert_eq!(
+            out,
+            r#"{"request_id":"123","error_code":"err","message":"oy!"}"#
+        );
+    }
+}