From b487c9543affbb71cf28496e0c73023368d26781 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Sat, 4 May 2024 06:49:39 +0100 Subject: [PATCH] Error messages (#431) --- .../Middleware/FileMiddleware.swift | 4 ++-- .../Hummingbird/Router/Parameters+UUID.swift | 10 +++++---- Sources/Hummingbird/Router/Parameters.swift | 22 +++++++++++-------- Sources/HummingbirdCore/Error/HTTPError.swift | 22 ++++++++++++++----- Sources/HummingbirdHTTP2/HTTP2Channel.swift | 2 +- .../MiddlewareTests.swift | 11 +++++++++- Tests/HummingbirdTests/ApplicationTests.swift | 22 +++++++++++++++++++ Tests/HummingbirdTests/MiddlewareTests.swift | 11 +++++++++- 8 files changed, 80 insertions(+), 24 deletions(-) diff --git a/Sources/Hummingbird/Middleware/FileMiddleware.swift b/Sources/Hummingbird/Middleware/FileMiddleware.swift index 7f9a8d1b5..d3a885611 100644 --- a/Sources/Hummingbird/Middleware/FileMiddleware.swift +++ b/Sources/Hummingbird/Middleware/FileMiddleware.swift @@ -96,7 +96,7 @@ public struct FileMiddleware UUID { - guard let param = self[s[...]], - let result = UUID(uuidString: String(param)) + guard let param = self[s[...]] else { + throw HTTPError(.badRequest, message: "Expected parameter does not exist") + } + guard let result = UUID(uuidString: String(param)) else { - throw HTTPError(.badRequest) + throw HTTPError(.badRequest, message: "Parameter '\(param)' can not be converted to the expected type (UUID)") } return result } @@ -53,7 +55,7 @@ extension Parameters { public func requireAll(_ s: String, as: UUID.Type) throws -> [UUID] { return try self[values: s[...]].map { guard let result = UUID(uuidString: String($0)) else { - throw HTTPError(.badRequest) + throw HTTPError(.badRequest, message: "One of the parameters '\($0)' can not be converted to the expected type (UUID)") } return result } diff --git a/Sources/Hummingbird/Router/Parameters.swift b/Sources/Hummingbird/Router/Parameters.swift index 840c784bc..dc9353e9f 100644 --- a/Sources/Hummingbird/Router/Parameters.swift +++ b/Sources/Hummingbird/Router/Parameters.swift @@ -46,7 +46,7 @@ public extension Parameters { /// - Parameter s: parameter id func require(_ s: String) throws -> String { guard let param = self[s[...]].map({ String($0) }) else { - throw HTTPError(.badRequest) + throw HTTPError(.badRequest, message: "Expected parameter does not exist") } return param } @@ -56,10 +56,12 @@ public extension Parameters { /// - s: parameter id /// - as: type we want returned func require(_ s: String, as: T.Type) throws -> T { - guard let param = self[s[...]], - let result = T(String(param)) + guard let param = self[s[...]] else { + throw HTTPError(.badRequest, message: "Expected parameter does not exist") + } + guard let result = T(String(param)) else { - throw HTTPError(.badRequest) + throw HTTPError(.badRequest, message: "Parameter '\(param)' can not be converted to the expected type (\(T.self))") } return result } @@ -69,10 +71,12 @@ public extension Parameters { /// - s: parameter id /// - as: type we want returned func require(_ s: String, as: T.Type) throws -> T where T.RawValue == String { - guard let param = self[s[...]], - let result = T(rawValue: String(param)) + guard let param = self[s[...]] else { + throw HTTPError(.badRequest, message: "Expected parameter does not exist") + } + guard let result = T(rawValue: String(param)) else { - throw HTTPError(.badRequest) + throw HTTPError(.badRequest, message: "Parameter '\(param)' can not be converted to the expected type (\(T.self))") } return result } @@ -107,7 +111,7 @@ public extension Parameters { func requireAll(_ s: String, as: T.Type) throws -> [T] { return try self[values: s[...]].map { guard let result = T(String($0)) else { - throw HTTPError(.badRequest) + throw HTTPError(.badRequest, message: "One of the parameters '\($0)' can not be converted to the expected type (\(T.self))") } return result } @@ -120,7 +124,7 @@ public extension Parameters { func requireAll(_ s: String, as: T.Type) throws -> [T] where T.RawValue == String { return try self[values: s[...]].map { guard let result = T(rawValue: String($0)) else { - throw HTTPError(.badRequest) + throw HTTPError(.badRequest, message: "One of the parameters '\($0)' can not be converted to the expected type (\(T.self))") } return result } diff --git a/Sources/HummingbirdCore/Error/HTTPError.swift b/Sources/HummingbirdCore/Error/HTTPError.swift index 926758834..97a0e7dd2 100644 --- a/Sources/HummingbirdCore/Error/HTTPError.swift +++ b/Sources/HummingbirdCore/Error/HTTPError.swift @@ -19,9 +19,19 @@ import NIOCore public struct HTTPError: Error, HTTPResponseError, Sendable { /// status code for the error public var status: HTTPResponse.Status - /// any addiitional headers required - public var headers: HTTPFields - /// error payload, assumed to be a string + /// internal representation of error headers without contentType + private var _headers: HTTPFields + /// headers + public var headers: HTTPFields { + get { + return self.body != nil ? self._headers + [.contentType: "application/json; charset=utf-8"] : self._headers + } + set { + self._headers = newValue + } + } + + /// error message public var body: String? /// Initialize HTTPError @@ -29,7 +39,7 @@ public struct HTTPError: Error, HTTPResponseError, Sendable { /// - status: HTTP status public init(_ status: HTTPResponse.Status) { self.status = status - self.headers = [:] + self._headers = [:] self.body = nil } @@ -39,13 +49,13 @@ public struct HTTPError: Error, HTTPResponseError, Sendable { /// - message: Associated message public init(_ status: HTTPResponse.Status, message: String) { self.status = status - self.headers = [.contentType: "text/plain; charset=utf-8"] + self._headers = [:] self.body = message } /// Get body of error as ByteBuffer public func body(allocator: ByteBufferAllocator) -> ByteBuffer? { - return self.body.map { allocator.buffer(string: $0) } + return self.body.map { allocator.buffer(string: "{\"error\":{\"message\":\"\($0)\"}}\n") } } } diff --git a/Sources/HummingbirdHTTP2/HTTP2Channel.swift b/Sources/HummingbirdHTTP2/HTTP2Channel.swift index 355d007e6..76acc6898 100644 --- a/Sources/HummingbirdHTTP2/HTTP2Channel.swift +++ b/Sources/HummingbirdHTTP2/HTTP2Channel.swift @@ -40,7 +40,7 @@ public struct HTTP2UpgradeChannel: HTTPChannelHandler { public init( tlsConfiguration: TLSConfiguration, additionalChannelHandlers: @escaping @Sendable () -> [any RemovableChannelHandler] = { [] }, - responder: @escaping @Sendable (Request, Channel) async throws -> Response = { _, _ in throw HTTPError(.notImplemented) } + responder: @escaping @Sendable (Request, Channel) async throws -> Response = { _, _ in throw HTTPError(.notFound) } ) throws { var tlsConfiguration = tlsConfiguration tlsConfiguration.applicationProtocols = NIOHTTP2SupportedALPNProtocols diff --git a/Tests/HummingbirdRouterTests/MiddlewareTests.swift b/Tests/HummingbirdRouterTests/MiddlewareTests.swift index daf388d45..03c036ef2 100644 --- a/Tests/HummingbirdRouterTests/MiddlewareTests.swift +++ b/Tests/HummingbirdRouterTests/MiddlewareTests.swift @@ -98,6 +98,14 @@ final class MiddlewareTests: XCTestCase { } func testMiddlewareRunWhenNoRouteFound() async throws { + /// Error message returned by Hummingbird + struct ErrorMessage: Codable { + struct Details: Codable { + let message: String + } + + let error: Details + } struct TestMiddleware: RouterMiddleware { func handle(_ request: Request, context: Context, next: (Request, Context) async throws -> Response) async throws -> Response { do { @@ -114,8 +122,9 @@ final class MiddlewareTests: XCTestCase { try await app.test(.router) { client in try await client.execute(uri: "/hello", method: .get) { response in - XCTAssertEqual(String(buffer: response.body), "Edited error") XCTAssertEqual(response.status, .notFound) + let error = try JSONDecoder().decode(ErrorMessage.self, from: response.body) + XCTAssertEqual(error.error.message, "Edited error") } } } diff --git a/Tests/HummingbirdTests/ApplicationTests.swift b/Tests/HummingbirdTests/ApplicationTests.swift index efc4ecc80..c1fc2063e 100644 --- a/Tests/HummingbirdTests/ApplicationTests.swift +++ b/Tests/HummingbirdTests/ApplicationTests.swift @@ -186,6 +186,28 @@ final class ApplicationTests: XCTestCase { } } + func testErrorOutput() async throws { + /// Error message returned by Hummingbird + struct ErrorMessage: Codable { + struct Details: Codable { + let message: String + } + + let error: Details + } + let router = Router() + router.get("error") { _, _ -> HTTPResponse.Status in + throw HTTPError(.badRequest, message: "BAD!") + } + let app = Application(router: router) + try await app.test(.router) { client in + try await client.execute(uri: "/error", method: .get) { response in + let error = try JSONDecoder().decode(ErrorMessage.self, from: response.body) + XCTAssertEqual(error.error.message, "BAD!") + } + } + } + func testResponseBody() async throws { let router = Router() router diff --git a/Tests/HummingbirdTests/MiddlewareTests.swift b/Tests/HummingbirdTests/MiddlewareTests.swift index db0fd6daf..e7a6c162d 100644 --- a/Tests/HummingbirdTests/MiddlewareTests.swift +++ b/Tests/HummingbirdTests/MiddlewareTests.swift @@ -91,6 +91,14 @@ final class MiddlewareTests: XCTestCase { } func testMiddlewareRunWhenNoRouteFound() async throws { + /// Error message returned by Hummingbird + struct ErrorMessage: Codable { + struct Details: Codable { + let message: String + } + + let error: Details + } struct TestMiddleware: RouterMiddleware { public func handle(_ request: Request, context: Context, next: (Request, Context) async throws -> Response) async throws -> Response { do { @@ -106,8 +114,9 @@ final class MiddlewareTests: XCTestCase { try await app.test(.router) { client in try await client.execute(uri: "/hello", method: .get) { response in - XCTAssertEqual(String(buffer: response.body), "Edited error") XCTAssertEqual(response.status, .notFound) + let error = try JSONDecoder().decode(ErrorMessage.self, from: response.body) + XCTAssertEqual(error.error.message, "Edited error") } } }