Skip to content

Commit 7cdf333

Browse files
colindignaziocolin-dignazioczechboy0
authored
Convert DecodingErrors thrown from request handling code to HTTP 400 … (#161)
### 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 <colin_dignazio@apple.com> Co-authored-by: Honza Dvorsky <honza@apple.com>
1 parent f04cc99 commit 7cdf333

6 files changed

Lines changed: 136 additions & 10 deletions

File tree

‎Sources/OpenAPIRuntime/Errors/RuntimeError.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
5454
// Body
5555
case missingRequiredRequestBody
5656
case missingRequiredResponseBody
57+
case failedToParseRequest(DecodingError)
5758

5859
// Multipart
5960
case missingRequiredMultipartFormDataContentType
@@ -72,6 +73,7 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
7273
var underlyingError: (any Error)? {
7374
switch self {
7475
case .transportFailed(let error), .handlerFailed(let error), .middlewareFailed(_, let error): return error
76+
case .failedToParseRequest(let decodingError): return decodingError
7577
default: return nil
7678
}
7779
}
@@ -119,6 +121,8 @@ internal enum RuntimeError: Error, CustomStringConvertible, LocalizedError, Pret
119121
return "Unexpected response, expected status code: \(expectedStatus), response: \(response)"
120122
case .unexpectedResponseBody(let expectedContentType, let body):
121123
return "Unexpected response body, expected content type: \(expectedContentType), body: \(body)"
124+
case .failedToParseRequest(let decodingError):
125+
return "An error occurred while attempting to parse the request: \(decodingError.prettyDescription)."
122126
}
123127
}
124128

@@ -160,7 +164,7 @@ extension RuntimeError: HTTPResponseConvertible {
160164
.invalidHeaderFieldName, .malformedAcceptHeader, .missingMultipartBoundaryContentTypeParameter,
161165
.missingOrMalformedContentDispositionName, .missingRequiredHeaderField,
162166
.missingRequiredMultipartFormDataContentType, .missingRequiredQueryParameter, .missingRequiredPathParameter,
163-
.missingRequiredRequestBody, .unsupportedParameterStyle:
167+
.missingRequiredRequestBody, .unsupportedParameterStyle, .failedToParseRequest:
164168
.badRequest
165169
case .handlerFailed, .middlewareFailed, .missingRequiredResponseBody, .transportFailed,
166170
.unexpectedResponseStatus, .unexpectedResponseBody:

‎Sources/OpenAPIRuntime/Errors/ServerError.swift‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import HTTPTypes
1616
import protocol Foundation.LocalizedError
1717

1818
/// An error thrown by a server handling an OpenAPI operation.
19-
public struct ServerError: Error {
19+
public struct ServerError: Error, HTTPResponseConvertible {
2020

2121
/// Identifier of the operation that threw the error.
2222
public var operationID: String
@@ -47,6 +47,15 @@ public struct ServerError: Error {
4747
/// The underlying error that caused the operation to fail.
4848
public var underlyingError: any Error
4949

50+
/// An HTTP status to return in the response.
51+
public var httpStatus: HTTPResponse.Status
52+
53+
/// The HTTP header fields of the response.
54+
public var httpHeaderFields: HTTPTypes.HTTPFields
55+
56+
/// The body of the HTTP response.
57+
public var httpBody: OpenAPIRuntime.HTTPBody?
58+
5059
/// Creates a new error.
5160
/// - Parameters:
5261
/// - operationID: The OpenAPI operation identifier.
@@ -68,6 +77,62 @@ public struct ServerError: Error {
6877
operationOutput: (any Sendable)? = nil,
6978
causeDescription: String,
7079
underlyingError: any Error
80+
) {
81+
let httpStatus: HTTPResponse.Status
82+
let httpHeaderFields: HTTPTypes.HTTPFields
83+
let httpBody: OpenAPIRuntime.HTTPBody?
84+
if let httpConvertibleError = underlyingError as? (any HTTPResponseConvertible) {
85+
httpStatus = httpConvertibleError.httpStatus
86+
httpHeaderFields = httpConvertibleError.httpHeaderFields
87+
httpBody = httpConvertibleError.httpBody
88+
} else {
89+
httpStatus = .internalServerError
90+
httpHeaderFields = [:]
91+
httpBody = nil
92+
}
93+
94+
self.init(
95+
operationID: operationID,
96+
request: request,
97+
requestBody: requestBody,
98+
requestMetadata: requestMetadata,
99+
operationInput: operationInput,
100+
operationOutput: operationOutput,
101+
causeDescription: causeDescription,
102+
underlyingError: underlyingError,
103+
httpStatus: httpStatus,
104+
httpHeaderFields: httpHeaderFields,
105+
httpBody: httpBody
106+
)
107+
}
108+
109+
/// Creates a new error.
110+
/// - Parameters:
111+
/// - operationID: The OpenAPI operation identifier.
112+
/// - request: The HTTP request provided to the server.
113+
/// - requestBody: The HTTP request body provided to the server.
114+
/// - requestMetadata: The request metadata extracted by the server.
115+
/// - operationInput: An operation-specific Input value.
116+
/// - operationOutput: An operation-specific Output value.
117+
/// - causeDescription: A user-facing description of what caused
118+
/// the underlying error to be thrown.
119+
/// - underlyingError: The underlying error that caused the operation
120+
/// to fail.
121+
/// - httpStatus: An HTTP status to return in the response.
122+
/// - httpHeaderFields: The HTTP header fields of the response.
123+
/// - httpBody: The body of the HTTP response.
124+
public init(
125+
operationID: String,
126+
request: HTTPRequest,
127+
requestBody: HTTPBody?,
128+
requestMetadata: ServerRequestMetadata,
129+
operationInput: (any Sendable)? = nil,
130+
operationOutput: (any Sendable)? = nil,
131+
causeDescription: String,
132+
underlyingError: any Error,
133+
httpStatus: HTTPResponse.Status,
134+
httpHeaderFields: HTTPTypes.HTTPFields,
135+
httpBody: OpenAPIRuntime.HTTPBody?
71136
) {
72137
self.operationID = operationID
73138
self.request = request
@@ -77,6 +142,9 @@ public struct ServerError: Error {
77142
self.operationOutput = operationOutput
78143
self.causeDescription = causeDescription
79144
self.underlyingError = underlyingError
145+
self.httpStatus = httpStatus
146+
self.httpHeaderFields = httpHeaderFields
147+
self.httpBody = httpBody
80148
}
81149

82150
// MARK: Private

‎Sources/OpenAPIRuntime/Interface/ErrorHandlingMiddleware.swift‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,10 @@ public struct ErrorHandlingMiddleware: ServerMiddleware {
5757
async throws -> (HTTPTypes.HTTPResponse, OpenAPIRuntime.HTTPBody?)
5858
) async throws -> (HTTPTypes.HTTPResponse, OpenAPIRuntime.HTTPBody?) {
5959
do { return try await next(request, body, metadata) } catch {
60-
if let serverError = error as? ServerError,
61-
let appError = serverError.underlyingError as? (any HTTPResponseConvertible)
62-
{
60+
if let serverError = error as? ServerError {
6361
return (
64-
HTTPResponse(status: appError.httpStatus, headerFields: appError.httpHeaderFields),
65-
appError.httpBody
62+
HTTPResponse(status: serverError.httpStatus, headerFields: serverError.httpHeaderFields),
63+
serverError.httpBody
6664
)
6765
} else {
6866
return (HTTPResponse(status: .internalServerError), nil)

‎Sources/OpenAPIRuntime/Interface/UniversalServer.swift‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,23 @@ import struct Foundation.URLComponents
119119
causeDescription = "Unknown"
120120
underlyingError = error
121121
}
122+
123+
let httpStatus: HTTPResponse.Status
124+
let httpHeaderFields: HTTPTypes.HTTPFields
125+
let httpBody: OpenAPIRuntime.HTTPBody?
126+
if let httpConvertibleError = underlyingError as? (any HTTPResponseConvertible) {
127+
httpStatus = httpConvertibleError.httpStatus
128+
httpHeaderFields = httpConvertibleError.httpHeaderFields
129+
httpBody = httpConvertibleError.httpBody
130+
} else if let httpConvertibleError = error as? (any HTTPResponseConvertible) {
131+
httpStatus = httpConvertibleError.httpStatus
132+
httpHeaderFields = httpConvertibleError.httpHeaderFields
133+
httpBody = httpConvertibleError.httpBody
134+
} else {
135+
httpStatus = .internalServerError
136+
httpHeaderFields = [:]
137+
httpBody = nil
138+
}
122139
return ServerError(
123140
operationID: operationID,
124141
request: request,
@@ -127,13 +144,18 @@ import struct Foundation.URLComponents
127144
operationInput: input,
128145
operationOutput: output,
129146
causeDescription: causeDescription,
130-
underlyingError: underlyingError
147+
underlyingError: underlyingError,
148+
httpStatus: httpStatus,
149+
httpHeaderFields: httpHeaderFields,
150+
httpBody: httpBody
131151
)
132152
}
133153
var next: @Sendable (HTTPRequest, HTTPBody?, ServerRequestMetadata) async throws -> (HTTPResponse, HTTPBody?) =
134154
{ _request, _requestBody, _metadata in
135155
let input: OperationInput = try await wrappingErrors {
136-
try await deserializer(_request, _requestBody, _metadata)
156+
do { return try await deserializer(_request, _requestBody, _metadata) } catch let decodingError
157+
as DecodingError
158+
{ throw RuntimeError.failedToParseRequest(decodingError) }
137159
} mapError: { error in
138160
makeError(error: error)
139161
}

‎Tests/OpenAPIRuntimeTests/Errors/Test_ClientError.swift‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ final class Test_ServerError: XCTestCase {
2525
requestBody: nil,
2626
requestMetadata: .init(),
2727
causeDescription: upstreamError.prettyDescription,
28-
underlyingError: upstreamError.underlyingError ?? upstreamError
28+
underlyingError: upstreamError.underlyingError ?? upstreamError,
29+
httpStatus: .internalServerError,
30+
httpHeaderFields: [:],
31+
httpBody: nil
2932
)
3033
XCTAssertEqual(
3134
"\(error)",

‎Tests/OpenAPIRuntimeTests/Interface/Test_UniversalServer.swift‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,37 @@ final class Test_UniversalServer: Test_Runtime {
101101
}
102102
}
103103

104+
func testErrorPropagation_deserializerWithDecodingError() async throws {
105+
let decodingError = DecodingError.dataCorrupted(
106+
.init(codingPath: [], debugDescription: "Invalid request body.")
107+
)
108+
do {
109+
let server = UniversalServer(handler: MockHandler())
110+
_ = try await server.handle(
111+
request: .init(soar_path: "/", method: .post),
112+
requestBody: MockHandler.requestBody,
113+
metadata: .init(),
114+
forOperation: "op",
115+
using: { MockHandler.greet($0) },
116+
deserializer: { request, body, metadata in throw decodingError },
117+
serializer: { output, _ in fatalError() }
118+
)
119+
} catch {
120+
let serverError = try XCTUnwrap(error as? ServerError)
121+
XCTAssertEqual(serverError.operationID, "op")
122+
XCTAssert(serverError.causeDescription.contains("An error occurred while attempting to parse the request"))
123+
XCTAssert(serverError.underlyingError is DecodingError)
124+
XCTAssertEqual(serverError.httpStatus, .badRequest)
125+
XCTAssertEqual(serverError.httpHeaderFields, [:])
126+
XCTAssertNil(serverError.httpBody)
127+
XCTAssertEqual(serverError.request, .init(soar_path: "/", method: .post))
128+
XCTAssertEqual(serverError.requestBody, MockHandler.requestBody)
129+
XCTAssertEqual(serverError.requestMetadata, .init())
130+
XCTAssertNil(serverError.operationInput)
131+
XCTAssertNil(serverError.operationOutput)
132+
}
133+
}
134+
104135
func testErrorPropagation_handler() async throws {
105136
do {
106137
let server = UniversalServer(handler: MockHandler(shouldFail: true))

0 commit comments

Comments
 (0)