Persist the unfinished frontier so --resume resumes (issue #36) - #79
Merged
Conversation
state.json held only the visited set. The pending frontier lives in the
cloner's channels and was never written out, and load() seeded both the
visited and seen maps from what it read.
Follow a resumed run from there. The only URL enqueued at startup is the
seed, plus anything from sitemap.xml. On a resumed run the seed was already
written, so isVisited() is true, enqueuePage returns false, and nothing is
queued. The frontier is otherwise rebuilt purely by re-rendering pages and
following their links, which resume guarantees never happens. The run prints
its summary and exits successfully with pages 0 and most of the site missing.
Only sites with a sitemap.xml appeared to resume, because those URLs are
seeded independently on every run, which is presumably why this survived
eleven releases. The README promises the opposite:
Hit Ctrl-C and it saves its place on the way out; run it again and it
picks up where it stopped.
Save the unfinished frontier next to the visited set and re-queue it at
startup. Each entry carries its depth, or --max-depth would silently change
meaning across a restart. A state file written before this loads fine and
resumes with nothing outstanding, which is what that run recorded.
A page is pending from the moment it is offered until it is written, so an
interrupted render, a page held back by --max-pages, and a page that failed
all carry over. Retrying failures is the "memory of what failed" the issue
asked for; until now the only way to pick them up was --refresh, which
re-renders the whole site. A page robots.txt disallows is marked done rather
than left pending, since a later run would only skip it again.
The run also reports what it is leaving behind, so a short run no longer
looks like a finished one.
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.
Fixes the main defect in #36. The CDP error in that issue's title was already fixed in
eb683ddthe day after it was filed; this is the aside underneath it, which turned out to be the more serious of the two.What the reporter said
--resumeis already on by default, so asking for--continueis the tell. They had to reach for--refreshbecause resume does not resume.What was happening
state.jsonheld only the visited set:The pending frontier lives in the cloner's channels and was never written out, and
loadseeded bothvisitedandseenfrom what it read.Follow a resumed run from there. The only URL enqueued at startup is the seed, plus anything from
sitemap.xml. On a resumed run the seed was already written, soisVisited(key)is true,enqueuePagereturns false, and nothing is queued. The frontier is otherwise rebuilt purely by re-rendering pages and following their links, which resume guarantees never happens.So the run printed its summary and exited successfully having done no work at all. On a 5,000 page site interrupted at page 500, restarting got you nothing, and the exit code and the output both said everything was fine.
Only sites with a
sitemap.xmlappeared to resume, since those URLs are seeded independently on every run. That is presumably why this survived eleven releases.--refreshworked becausecloner.go:131deliberately skips loading the state, so the seed enqueues normally and everything is re-rendered from scratch. That is exactly what the reporter observed and reasonably misread as "helped to scrap more".The README promises the opposite:
Reproduced on
mainagainst a local 8 page chain site:The fix
Save the unfinished frontier next to the visited set and re-queue it at startup.
Each entry carries its depth, because
--max-depthis measured from the seed and a resumed run has no way to recompute it. A state file written by an older kage has nopendingkey, loads fine, and resumes with nothing outstanding, which is exactly what that run recorded.A page is pending from the moment it is offered until it is written, which falls out nicely:
--max-pagescarries over, so-p 20to look at a site and then a plain run to finish it is now a workflow rather than a dead end--refresh, which re-renders everything.A page
robots.txtdisallows is marked done rather than left pending, since a later run would only fetchrobots.txtand skip it again.The run also says what it is leaving behind, so a short run stops looking like a finished one:
Verified
Same site, with the fix:
New tests:
TestCloneResumeFinishesTheCrawldrives real Chrome over a five page chain, stops the first run partway, and asserts the resumed run walks the rest and that a third run then has nothing to do. This fails onmain.TestCloneResumeRetriesFailuresdrops the connection on one page during the first run, brings it back for the second, and asserts the page is retried. The connection is dropped rather than answered with a 5xx because Chrome renders an error page happily.TestFrontierPersistsUnfinishedWork,TestFrontierMarkDoneDropsWorkandTestFrontierLoadsStateWrittenBeforePendingcover the state file without Chrome.TestCloneResumeSkipsVisitedstill passes unchanged: after a complete crawl there is nothing pending, so a resumed run correctly does nothing.go build ./...,go vet ./...and the fullgo test ./...pass with Chrome present.Also worth knowing
The rest of #36 is not addressed here and should be split off:
Object reference chain is too longerror in the title was fixed ineb683ddon 2026-06-17, the day after the report. The reporter was never told, and the issue is still open. Worth one comment and a question about whether 161 occurrences on one site still reproduce, since that number is not random.