Error messages (#431)

hummingbird-project · May 4, 2024 · b487c95 · b487c95
1 parent 972f81d
commit b487c95
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 24 deletions.
diff --git a/Sources/Hummingbird/Middleware/FileMiddleware.swift b/Sources/Hummingbird/Middleware/FileMiddleware.swift
@@ -96,7 +96,7 @@ public struct FileMiddleware<Context: BaseRequestContext, Provider: FileProvider
 
             // Remove percent encoding from URI path
             guard let path = request.uri.path.removingPercentEncoding else {
-                throw HTTPError(.badRequest)
+                throw HTTPError(.badRequest, message: "Invalid percent encoding in URL")
             }
 
             // file paths that contain ".." are considered illegal
@@ -219,7 +219,7 @@ extension FileMiddleware {
 
         if let rangeHeader = request.headers[.range] {
             guard let range = getRangeFromHeaderValue(rangeHeader) else {
-                throw HTTPError(.rangeNotSatisfiable)
+                throw HTTPError(.rangeNotSatisfiable, message: "Unable to read range requested from file")
             }
             // range request conditional on etag or modified date being equal to value in if-range
             if let ifRange = request.headers[.ifRange], ifRange != headers[.eTag], ifRange != headers[.lastModified] {

diff --git a/Sources/Hummingbird/Router/Parameters+UUID.swift b/Sources/Hummingbird/Router/Parameters+UUID.swift
@@ -30,10 +30,12 @@ extension Parameters {
     ///   - s: parameter id
     ///   - as: type we want returned
     public func require(_ s: String, as: UUID.Type) throws -> 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
@@ -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<T: LosslessStringConvertible>(_ 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<T: RawRepresentable>(_ 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<T: LosslessStringConvertible>(_ 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<T: RawRepresentable>(_ 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
@@ -19,17 +19,27 @@ 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
     /// - Parameters:
     ///   - 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
@@ -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
@@ -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<Context: BaseRequestContext>: 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
@@ -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
@@ -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<Context: BaseRequestContext>: 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")
             }
         }
     }