Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log the request path of a failed request in the ErrorMiddleware. #2170

Merged
merged 4 commits into from Feb 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 {
tanner0101 marked this conversation as resolved.
Show resolved Hide resolved

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)
}
}