Description
pkg/errors/error.go contains two related forms of dead code that mislead
readers about how the CLI signals failure.
1. os.Exit(exitcode) is unreachable
func Fatal(exitcode int, args ...interface{}) {
log.Fatal(args...) // already calls os.Exit(1) internally
os.Exit(exitcode) // unreachable
}
Go's stdlib defines log.Fatal as:
// log/log.go
func Fatal(v ...any) {
std.Output(2, fmt.Sprint(v...))
os.Exit(1)
}
Execution never reaches os.Exit(exitcode). The exitcode argument passed by
callers is silently discarded — the binary always exits with code 1.
2. Four of the five Error* constants are unreferenced
Grep across the whole repo:
| Constant |
Value |
References outside error.go |
ErrorCommandSpecific |
1 |
0 |
ErrorConnectionFailure |
11 |
0 |
ErrorAPIResponse |
12 |
0 |
ErrorResourceDoesNotExist |
13 |
0 |
ErrorGeneric |
20 |
1 (CheckError, same file) |
The constants suggest a differentiated-exit-codes design that was never
actually wired up (the only caller of Fatal always passes ErrorGeneric,
and the chosen code is never reached anyway because of issue 1).
Impact
- Misleading: reading the file gives the false impression that microcks-cli
returns differentiated exit codes (11/12/13/20). It always returns 1.
- Dead code rot: future readers may write callers expecting these codes to
matter.
- Lint noise: tools like
staticcheck flag unreachable code (SA4006).
Proposed fix
Two paths:
(A) Implement differentiated exit codes properly. Replace log.Fatal with
log.Println, let os.Exit(exitcode) actually run, audit every
CheckError call site to pass the right constant. This would alter
observable CLI exit codes (currently always 1), which could break downstream
CI scripts.
(B) Remove the dead code, keep behavior identical. Strip the unreachable
line, the unused constants, and the misleading exitcode parameter on
Fatal. No behavioral change.
I'd like to propose (B) as the minimal, safe cleanup. (A) can be tracked
separately as a feature if maintainers want differentiated exit codes.
Happy to send a PR with (B) once approach is approved.
Environment
- Branch:
master (development version 1.0.3)
- File:
pkg/errors/error.go
Description
pkg/errors/error.gocontains two related forms of dead code that misleadreaders about how the CLI signals failure.
1.
os.Exit(exitcode)is unreachableGo's stdlib defines
log.Fatalas:Execution never reaches
os.Exit(exitcode). Theexitcodeargument passed bycallers is silently discarded — the binary always exits with code 1.
2. Four of the five
Error*constants are unreferencedGrep across the whole repo:
error.goErrorCommandSpecificErrorConnectionFailureErrorAPIResponseErrorResourceDoesNotExistErrorGenericThe constants suggest a differentiated-exit-codes design that was never
actually wired up (the only caller of
Fatalalways passesErrorGeneric,and the chosen code is never reached anyway because of issue 1).
Impact
returns differentiated exit codes (11/12/13/20). It always returns 1.
matter.
staticcheckflag unreachable code (SA4006).Proposed fix
Two paths:
(A) Implement differentiated exit codes properly. Replace
log.Fatalwithlog.Println, letos.Exit(exitcode)actually run, audit everyCheckErrorcall site to pass the right constant. This would alterobservable CLI exit codes (currently always 1), which could break downstream
CI scripts.
(B) Remove the dead code, keep behavior identical. Strip the unreachable
line, the unused constants, and the misleading
exitcodeparameter onFatal. No behavioral change.I'd like to propose (B) as the minimal, safe cleanup. (A) can be tracked
separately as a feature if maintainers want differentiated exit codes.
Happy to send a PR with (B) once approach is approved.
Environment
master(development version1.0.3)pkg/errors/error.go