A mirror that aborts on a fatal error still exits 0 - #1419
Merged
Conversation
hts_main2() returned 0 whatever the mirror did, so a script could not tell a finished mirror from one the engine gave up on halfway. It now returns HTS_EXIT_MIRROR_ABORTED (2) for a mirror that started and did not finish, covering all nine sites that set exit_xh = -1 plus the httpmirror() bailout that already printed an error before exiting 0. A mirror the caller ended on purpose keeps returning 0: hts_request_stop(), a callback returning 0, SIGINT and WebHTTrack's cancel all set exit_xh to 1, and reporting those as failures would break the cancel button. httrack.c prints hts_errmsg() on a non-zero return and that buffer was empty here, so the abort now fills it. The man page gains an EXIT STATUS section, and httrack-library.h documents the contract for embedders. Closes #1394 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Its crawl_onto_full helper ran local_crawl under set -e without meaning to read the status, so the abort it provokes on purpose ended the script once an aborted mirror stopped exiting 0. Capture the status on the line itself, before the log read clobbers $?, and assert it; the EPIPE arm, where a non-fatal write error leaves the mirror running, asserts 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
htsftp.c:939 is commented-out code, so there are 8 live exit_xh = -1 producers, not 9; the count in httrack-library.h claimed an FTP fatal the engine cannot raise, which is the worst place for it since embedders read that header. The -#L link cap reported two different statuses depending on which path found it: htsAddLink aborted, the parser at htsparse.c:3182 logged "Too many URLs" and returned quietly. It aborts now, which is the ninth producer and the first one a test can reach without a full disk. 350's SIGTERM arm and 240's two passes assert the status, pinning the exit_xh == 1 carve-out (widening the check to != 0 would put an error page on WebHTTrack's cancel button) and the rollback that is not an abort. The header and the man page now say the exit status and exit_xh are separate channels that happen to share the integer 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
hts_is_exiting() already returns 2, for a session that transferred nothing and was rolled back, which exits 0. Both functions are exported, so 2 would have meant the opposite thing on the other channel. 3 is free: hts_main2() only ever returned -1, 0 and 1, main() returns that value and nothing else, and exit_xh holds only -1, 0, 1 and 2, so the two value spaces are now disjoint. The header and the man page keep saying the exit status and exit_xh are separate channels, so a fourth value does not reopen this. The tests assert through their own named constant, so both mutants still fire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
A -#L link cap exits 3 while --max-time and --max-size exit 0, so a user who meets one and then the other would read the status as unreliable. The caps stop through hts_request_stop(), the same channel as ^C, because meeting a budget is the outcome that was asked for; -#L cuts short work that was asked for. Documented in the man page's EXIT STATUS section and beside HTS_EXIT_MIRROR_ABORTED. The log file calls a capped mirror aborted either way, which the man page now warns about rather than leaving to be discovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
xroche
enabled auto-merge (squash)
August 25, 2026 08:07
xroche
added a commit
that referenced
this pull request
Aug 25, 2026
`httpmirror()` already knows whether the crawl reached its end. It computes `aborted` next to the purge decision (`src/htscore.c:2076`), turns it into `completed`, uses that to decide whether the change report is real, and then throws it away, returning 1 for a finished mirror and an abandoned one alike. `main` re-derived completion from that return code, so it printed `Done.` for a mirror the engine had given up on. Re-deriving it from `exit_xh` instead, which is where this PR started, is wrong in the same way and for the same reason: `sig_leave` (^C) and `back_checkmirror` (a `--max-time` or `--max-size` cap) raise `state.stop` and never touch `exit_xh`, so the two aborts users actually hit would still have said `Done.` So `httpmirror()` now hands the verdict back through a `completed` out-param and `main` renders it. False for every bailout and for any stop however it arrived; true at the natural end, and for `--why`, which answers a filter question without mirroring anything. That also settles a reporting inconsistency. The `-E`/`-M` caps are two-stage (#77/#481): the cap raises `state.stop`, and only an overrun past the grace reaches `exit_xh`. A crawl that drained inside the grace and one that did not would have printed different things for the same command. The engine's own verdict makes both stages read alike. The two-stage behaviour itself is untouched and still worth fixing separately. On the shell protocol, since the issue named it as the risk: nothing consumes `TRANSFER DONE`, and nothing can, because it never reaches a stream. `HT_PRINT` expands to `strcatbuff(opt->state.HTbuff, ...)` and `HTT_REQUEST_END` to nothing, so the token is appended to a scratch buffer nothing reads after the mirror (`ask_continue()` is the only later reader and clears it first). One hit in the tree, the write itself. WebHTTrack never reaches that arm either: `htsweb.c` calls `hts_main2()` in-process with callbacks and never sets `opt->shell`, and `htsserver.c` spawns no crawl. What a shell-mode consumer parses is the per-second `TIME`/`TOTAL`/`RATE`/`SOCKET`/`LINK`/`INMEM` block in `htsparse.c`. Measured rather than assumed: the same crawl through a master binary and a patched one, once completing and once aborting, gives identical `-#S` stdout apart from the timestamp. Now that #1419 has landed, measured per class on the merged tree rather than claimed as a blanket: a full disk and a link table past `-#L` give `Mirror not completed` on stdout, `* mirror aborted before completion` on stderr and exit 3; a `--max-time` cap and a ^C give the same stdout line, a silent stderr and exit 0. That last pair is not a contradiction with #1419's reasoning that meeting a budget is the outcome that was asked for. The status answers whether anything failed, and nothing did; the stdout line answers whether the site was covered, and it was not. `Done.` there would be the misleading half. 364 covers all three ways an abort reaches the engine (a `-#L` bailout, a full disk for `exit_xh`, and a cap and a ^C for `state.stop`), plus `--why` and the shell stream. The cap and ^C arms were confirmed red against the previous HEAD before the fix landed. It runs rather than skips on Windows: the `-#L` and cap arms need neither `/dev/full` nor deliverable signals, so the two arms that do are guarded and report what they could not cover instead of skipping the file. Mutants, re-run after merging master: dropping `state.stop` from `aborted` reds the cap arm, dropping `exit_xh` reds the disk arm, never printing `Done.` reds the completed arm, dropping `--why`'s flag reds its arm, and giving the shell arm a real `printf` reds the shell arm. Closes #1420 --------- Signed-off-by: Xavier Roche <roche@httrack.com> Co-authored-by: Claude Opus 5 (1M context) <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.
hts_main2()returned 0 whatever the mirror did, so a script could not tell a finished mirror from one the engine gave up on halfway. It now returns 3 for a mirror that started and did not finish. This changes an exported API's documented return, which Xavier approved ahead of the work.By class:
^C,hts_request_stop(), a callback returning 0, WebHTTrack's cancel); or nothing transferred and the session was rolled back to the last good one-#L-#E/-#Rcache operation failed. No mirror ranNot every user-set limit aborts, and that is worth your eye:
-#Lexits 3 while--max-timeand--max-sizeexit 0, because the caps stop throughhts_request_stop(), the same channel as^C, on the reading that meeting a budget is the outcome asked for while a link cap cuts short work that was asked for. Both the man page and the header say so, since a user who meets one and then the other would otherwise call the status unreliable. (The log calls a capped mirror aborted either way, and stdout saysDone.for both, which is #1420.)3 and not 2, on Xavier's call after review.
hts_is_exiting()already returns 2 for the rolled-back session, which exits 0, so 2 would have meant the opposite thing on the other channel and an embedder reading one as the other would get it backwards. 3 is free:hts_main2()only ever returned -1, 0 and 1;main()returns that value and nothing else;exit_xh, which is whathts_is_exiting()hands back, holds only -1, 0, 1 and 2, so the two value spaces are now disjoint rather than merely renumbered. The header and the man page both say the two are separate channels, so a fourth value does not reopen this.Nine live sites set
exit_xh = -1and all reach that status. Eight already existed; the ninth is added here, because the parser hit the-#Lcap athtsparse.c:3182, loggedToo many URLs, and returned quietly, whilehtsAddLink's identical refusal aborted. One limit, two answers.htsserver.cbranches on this status to render its error block, which is why an exit the caller asked for stays 0: an abort there would put a red error page on the cancel button.hts_errmsg()was empty on this path, so the abort now fills it; without that the status arrived as a bare*on stderr.No soname bump, and I do not read this as an ABI break: the signature is unchanged, no installed struct changed shape (the
htsopt.hedit is a comment), andHTS_EXIT_MIRROR_ABORTEDis a new macro. Only the value a caller reads back moved.Three tests carry it, all through the constant rather than the literal. 360 asserts the abort across four producers and 0 for a mirror that finishes and one that fits under the link cap, and requires a refused command line to exit 255 exactly. 350 asserts it on its
/dev/fullarms, including the four #1417 added, and 0 on both its EPIPE and SIGTERM arms. 240 already called its-#Lpass an aborted mirror in its own header, so it asserts the abort there and 0 on the dead-server pass, where the session is rolled back rather than left half written. Mutants: reverting the return reds all three files, always aborting reds the clean-mirror control, and widening the check toexit_xh != 0reds 350's SIGTERM arm. Thehttpmirror() == 0branch also sets the status and is live rather than dead, but all six returns behind it are allocation or size-overflow guards, so no black-box arm reaches them.Done.still goes to stdout for an aborted mirror: that is #1420, whose fix lands on theopt->shellTRANSFER DONEtoken WinHTTrack and htsserver parse.Closes #1394