Phase-5 wrap-up for plans/exit.md. The DSL-runtime cluster (pkg/dsl/cst: hofs.go, udf.go, evaluable.go) is deferred rather than converted: 58 regression cases pin the current loud-failure behavior (exact stderr plus exit 1), and the planned error-Mlrval mechanism would make HOF/UDF misuse fail silently as bare '(error)' with exit 0 -- a debugging-UX regression. The alternative (typed panic recovered at the Execute* boundaries) preserves behavior but introduces panic/recover to a codebase that has none. The decision belongs with the issue-440 strict-mode design, where the fatal-vs-data error taxonomy gets decided anyway; plans/exit.md now records the rationale, the phase-by-phase status (#2198, #2202, #2204, #2205), and the final intentional keep-list. Also converted here, since they're squarely in prior phases' patterns rather than the deferred expression-depth cluster: - RootNode.ProcessEndOfStream (a phase-3 leftover on the put/filter Transform path) returns an error instead of printing-and-exiting on end-of-stream close failures; first error returned, any others printed at the site. - The three CompileMillerRegexOrDie calls in option_parse.go (inside error-returning parser closures since phase 2) use CompileMillerRegex and return the error; 'mlr --ifs-regex (' output and exit code are unchanged. All 4779 regression cases pass; make lint 0 issues. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
14 KiB
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.
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 (DSL
exitstatement) — anexit 0/exit Nfrom inside aputexpression must shut the record stream down cleanly (flush writers, stop readers) and carry a chosen exit code up tomain. Rawos.Exitmid-goroutine can't do that. - Pre-work for #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: sentinelscli.ErrHelpRequested(→ exit 0) andcli.ErrUsagePrinted(→ exit 1).pkg/cli/verb_utils.go: error-returning verb-arg helpers (VerbCheckArgCount,VerbGetStringArg, …,VerbErrorf) — the stated replacement for exit-on-failure helpers.TransformerParseCLIFuncalready returns(RecordTransformer, error)(pkg/transformers/aaa_record_transformer.go).pkg/climain/errors_json.go: typedCLIError{Kind,Token,Msg}+--errors-jsonstructured emission.mlrval.FromErrorStringerror-valued Mlrvals +state.NoExitOnFunctionNotFound(used inpkg/dsl/cst/udf.go) — the DSL-layer error convention.- REPL precedent:
pkg/terminals/repl/dsl.goprints DSL errors and continues (#1976), while the same errors fromcst.ExecuteMainBlockmakeput_or_filter.goexit.
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:
// 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:
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):
// 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_ERRORalready gives stack traces.)asserting_*DSL builtins (pkg/bifs/types.goassertingCommon) — abort on assertion failure is their documented contract.- Top-of-stack dispatchers
os.Exit(entry.main(args))inauxents.go/terminals.go— these are the immediate-callee exits; inner exits within each aux/terminal convert toreturn <code>up to theirmain(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
RecordTransformerFuncinternal dispatch methods were converted along withTransform, and the step verb'stStepperAllocatortable gained an error return. - Phase 4: done — #2205. Went one step further than planned: the
auxents/terminalsdispatchers themselves now return exit codes instead of callingos.Exit, soentrypoint.Mainis the single exit point belowmain. 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.goexitOnError— the sanctioned exit point.pkg/lib/logger.goInternalCodingErrorIffamily — should-never-happen assertions (MLR_PANIC_ON_INTERNAL_ERRORgives stack traces).pkg/bifs/types.goassertingCommon— theasserting_*DSL builtins, whose documented contract is abort-on-failure.pkg/mlrval/mlrval_get.go(GetNumericToFloatValueOrDie,StrictModeCheck) andmlrval_output.go(String()path) — insidefmt.Stringerand 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.goCompileMillerRegexOrDie(remaining callers arelib-internal andbifshot paths, both on theEvaluate-shaped signatures deferred with phase 5; theoption_parsecallers 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 inentrypoint.gobecome the single mapping point; introduceExitRequest.mlrcli_parse.govalidation exits → returnedCLIErrors;loadMlrrcOrDie→loadMlrrc(...) error; move help/usage-sentinel exit translation to entrypoint.- Transformer ParseCLI constructor exits →
return nil, err(split.go:234et al.). - Migrate callers to existing error siblings:
CompileMillerRegexOrDie→CompileMillerRegex, mlrval...OrDiegetters →...OrError,WriteTempFileOrDie→ error-returning version;mlrcli_util.go CheckArgCountcallers →VerbCheckArgCountstyle 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 inlineparser:closures inoption_parse.goand the fourParsecall sites (mlrcli_parse.go,mlrcli_mlrrc.go,transformers/split.go).- Tedious but rote; also lets
pkg/terminals/completiondrop its "parsers call os.Exit" workaround (noted in its header comment).
Phase 3 — streaming interface (cluster 3b) ← the load-bearing phase for #341/#440
Transformreturnserror; all ~55 verbs getreturn nilon happy paths.ChainTransformer/runSingleTransformer+ChannelWriterpropagate errors viachan error; retiredataProcessingErrorChannel chan bool;stream.Streamselects and returns.- Convert runtime exits in
tee.go,split.go,step.go,join_bucket_keeper.go, andput_or_filter.go(theExecuteBeginBlocks/ExecuteMainBlock/ExecuteEndBlockserr → exit sites) into returned errors.
Phase 4 — sub-entrypoints (cluster 6)
- Per aux/terminal: usage funcs stop taking an
exitCode intand exiting; inner I/O helpers return errors; eachmain(args []string) intreturns 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...OrDiehelpers → error-Mlrvals per D3; deleteNoExitOnFunctionNotFoundspecial-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) andmake lintper phase.- Exit-code checks:
mlr --version; echo $?(0), unknown verb/flag (1),mlr put 'syntax error'(1),mlr joinwith unreadable left file (1, per #2171), broken pipemlr cat big | head -1(code unchanged),--errors-jsonoutput shape. mlr replstill print-and-continues on DSL errors.- Grep gate per phase:
grep -rn os.Exit pkg/ | grep -v _testshrinks to the documented keep-list; final state ≈entrypoint.go,logger.go,bifs/types.go(asserting),auxents.go/terminals.godispatchers.