Repository navigation
Give the batch commands one exit status and one JSON document - #39
Merged
Merged
Conversation
`upload`, `download` and `edit` each ran a batch and each decided for itself what a refusal meant. An edit the server refused entirely printed "Edited 0 photographs." and exited 0. `download --json` returned before the failure was raised and exited 0 where the same run in prose exited 1. `upload --json` wrote its summary and then let the failure write a second document to the same stdout, which no ordinary parser will read. One rule now, in `finish_batch`: - Nothing refused exits 0; nothing got through exits 1; some of each exits 3. "Nothing worked" and "most of it worked" ask different things of the script that called, and 2 is left to clap's usage error. - `--json` puts exactly one document on stdout whatever happened. On a failure the command writes nothing there: the summary travels out with `BatchFailure` and `main` writes it once, with the sentence added under `error` so a reader looking only for that key is not misled. This is a behaviour change for scripts. A run that today exits 0 because every edit was refused, or because a download failed under `--json`, now exits non-zero, and a partly refused run exits 3 rather than 1. The tests run the real program against a stub server, because `$?` and the bytes on stdout only exist once there is a process; stdout is parsed as a stream and the documents are counted, since a second one is invisible to a grep. Closes #27 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bullet above it promises `{"error", "causes"}` for a `--json` error,
and a batch failure's document is the run's summary with `error` added —
no `causes`, because the run did not fail, some of its items did. A
reader keying off `causes` had no way to know that from the page.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`edit` printed "Edited 0 photographs." in green and then, now that the run is non-zero, `error: 2 photographs failed` under it — the success line contradicting the exit status. `upload` and `download` had the same shape. All three now leave the closing line out when nothing got through: the warnings and the sentence `main` prints are the whole of what happened. Also: the README claimed one document reaches stdout whatever happened, which an empty batch does not do — it says so on stderr and writes nothing. And the partly-refused upload test picked its victim by request ordinal, so a preflight the program learns to make one day would have become the refused request; it keys on the endpoint instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two `report`s in one function, the file handle and the JSON answer. The JSONL writes all happen before the shadow, so nothing was wrong today — it is the next edit that would find out. The answer is `summary`, which is what `finish_batch` calls its parameter, and the prose line is the `sentence`. The README's guarantee also covered `--dry-run`, which lists a plan and has no `failed` to give. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One promised `causes` on every JSON error; the other said a batch has none. The first now says which kind of failure it describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #27.
upload,downloadandediteach ran a batch and each decided for itself what a refusal meant:editexited 0 when every edit was refused. It printedEdited 0 photographs.and returnedOk(()). A script that submitted a hundred edits and had a hundred refused saw success — which is what undercut assets edit drops the fields a 400 named, and prints raw JSON at the person #23, since the rejection said which field was wrong and$?still said nothing was.upload --jsonwrote two documents when a file failed. The summary, thenmain::report's{"error":"N files failed",…}after it, on the same stdout.jqand every ordinary parser choke on the second.download --jsonexited 0 where the identical run in prose exited 1. Its JSON branch returned before thebail!.The rule
One
finish_batchinsrc/commands/mod.rs, called by all three.Exit status. 0 when nothing was refused; 1 when nothing got through; 3 when some items succeeded and some were refused.
The partial case is the decision the three drifted apart over, so it is worth saying why 3. A script that fires a hundred uploads wants to know which failure it has: "the server is down / the token is wrong / nothing landed" calls for stopping and shouting, and "ninety-seven landed, three were refused" calls for reading
failuresand retrying those three. Collapsing both into 1 throws that away and costs a document read to recover. 2 is not used, because clap already exits with it for a command line it could not parse, and a script cannot tell a usage error from a batch outcome if they share a code. A file the server already had, and a download skipped because it was already on disk, count as got through: the caller asked for the file to be there and it is.One document. In
--jsona failing batch writes nothing to stdout itself. The summary travels out withBatchFailureandmain::reportwrites it once, with the sentence added undererrorso a reader looking only for that key is not misled by a document full of items. Success still writes the summary from the command. Whichever way the run went, stdout parses as exactly one document.Prose. A run that got nothing through no longer closes with a green summary line saying so —
Edited 0 photographs.immediately aboveerror: 2 photographs failedwas the success line contradicting the exit status. All three leave it out when nothing succeeded.This is a behaviour change for scripts
Someone may be relying on today's zero. Plainly:
editwith every edit refusededitpartly refuseddownload --jsonwith a failureupload/downloadprose, all refusedupload/downloadpartly refusedThe prose sentence for a wholly failed run is unchanged (
error: 3 files failed); a partial one now readserror: 1 of 5 files failed.Tests
Written first and watched fail — all three failed against
mainfor exactly the reason #27 gives, and the upload one printed the two concatenated documents verbatim.They run the real program against a stub HTTP server (std
TcpListener, no new dependency), because$?and the bytes on stdout only exist once there is a process. The single-document property is asserted by parsing the whole of stdout as a JSON stream and counting what comes out — a second document is invisible to a grep and obvious to a parser.cargo test --locked && cargo fmt --check && cargo clippy --all-targets --locked -- -D warnings && cargo build --release --lockedis green: 137 unit, 4 integration.Found, not fixed
--jsondocument.upload.rsrenders it withctx.out.warn(format!("Could not fill an album: {error}"))and never makes it aFailure, sofinish_batchcounts none. This is explicitly Two seams left by the error-reporting rework: a lost download path, and an un-migrated album failure #34's second half, which says it should "participate in whatever exit rule The batch commands disagree about exit status, and upload --json emits two documents #27 settles" — it is a few lines above thefinish_batchcall this PR added, and deliberately left where it is. Whoever takes Two seams left by the error-reporting rework: a lost download path, and an un-migrated album failure #34 gets a one-line landing site.--jsonbatch writes no document at all.Nothing to upload./Nothing matched.go to stderr and stdout stays empty, so a script gets nothing rather than a summary of zeros. Not a parse failure, so it is not part of The batch commands disagree about exit status, and upload --json emits two documents #27's defect; the README now says so rather than over-promising. Settling it means settling the--dry-runshapes alongside it.🤖 Generated with Claude Code