Skip to content

Commit

Permalink
Log the request path of a failed request in the ErrorMiddleware. (#2170)
Browse files Browse the repository at this point in the history
* Log the request path in the ErrorMiddleware.

* Added logging tests.

* ...and allTests.

* Review changes.
  • Loading branch information
maciejtrybilo committed Feb 15, 2020
1 parent 82ca880 commit 642f3d4
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 3 deletions.
11 changes: 10 additions & 1 deletion Sources/Vapor/Logging/Logger+LogError.swift
Expand Up @@ -9,6 +9,7 @@ extension Logger {
/// Defaults to `true`.
public func report(
error e: Error,
request: Request? = nil,
verbose: Bool = true,
file: String = #file,
function: String = #function,
Expand All @@ -17,7 +18,15 @@ extension Logger {
) {
switch e {
case let debuggable as Debuggable:
let errorString = "\(debuggable.fullIdentifier): \(debuggable.reason)"

let requestString: String
if let request = request {
requestString = " \(request.http.method) \(request.http.urlString)"
} else {
requestString = ""
}

let errorString = "\(debuggable.fullIdentifier):\(requestString) \(debuggable.reason)"
if let source = debuggable.sourceLocation {
error(errorString, file: source.file.lastPart, function: source.function, line: source.line, column: source.column)
} else {
Expand Down
8 changes: 6 additions & 2 deletions Sources/Vapor/Middleware/ErrorMiddleware.swift
Expand Up @@ -23,8 +23,12 @@ public final class ErrorMiddleware: Middleware, ServiceType {
public static func `default`(environment: Environment, log: Logger) -> ErrorMiddleware {
return .init { req, error in
// log the error
log.report(error: error, verbose: !environment.isRelease)

log.report(
error: error,
request: req,
verbose: !environment.isRelease
)

// variables to determine
let status: HTTPResponseStatus
let reason: String
Expand Down
1 change: 1 addition & 0 deletions Tests/LinuxMain.swift
Expand Up @@ -7,6 +7,7 @@ XCTMain([
// Vapor
testCase(ApplicationTests.allTests),
testCase(MiddlewareTests.allTests),
testCase(LoggingTests.allTests)
])

#endif
64 changes: 64 additions & 0 deletions Tests/VaporTests/LoggingTests.swift
@@ -0,0 +1,64 @@
import Vapor
import XCTest

class LoggingTests : XCTestCase {

func testNotFoundLogging() throws {

var services = Services.default()
var config = Config.default()
let loggerProvider = TestLoggerProvider()

try services.register(loggerProvider)
config.prefer(TestLogger.self, for: Logger.self)

let app = try Application(config: config, services: services)

let req = Request(
http: HTTPRequest(method: .GET, url: "/hello/vapor"),
using: app
)

do {
_ = try app.make(Responder.self).respond(to: req).wait()
} catch {}

XCTAssert(loggerProvider.logger.didLog(string: "Abort.404: GET /hello/vapor Not Found"))
}

func testInternalServerErrorLogging() throws {

var services = Services.default()
var config = Config.default()
let loggerProvider = TestLoggerProvider()

try services.register(loggerProvider)
config.prefer(TestLogger.self, for: Logger.self)

let router = EngineRouter.default()

router.post("fail/me") { (_) -> String in
throw Abort(.internalServerError)
}

services.register(router, as: Router.self)

let app = try Application(config: config, services: services)

let req = Request(
http: HTTPRequest(method: .POST, url: "/fail/me"),
using: app
)

do {
_ = try app.make(Responder.self).respond(to: req).wait()
} catch {}

XCTAssert(loggerProvider.logger.didLog(string: "Abort.500: POST /fail/me Internal Server Error"))
}

static let allTests = [
("testNotFoundLogging", testNotFoundLogging),
("testInternalServerErrorLogging", testInternalServerErrorLogging)
]
}
33 changes: 33 additions & 0 deletions Tests/VaporTests/LoggingTestsMocks.swift
@@ -0,0 +1,33 @@
import Foundation
import Vapor

final class TestLogger: Logger {

var logs = [String]()

public func log(_ string: String, at level: LogLevel, file: String, function: String, line: UInt, column: UInt) {

logs += [string]
}

func didLog(string: String) -> Bool {
return logs.contains(string)
}
}

extension TestLogger: Service {}

final class TestLoggerProvider: Provider {

let logger = TestLogger()

func register(_ services: inout Services) throws {
services.register(Logger.self) { container -> TestLogger in
return self.logger
}
}

func didBoot(_ container: Container) throws -> EventLoopFuture<Void> {
return .done(on: container)
}
}

0 comments on commit 642f3d4

Please sign in to comment.