From 71190d3d9941058e0292a00a8c8165e8278b008a Mon Sep 17 00:00:00 2001 From: John Kerl Date: Wed, 15 Jul 2026 16:05:53 -0400 Subject: [PATCH] plans/exit.md: record final status; defer phase 5; convert stragglers (#2207) 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 --- pkg/cli/option_parse.go | 18 ++++++++++-- pkg/dsl/cst/root.go | 8 ++++-- pkg/transformers/put_or_filter.go | 4 ++- plans/exit.md | 46 +++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/pkg/cli/option_parse.go b/pkg/cli/option_parse.go index e9e24036b..4efb6d459 100644 --- a/pkg/cli/option_parse.go +++ b/pkg/cli/option_parse.go @@ -59,7 +59,11 @@ func FinalizeReaderOptions(readerOptions *TReaderOptions) error { // and spaces, that should now be the default for NIDX. But *only* for NIDX format, // and if IFS wasn't specified. if readerOptions.InputFileFormat == "nidx" && !readerOptions.ifsWasSpecified { - readerOptions.IFSRegex = lib.CompileMillerRegexOrDie(WHITESPACE_REGEX) + regex, err := lib.CompileMillerRegex(WHITESPACE_REGEX) + if err != nil { + return err + } + readerOptions.IFSRegex = regex } else { if allowRepeatIFS, ok := defaultAllowRepeatIFSes[readerOptions.InputFileFormat]; ok { readerOptions.AllowRepeatIFS = allowRepeatIFS @@ -285,7 +289,11 @@ var SeparatorFlagSection = FlagSection{ // LF vs CR/LF line endings is handled within Go libraries so // we needn't do anything ourselves. if args[*pargi+1] != "auto" { - options.ReaderOptions.IFSRegex = lib.CompileMillerRegexOrDie(SeparatorRegexFromArg(args[*pargi+1])) + regex, err := lib.CompileMillerRegex(SeparatorRegexFromArg(args[*pargi+1])) + if err != nil { + return err + } + options.ReaderOptions.IFSRegex = regex options.ReaderOptions.ifsWasSpecified = true } *pargi += 2 @@ -316,7 +324,11 @@ var SeparatorFlagSection = FlagSection{ if err := CheckArgCount(args, *pargi, argc, 2); err != nil { return err } - options.ReaderOptions.IPSRegex = lib.CompileMillerRegexOrDie(SeparatorRegexFromArg(args[*pargi+1])) + regex, err := lib.CompileMillerRegex(SeparatorRegexFromArg(args[*pargi+1])) + if err != nil { + return err + } + options.ReaderOptions.IPSRegex = regex options.ReaderOptions.ipsWasSpecified = true *pargi += 2 return nil diff --git a/pkg/dsl/cst/root.go b/pkg/dsl/cst/root.go index 17ba89896..d90014ac2 100644 --- a/pkg/dsl/cst/root.go +++ b/pkg/dsl/cst/root.go @@ -411,11 +411,12 @@ func (root *RootNode) RegisterOutputHandlerManager( root.outputHandlerManagers = append(root.outputHandlerManagers, outputHandlerManager) } -func (root *RootNode) ProcessEndOfStream() { +func (root *RootNode) ProcessEndOfStream() error { for _, outputHandlerManager := range root.outputHandlerManagers { errs := outputHandlerManager.Close() if len(errs) != 0 { - for _, err := range errs { + // Print any additional errors here; return the first one. + for _, err := range errs[1:] { fmt.Fprintf( os.Stderr, "%s: error on end-of-stream close: %v\n", @@ -423,9 +424,10 @@ func (root *RootNode) ProcessEndOfStream() { err, ) } - os.Exit(1) + return fmt.Errorf("mlr: error on end-of-stream close: %v", errs[0]) } } + return nil } func (root *RootNode) ExecuteBeginBlocks(state *runtime.State) error { diff --git a/pkg/transformers/put_or_filter.go b/pkg/transformers/put_or_filter.go index f026f5306..fa751e104 100644 --- a/pkg/transformers/put_or_filter.go +++ b/pkg/transformers/put_or_filter.go @@ -615,7 +615,9 @@ func (tr *TransformerPut) Transform( // Send all registered OutputHandlerManager instances the end-of-stream // indicator. - tr.cstRootNode.ProcessEndOfStream() + if err := tr.cstRootNode.ProcessEndOfStream(); err != nil { + return err + } *outputRecordsAndContexts = append(*outputRecordsAndContexts, types.NewEndOfStreamMarker(&context)) } diff --git a/plans/exit.md b/plans/exit.md index 296d71638..5fd0a1851 100644 --- a/plans/exit.md +++ b/plans/exit.md @@ -140,6 +140,52 @@ 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)