feat: log standard output and error messages through the logger#165
feat: log standard output and error messages through the logger#165
Conversation
fortmarek
left a comment
There was a problem hiding this comment.
For both scenarios, the output is already passed to the consumer and I think the consumer should deal with logging to stdout and stderror via the AsyncThrowingStream instead of us doing that implicitly when the logger instance is passed.
I'd keep the logger calls that we do inside CommandRunner scoped to logger.debug only.
At least as things are done now in tuist/tuist, this would easily cause duplicated output in our CLI.
Log the standard output and error messages through the logger to allow the caller hook a loggers like syslog loggers to ease debugging.
5d634e8 to
69c080f
Compare
I forgot that we are conflating UI and logs currently in
|
| } | ||
| } catch { | ||
| logger?.error("Error reading stdout: \(error)") | ||
| logger?.error("Error reading stdout: \(error)", metadata: loggerMetadata) |
There was a problem hiding this comment.
@fortmarek I ended up using metadata to pass the information of the command that's being run. The log handler can then output that information.
| } | ||
| } catch { | ||
| logger?.error("Error reading stdout: \(error)") | ||
| logger?.error("Error reading stdout: \(error)", metadata: loggerMetadata) |
We allowed passing a
loggerinstance to theCommandRunner, but we were only using it for logging the run commands, which might not be sufficient to debug failing workflows. This PR extends the usage to also log stdout and stderr .utf8 events.