Simplify logging `Error` types

[sukhrobkhakimov]

While developing a server-side web app, it is very common to log errors. But, the library doesn't automatically extract the localizedDescription from Error types.

Given

let error = // some `Error` type thrown
let logger = Logger(label: "com.example")

Expected behavior

logger.error(error)

Actual behavior

logger.error(error.localizedDescription) // This doesn't even compile
logger.error("\(error.localizedDescription)") // It works but it is very cumbersome

Solution

Add an extra error() method to handle Error types automatically.

[ktoso]

Thanks for opening the issue, I don't think that's right quite to be honest.

For example, here you want to log localizedDescription -- that's specifically an NSError thing and does not exist on Linux. So eve if we were to add this, it'd have to be #if darwin platforms. Then using it would also have to be #if-ed around.

In addition to that, this library is primarily for the server ecosystem -- on Darwin you should be using OSLog after all. Making such "only for Darwin" addition somewhat weird.

I would also encourage using structured logging:

logger.error("Error doing something", metadata: ["error": "\(error)"])

rather the "just log the error" which yields better results in servers, and in general, gives more context about what the error is.

Summing up I don't think this is a good addition to the core API, though you're ofc free to make an extension Logger and add such method if you want to use it.

[tomerd]

@ktoso agreed on the structured metadata point. one other thing to point out is that MetadataValue accepts CustomStringConvertible so if the Error conforms to that it can be sent directly eg

MyError: Error, CustomStringConvertible {
   ...
}

logger.error("Error doing something", metadata: ["error": error])

[Mordil]

@tomerd Couldn't we provide a default implementation for Error types? Or is that asking for potential footguns where an error may contain more data than we intend to leak out in logs?

Most often, I write code like this, so there's no way to know if it conforms to CustomStringConvertible without additional boilerplate

catch {
  logger.error("failed to do X", metadata: "\(error)")
}

[tomerd]

right, I ran into many cases where the intuitive "\(error)" yielded undesired results due to the nature of the underlying Error implementation