Respect robots.txt by default - #821
Conversation
There was a problem hiding this comment.
I checked this against #614 locally. The direction looks consistent with the issue, but the full suite needs one follow-up before this can merge cleanly.
What I ran:
GOCACHE=.codex-tmp/go-cache GOTMPDIR=.codex-tmp/go-tmp GOMODCACHE=.codex-tmp/go-mod go test . -run 'TestRobots|Robots|IgnoreRobots|NewCollector' -count=1 -v
GOCACHE=.codex-tmp/go-cache GOTMPDIR=.codex-tmp/go-tmp GOMODCACHE=.codex-tmp/go-mod go test ./... -count=1
git diff --check upstream/master...HEADThe focused robots/default tests pass, and git diff --check is clean. GitHub reports no checks for this PR at the moment.
go test ./... fails in queue/TestQueue:
queue_test.go:72: wrong Queue implementation: items = 3000, requests = 0, success = 0, failure = 0
That test creates an httptest server that only handles /delay. With robots respected by default, the collector probes /robots.txt, the test server closes that unsupported route, and the queued URLs never reach OnRequest. I temporarily added colly.IgnoreRobotsTxt() to that test collector and confirmed go test ./queue -run TestQueue -count=1 -v passes, so this looks like a test-suite alignment issue rather than a blocker to the behavior change itself.
I’d suggest updating tests that intentionally expect the old ignore-robots behavior to opt out explicitly, and adding a small assertion around the new NewCollector() default. Since this changes default crawler behavior, a docs/changelog note would also help downstream users notice the migration path.
- Add TestNewCollector/Defaults and TestRobotsDisallowedByDefault to cover the new default - Opt queue.TestQueue out of robots.txt, as its test server only serves /delay - Document the IgnoreRobotsTxt default on the field
407e13f to
aa80da7
Compare
|
Confirmed. The queue test server only answers I went with For the default, I added two tests: On docs: the default is now stated on the Worth flagging while you're looking at it: respecting robots.txt also means a 5xx on No checks showing up on the PR, so I ran CI's sequence locally: |
These changes aim to address #614.
Ignoring the
robots.txtfile by default raises ethical concerns.Additionally, the documentation regarding the default configuration is arguably incomplete. Considering this, the existence of
IgnoreRobotsTxt()function implies that the default behavior is to respect it in the first place.Important
This alters default behavior and so presumably warrants a note in the release notes whenever it is released. A collector created through
NewCollector()now consultsrobots.txtand will decline URLs its target disallows.Two further consequences seem worth stating plainly, as neither is obvious. A
5xxresponse to/robots.txtrenders a host entirely uncrawlable, since it is read as a full disallow, and a failed connection causes the request to error out. A404maps to allow-all and changes nothing. None of this is new logic, it simply was not reachable by default until now.The previous behavior remains available per collector
or globally through the environment
export COLLY_IGNORE_ROBOTSTXT=yThe change itself is confined to
Collector.Init(), which now leavesIgnoreRobotsTxtatfalse. The field's documentation has been amended to state that default explicitly. Tests covering the new default have been added, and the one existing test that relied on the old one now opts out explicitly.The changes can be exercised with the focused tests
and the full suite, together with the remaining checks CI performs, with
gofmt -l -d ./ golint -set_exit_status go vet ./... go test -race -coverprofile=coverage.txt -covermode=atomic ./...