# Plan: survey & phased removal of scattered `os.Exit` calls

## Context

Miller has ~170 `os.Exit` call sites scattered through non-test Go code under `pkg/`
(plus a handful in `cmd/experiments/`, which are throwaway sandboxes and out of scope).
The north star: `os.Exit` should happen only in `main` or its immediate callee
(`entrypoint.Main`), with everything below returning `error`. Not every last site must
be eliminated — genuine internal-coding-error assertions can stay — but user-facing
error and exit paths should propagate.

See also [issue 2200](https://github.com/johnkerl/miller/issues/2200).

Motivation:

- **General hygiene** — exits deep in library code make the packages unusable as a
  library, skip deferred cleanup (e.g. the pprof defers in `cmd/mlr/main.go`), and
  create unreachable/untestable code paths.
- **Pre-work for [#341](https://github.com/johnkerl/miller/issues/341)** (DSL `exit`
  statement) — an `exit 0`/`exit N` from inside a `put` expression must shut the
  record stream down cleanly (flush writers, stop readers) and carry a *chosen* exit
  code up to `main`. Raw `os.Exit` mid-goroutine can't do that.
- **Pre-work for [#440](https://github.com/johnkerl/miller/issues/440)** (strict mode)
  — missing-field reads become *errors*, which need a propagation path from the DSL
  evaluator up through the transformer chain to the process exit code.

Prior art already in-tree (this effort is a continuation, not a fresh start):

- Commit `6b32e1f41` "Reduce number of os.Exit callsites, part 1 of n" (#1055).
- `pkg/cli/errors.go`: sentinels `cli.ErrHelpRequested` (→ exit 0) and
  `cli.ErrUsagePrinted` (→ exit 1).
- `pkg/cli/verb_utils.go`: error-returning verb-arg helpers (`VerbCheckArgCount`,
  `VerbGetStringArg`, …, `VerbErrorf`) — the stated replacement for exit-on-failure
  helpers.
- `TransformerParseCLIFunc` already returns `(RecordTransformer, error)`
  (`pkg/transformers/aaa_record_transformer.go`).
- `pkg/climain/errors_json.go`: typed `CLIError{Kind,Token,Msg}` + `--errors-json`
  structured emission.
- `mlrval.FromErrorString` error-valued Mlrvals + `state.NoExitOnFunctionNotFound`
  (used in `pkg/dsl/cst/udf.go`) — the DSL-layer error convention.
- REPL precedent: `pkg/terminals/repl/dsl.go` prints DSL errors and continues (#1976),
  while the same errors from `cst.ExecuteMainBlock` make `put_or_filter.go` exit.

## Survey: where the ~170 sites live

Counts are non-test `.go` under `pkg/`. Breakdown: 151 × `os.Exit(1)`, 15 ×
`os.Exit(0)`, plus `os.Exit(exitCode)` / `os.Exit(entry.main(args))` dispatchers.

| Cluster | Sites (approx) | Containing signature returns error today? | Difficulty |
|---|---|---|---|
| 1. `pkg/cli` main-flag parsing: `option_parse.go` (14), `flag_types.go`, `mlrcli_util.go` (`CheckArgCount`) | ~18 | No — `FlagParser` is void (`flag_types.go:58`); `FlagTable.Parse` returns bool | Signature change, then mechanical |
| 2. `pkg/climain`: `mlrcli_parse.go` (11), `mlrcli_mlrrc.go` (`loadMlrrcOrDie`) | 12 | Yes — `ParseCommandLine` and both passes return error | Mechanical |
| 3a. Transformer ParseCLI leftovers: constructor-error exits (e.g. `split.go`, `tee.go`, `subs.go`, `nest.go`, `join.go`, `surv.go`) | ~15 | Yes — `TransformerParseCLIFunc` returns error | Mechanical |
| 3b. Transformer **runtime** (`Transform` path): `tee.go`, `split.go`, `step.go`, `put_or_filter.go`, `utils/join_bucket_keeper.go` (5) | ~20 | No — `RecordTransformer.Transform` is void | Interface change |
| 4. DSL runtime: `pkg/dsl/cst/hofs.go` (14), `udf.go` (8), `root.go`, `evaluable.go` | ~25 | No — `IEvaluable.Evaluate` returns only `*mlrval.Mlrval` | Semantic; use error-Mlrval convention |
| 5. Writers/values: `record_writer_yaml.go` (4), `record_writer_json_jsonl.go` (2), `mlrval_output.go`, `mlrval_get.go`; `pkg/lib` (`WriteTempFileOrDie`, `CompileMillerRegexOrDie`, `mlrmath.go`) | ~18 | Mixed — many have `...OrError` siblings already | Mostly mechanical |
| 6. Sub-entrypoints: `pkg/auxents/*` (hex, unhex, lecat, termcvt), `pkg/terminals/*` (repl, script, regtest, completion, mcp) | ~40 | `main(args []string) int` per aux/terminal | Self-contained, mechanical |
| 7. Deliberate exit-0: `--version`, help, `--list-color-*`, `put -e`/`--explain` "valid" paths | ~15 | Varies | Convert to sentinels mapped at entrypoint |
| Keep as-is | — | `lib.InternalCodingErrorIf` family (~365 assertion call sites), `asserting_*` builtins (`pkg/bifs/types.go` — abort is their specified behavior), `os.Exit(entry.main(args))` dispatchers in `auxents.go`/`terminals.go` | — |

## Design decisions

### D1. Single exit point

`cmd/mlr/main.go` stays exit-free (so its pprof/trace defers run); `entrypoint.Main`
becomes the one place that calls `os.Exit`, mapping errors to codes:

```go
// pkg/entrypoint: nil → 0; lib.ExitRequest{Code} → Code;
// cli.ErrHelpRequested → 0; everything else → print (or EmitStructuredError) + 1
```

The `ErrHelpRequested`/`ErrUsagePrinted` → exit translation currently mid-stack in
`mlrcli_parse.go:406-409` moves up to this boundary.

### D2. `Transform` returns `error`; exit codes ride a typed sentinel

Change the streaming interface:

```go
type RecordTransformer interface {
    Transform(inrecAndContext *types.RecordAndContext,
        outputRecordsAndContexts *[]*types.RecordAndContext,
        inputDownstreamDoneChannel <-chan bool,
        outputDownstreamDoneChannel chan<- bool,
    ) error
}
```

`runSingleTransformer`/`ChainTransformer` forward a non-nil error to a new
`chan error` consumed by `stream.Stream`'s select loop (alongside the existing
`inputErrorChannel`); the anemic `dataProcessingErrorChannel chan bool` is subsumed.
This also fixes the standing `XXX ... exit 1 & goroutine cleanup` in
`pkg/output/channel_writer.go`, whose print-and-signal-bool becomes send-the-error.

**Exit(0) vs exit(1) disambiguation for #341 lives in the error's *type*, not the
transport** — the `io.EOF` pattern (sentinel error as control flow):

```go
// pkg/lib (or pkg/types): satisfies error
type ExitRequest struct{ Code int }
```

A DSL `exit N` statement extends the existing `BlockExitPayload` machinery
(`pkg/dsl/cst/types.go:101-105` — BREAK/CONTINUE/RETURN already propagate out of
nested blocks) with a `BLOCK_EXIT_EXIT` status carrying the code; at the block
boundary it becomes `ExitRequest{N}` returned as an error from `ExecuteMainBlock` →
`put_or_filter.Transform` → chain → `stream.Stream` → `entrypoint.Main`, which exits
with `Code` — after the normal flush path.

The synchronous error return is *more* amenable to #341 than a side-channel: a
free-standing error channel races with `doneWritingChannel` in the Stream select loop
and gives no ordering guarantee that records emitted *before* the `exit` statement get
flushed. A returned error lets `ChainTransformer` forward already-produced records,
signal downstream-done (the same drain mechanism `head -n` uses), and only then
surface the `ExitRequest`.

### D3. DSL runtime errors use the existing error-Mlrval convention

The `...OrDie` helpers in `hofs.go`/`udf.go` return `mlrval.FromErrorString(...)`
values instead of exiting (generalizing what `state.NoExitOnFunctionNotFound` already
does at `udf.go:120-132`). `cst.Execute*Blocks` already return `error`;
`put_or_filter.go` stops swallowing those with `os.Exit(1)` and returns them from
`Transform` (per D2). This is exactly the plumbing #440 strict mode needs.

### D4. What stays

- `lib.InternalCodingErrorIf` / `...WithMessageIf` — should-never-happen assertions,
  by their own doc comment. (`MLR_PANIC_ON_INTERNAL_ERROR` already gives stack traces.)
- `asserting_*` DSL builtins (`pkg/bifs/types.go` `assertingCommon`) — abort on
  assertion failure is their documented contract.
- Top-of-stack dispatchers `os.Exit(entry.main(args))` in `auxents.go`/`terminals.go`
  — these *are* the immediate-callee exits; inner exits within each aux/terminal
  convert to `return <code>` up to their `main(args) int`.

### Behavioral invariants

Stderr message text and process exit codes must not change (the regression suite
asserts on both; see also recent exit-code fixes #2171, #2146). `--errors-json`
categorization must keep working — and gains coverage, since stream-time errors that
today bypass `EmitStructuredError` will now arrive at the entrypoint as errors.

## Status (2026-07)

- Phase 1: done — #2198.
- Phase 2: done — #2202.
- Phase 3: done — #2204. Additionally, the `RecordTransformerFunc` internal
  dispatch methods were converted along with `Transform`, and the step verb's
  `tStepperAllocator` table gained an error return.
- Phase 4: done — #2205. Went one step further than planned: the
  `auxents`/`terminals` dispatchers themselves now return exit codes instead
  of calling `os.Exit`, so `entrypoint.Main` is the single exit point below
  `main`. Also converted in the phase-5 wrap-up PR: `RootNode.ProcessEndOfStream`
  (a phase-3 leftover on the put/filter Transform path) returns an error.
- Phase 5: **deferred** — see below.

### Phase 5 deferral rationale

The sketch below (error-Mlrvals via `FromErrorString`) turns out to have costs
not visible at planning time: 58 regression cases pin the current
loud-failure behavior (exact stderr message plus exit 1 via `should-fail`),
and error-Mlrvals display as bare `(error)` — so HOF/UDF misuse would fail
*silently* with exit 0 unless `--fail-on-data-error` is set, a debugging-UX
regression. An alternative mechanism (a typed panic recovered at the four
`Execute*` boundaries, then riding the phase-3 error path) would preserve
behavior exactly, but introduces panic/recover to a codebase that has none.
Rather than pick under time pressure, the DSL cluster is deferred to the
issue-440 strict-mode design, where the fatal-vs-data error taxonomy has to
be decided anyway. The remaining sites are all in `pkg/dsl/cst`: `hofs.go`
(14), `udf.go` (8), `evaluable.go` (1), plus `builtin_function_manager.go`'s
init-time duplicate-name assertion.

### Final keep-list (intentional, documented)

- `pkg/entrypoint/entrypoint.go` `exitOnError` — the sanctioned exit point.
- `pkg/lib/logger.go` `InternalCodingErrorIf` family — should-never-happen
  assertions (`MLR_PANIC_ON_INTERNAL_ERROR` gives stack traces).
- `pkg/bifs/types.go` `assertingCommon` — the `asserting_*` DSL builtins,
  whose documented contract is abort-on-failure.
- `pkg/mlrval/mlrval_get.go` (`GetNumericToFloatValueOrDie`,
  `StrictModeCheck`) and `mlrval_output.go` (`String()` path) — inside
  `fmt.Stringer` and hot-path value accessors; error returns don't fit the
  signatures. Candidates for the 440 redesign.
- `pkg/lib/mlrmath.go` (eigensolver non-convergence), `pkg/lib/regex.go`
  `CompileMillerRegexOrDie` (remaining callers are `lib`-internal and `bifs`
  hot paths, both on the `Evaluate`-shaped signatures deferred with phase 5;
  the `option_parse` callers were converted in the wrap-up PR).

## Phases (one PR each, each green under `make dev` + `make lint`)

### Phase 1 — top level + all mechanical swaps (clusters 2, 3a, 5, 7 partial)
- `entrypoint.Main() (MainReturn, error)` (or int code); the two exits in
  `entrypoint.go` become the single mapping point; introduce `ExitRequest`.
- `mlrcli_parse.go` validation exits → returned `CLIError`s; `loadMlrrcOrDie` →
  `loadMlrrc(...) error`; move help/usage-sentinel exit translation to entrypoint.
- Transformer ParseCLI constructor exits → `return nil, err` (`split.go:234` et al.).
- Migrate callers to existing error siblings: `CompileMillerRegexOrDie` →
  `CompileMillerRegex`, mlrval `...OrDie` getters → `...OrError`, `WriteTempFileOrDie`
  → error-returning version; `mlrcli_util.go CheckArgCount` callers → `VerbCheckArgCount`
  style where signatures already allow.
- Deliberate exit-0 sites reachable from error-returning frames (`--version`, `put -e`
  "valid", `subs.go:180`) → sentinel returns.

### Phase 2 — `FlagParser` signature (cluster 1)
- `type FlagParser func(...) error`; `FlagTable.Parse` → `(bool, error)`; update the
  inline `parser:` closures in `option_parse.go` and the four `Parse` call sites
  (`mlrcli_parse.go`, `mlrcli_mlrrc.go`, `transformers/split.go`).
- Tedious but rote; also lets `pkg/terminals/completion` drop its
  "parsers call os.Exit" workaround (noted in its header comment).

### Phase 3 — streaming interface (cluster 3b) **← the load-bearing phase for #341/#440**
- `Transform` returns `error`; all ~55 verbs get `return nil` on happy paths.
- `ChainTransformer`/`runSingleTransformer` + `ChannelWriter` propagate errors via
  `chan error`; retire `dataProcessingErrorChannel chan bool`; `stream.Stream` selects
  and returns.
- Convert runtime exits in `tee.go`, `split.go`, `step.go`, `join_bucket_keeper.go`,
  and `put_or_filter.go` (the `ExecuteBeginBlocks`/`ExecuteMainBlock`/
  `ExecuteEndBlocks` err → exit sites) into returned errors.

### Phase 4 — sub-entrypoints (cluster 6)
- Per aux/terminal: usage funcs stop taking an `exitCode int` and exiting; inner I/O
  helpers return errors; each `main(args []string) int` returns codes to the
  dispatcher. `repl/entry.go`, `script/entry.go`, `regtest`, `hex/unhex/lecat/termcvt`.

### Phase 5 — DSL runtime (cluster 4)
- `hofs.go`/`udf.go` `...OrDie` helpers → error-Mlrvals per D3; delete
  `NoExitOnFunctionNotFound` special-casing once return-errors is the only behavior.
- Behavior note to document: HOF misuse now surfaces as `mlr: <msg>` via error
  propagation (same message, same exit 1 at top) rather than instant abort; REPL keeps
  its print-and-continue behavior for free.

## Verification

- `make dev` (fmt, build, unit + regression, docs) and `make lint` per phase.
- Exit-code checks: `mlr --version; echo $?` (0), unknown verb/flag (1), `mlr put
  'syntax error'` (1), `mlr join` with unreadable left file (1, per #2171), broken
  pipe `mlr cat big | head -1` (code unchanged), `--errors-json` output shape.
- `mlr repl` still print-and-continues on DSL errors.
- Grep gate per phase: `grep -rn os.Exit pkg/ | grep -v _test` shrinks to the
  documented keep-list; final state ≈ `entrypoint.go`, `logger.go`, `bifs/types.go`
  (asserting), `auxents.go`/`terminals.go` dispatchers.
