Convert DecodingErrors thrown from request handling code to HTTP 400 … (#161)

colindignazio · colin-dignazio · czechboy0 · web-flow · commit 7cdf33371bf8 · 2025-11-04T19:15:49.000+01:00
### Motivation

The library does not gracefully handle when clients send requests with
malformed request bodies, parameters, etc. If a client sends a request
that for example is missing a required field or has a field that cannot
be converted to the correct type, the server should respond with a `400`
indicating to the client that **they** have done something wrong.
Instead, the library throws a `DecodingError` which propagates all the
way up resulting in a `500` response which incorrectly tells the client
that something went wrong on the server.

### Modifications

* Added a layer of handling to each call that is made to decode request
objects which wraps `DecodingErrors` into `RuntimeErrors`.
* Make `ServerError` conform to `HTTPResponseConvertible`
* Modify `ErrorHandlingMiddleware` to first check if the underlying
error conforms to `HTTPResponseConvertible` and if not use the values
from `ServerError`. This allows the `HTTPResponseConvertible` values set
in a `RuntimeError` to be honoured after the `RuntimeError` is
transformed into a `ServerError`.

### Result

Since `RuntimeError` conforms to `HTTPResponseConvertible` these errors
will get converted to `400` responses by the `ErrorHandlingMiddleware`.
This isn't a perfect solution because consumers of the library have to
opt-in to the `ErrorHandlingMiddleware` to avoid returning `500`.

### Test Plan

* Added unit tests for each modification.
* Verified in my own service that with this change malformed requests
get converted into `400` responses.

---------

Co-authored-by: colin-dignazio &lt;colin_dignazio@apple.com&gt;
Co-authored-by: Honza Dvorsky &lt;honza@apple.com&gt;
diff --git a/Sources/OpenAPIRuntime/Errors/RuntimeError.swift b/Sources/OpenAPIRuntime/Errors/RuntimeError.swift
@@ -54,6 +54,7 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
     // Body
     case missingRequiredRequestBody
     case missingRequiredResponseBody
+    case failedToParseRequest(DecodingError)
 
     // Multipart
     case missingRequiredMultipartFormDataContentType
@@ -72,6 +73,7 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
     var underlyingError: (any Error)? {
         switch self {
         case .transportFailed(let error), .handlerFailed(let error), .middlewareFailed(_, let error): return error
+        case .failedToParseRequest(let decodingError): return decodingError
         default: return nil
         }
     }
@@ -119,6 +121,8 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
             return "Unexpected response, expected status code: \(expectedStatus), response: \(response)"
         case .unexpectedResponseBody(let expectedContentType, let body):
             return "Unexpected response body, expected content type: \(expectedContentType), body: \(body)"
+        case .failedToParseRequest(let decodingError):
+            return "An error occurred while attempting to parse the request: \(decodingError.prettyDescription)."
         }
     }
 
@@ -160,7 +164,7 @@ extension RuntimeError: HTTPResponseConvertible {
             .invalidHeaderFieldName, .malformedAcceptHeader, .missingMultipartBoundaryContentTypeParameter,
             .missingOrMalformedContentDispositionName, .missingRequiredHeaderField,
             .missingRequiredMultipartFormDataContentType, .missingRequiredQueryParameter, .missingRequiredPathParameter,
-            .missingRequiredRequestBody, .unsupportedParameterStyle:
+            .missingRequiredRequestBody, .unsupportedParameterStyle, .failedToParseRequest:
             .badRequest
         case .handlerFailed, .middlewareFailed, .missingRequiredResponseBody, .transportFailed,
             .unexpectedResponseStatus, .unexpectedResponseBody:
diff --git a/Sources/OpenAPIRuntime/Errors/ServerError.swift b/Sources/OpenAPIRuntime/Errors/ServerError.swift
@@ -16,7 +16,7 @@ import HTTPTypes
 import protocol Foundation.LocalizedError
 
 /// An error thrown by a server handling an OpenAPI operation.
-public struct ServerError: Error {
+public struct ServerError: Error, HTTPResponseConvertible {
 
     /// Identifier of the operation that threw the error.
     public var operationID: String
@@ -47,6 +47,15 @@ public struct ServerError: Error {
     /// The underlying error that caused the operation to fail.
     public var underlyingError: any Error
 
+    /// An HTTP status to return in the response.
+    public var httpStatus: HTTPResponse.Status
+
+    /// The HTTP header fields of the response.
+    public var httpHeaderFields: HTTPTypes.HTTPFields
+
+    /// The body of the HTTP response.
+    public var httpBody: OpenAPIRuntime.HTTPBody?
+
     /// Creates a new error.
     /// - Parameters:
     ///   - operationID: The OpenAPI operation identifier.
@@ -68,6 +77,62 @@ public struct ServerError: Error {
         operationOutput: (any Sendable)? = nil,
         causeDescription: String,
         underlyingError: any Error
+    ) {
+        let httpStatus: HTTPResponse.Status
+        let httpHeaderFields: HTTPTypes.HTTPFields
+        let httpBody: OpenAPIRuntime.HTTPBody?
+        if let httpConvertibleError = underlyingError as? (any HTTPResponseConvertible) {
+            httpStatus = httpConvertibleError.httpStatus
+            httpHeaderFields = httpConvertibleError.httpHeaderFields
+            httpBody = httpConvertibleError.httpBody
+        } else {
+            httpStatus = .internalServerError
+            httpHeaderFields = [:]
+            httpBody = nil
+        }
+
+        self.init(
+            operationID: operationID,
+            request: request,
+            requestBody: requestBody,
+            requestMetadata: requestMetadata,
+            operationInput: operationInput,
+            operationOutput: operationOutput,
+            causeDescription: causeDescription,
+            underlyingError: underlyingError,
+            httpStatus: httpStatus,
+            httpHeaderFields: httpHeaderFields,
+            httpBody: httpBody
+        )
+    }
+
+    /// Creates a new error.
+    /// - Parameters:
+    ///   - operationID: The OpenAPI operation identifier.
+    ///   - request: The HTTP request provided to the server.
+    ///   - requestBody: The HTTP request body provided to the server.
+    ///   - requestMetadata: The request metadata extracted by the server.
+    ///   - operationInput: An operation-specific Input value.
+    ///   - operationOutput: An operation-specific Output value.
+    ///   - causeDescription: A user-facing description of what caused
+    ///     the underlying error to be thrown.
+    ///   - underlyingError: The underlying error that caused the operation
+    ///     to fail.
+    ///   - httpStatus: An HTTP status to return in the response.
+    ///   - httpHeaderFields: The HTTP header fields of the response.
+    ///   - httpBody: The body of the HTTP response.
+    public init(
+        operationID: String,
+        request: HTTPRequest,
+        requestBody: HTTPBody?,
+        requestMetadata: ServerRequestMetadata,
+        operationInput: (any Sendable)? = nil,
+        operationOutput: (any Sendable)? = nil,
+        causeDescription: String,
+        underlyingError: any Error,
+        httpStatus: HTTPResponse.Status,
+        httpHeaderFields: HTTPTypes.HTTPFields,
+        httpBody: OpenAPIRuntime.HTTPBody?
     ) {
         self.operationID = operationID
         self.request = request
@@ -77,6 +142,9 @@ public struct ServerError: Error {
         self.operationOutput = operationOutput
         self.causeDescription = causeDescription
         self.underlyingError = underlyingError
+        self.httpStatus = httpStatus
+        self.httpHeaderFields = httpHeaderFields
+        self.httpBody = httpBody
     }
 
     // MARK: Private
diff --git a/Sources/OpenAPIRuntime/Interface/ErrorHandlingMiddleware.swift b/Sources/OpenAPIRuntime/Interface/ErrorHandlingMiddleware.swift
@@ -57,12 +57,10 @@ public struct ErrorHandlingMiddleware: ServerMiddleware {
             async throws -> (HTTPTypes.HTTPResponse, OpenAPIRuntime.HTTPBody?)
     ) async throws -> (HTTPTypes.HTTPResponse, OpenAPIRuntime.HTTPBody?) {
         do { return try await next(request, body, metadata) } catch {
-            if let serverError = error as? ServerError,
-                let appError = serverError.underlyingError as? (any HTTPResponseConvertible)
-            {
+            if let serverError = error as? ServerError {
                 return (
-                    HTTPResponse(status: appError.httpStatus, headerFields: appError.httpHeaderFields),
-                    appError.httpBody
+                    HTTPResponse(status: serverError.httpStatus, headerFields: serverError.httpHeaderFields),
+                    serverError.httpBody
                 )
             } else {
                 return (HTTPResponse(status: .internalServerError), nil)
diff --git a/Sources/OpenAPIRuntime/Interface/UniversalServer.swift b/Sources/OpenAPIRuntime/Interface/UniversalServer.swift
@@ -119,6 +119,23 @@ import struct Foundation.URLComponents
                 causeDescription = "Unknown"
                 underlyingError = error
             }
+
+            let httpStatus: HTTPResponse.Status
+            let httpHeaderFields: HTTPTypes.HTTPFields
+            let httpBody: OpenAPIRuntime.HTTPBody?
+            if let httpConvertibleError = underlyingError as? (any HTTPResponseConvertible) {
+                httpStatus = httpConvertibleError.httpStatus
+                httpHeaderFields = httpConvertibleError.httpHeaderFields
+                httpBody = httpConvertibleError.httpBody
+            } else if let httpConvertibleError = error as? (any HTTPResponseConvertible) {
+                httpStatus = httpConvertibleError.httpStatus
+                httpHeaderFields = httpConvertibleError.httpHeaderFields
+                httpBody = httpConvertibleError.httpBody
+            } else {
+                httpStatus = .internalServerError
+                httpHeaderFields = [:]
+                httpBody = nil
+            }
             return ServerError(
                 operationID: operationID,
                 request: request,
@@ -127,13 +144,18 @@ import struct Foundation.URLComponents
                 operationInput: input,
                 operationOutput: output,
                 causeDescription: causeDescription,
-                underlyingError: underlyingError
+                underlyingError: underlyingError,
+                httpStatus: httpStatus,
+                httpHeaderFields: httpHeaderFields,
+                httpBody: httpBody
             )
         }
         var next: @Sendable (HTTPRequest, HTTPBody?, ServerRequestMetadata) async throws -> (HTTPResponse, HTTPBody?) =
             { _request, _requestBody, _metadata in
                 let input: OperationInput = try await wrappingErrors {
-                    try await deserializer(_request, _requestBody, _metadata)
+                    do { return try await deserializer(_request, _requestBody, _metadata) } catch let decodingError
+                        as DecodingError
+                    { throw RuntimeError.failedToParseRequest(decodingError) }
                 } mapError: { error in
                     makeError(error: error)
                 }
diff --git a/Tests/OpenAPIRuntimeTests/Errors/Test_ClientError.swift b/Tests/OpenAPIRuntimeTests/Errors/Test_ClientError.swift
@@ -25,7 +25,10 @@ final class Test_ServerError: XCTestCase {
             requestBody: nil,
             requestMetadata: .init(),
             causeDescription: upstreamError.prettyDescription,
-            underlyingError: upstreamError.underlyingError ?? upstreamError
+            underlyingError: upstreamError.underlyingError ?? upstreamError,
+            httpStatus: .internalServerError,
+            httpHeaderFields: [:],
+            httpBody: nil
         )
         XCTAssertEqual(
             "\(error)",
diff --git a/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift b/Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift
@@ -101,6 +101,37 @@ final class Test_UniversalServer: Test_Runtime {
         }
     }
 
+    func testErrorPropagation_deserializerWithDecodingError() async throws {
+        let decodingError = DecodingError.dataCorrupted(
+            .init(codingPath: [], debugDescription: "Invalid request body.")
+        )
+        do {
+            let server = UniversalServer(handler: MockHandler())
+            _ = try await server.handle(
+                request: .init(soar_path: "/", method: .post),
+                requestBody: MockHandler.requestBody,
+                metadata: .init(),
+                forOperation: "op",
+                using: { MockHandler.greet($0) },
+                deserializer: { request, body, metadata in throw decodingError },
+                serializer: { output, _ in fatalError() }
+            )
+        } catch {
+            let serverError = try XCTUnwrap(error as? ServerError)
+            XCTAssertEqual(serverError.operationID, "op")
+            XCTAssert(serverError.causeDescription.contains("An error occurred while attempting to parse the request"))
+            XCTAssert(serverError.underlyingError is DecodingError)
+            XCTAssertEqual(serverError.httpStatus, .badRequest)
+            XCTAssertEqual(serverError.httpHeaderFields, [:])
+            XCTAssertNil(serverError.httpBody)
+            XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post))
+            XCTAssertEqual(serverError.requestBody, MockHandler.requestBody)
+            XCTAssertEqual(serverError.requestMetadata, .init())
+            XCTAssertNil(serverError.operationInput)
+            XCTAssertNil(serverError.operationOutput)
+        }
+    }
+
     func testErrorPropagation_handler() async throws {
         do {
             let server = UniversalServer(handler: MockHandler(shouldFail: true))
