microcks / microcks/microcks-cli

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

Offen Anfängerfreundlich
#443 2 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

stale
Vorherrschende Sprache
Go
Sterne
52
Forks
68
Ø Merge
6 Std. 54 Min.
Gemergte PRs (30 T.)
10

Beschreibung

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

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne mit pkg/errors/error.go und durchsuche dann das Repository mit grep nach den Aufrufstellen von CheckError und Fatal, um die ungenutzten Konstanten und das aktuelle Exit-Verhalten zu bestätigen. Erledigt ist die Aufgabe, wenn die vereinbarte minimale Bereinigung angewendet wurde, ohne das CLI-Verhalten zu ändern, und staticcheck oder die im Projekt verfügbaren Prüfungen bestätigen, dass keine Probleme mit unerreichbarem Code oder ungenutzten Konstanten verbleiben.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
go
Bereich
cli
Issue-Typ
Refactoring
Schwierigkeit
2/5
Geschätzter Aufwand
1-3 Stunden
Aktivitätsstatus
Ruhig
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
70/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.