Skip to content

Fix crawler hanging indefinitely when a sitemap request fails - #54

Closed
and-ri wants to merge 2 commits into
StJudeWasHere:mainfrom
and-ri:fix/sitemap-waitgroup-deadlock
Closed

and-ri wants to merge 2 commits into
StJudeWasHere:mainfrom
and-ri:fix/sitemap-waitgroup-deadlock

Conversation

@and-ri

@and-ri and-ri commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Problem

A crawl can hang forever before a single URL is fetched. The project stays in
the "Crawling" state indefinitely, with 0 crawled URLs, and never recovers.

I hit this on three WordPress sites whose Yoast sitemap index takes longer than
the 10s ClientTimeout to generate. Goroutine dump after ~48 hours:

goroutine 367 gp=0xc0008cd500 m=nil [sync.WaitGroup.Wait, 2876 minutes]:
sync.(*WaitGroup).Wait(0xc0003bf4c0)
/usr/local/go/src/sync/waitgroup.go:206 +0x85
github.com/stjudewashere/seonaut/internal/crawler.(*SitemapChecker).ParseSitemaps(...)
/app/internal/crawler/sitemap_checker.go:83 +0xcc
github.com/stjudewashere/seonaut/internal/crawler.(*Crawler).Start(0xc000a30300)
/app/internal/crawler/crawler.go:142 +0xe9

Root cause

In ParseSitemaps, wg.Done() is the last statement of the goroutine, so the
early return on a failed request skips it:

wg.Add(1)
go func(s string) {
    resp, err := sc.client.Get(s)
    if err != nil {
        return          // wg.Done() is never reached
    }
    ...
    wg.Done()
}(s)

The counter stays above zero and wg.Wait() blocks forever.

The 2h context.WithTimeout in NewCrawler does not help. Start() calls
ParseSitemaps before the crawl loop, and the context is only observed inside
crawl(), which is never reached:

c.setupSitemaps()
if c.sitemapExists && c.options.CrawlSitemap {
    c.sitemapChecker.ParseSitemaps(...)   // blocks here
}
...
for rm := range c.crawl() { ... }          // never reached

Second issue

checkIndex calls sitemap.ParseIndexFromSite, which fetches through
http.Get on http.DefaultClient - no timeout at all. An unresponsive sitemap
index hangs the crawl with no stack trace pointing anywhere useful. That request
also bypassed the project's User-Agent and basic auth settings.

Changes

  • defer wg.Done() as the first statement of the goroutine in ParseSitemaps.
  • checkIndex fetches the index through sc.client and parses it with
    sitemap.ParseIndex, so the configured timeout, User-Agent and basic auth
    apply.
  • Regression test with a client that always fails, asserting ParseSitemaps
    returns instead of blocking.

Reproducing

Any site whose sitemap takes longer than ClientTimeout (10s) to respond, with
"Crawl sitemap" enabled. A large WordPress install with an uncached Yoast
sitemap index is the common case. The new test reproduces it without network
access - on main it hangs until the test timeout.

Notes

The two fixes are independent and are in separate commits, in case you want only
the first one.

ClientTimeout is left at 10s. After this change a slow sitemap is skipped
rather than crawled, which may deserve a separate discussion - a longer timeout
just for sitemap requests would be reasonable, since they are far larger than a
typical page.

…lock

ParseSitemaps returned from the goroutine without calling wg.Done() when
sc.client.Get failed, leaving the WaitGroup counter permanently above zero.
Crawler.Start blocks on wg.Wait() before reaching the crawl loop, so the
crawl never starts and never times out: the context deadline is only
checked inside crawl(). The project stays in "Crawling" state forever with
zero crawled URLs.
@and-ri

and-ri commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, I didn't notice that such a PR already exists.

@and-ri and-ri closed this Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant