A timeout storm reports Success and withdraws the resume offer - #221
Merged
Merged
Conversation
The engine counts a link the server refused and a link whose transfer failed into the same stat_errors, because both reach one hts_log_print call. So a crawl that lost every link to timeouts returned 0 with a clean queue, exactly as a finished crawl does. The pane said Success, and leavesPendingWork() read the same 0 and cleared the interrupted marker, so the user was not offered the retry that would have filled the holes. Engine PR #1676 adds stat_transport_failures, counted where back_transfer_failed() already decides. This bumps the pin to it and reads the count in three places: the JNI copies it, HTTrackStats carries it, and leavesPendingWork() takes it as a third input. MirrorOutcome gains INCOMPLETE, which names both counts and keeps the folder link, and it is weighed after every named ending so a user stop or an abort still wins. The pin moves from 5f71d45 to 9b15ccd, which is two commits. The other one only touches engine tests. HTTRACK_VERSIONID stays 3.50.2, so the versionName prefix does not move, and the engine added no source file, so Android.mk needs no new entry. Verified by building the APK, which is what proves the JNI reads a field that exists: at the old pin the same build fails with "no member named 'stat_transport_failures'". Five mutants of the two new decisions were all killed, covering a dropped term, a polarity flip, a reordering against the user stop, and a swap of the two counts in the message. Signed-off-by: Xavier Roche <xroche@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
…r tests The null check on lastStats was dead. The JNI runs its final stats refresh unconditionally after hts_main2 returns, and its else branch throws rather than returning. So lastStats is always set wherever that line runs. The identical guard further down IS live, because an IOException thrown before engine.main() reaches it with lastStats still null. Same field, different reachability, so the guard was copied to a place that cannot use it. The Incomplete message no longer prints an error count. fspc() only increments stat_errors inside its `if (fp != NULL)` branch, and Android emits -Q when the user turns logging off. So the pane could read "5 links failed to transfer, 0 errors". The transport count is not log-derived, so it stays. A mutation matrix over every clause showed most of the new test rows were subsets of each other. One was a byte-for-byte copy of a row already on master. Test lines drop from 93 to 55 with no mutation left unkilled. The end-to-end marker test called leavesPendingWork itself, so it never read what CrawlRun passes. Reverting CrawlRun to a literal left it green. It is gone, and the source-text guard that can see the argument now asserts it. Signed-off-by: Xavier Roche <xroche@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
Round 2 cut a test row too many. No row paired failed transfers with an empty mirror, so conjoining the INCOMPLETE branch with filesWritten != 0 survived every test. The row is back, and that mutant now dies. Two agents disagreed about a second restore, the abort rows. A matrix over 23 mutants settled it: the user-stop and cap rows already catch all three hoist positions, so those rows stay deleted. The stats() short form had one caller left, which is why the 3-argument reopensOnContinue went in the first place. The @PARAM on leavesPendingWork said what HTTrackStats.transportFailures already says, so it is a pointer now. Signed-off-by: Xavier Roche <xroche@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The comment said the verdict omits the error count because -Q leaves stat_errors at 0. Engine commit f6426310 fixes that, so the reason dies at the next pin bump. State the durable one instead, because stat_errors also counts errors the server answered, and those are not holes to refill. Also make the INCOMPLETE summary a full sentence like its siblings. And name the trailing argument order of check() and verdict(), which reads as three bare numbers at about thirty call sites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
xroche
enabled auto-merge (squash)
September 16, 2026 07:17
xroche
added a commit
that referenced
this pull request
Sep 16, 2026
The abort loop in `MirrorOutcomeTest` passed a zero failure count, so an abort that lost to a failed transfer went unnoticed. That mutant survives master and dies here, measured both ways. The one surviving `Incomplete` row had a zero error count, so a message adding the two counts together read correctly by luck. Raising that row's error count to 5 against 3 failures kills it, and adds no row. #221's review also asked for a null guard on `lastStats`, and this drops it. The value cannot be null there. The guard also let the next line fault after committing `pendingWork = false`, which cleared the resume marker on the path it claimed to protect. --------- 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 crawl that lost every link to a timeout reported
Success, then cleared the interrupted marker. So the user was never offered the retry that would have filled the holes. The engine counts a refused link and a failed transfer into the samestat_errors. Nothing on the Android side could tell the two apart.Engine PR #1676 adds
stat_transport_failures. This bumps the pin to it, copies the count through the JNI ontoHTTrackStats, and givesleavesPendingWorka third input. A run that lost links now readsIncompleteand keeps its folder link. A user stop or an abort still wins, becauseof()weighs the new branch last.The pin moves two commits, and the other one only touches engine tests.
HTTRACK_VERSIONIDstays3.50.2, so theversionNameprefix does not move.Building the APK proves the JNI reads a field that exists. At the old pin the same build fails with
no member named 'stat_transport_failures'. 23 mutants of the new branches all died.Closes #74, phase 2 of 4 and the last one.