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 3 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,
path: String? = nil,
tanner0101 marked this conversation as resolved.
Show resolved Hide resolved
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 pathString: String
if let path = path {
pathString = " \(path)"
} else {
pathString = ""
}

let errorString = "\(debuggable.fullIdentifier):\(pathString) \(debuggable.reason)"
tanner0101 marked this conversation as resolved.
Show resolved Hide resolved
if let source = debuggable.sourceLocation {
error(errorString, file: source.file.lastPart, function: source.function, line: source.line, column: source.column)
} else {
Expand Down
4 changes: 3 additions & 1 deletion Sources/Vapor/Middleware/ErrorMiddleware.swift
Expand Up @@ -23,7 +23,9 @@ 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,
tanner0101 marked this conversation as resolved.
Show resolved Hide resolved
path: req.http.urlString,
verbose: !environment.isRelease)

// variables to determine
let status: HTTPResponseStatus
Expand Down
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: /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.get("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: .GET, url: "/fail/me"),
using: app
)

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

XCTAssert(loggerProvider.logger.didLog(string: "Abort.500: /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)
}
}