fix: report the real error when the log target cannot be opened - #670
Closed
l1a wants to merge 1 commit into
Closed
Conversation
The inner `err` from the `if closer, options, err := setupTargetLogger(...)`
statement is scoped to that `if` only, so the `clog.Errorf` call after the
block referenced `main()`'s outer `var err error` (main.go:39), which is
still nil at that point. The result was that any failure to open a log
target printed the useless
cannot open log target: %!s(<nil>)
and discarded the real reason. Hoist the call out of the `if` init so the
actual error is in scope where it is logged.
Before: cannot open log target: %!s(<nil>)
After: cannot open log target: open /nonexistent/test.log: no such file
or directory
Assisted-By: Claude Opus 5
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.
Problem
When a
logtarget cannot be opened, the error message discards the actual reason:I hit this with a scheduled profile whose
logpointed into a directory that did not exist. The message gave me nothing to go on, and the real cause (no such file or directory) was never printed.Cause
In
main.go,erris declared in theifstatement's init:Go scopes that
errto theifstatement, so it is out of scope by the timeclog.Errorfruns. The call therefore resolves tomain()'s outervar err error(main.go:39), which is stillnilat that point — hence%!s(<nil>). It compiles precisely because that outer variable exists.Fix
Hoist the call out of the
ifinit so the error is in scope where it is logged. Two lines, no behaviour change beyond the message.Verification
Same command, before and after:
make buildandmake lintpass (0 issues on darwin, linux and windows).make testshows no new failures.I did not add a unit test: the affected code is a closure inside
main(), so covering it would require refactoringsetupLoggingout ofmain— happy to do that in this PR if you would prefer it.