fix(log): send Debug and Trace output to stderr (#851)#860
Open
gustcol wants to merge 2 commits intopeak:masterfrom
Open
fix(log): send Debug and Trace output to stderr (#851)#860gustcol wants to merge 2 commits intopeak:masterfrom
gustcol wants to merge 2 commits intopeak:masterfrom
Conversation
Fixes peak#851 Debug and Trace calls both used os.Stdout as the write target. When users piped 's5cmd --log debug cat s3://...' into another tool (e.g. xmlstarlet), the diagnostic messages were interleaved with the object payload and corrupted the stream. Only Error already used os.Stderr. This commit applies the same convention to Debug and Trace: diagnostics go to stderr, data goes to stdout. Reproducer: s5cmd --log debug cat s3://bucket/object.xml | xmlstarlet sel ... After this change: Debug/trace lines appear on stderr; stdout carries only object content.
Companion to the log fix: e2e tests that used --log debug were asserting DEBUG lines in result.Stdout(). Now that Debug and Trace write to stderr, move those expectations to result.Stderr() and assert stdout is empty (or contains only the non-debug lines for mixed-output tests).
Author
Test resultsUnit tests ( Changes included in this branch beyond the original fix:
Wasabi smoke test: Confirmed: DEBUG lines go to stderr; stdout stays clean. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fixes #851
Root cause
Trace()andDebug()both calledglobal.printf(..., os.Stdout).When a user ran
s5cmd --log debug cat s3://...and piped the outputinto another program, diagnostic messages were interleaved with the
object payload, corrupting the stream.
Fix
Changed
TraceandDebugto write toos.Stderr.Infocontinuesto write to
os.Stdout(it carries functional output). OnlyErroralready used
os.Stderr; this brings Debug and Trace in line with thesame convention.
Files changed:
log/log.goTests
TestDebugWritesToStderr— asserts Debug output appears on stderr and nothing on stdout.TestTraceWritesToStderr— same for Trace.TestInfoWritesToStdout— regression guard confirming Info still goes to stdout.Manual verification