microcks / microcks/microcks-cli

Dead code in pkg/errors: Fatal()'s os.Exit() is unreachable; four ErrorX constants unused

Open Beginner friendly
#443 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

stale
Dominant language
Go
Stars
52
Forks
68
Avg merge
6h 54m
Merged PRs (30d)
10

Description

## 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

```go
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:

```go
// 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`

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with pkg/errors/error.go, then grep the repository for CheckError and Fatal call sites to confirm the unused constants and current exit behavior. Done means the agreed minimal cleanup is applied without changing CLI behavior, with staticcheck or the project’s available checks confirming no unreachable-code or unused-constant issues remain.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.