report exits 0 when stdout is closed, instead of failing the write #30

New Issue

clawbot · 2026-08-09T04:29:07+02:00

clawbot commented

2026-08-09 04:29:07 +02:00

sfdupes report >&- — stdout closed outright rather than redirected — exits 0. README §Error handling lists "stdout write failure" as an exit-1 fatal condition, so this is a straight spec violation.

The mechanism makes it worse than a wrong exit code. With fd 1 closed at exec, the next file the process opens is handed fd 1. The database is opened early, so report output can be written into the database file descriptor rather than to a pipe. The command then reports success.

The general case matters more than the closed-fd edge case: sfdupes report | head -5 produces EPIPE on the writes that follow head exiting, and a full disk produces ENOSPC. Neither is currently detected, so a truncated report is indistinguishable from a complete one — for a tool whose entire product is a machine-readable list an operator acts on, silently losing rows is the worst failure mode available.

Found during the implementation of #4 and deliberately left out of that PR: it predates the change, is unrelated to database lifetime, and belongs on its own.

Definition of done

Every write to stdout is error-checked in report and trees — including the header line, every row, and the final flush if buffering is introduced.
A write failure is fatal: one-line diagnostic on stderr, exit 1.
EPIPE is handled deliberately and documented. Decide whether sfdupes report | head is an error or an expected early exit, then make the code and README agree; the common Unix convention is to die quietly on SIGPIPE/EPIPE rather than treat it as an error, but README currently says otherwise and one of the two must move.
If output is buffered, the buffer is flushed before exit and the flush error is checked — an unchecked Flush reintroduces the same bug one layer up.
Tests: report and trees with stdout closed assert exit 1 and a stderr diagnostic; a test writing to a deliberately failing writer asserts the error propagates.
README §Error handling is updated if the EPIPE decision changes the documented contract.
make check and make docker green.

`sfdupes report >&-` — stdout closed outright rather than redirected — exits 0. README §Error handling lists "stdout write failure" as an exit-1 fatal condition, so this is a straight spec violation. The mechanism makes it worse than a wrong exit code. With fd 1 closed at exec, the next file the process opens is handed fd 1. The database is opened early, so report output can be written into the database file descriptor rather than to a pipe. The command then reports success. The general case matters more than the closed-fd edge case: `sfdupes report | head -5` produces `EPIPE` on the writes that follow `head` exiting, and a full disk produces `ENOSPC`. Neither is currently detected, so a truncated report is indistinguishable from a complete one — for a tool whose entire product is a machine-readable list an operator acts on, silently losing rows is the worst failure mode available. Found during the implementation of #4 and deliberately left out of that PR: it predates the change, is unrelated to database lifetime, and belongs on its own. ## Definition of done 1. Every write to stdout is error-checked in `report` and `trees` — including the header line, every row, and the final flush if buffering is introduced. 2. A write failure is fatal: one-line diagnostic on stderr, exit 1. 3. `EPIPE` is handled deliberately and documented. Decide whether `sfdupes report | head` is an error or an expected early exit, then make the code and README agree; the common Unix convention is to die quietly on `SIGPIPE`/`EPIPE` rather than treat it as an error, but README currently says otherwise and one of the two must move. 4. If output is buffered, the buffer is flushed before exit and the flush error is checked — an unchecked `Flush` reintroduces the same bug one layer up. 5. Tests: `report` and `trees` with stdout closed assert exit 1 and a stderr diagnostic; a test writing to a deliberately failing writer asserts the error propagates. 6. README §Error handling is updated if the `EPIPE` decision changes the documented contract. 7. `make check` and `make docker` green.

clawbot added this to the 1.0.0 milestone 2026-08-09 04:29:07 +02:00

clawbot referenced this issue

2026-08-09 04:29:19 +02:00

Guarantee the database is closed on every fatal exit path (closes #4) #29

clawbot referenced this issue

2026-08-09 06:56:15 +02:00

Unwind the hash worker pool instead of abandoning it (closes #6) #31

clawbot referenced this issue

2026-08-09 07:20:50 +02:00

Unwind the hash worker pool instead of abandoning it (closes #6) #31

clawbot referenced this issue

2026-08-09 07:45:39 +02:00