--purge-old deleted a file whose re-fetch never got a response - #765
Merged
Conversation
… debris A transfer that dies before a complete response has no body, yet the save path still consulted r.adr, which at that point holds whatever the aborted header read left behind: raw status-line bytes, or an empty buffer that truncated the file to zero on macOS. Require a successful transfer, as the empty-body half of the condition already did. Closes #748 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Test 93 filtered reset.bin out of its bucket lists because a connection killed before the status line surfaced differently per platform. It no longer does, so assert the resource like any other: unchanged, with the bytes pass 1 mirrored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The new test passed with no second pass at all, so a regression that stopped re-fetching would have looked green. Assert the failure the fixture provokes, give stay.bin a fresh pass-2 body so a fix that stopped overwriting anything fails, and compare reset.bin by checksum rather than by length. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
A transfer that dies on the wire leaves the previously mirrored copy in place, but back_finalize() returned without noting it, so the URL fell out of new.lst and the end-of-update purge treated the file like a page that had vanished from the site. Note the surviving copy, the way the incomplete-transfer branch above already does, for the connection-level failures htsparse.c retries on. A deliberate skip (too big, MIME-excluded, cancelled) keeps its current fate. Closes #746 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
A malformed status line, an oversized declared length or a mid-flight abort all land on STATUSCODE_INVALID, outside the retryable set, and the purge still ate the file. Invert the test: any failure keeps the copy except the codes that mean the engine passed the resource over on purpose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
xroche
force-pushed
the
fix/failed-refetch-keeps-copy
branch
from
July 27, 2026 07:23
2f8a07a to
14cc7a1
Compare
The message a cut connection produces depends on whether any bytes arrived, so matching it would fail on a runner that sees none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Signed-off-by: Xavier Roche <roche@httrack.com> # Conflicts: # tests/96_local-refetch-keep.test
Signed-off-by: Xavier Roche <roche@httrack.com> # Conflicts: # src/htsback.c # tests/96_local-refetch-keep.test
xroche
added a commit
that referenced
this pull request
Jul 27, 2026
Under `set -e` a failing command in an EXIT trap becomes the script's exit status, so a hiccup while tearing down fixtures fails a test whose assertions all passed. That is what turned `57_local-proxy-connect.test` red on the Windows x64 leg of #765: five OK lines, no FAIL, exit 1. Every EXIT trap in the suite now runs teardown with errexit off, and the signal traps keep their own `trap` line, since sharing `set +e` with HUP/INT/QUIT/PIPE/TERM would leave errexit off for the rest of a signalled run and let a torn-down test still report success. A `|| true` on the `rm` would have been smaller, but it throws away the only diagnostic, and the evidence does not say which teardown command failed: a blocked `rm -rf` on the Windows runner exits 1 and prints "Device or resource busy", while the log shows exit 1 and nothing at all. The sharing violation in the issue is the plausible mechanism rather than a confirmed one, so whatever it really is now prints its own error. `99_teardown-status.test` pins the semantics both ways and scans the suite so a new test cannot reintroduce the shape, the leaky combined trap included. The `return 0` that three `cleanup()` bodies ended with never protected anything, since errexit fires at the failing command before it is reached. Closes #773 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
1 participant
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.
A re-fetch that fails never touches the mirrored file, but
back_finalize()returned without noting the surviving copy, so the URL fell out ofnew.lstand the end-of-update purge deleted it as if the page had vanished from the site. Same family as #562 and #584, which covered the short-body path; a fetch that dies before a usable response takes a different exit and was still losing files.The fix notes the copy for any failure except the codes meaning the engine passed the resource over on purpose (too big, MIME-excluded, link test). That is wider than the reported case needs, deliberately: a malformed status line and an oversized declared length both surface as
STATUSCODE_INVALIDand lose the file the same way. An HTTP error on an update already masks itself as a 304 and keeps its copy, which is what this brings failures in line with.Test 96 now runs its update pass with purging on, and fails without the fix.
Closes #746