Merge pull request #15 from brokenhandsio/vapor4

Vapor 4

.github/workflows/ci.yml

@@ -4,23 +4,23 @@ on:
 jobs:
   xenial:
     container: 
-      image: vapor/swift:5.1-xenial
+      image: vapor/swift:5.2-xenial
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@master
     - run: swift test --enable-test-discovery --enable-code-coverage
   bionic:
     container: 
-      image: vapor/swift:5.1-bionic
+      image: vapor/swift:5.2-bionic
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@master
     - name: Run Bionic Tests
       run: swift test --enable-test-discovery --enable-code-coverage
     - name: Setup container for codecov upload
-      run: apt-get update && apt-get install curl
+      run: apt-get update && apt-get install -y curl
     - name: Process coverage file
-      run: llvm-cov show .build/x86_64-unknown-linux/debug/LeafErrorMiddlewarePackageTests.xctest -instr-profile=.build/x86_64-unknown-linux/debug/codecov/default.profdata > coverage.txt
+      run: llvm-cov show .build/x86_64-unknown-linux-gnu/debug/LeafErrorMiddlewarePackageTests.xctest -instr-profile=.build/debug/codecov/default.profdata > coverage.txt
     - name: Upload code coverage
       uses: codecov/codecov-action@v1
       with:

Package.swift

@@ -1,16 +1,21 @@
-// swift-tools-version:4.0
+// swift-tools-version:5.2
 import PackageDescription
 
 let package = Package(
     name: "LeafErrorMiddleware",
+    platforms: [
+       .macOS(.v10_15),
+    ],
     products: [
         .library(name: "LeafErrorMiddleware", targets: ["LeafErrorMiddleware"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/vapor/vapor.git", from: "3.0.0"),
+        .package(url: "https://github.com/vapor/vapor.git", from: "4.0.0-rc"),
     ],
     targets: [
-        .target(name: "LeafErrorMiddleware", dependencies: ["Vapor"]),
+        .target(name: "LeafErrorMiddleware", dependencies: [
+            .product(name: "Vapor", package: "vapor"),
+        ]),
         .testTarget(name: "LeafErrorMiddlewareTests", dependencies: ["LeafErrorMiddleware"]),
     ]
 )

README.md

@@ -3,7 +3,7 @@
     <br>
     <br>
     <a href="https://swift.org">
-        <img src="http://img.shields.io/badge/Swift-4.1-brightgreen.svg" alt="Language">
+        <img src="http://img.shields.io/badge/Swift-5.2-brightgreen.svg" alt="Language">
     </a>
     <a href="https://github.com/brokenhandsio/leaf-error-middleware/actions">
         <img src="https://github.com/brokenhandsio/leaf-error-middleware/workflows/CI/badge.svg?branch=master" alt="Build Status">
@@ -27,35 +27,21 @@ First, add LeafErrorMiddleware as a dependency in your `Package.swift` file:
 ```swift
 dependencies: [
     // ...,
-    .package(url: "https://github.com/brokenhandsio/leaf-error-middleware.git", from: "1.0.0")
+    .package(name: "LeafErrorMiddleware", url: "https://github.com/brokenhandsio/leaf-error-middleware.git", from: "2.0.0")
 ],
 targets: [
     .target(name: "App", dependencies: ["Vapor", ..., "LeafErrorMiddleware"]),
     // ...
 ]
 ```
 
-To use the LeafErrorMiddleware, register the middleware service in `configure.swift` (make sure you `import LeafErrorMiddleware` at the top):
+To use the LeafErrorMiddleware, register the middleware service in `configure.swift` to your `Application`'s middleware (make sure you `import LeafErrorMiddleware` at the top):
 
 ```swift
-// You must set the preferred renderer:
-config.prefer(LeafRenderer.self, for: ViewRenderer.self)
-
-services.register { worker in
-    return try LeafErrorMiddleware(environment: worker.environment)
-}
-```
-
-Then add it to your `MiddlewareConfig`:
-
-```swift
-var middlewares = MiddlewareConfig()
-middlewares.use(LeafErrorMiddleware.self)
-// ...
-services.register(middlewares)
+app.middleware.use(LeafErrorMiddleware())
 ```
 
-This replaces the default error middleware in Vapor, so ***do not*** add the default `ErrorMiddleware` to your `MiddlewareConfig`.
+Make sure it appears before all other middleware to catch errors.
 
 # Setting Up
 
@@ -64,7 +50,10 @@ You need to include two [Leaf](https://github.com/vapor/leaf) templates in your
 * `404.leaf`
 * `serverError.leaf`
 
-When Leaf Error Middleware catches a 404 error, it will return the `404.leaf` template. Any other error caught will return the `serverError.leaf` template. The `serverError.leaf` template will be passed two parameters:
+When Leaf Error Middleware catches a 404 error, it will return the `404.leaf` template. Any other error caught will return the `serverError.leaf` template. The `serverError.leaf` template will be passed up to three parameters in its context:
 
 * `status` - the status code of the error caught
 * `statusMessage` - a reason for the status code
+* `reason` - the reason for the error, if known. Otherwise this won't be passed in.
+
+The `404.leaf` template will get a `reason` parameter in the context if one is known.

Sources/LeafErrorMiddleware/LeafErrorMiddleware.swift

@@ -1,94 +1,78 @@
 import Vapor
 
 /// Captures all errors and transforms them into an internal server error.
-public final class LeafErrorMiddleware: Middleware, Service {
-    /// The environment to respect when presenting errors.
-    let environment: Environment
-
-    /// Create a new ErrorMiddleware for the supplied environment.
-    public init(environment: Environment) {
-        self.environment = environment
-    }
-
+public final class LeafErrorMiddleware: Middleware {
+
+    public init() {}
+
     /// See `Middleware.respond`
-    public func respond(to req: Request, chainingTo next: Responder) throws -> Future<Response> {
-        do {
-            return try next.respond(to: req).flatMap(to: Response.self) { res in
-                if res.http.status.code >= HTTPResponseStatus.badRequest.code {
-                    return try self.handleError(for: req, status: res.http.status)
-                } else {
-                    return try res.encode(for: req)
-                }
-            }.catchFlatMap { error in
-                try? req.make(Logger.self).report(error: error, verbose: true)
-                switch (error) {
-                case let abort as AbortError:
-                    guard
-                        abort.status.representsError
+    public func respond(to request: Request, chainingTo next: Responder) -> EventLoopFuture<Response> {
+        return next.respond(to: request).flatMap { res in
+            if res.status.code >= HTTPResponseStatus.badRequest.code {
+                return self.handleError(for: request, status: res.status, error: Abort(res.status))
+            } else {
+                return res.encodeResponse(for: request)
+            }
+        }.flatMapError { error in
+            request.logger.report(error: error)
+            switch (error) {
+            case let abort as AbortError:
+                guard
+                    abort.status.representsError
                     else {
                         if let location = abort.headers[.location].first {
-                            return req.future(req.redirect(to: location))
+                            return request.eventLoop.future(request.redirect(to: location))
                         } else {
-                            return try self.handleError(for: req, status: abort.status)
+                            return self.handleError(for: request, status: abort.status, error: error)
                         }
-                    }
-                    return try self.handleError(for: req, status: abort.status)
-                default:
-                    return try self.handleError(for: req, status: .internalServerError)
                 }
+                return self.handleError(for: request, status: abort.status, error: error)
+            default:
+                return self.handleError(for: request, status: .internalServerError, error: error)
             }
-        } catch {
-            return try handleError(for: req, status: HTTPStatus(error))
         }
     }
-
-    private func handleError(for req: Request, status: HTTPStatus) throws -> Future<Response> {
-        let renderer = try req.make(ViewRenderer.self)
-
+
+    private func handleError(for req: Request, status: HTTPStatus, error: Error) -> EventLoopFuture<Response> {
         if status == .notFound {
-            return try renderer.render("404").encode(for: req).map(to: Response.self) { res in
-                res.http.status = status
+            var parameters = [String:String]()
+            if let abortError = error as? AbortError {
+                parameters["reason"] = abortError.reason
+            }
+            return req.view.render("404", parameters).encodeResponse(for: req).map { res in
+                res.status = status
                 return res
-            }.catchFlatMap { _ in
-                return try self.renderServerErrorPage(for: status, request: req, renderer: renderer)
+            }.flatMapError { newError in
+                return self.renderServerErrorPage(for: status, request: req, error: newError)
             }
         }
-
-        return try renderServerErrorPage(for: status, request: req, renderer: renderer)
+        
+        return renderServerErrorPage(for: status, request: req, error: error)
     }
-
-    private func renderServerErrorPage(for status: HTTPStatus, request: Request, renderer: ViewRenderer) throws -> Future<Response> {
-        let parameters: [String:String] = [
+    
+    private func renderServerErrorPage(for status: HTTPStatus, request: Request, error: Error) -> EventLoopFuture<Response> {
+        var parameters: [String:String] = [
             "status": status.code.description,
             "statusMessage": status.reasonPhrase
         ]
-
-        let logger = try request.make(Logger.self)
-        logger.error("Internal server error. Status: \(status.code) - path: \(request.http.url)")
-
-        return try renderer.render("serverError", parameters).encode(for: request).map(to: Response.self) { res in
-            res.http.status = status
-            return res
-            }.catchFlatMap { error -> Future<Response> in
-                let body = "<h1>Internal Error</h1><p>There was an internal error. Please try again later.</p>"
-                let logger = try request.make(Logger.self)
-                logger.error("Failed to render custom error page - \(error)")
-                return try body.encode(for: request)
-                    .map(to: Response.self) { res in
-                        res.http.status = status
-                        res.http.headers.replaceOrAdd(name: .contentType, value: "text/html; charset=utf-8")
-                        return res
-                }
+
+        if let abortError = error as? AbortError {
+            parameters["reason"] = abortError.reason
         }
-    }
-}
-
-extension HTTPStatus {
-    internal init(_ error: Error) {
-        if let abort = error as? AbortError {
-            self = abort.status
-        } else {
-            self = .internalServerError
+
+        request.logger.error("Internal server error. Status: \(status.code) - path: \(request.url)")
+
+        return request.view.render("serverError", parameters).encodeResponse(for: request).map { res in
+            res.status = status
+            return res
+        }.flatMapError { error -> EventLoopFuture<Response> in
+            let body = "<h1>Internal Error</h1><p>There was an internal error. Please try again later.</p>"
+            request.logger.error("Failed to render custom error page - \(error)")
+            return body.encodeResponse(for: request).map { res in
+                res.status = status
+                res.headers.replaceOrAdd(name: .contentType, value: "text/html; charset=utf-8")
+                return res
+            }
         }
     }
 }

Tests/LeafErrorMiddlewareTests/Fakes/CapturingLogger.swift

@@ -1,14 +1,23 @@
 import Vapor
 
-class CapturingLogger: Logger, Service {
-
-    var enabled: [LogLevel] = []
+class CapturingLogger: LogHandler {
+
+    subscript(metadataKey key: String) -> Logger.Metadata.Value? {
+        get { return self.metadata[key] }
+        set { self.metadata[key] = newValue }
+    }
+
+    var metadata: Logger.Metadata = [:]
+
+    var logLevel: Logger.Level = .trace
+
+    var enabled: [Logger.Level] = []
 
     private(set) var message: String?
-    private(set) var logLevel: LogLevel?
-
-    func log(_ string: String, at level: LogLevel, file: String, function: String, line: UInt, column: UInt) {
-        self.message = string
-        self.logLevel = level
+    private(set) var logLevelUsed: Logger.Level?
+    
+    func log(level: Logger.Level, message: Logger.Message, metadata: Logger.Metadata?, file: String, function: String, line: UInt) {
+        self.message = message.description
+        self.logLevelUsed = level
     }
 }

Tests/LeafErrorMiddlewareTests/Fakes/ThrowingViewRenderer.swift

@@ -1,23 +1,30 @@
 import Vapor
 
-class ThrowingViewRenderer: ViewRenderer, Service {
+class ThrowingViewRenderer: ViewRenderer {
 
     var shouldCache = false
-    var worker: Worker
+    var eventLoop: EventLoop
     var shouldThrow = false
 
-    init(worker: Worker) {
-        self.worker = worker
+    init(eventLoop: EventLoop) {
+        self.eventLoop = eventLoop
     }
 
     private(set) var capturedContext: Encodable? = nil
     private(set) var leafPath: String? = nil
-    func render<E>(_ path: String, _ context: E, userInfo: [AnyHashable : Any]) -> EventLoopFuture<View> where E : Encodable {
+    func render<E>(_ name: String, _ context: E) -> EventLoopFuture<View> where E : Encodable {
         self.capturedContext = context
-        self.leafPath = path
+        self.leafPath = name
         if shouldThrow {
-            return Future.map(on: worker) { throw TestError() }
+            return self.eventLoop.makeFailedFuture(TestError())
         }
-        return Future.map(on: worker) { return View(data: "Test".convertToData()) }
+        let response = "Test"
+        var byteBuffer = ByteBufferAllocator().buffer(capacity: response.count)
+        byteBuffer.writeString(response)
+        return self.eventLoop.future(View(data: byteBuffer))
+    }
+
+    func `for`(_ request: Request) -> ViewRenderer {
+        return self
     }
 }