fix(robots): cache misses and share in-flight robots.txt fetches

[Mr-Neutr0n]

Two gaps in the robots.txt cache, both of which cost real time during a crawl. I hit this with respect_robots=true over a domain that has no robots.txt.

1. Misses are never cached

_get_robots_parser only writes to _robots_cache on the success path. When _fetch_robots_txt returns None — domain has no robots.txt, or it is temporarily unreachable — nothing is cached, so the next URL on that domain fetches again. And again.

That is not just a wasted request. _fetch_robots_txt tries the HTTPSession path first and then falls through to the urllib fallback, each with _FETCH_TIMEOUT = 10. On a domain that black-holes the request, every single URL in the crawl can pay up to ~20s before the fetch is given up on and the URL is allowed by default.

2. Concurrent checks on one domain each fetch separately

The cache lock is released before the fetch and re-taken after it:

async with lock:
    ...check cache...
raw = await _fetch_robots_txt(domain)      # lock not held
...
async with lock:
    _robots_cache[domain] = (parser, now_ts)

So every caller that arrives while the first fetch is in flight misses the cache and starts its own. crawl.py and bulk_fetch check URLs on one domain concurrently, which is exactly the shape that triggers it.

Measured against main, with _fetch_robots_txt stubbed to count calls:

concurrent burst on one domain -> robots.txt fetches: 20     (20 URLs)
5 later sequential checks      -> robots.txt fetches: 5      (unreachable robots.txt)
reachable, concurrent burst    -> robots.txt fetches: 20

After this change:

concurrent burst on one domain -> robots.txt fetches: 1
5 later sequential checks      -> robots.txt fetches: 0
reachable, concurrent burst    -> robots.txt fetches: 1

The change

• _robots_cache now stores RobotFileParser | None, so a miss is a cache entry rather than the absence of one. Misses expire on a separate _ROBOTS_MISS_TTL (5 min) rather than the 1 hour used for hits — an unreachable robots.txt is often transient, and I did not want a five-second blip to suppress robots for the next hour.
• The fetch+parse moved into _load_robots_parser, run as one shared task per domain and tracked in _robots_inflight. Callers that arrive mid-fetch await the existing task instead of starting a second one. The task clears its own entry in a finally.
• asyncio.shield on the await, so one caller being cancelled does not cancel the fetch the other callers are waiting on.
• The cache timestamp is now taken after the fetch rather than before, so a slow fetch no longer eats into its own TTL.

Single-flight is keyed per domain, so unrelated domains still fetch in parallel — there is a test for that, since it would be an easy thing to break with a single global lock.

Behaviour is otherwise unchanged: unreachable and unparseable robots.txt still allow by default, and clear_robots_cache() still clears everything.

Tests

Four added to TestIsAllowed, alongside the existing test_cache_prevents_refetch:

• test_unreachable_robots_is_cached — 5 URLs, 1 fetch
• test_concurrent_checks_share_one_fetch — 20 concurrent URLs, 1 fetch
• test_concurrent_checks_on_distinct_domains_are_not_serialized — guards against fixing this with a global lock
• test_cached_miss_expires — miss TTL is honoured

The first two fail on main with assert 5 == 1 and assert 20 == 1.

pytest tests/test_robots.py → 15 passed. Full suite 761 → 765 passed with the same 2 pre-existing test_browser_lifecycle failures before and after (they need a real playwright install).

---

I use AI assistance in my workflow; the numbers above are from runs against 86d1b13.