Write diagnostic warnings to stderr instead of stdout#1682
Write diagnostic warnings to stderr instead of stdout#1682
Conversation
Expiration warnings, profile-not-found warnings, and config permission warnings were written to data.Output (stdout), breaking JSON parsing when --json was used.
philippschulte
left a comment
There was a problem hiding this comment.
Do the tests really verify the regression, because most of them set ErrOutput to the same buffer as Output. I don't think they catch accidental writes to stdout.
Maybe it would be good to add a test with separate stdout/stderr buffers and assert that warnings land only in stderr while stdout remains valid JSON?
Added, thanks! |
philippschulte
left a comment
There was a problem hiding this comment.
Looks good to me — routing these diagnostics to stderr is the right fix, and the added tests cover the main stdout-vs-stderr behavior well.
One small thing to double-check before merging: since --json no longer sets Quiet=true and instead uses Flags.JSON, can you confirm there are no remaining command-specific !Flags.Quiet stdout messages on any JSON-capable commands?
From a quick repo search, the global paths in pkg/app/run.go look covered by this PR, so this is mostly just a final sanity check that no command-level informational output can still leak into stdout and interfere with JSON parsing.
Change summary
Expiration warnings, profile-not-found warnings, and config permission warnings were written to
stdout,which broke JSON parsing when--jsonwas used. Now warnings go to stderr instead.All Submissions:
Changes to Core Features:
User Impact
Users piping CLI output through
jqor other JSON parsers will no longer encounter parse errors caused by diagnostic warnings mixed intostdout.Are there any considerations that need to be addressed for release?
No breaking changes.