Make HTTP handler non-throwing. Application should do error handling (#…

…462)

* Make HTTP responder non-throwing

* Move HTTPError to Hummingbird

* remove commented out code

* Log unrecognised errors

* Rework logger message

Sources/Hummingbird/Application.swift

@@ -115,6 +115,7 @@ extension ApplicationProtocol {
                     response = httpError.response(allocator: channel.allocator)
                 default:
                     // this error has not been recognised
+                    context.logger.debug("Unrecognised Error", metadata: ["error": "\(error)"])
                     response = Response(
                         status: .internalServerError,
                         body: .init()

Sources/Hummingbird/Deprecations.swift

@@ -97,3 +97,8 @@ public typealias HBMemoryPersistDriver = MemoryPersistDriver
 public typealias HBPersistDriver = PersistDriver
 @_documentation(visibility: internal) @available(*, deprecated, renamed: "PersistError")
 public typealias HBPersistError = PersistError
+
+@_documentation(visibility: internal) @available(*, deprecated, renamed: "HTTPError")
+public typealias HBHTTPError = HTTPError
+@_documentation(visibility: internal) @available(*, deprecated, renamed: "HTTPResponseError")
+public typealias HBHTTPResponseError = HTTPResponseError

Sources/Hummingbird/Exports.swift

@@ -13,8 +13,6 @@
 //===----------------------------------------------------------------------===//
 
 @_exported import struct HummingbirdCore.BindAddress
-@_exported import struct HummingbirdCore.HTTPError
-@_exported import protocol HummingbirdCore.HTTPResponseError
 @_exported import struct HummingbirdCore.Request
 @_exported import struct HummingbirdCore.RequestBody
 @_exported import struct HummingbirdCore.Response
@@ -32,8 +30,6 @@
 @_exported @_documentation(visibility: internal) import struct HTTPTypes.HTTPResponse
 
 // Temporary exports or deprecated typealiases
-@_exported import struct HummingbirdCore.HBHTTPError
-@_exported import protocol HummingbirdCore.HBHTTPResponseError
 @_exported import struct HummingbirdCore.HBRequest
 @_exported import struct HummingbirdCore.HBRequestBody
 @_exported import struct HummingbirdCore.HBResponse

Sources/HummingbirdCore/Deprecations.swift

@@ -28,10 +28,6 @@ public typealias HBResponse = Response
 public typealias HBResponseBody = ResponseBody
 @_documentation(visibility: internal) @available(*, deprecated, renamed: "ResponseBodyWriter")
 public typealias HBResponseBodyWriter = ResponseBodyWriter
-@_documentation(visibility: internal) @available(*, deprecated, renamed: "HTTPError")
-public typealias HBHTTPError = HTTPError
-@_documentation(visibility: internal) @available(*, deprecated, renamed: "HTTPResponseError")
-public typealias HBHTTPResponseError = HTTPResponseError
 @_documentation(visibility: internal) @available(*, deprecated, renamed: "Server")
 public typealias HBServer = Server
 @_documentation(visibility: internal) @available(*, deprecated, renamed: "ServerConfiguration")

Sources/HummingbirdCore/Server/HTTP/HTTP1Channel.swift

@@ -27,7 +27,7 @@ public struct HTTP1Channel: ServerChildChannel, HTTPChannelHandler {
     ///   - responder: Function returning a HTTP response for a HTTP request
     ///   - additionalChannelHandlers: Additional channel handlers to add to channel pipeline
     public init(
-        responder: @escaping @Sendable (Request, Channel) async throws -> Response,
+        responder: @escaping HTTPChannelHandler.Responder,
         additionalChannelHandlers: @escaping @Sendable () -> [any RemovableChannelHandler] = { [] }
     ) {
         self.additionalChannelHandlers = additionalChannelHandlers
@@ -66,7 +66,7 @@ public struct HTTP1Channel: ServerChildChannel, HTTPChannelHandler {
         await handleHTTP(asyncChannel: asyncChannel, logger: logger)
     }
 
-    public let responder: @Sendable (Request, Channel) async throws -> Response
+    public let responder: HTTPChannelHandler.Responder
     let additionalChannelHandlers: @Sendable () -> [any RemovableChannelHandler]
 }
 

Sources/HummingbirdCore/Server/HTTP/HTTPChannelHandler.swift

@@ -21,7 +21,7 @@ import ServiceLifecycle
 
 /// Protocol for HTTP channels
 public protocol HTTPChannelHandler: ServerChildChannel {
-    typealias Responder = @Sendable (Request, Channel) async throws -> Response
+    typealias Responder = @Sendable (Request, Channel) async -> Response
     var responder: Responder { get }
 }
 
@@ -60,12 +60,7 @@ extension HTTPChannelHandler {
 
                             let bodyStream = NIOAsyncChannelRequestBody(iterator: iterator)
                             let request = Request(head: head, body: .init(asyncSequence: bodyStream))
-                            let response: Response
-                            do {
-                                response = try await self.responder(request, asyncChannel.channel)
-                            } catch {
-                                response = self.getErrorResponse(from: error, allocator: asyncChannel.channel.allocator)
-                            }
+                            let response: Response = await self.responder(request, asyncChannel.channel)
                             do {
                                 try await outbound.write(.head(response.head))
                                 let tailHeaders = try await response.body.write(responseWriter)
@@ -116,20 +111,6 @@ extension HTTPChannelHandler {
             logger.trace("Failed to read/write to Channel. Error: \(error)")
         }
     }
-
-    func getErrorResponse(from error: Error, allocator: ByteBufferAllocator) -> Response {
-        switch error {
-        case let httpError as HTTPResponseError:
-            // this is a processed error so don't log as Error
-            return httpError.response(allocator: allocator)
-        default:
-            // this error has not been recognised
-            return Response(
-                status: .internalServerError,
-                body: .init()
-            )
-        }
-    }
 }
 
 /// Writes ByteBuffers to AsyncChannel outbound writer

Sources/HummingbirdHTTP2/HTTP2Channel.swift

@@ -33,7 +33,7 @@ public struct HTTP2UpgradeChannel: HTTPChannelHandler {
     private let sslContext: NIOSSLContext
     private let http1: HTTP1Channel
     private let additionalChannelHandlers: @Sendable () -> [any RemovableChannelHandler]
-    public var responder: @Sendable (Request, Channel) async throws -> Response { http1.responder }
+    public var responder: HTTPChannelHandler.Responder { self.http1.responder }
 
     ///  Initialize HTTP1Channel
     /// - Parameters:
@@ -43,7 +43,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(.notFound) }
+        responder: @escaping HTTPChannelHandler.Responder
     ) throws {
         var tlsConfiguration = tlsConfiguration
         tlsConfiguration.applicationProtocols = NIOHTTP2SupportedALPNProtocols

Sources/HummingbirdTLS/TLSChannel.swift

@@ -58,8 +58,8 @@ public struct TLSChannel<BaseChannel: ServerChildChannel>: ServerChildChannel {
 }
 
 extension TLSChannel: HTTPChannelHandler where BaseChannel: HTTPChannelHandler {
-    public var responder: @Sendable (Request, Channel) async throws -> Response {
-        baseChannel.responder
+    public var responder: HTTPChannelHandler.Responder {
+        self.baseChannel.responder
     }
 }
 

Tests/HummingbirdCoreTests/CoreTests.swift

@@ -70,7 +70,7 @@ class HummingBirdCoreTests: XCTestCase {
 
     func testError() async throws {
         try await testServer(
-            responder: { _, _ in throw HTTPError(.unauthorized) },
+            responder: { _, _ in .init(status: .unauthorized) },
             httpChannelSetup: .http1(),
             configuration: .init(address: .hostname(port: 0)),
             eventLoopGroup: Self.eventLoopGroup,
@@ -85,8 +85,12 @@ class HummingBirdCoreTests: XCTestCase {
     func testConsumeBody() async throws {
         try await testServer(
             responder: { request, _ in
-                let buffer = try await request.body.collect(upTo: .max)
-                return Response(status: .ok, body: .init(byteBuffer: buffer))
+                do {
+                    let buffer = try await request.body.collect(upTo: .max)
+                    return Response(status: .ok, body: .init(byteBuffer: buffer))
+                } catch {
+                    return Response(status: .contentTooLarge)
+                }
             },
             configuration: .init(address: .hostname(port: 0)),
             eventLoopGroup: Self.eventLoopGroup,
@@ -190,20 +194,27 @@ class HummingBirdCoreTests: XCTestCase {
     }
 
     func testChannelHandlerErrorPropagation() async throws {
+        struct TestChannelHandlerError: Error {}
         class CreateErrorHandler: ChannelInboundHandler, RemovableChannelHandler {
             typealias InboundIn = HTTPRequestPart
 
             var seen: Bool = false
             func channelRead(context: ChannelHandlerContext, data: NIOAny) {
                 if case .body = self.unwrapInboundIn(data) {
-                    context.fireErrorCaught(HTTPError(.unavailableForLegalReasons))
+                    context.fireErrorCaught(TestChannelHandlerError())
                 }
                 context.fireChannelRead(data)
             }
         }
         try await testServer(
             responder: { request, _ in
-                _ = try await request.body.collect(upTo: .max)
+                do {
+                    _ = try await request.body.collect(upTo: .max)
+                } catch is TestChannelHandlerError {
+                    return Response(status: .unavailableForLegalReasons)
+                } catch {
+                    return Response(status: .contentTooLarge)
+                }
                 return Response(status: .ok)
             },
             httpChannelSetup: .http1(additionalChannelHandlers: [CreateErrorHandler()]),
@@ -269,7 +280,11 @@ class HummingBirdCoreTests: XCTestCase {
         }
         try await testServer(
             responder: { request, _ in
-                _ = try await request.body.collect(upTo: .max)
+                do {
+                    _ = try await request.body.collect(upTo: .max)
+                } catch {
+                    return Response(status: .contentTooLarge)
+                }
                 return .init(status: .ok)
             },
             httpChannelSetup: .http1(additionalChannelHandlers: [HTTPServerIncompleteRequest(), IdleStateHandler(readTimeout: .seconds(1))]),
@@ -292,7 +307,11 @@ class HummingBirdCoreTests: XCTestCase {
     func testWriteIdleTimeout() async throws {
         try await testServer(
             responder: { request, _ in
-                _ = try await request.body.collect(upTo: .max)
+                do {
+                    _ = try await request.body.collect(upTo: .max)
+                } catch {
+                    return Response(status: .contentTooLarge)
+                }
                 return .init(status: .ok)
             },
             httpChannelSetup: .http1(additionalChannelHandlers: [IdleStateHandler(writeTimeout: .seconds(1))]),
@@ -320,7 +339,7 @@ class HummingBirdCoreTests: XCTestCase {
                 logger: logger
             ) { request, _ in
                 await handlerPromise.complete(())
-                try await Task.sleep(for: .milliseconds(500))
+                try? await Task.sleep(for: .milliseconds(500))
                 return Response(status: .ok, body: .init(asyncSequence: request.body.delayed()))
             } onServerRunning: {
                 await portPromise.complete($0.localAddress!.port!)