[samuelmurray]

In swift-log, a user can attach an error object to a log entry, which is passed as-is to the LogHandler for it to serialise. ConsoleLogger, and the default fragment, is updated to use this property, serialising it as [ Fully.Qualified.Type: string representation ].

Example:

do {
  try throwingFunc()
} catch {
  logger.warning("Something went wrong", error: error) // Outputs "[...] Something went wrong [ ThrownError: description ] [...]"
}

Fixes #239

[samuelmurray]

I'm assuming breaking changes are ok, since it's a beta release?

[samuelmurray]

I didn't spend too much time thinking of a good output format for the error, so I'm happy to adjust if you have any suggestions. One alternative is to simply treat it as metadata, and use some sensible keys, eg. error.type and error.message. This is how StreamLogHandler in swift-log does it (https://github.com/apple/swift-log/blob/main/Sources/Logging/Handlers/StreamLogHandler.swift#L197-L200), though I'm not suggesting Vapor has to do the same.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.47%. Comparing base (df9e94c) to head (2c86d71).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #245      +/-   ##
==========================================
+ Coverage   82.37%   82.47%   +0.09%     
==========================================
  Files          29       29              
  Lines         891      896       +5     
==========================================
+ Hits          734      739       +5     
  Misses        157      157              

Files with missing lines	Coverage Δ
Sources/ConsoleLogger/ConsoleLogger.swift	87.27% <100.00%> (+0.23%)	⬆️
Sources/ConsoleLogger/LogRecord.swift	100.00% <100.00%> (ø)
...ConsoleLogger/LoggerFragments/LoggerFragment.swift	91.55% <100.00%> (+0.16%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fpseverino]

LGTM, the placement of the ErrorFragment is correct IMO, I'm not too sure about the output format, but we can change that later without breaking API

[0xTim]

Error output should definitely go in metadata. Only static strings should be added to the actual message as those are what are queried so need to be the same

[samuelmurray]

Error output should definitely go in metadata. Only static strings should be added to the actual message as those are what are queried so need to be the same

@0xTim would you mind clarifying your suggestion? Do you mean we should skip adding a ErrorFragment struct, and instead modify MetadataFragment to also serialize the error parameter?

[0xTim]

@samuelmurray yep exactly. Having a separate ErrorFragment complicates how we actually output information in the console, which then affects any tooling that relies on it. We want the error itself to be added to the metadata, which is already rendered, not as an additional fragment

[samuelmurray]

@0xTim have you had time to look at my changes? Let me know if you have any other suggestions!

[0xTim]

Thanks!

[0xTim]

@samuelmurray you might need to update the branch to trigger CI, I don't have permission to update the branch in the fork I think

[samuelmurray]

@0xTim I did a merge from main, let's see if that triggers a build