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

Improve error logging #2201

Merged
merged 3 commits into from
Mar 1, 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
32 changes: 32 additions & 0 deletions Sources/Development/routes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,41 @@ public func routes(_ app: Application) throws {
req.view.render("hello.txt", ["name": "world"])
}

app.get("error") { req -> String in
throw TestError()
}

app.get("secret") { (req) -> EventLoopFuture<String> in
return Environment
.secret(key: "PASSWORD_SECRET", fileIO: req.application.fileio, on: req.eventLoop)
.unwrap(or: Abort(.badRequest))
}
}

struct TestError: AbortError {
var status: HTTPResponseStatus {
.internalServerError
}

var reason: String {
"This is a test."
}

let file: String
let function: String
let line: Int

var source: ErrorSource? {
ErrorSource(file: self.file, function: self.function, line: self.line)
}

init(
file: String = #file,
function: String = #function,
line: Int = #line
) {
self.file = file
self.function = function
self.line = line
}
}
15 changes: 3 additions & 12 deletions Sources/Vapor/Error/Abort.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,12 @@ public struct Abort: AbortError {
public var function: String

/// The line where the error is thrown
public var line: UInt

/// The column where the errror is thrown
public var column: UInt
public var line: Int

/// Wrap this error's source location into a usable struct for
/// the `AbortError` protocol.
public var source: ErrorSource? {
return .init(file: self.file, line: self.line, function: self.function)
}

public var description: String {
return "Abort \(self.status.code): \(self.reason)"
ErrorSource(file: self.file, function: self.function, line: self.line)
}

/// Create a new `Abort`, capturing current source location info.
Expand All @@ -59,8 +52,7 @@ public struct Abort: AbortError {
suggestedFixes: [String] = [],
file: String = #file,
function: String = #function,
line: UInt = #line,
column: UInt = #column
line: Int = #line
) {
self.identifier = status.code.description
self.headers = headers
Expand All @@ -69,6 +61,5 @@ public struct Abort: AbortError {
self.file = file
self.function = function
self.line = line
self.column = column
}
}
8 changes: 6 additions & 2 deletions Sources/Vapor/Error/AbortError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ public protocol AbortError: LocalizedError, CustomStringConvertible {

public struct ErrorSource {
public let file: String
public let line: UInt
public let function: String
public let line: Int

public init(file: String = #file, line: UInt = #line, function: String = #function) {
public init(file: String = #file, function: String = #function, line: Int = #line) {
self.file = file
self.line = line
self.function = function
Expand All @@ -39,6 +39,10 @@ extension AbortError {
return [:]
}

public var description: String {
return "\(Self.self) \(self.status.code): \(self.reason)"
}

public var errorDescription: String? {
return self.description
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
extension Logger {
/// Reports an `Error` to this `Logger`, first checking if it is `Debuggable`
/// for improved debug info.
/// Reports an `Error` to this `Logger`.
///
/// - parameters:
/// - error: `Error` to log.
/// - verbose: If `true`, extra lines of debug information will be printed containing
/// things like suggested fixes, possible causes, or other info.
/// Defaults to `true`.
/// - request: Optional `Request` associated with this error.
public func report(
error: Error,
verbose: Bool = true,
file: String = #file,
function: String = #function,
line: UInt = #line
line: Int = #line
) {
let source: ErrorSource?
if let abort = error as? AbortError {
source = abort.source
} else {
source = nil
}

let reason: String
switch error {
case let localized as LocalizedError:
Expand All @@ -26,9 +29,9 @@ extension Logger {
self.log(
level: .error,
.init(stringLiteral: reason),
file: file,
function: function,
line: line
file: source?.file ?? file,
function: source?.function ?? function,
line: numericCast(source?.line ?? line)
)
}
}
12 changes: 2 additions & 10 deletions Sources/Vapor/Middleware/ErrorMiddleware.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public final class ErrorMiddleware: Middleware {
let status: HTTPResponseStatus
let reason: String
let headers: HTTPHeaders
let source: ErrorSource?

// inspect the error type
switch error {
Expand All @@ -30,29 +29,22 @@ public final class ErrorMiddleware: Middleware {
reason = abort.reason
status = abort.status
headers = abort.headers
source = abort.source
case let error as LocalizedError where !environment.isRelease:
// if not release mode, and error is debuggable, provide debug
// info directly to the developer
reason = error.localizedDescription
status = .internalServerError
headers = [:]
source = nil
default:
// not an abort error, and not debuggable or in dev mode
// just deliver a generic 500 to avoid exposing any sensitive error info
reason = "Something went wrong."
status = .internalServerError
headers = [:]
source = nil
}

// Report with the values from the error
if let source = source {
req.logger.report(error: error, verbose: !environment.isRelease, file: source.file, function: source.function, line: source.line)
} else {
req.logger.report(error: error, verbose: !environment.isRelease)
}
// Report the error to logger.
req.logger.report(error: error)

// create a Response with appropriate status
let response = Response(status: status, headers: headers)
Expand Down