From 5f152d7669d95b0e9a235ab64c3c671c21a58038 Mon Sep 17 00:00:00 2001 From: John Kerl Date: Mon, 6 Jul 2026 17:34:33 -0400 Subject: [PATCH] join: exit nonzero when the left file cannot be read (#2169) (#2171) The join verb's left-file ingest silently swallowed read errors: a nonexistent or malformed left file produced empty (or partial) output with exit code 0. Two defects, one per ingest path: * Unsorted (half-streaming) path, ingestLeftFile in pkg/transformers/join.go: errors from the record-reader's error channel were dropped ("TODO: propagate error to caller"), as were record-reader construction errors. All left-file read failures were silent. * Sorted (-s) path, readRecord in pkg/transformers/utils/join_bucket_keeper.go: the error channel was consulted, but the record-reader goroutine sends the error and the end-of-stream marker on separate channels, so the select could see end-of-stream first and drop the error. A nonexistent left file with -s exited 0 about half the time (racy). Both paths now print "mlr: " to stderr and exit 1, matching how read errors on regular (right) input files are reported. On receipt of the end-of-stream marker, both consumers now do a final non-blocking drain of the error channel, which is deterministic since the reader sends any error before the marker. Discovered while verifying #377. Co-authored-by: Claude Fable 5 --- pkg/transformers/join.go | 20 ++++++++++++++----- pkg/transformers/utils/join_bucket_keeper.go | 11 ++++++++++ .../verb-join/left-file-malformed-sorted/cmd | 1 + .../left-file-malformed-sorted/experr | 1 + .../left-file-malformed-sorted/expout | 0 .../left-file-malformed-sorted/should-fail | 0 .../left-file-malformed-unsorted/cmd | 1 + .../left-file-malformed-unsorted/experr | 1 + .../left-file-malformed-unsorted/expout | 0 .../left-file-malformed-unsorted/should-fail | 0 .../cmd | 1 + .../experr | 1 + .../expout | 0 .../should-fail | 0 .../cmd | 1 + .../experr | 1 + .../expout | 0 .../should-fail | 0 test/input/join-left-ragged.csv | 3 +++ 19 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 test/cases/verb-join/left-file-malformed-sorted/cmd create mode 100644 test/cases/verb-join/left-file-malformed-sorted/experr create mode 100644 test/cases/verb-join/left-file-malformed-sorted/expout create mode 100644 test/cases/verb-join/left-file-malformed-sorted/should-fail create mode 100644 test/cases/verb-join/left-file-malformed-unsorted/cmd create mode 100644 test/cases/verb-join/left-file-malformed-unsorted/experr create mode 100644 test/cases/verb-join/left-file-malformed-unsorted/expout create mode 100644 test/cases/verb-join/left-file-malformed-unsorted/should-fail create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-sorted/cmd create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-sorted/experr create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-sorted/expout create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-sorted/should-fail create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-unsorted/cmd create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-unsorted/experr create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-unsorted/expout create mode 100644 test/cases/verb-join/non-windows-left-file-not-found-unsorted/should-fail create mode 100644 test/input/join-left-ragged.csv diff --git a/pkg/transformers/join.go b/pkg/transformers/join.go index b9cb312da..0fba89c3c 100644 --- a/pkg/transformers/join.go +++ b/pkg/transformers/join.go @@ -491,8 +491,8 @@ func (tr *TransformerJoin) ingestLeftFile() { // TODO: perhaps increase recordsPerBatch, and/or refactor recordReader, err := input.Create(readerOpts, 1) if err != nil { - // TODO: propagate error to caller - return + fmt.Fprintf(os.Stderr, "mlr: %v\n", err) + os.Exit(1) } // Set the initial context for the left-file. @@ -521,9 +521,8 @@ func (tr *TransformerJoin) ingestLeftFile() { select { case err := <-errorChannel: - // TODO: propagate error to caller - _ = err - return + fmt.Fprintf(os.Stderr, "mlr: %v\n", err) + os.Exit(1) case leftrecsAndContexts := <-readerChannel: // TODO: temp for batch-reader refactor @@ -532,6 +531,17 @@ func (tr *TransformerJoin) ingestLeftFile() { leftrecAndContext.Record = utils.KeepLeftFieldNames(leftrecAndContext.Record, tr.leftKeepFieldNameSet) if leftrecAndContext.EndOfStream { + // The record-reader may have sent an error (e.g. the left file + // is missing or unreadable) immediately before its end-of-stream + // marker. Since those are separate channels, the select can see + // the end-of-stream marker first -- so, check the error channel + // before declaring the ingest complete. + select { + case err := <-errorChannel: + fmt.Fprintf(os.Stderr, "mlr: %v\n", err) + os.Exit(1) + default: + } done = true break // breaks the switch, not the for, in Golang } diff --git a/pkg/transformers/utils/join_bucket_keeper.go b/pkg/transformers/utils/join_bucket_keeper.go index 52c8d47fe..795abf9f0 100644 --- a/pkg/transformers/utils/join_bucket_keeper.go +++ b/pkg/transformers/utils/join_bucket_keeper.go @@ -553,6 +553,17 @@ func (keeper *JoinBucketKeeper) readRecord() *types.RecordAndContext { leftrecAndContext := leftrecsAndContexts[0] leftrecAndContext.Record = KeepLeftFieldNames(leftrecAndContext.Record, keeper.leftKeepFieldNameSet) if leftrecAndContext.EndOfStream { // end-of-stream marker + // The record-reader may have sent an error (e.g. the left file is + // missing or unreadable) immediately before its end-of-stream + // marker. Since those are separate channels, the select can see + // the end-of-stream marker first -- so, check the error channel + // before declaring the left-file read complete. + select { + case err := <-keeper.errorChannel: + fmt.Fprintf(os.Stderr, "mlr: %v\n", err) + os.Exit(1) + default: + } keeper.recordReaderDone = true return nil } diff --git a/test/cases/verb-join/left-file-malformed-sorted/cmd b/test/cases/verb-join/left-file-malformed-sorted/cmd new file mode 100644 index 000000000..340fbb573 --- /dev/null +++ b/test/cases/verb-join/left-file-malformed-sorted/cmd @@ -0,0 +1 @@ +mlr --icsv --opprint join -s -j a -f test/input/join-left-ragged.csv test/input/abixy.csv diff --git a/test/cases/verb-join/left-file-malformed-sorted/experr b/test/cases/verb-join/left-file-malformed-sorted/experr new file mode 100644 index 000000000..d082c879e --- /dev/null +++ b/test/cases/verb-join/left-file-malformed-sorted/experr @@ -0,0 +1 @@ +mlr: CSV header/data length mismatch 2 != 4 at filename test/input/join-left-ragged.csv row 3 diff --git a/test/cases/verb-join/left-file-malformed-sorted/expout b/test/cases/verb-join/left-file-malformed-sorted/expout new file mode 100644 index 000000000..e69de29bb diff --git a/test/cases/verb-join/left-file-malformed-sorted/should-fail b/test/cases/verb-join/left-file-malformed-sorted/should-fail new file mode 100644 index 000000000..e69de29bb diff --git a/test/cases/verb-join/left-file-malformed-unsorted/cmd b/test/cases/verb-join/left-file-malformed-unsorted/cmd new file mode 100644 index 000000000..6bbb82d41 --- /dev/null +++ b/test/cases/verb-join/left-file-malformed-unsorted/cmd @@ -0,0 +1 @@ +mlr --icsv --opprint join -j a -f test/input/join-left-ragged.csv test/input/abixy.csv diff --git a/test/cases/verb-join/left-file-malformed-unsorted/experr b/test/cases/verb-join/left-file-malformed-unsorted/experr new file mode 100644 index 000000000..d082c879e --- /dev/null +++ b/test/cases/verb-join/left-file-malformed-unsorted/experr @@ -0,0 +1 @@ +mlr: CSV header/data length mismatch 2 != 4 at filename test/input/join-left-ragged.csv row 3 diff --git a/test/cases/verb-join/left-file-malformed-unsorted/expout b/test/cases/verb-join/left-file-malformed-unsorted/expout new file mode 100644 index 000000000..e69de29bb diff --git a/test/cases/verb-join/left-file-malformed-unsorted/should-fail b/test/cases/verb-join/left-file-malformed-unsorted/should-fail new file mode 100644 index 000000000..e69de29bb diff --git a/test/cases/verb-join/non-windows-left-file-not-found-sorted/cmd b/test/cases/verb-join/non-windows-left-file-not-found-sorted/cmd new file mode 100644 index 000000000..3fc46bd0e --- /dev/null +++ b/test/cases/verb-join/non-windows-left-file-not-found-sorted/cmd @@ -0,0 +1 @@ +mlr --icsv --opprint join -s -j a -f /nonesuch/nope/never test/input/abixy.csv diff --git a/test/cases/verb-join/non-windows-left-file-not-found-sorted/experr b/test/cases/verb-join/non-windows-left-file-not-found-sorted/experr new file mode 100644 index 000000000..81b3580c0 --- /dev/null +++ b/test/cases/verb-join/non-windows-left-file-not-found-sorted/experr @@ -0,0 +1 @@ +mlr: open /nonesuch/nope/never: no such file or directory diff --git a/test/cases/verb-join/non-windows-left-file-not-found-sorted/expout b/test/cases/verb-join/non-windows-left-file-not-found-sorted/expout new file mode 100644 index 000000000..e69de29bb diff --git a/test/cases/verb-join/non-windows-left-file-not-found-sorted/should-fail b/test/cases/verb-join/non-windows-left-file-not-found-sorted/should-fail new file mode 100644 index 000000000..e69de29bb diff --git a/test/cases/verb-join/non-windows-left-file-not-found-unsorted/cmd b/test/cases/verb-join/non-windows-left-file-not-found-unsorted/cmd new file mode 100644 index 000000000..20e60c307 --- /dev/null +++ b/test/cases/verb-join/non-windows-left-file-not-found-unsorted/cmd @@ -0,0 +1 @@ +mlr --icsv --opprint join -j a -f /nonesuch/nope/never test/input/abixy.csv diff --git a/test/cases/verb-join/non-windows-left-file-not-found-unsorted/experr b/test/cases/verb-join/non-windows-left-file-not-found-unsorted/experr new file mode 100644 index 000000000..81b3580c0 --- /dev/null +++ b/test/cases/verb-join/non-windows-left-file-not-found-unsorted/experr @@ -0,0 +1 @@ +mlr: open /nonesuch/nope/never: no such file or directory diff --git a/test/cases/verb-join/non-windows-left-file-not-found-unsorted/expout b/test/cases/verb-join/non-windows-left-file-not-found-unsorted/expout new file mode 100644 index 000000000..e69de29bb diff --git a/test/cases/verb-join/non-windows-left-file-not-found-unsorted/should-fail b/test/cases/verb-join/non-windows-left-file-not-found-unsorted/should-fail new file mode 100644 index 000000000..e69de29bb diff --git a/test/input/join-left-ragged.csv b/test/input/join-left-ragged.csv new file mode 100644 index 000000000..11cb35f9c --- /dev/null +++ b/test/input/join-left-ragged.csv @@ -0,0 +1,3 @@ +a,l +pan,one +eks,two,three,four