Make a failed Scholar scrape say what it actually got (#8)

The block detection added in #7 did not work. It waited for challenge
elements guessed at without being able to observe the real page - Scholar
is unreachable from the development sandbox - and matched none of them, so
a blocked scrape still cost the full 30s timeout and still reported a bare
"Timeout exceeded". Guessing again would repeat the mistake, so this
changes the shape of the failure instead.

An unclassifiable page now raises GoogleScholarUnavailableError carrying
the final URL, the page title and a text excerpt, so the next live call
identifies whatever Google is actually serving.

Two real defects in the first attempt, both now covered: challenge
elements were matched by visibility, but reCAPTCHA's container is commonly
zero-height until it loads, so detection sat through the entire timeout on
exactly the pages it was written to catch; and the evidence was purely
markup-based, where a /sorry redirect survives Google restyling the page.

The hidden-element test originally asserted only that the right exception
came out, which a visibility-based version also satisfies - just slowly.
Since speed is the point of racing the selectors, it now asserts timing.

260 tests, lint clean, four mutants aimed at this code all caught.

CONTRIBUTING.md

@@ -30,7 +30,7 @@ ruff check .   # same lint CI runs
 pytest
 ```
 
-257 tests, a few seconds, fully offline. All outbound HTTP is mocked with
+260 tests, a few seconds, fully offline. All outbound HTTP is mocked with
 `respx`; the Playwright-driven scrapers (Bing, Brave, Google Scholar,
 Google Patents search) run against a real headless Chromium but navigate to
 local fixture HTML instead of the live sites — see `tests/helpers.py` for

serp.py

@@ -71,6 +71,17 @@ class GoogleScholarBlockedException(Exception):
             "instead of results); the deployment's IP is likely rate-limited.")
 
 
+class GoogleScholarUnavailableError(Exception):
+    """Scholar returned neither results nor anything we can classify.
+
+    Carries what the page actually was - final URL, title, a text excerpt -
+    because the alternative is a bare selector timeout that says nothing.
+    The first attempt at block detection here guessed at Google's challenge
+    markup and matched none of it; the error was useless for working out
+    why. Whatever we fail to classify, we report.
+    """
+
+
 class BrowserUnavailableError(Exception):
     """Raised when a Playwright-backed query is attempted but the browser failed to start."""
 
@@ -141,17 +152,28 @@ async def _block_stylesheet_and_image_resources(route, request):
 SCHOLAR_SELECTOR_TIMEOUT_MS = 30_000
 
 _SCHOLAR_RESULT_SELECTOR = "div.gs_ri"
-# Elements Google's challenge page carries. Waiting for these *alongside*
-# the results selector is what makes a block fail fast instead of costing a
-# full timeout.
-_SCHOLAR_BLOCK_SELECTOR = "form#captcha-form, #recaptcha, .g-recaptcha, #infoDiv"
-# Fallback for a challenge page carrying none of the above - Google varies
-# the markup, so match on the wording it has used consistently.
+# Elements Google's challenge pages have carried. Waiting for these
+# *alongside* the results selector is what lets a block fail fast instead
+# of costing a full timeout. Matched on presence rather than visibility:
+# reCAPTCHA renders in an iframe and its container is commonly zero-height
+# until it loads, so a challenge element can be attached but never visible.
+_SCHOLAR_BLOCK_SELECTOR = (
+    "form#captcha-form, #recaptcha, .g-recaptcha, #infoDiv, "
+    "form[action*='sorry'], #gs_captcha_f, img[src*='sorry'], "
+    "div#af-error-container, input[name='captcha']"
+)
+# Wording Google's interstitials have used. Only consulted once the
+# selectors have failed - a legitimate Scholar search for "unusual traffic"
+# returns papers whose snippets contain that phrase.
 _SCHOLAR_BLOCK_TEXT_MARKERS = (
     "unusual traffic",
     "not a robot",
     "/sorry/index",
+    "automated queries",
+    "your computer network",
 )
+# A /sorry redirect is markup-independent evidence, unlike any selector.
+_SCHOLAR_BLOCK_URL_MARKERS = ("/sorry", "/challenge")
 
 
 def _looks_like_scholar_block(page_content: str) -> bool:
@@ -159,24 +181,64 @@ def _looks_like_scholar_block(page_content: str) -> bool:
     return any(marker in lowered for marker in _SCHOLAR_BLOCK_TEXT_MARKERS)
 
 
+def _url_looks_blocked(url: Optional[str]) -> bool:
+    return bool(url) and any(m in url.lower() for m in _SCHOLAR_BLOCK_URL_MARKERS)
+
+
+async def _describe_page(page: Page, final_url: Optional[str]) -> str:
+    """A short, human-readable summary of whatever we actually got.
+
+    Deliberately best-effort: this runs on an error path, so a failure to
+    read the page must not replace the original problem with a new one.
+    """
+    parts = [f"url={final_url or getattr(page, 'url', 'unknown')!r}"]
+    try:
+        parts.append(f"title={await page.title()!r}")
+    except Exception:
+        parts.append("title=<unreadable>")
+    try:
+        text = " ".join((await page.inner_text("body")).split())
+        parts.append(f"text={text[:300]!r}")
+    except Exception:
+        parts.append("text=<unreadable>")
+    return ", ".join(parts)
+
+
 async def _extract_google_scholar_results(page: Page, n_results: int,
-                                          timeout_ms: int = SCHOLAR_SELECTOR_TIMEOUT_MS) -> list[dict]:
+                                          timeout_ms: int = SCHOLAR_SELECTOR_TIMEOUT_MS,
+                                          final_url: Optional[str] = None) -> list[dict]:
     """Extract results from an already-loaded Google Scholar results page.
 
-    Raises GoogleScholarBlockedException when Google served its anti-bot
-    interstitial instead of results. A page with neither results nor a
-    recognisable challenge still raises the underlying timeout, so a real
-    selector regression can't hide behind the block excuse.
+    Raises GoogleScholarBlockedException when Google served an anti-bot
+    interstitial instead of results, and GoogleScholarUnavailableError -
+    carrying the page's URL, title and a text excerpt - when it served
+    something we can't classify. A bare selector timeout is never the
+    outcome, because it tells whoever reads the logs nothing about why.
     """
+    url = final_url if final_url is not None else getattr(page, "url", None)
+
+    async def _blocked() -> bool:
+        if _url_looks_blocked(url):
+            return True
+        try:
+            if await page.locator(_SCHOLAR_BLOCK_SELECTOR).count():
+                return True
+        except Exception:
+            pass
+        return _looks_like_scholar_block(await page.content())
+
     try:
         await page.wait_for_selector(
-            f"{_SCHOLAR_RESULT_SELECTOR}, {_SCHOLAR_BLOCK_SELECTOR}", timeout=timeout_ms)
+            f"{_SCHOLAR_RESULT_SELECTOR}, {_SCHOLAR_BLOCK_SELECTOR}",
+            state="attached", timeout=timeout_ms)
     except PlaywrightTimeoutError:
-        if _looks_like_scholar_block(await page.content()):
+        if await _blocked():
             raise GoogleScholarBlockedException() from None
-        raise
+        raise GoogleScholarUnavailableError(
+            "Google Scholar returned neither results nor a recognisable "
+            f"challenge page ({await _describe_page(page, url)})") from None
 
-    if await page.locator(_SCHOLAR_BLOCK_SELECTOR).count():
+    if await page.locator(_SCHOLAR_RESULT_SELECTOR).count() == 0 or await _blocked():
         raise GoogleScholarBlockedException()
 
     items = await page.locator(_SCHOLAR_RESULT_SELECTOR).all()

tests/test_serp_playwright.py

@@ -21,6 +21,7 @@ from serp import (
     BraveSearchBlockedException,
     BrowserUnavailableError,
     GoogleScholarBlockedException,
+    GoogleScholarUnavailableError,
     _block_stylesheet_and_image_resources,
     _extract_bing_results,
     _extract_brave_results,
@@ -223,17 +224,71 @@ async def test_google_scholar_block_is_detected_from_page_text_alone(page_factor
         await _extract_google_scholar_results(page, 10, timeout_ms=1000)
 
 
-async def test_a_page_with_neither_results_nor_a_block_still_times_out(page_factory):
-    """An unrecognised empty page must not be silently reported as a block -
-    that would hide a genuine selector regression behind a plausible excuse.
-    """
-    from playwright.async_api import TimeoutError as PlaywrightTimeoutError
-
+async def test_a_page_with_neither_results_nor_a_block_is_not_called_a_block(page_factory):
+    """An unrecognised page must not be reported as a block - that would
+    hide a genuine selector regression behind a plausible excuse."""
     page = await page_factory("<html><body><p>something else entirely</p></body></html>")
 
-    with pytest.raises(PlaywrightTimeoutError):
+    with pytest.raises(GoogleScholarUnavailableError) as exc_info:
+        await _extract_google_scholar_results(page, 10, timeout_ms=1000)
+
+    assert not isinstance(exc_info.value, GoogleScholarBlockedException)
+
+
+async def test_an_unrecognised_page_reports_what_it_actually_saw(page_factory):
+    """The first attempt at this fix guessed at Google's challenge markup
+    and matched none of it, and the resulting error - a bare selector
+    timeout - said nothing about what was actually served. Whatever we
+    fail to classify, the exception must carry enough of the page to
+    identify it from a single live call, rather than needing another
+    round of guesswork.
+    """
+    page = await page_factory(
+        "<html><head><title>Totally Unexpected Page</title></head>"
+        "<body><p>a distinctive sentence that identifies this page</p></body></html>")
+
+    with pytest.raises(GoogleScholarUnavailableError) as exc_info:
         await _extract_google_scholar_results(page, 10, timeout_ms=1000)
 
+    message = str(exc_info.value)
+    assert "Totally Unexpected Page" in message, "page title must be reported"
+    assert "a distinctive sentence" in message, "page text excerpt must be reported"
+
+
+async def test_a_hidden_challenge_element_still_counts_as_a_block(page_factory):
+    """reCAPTCHA renders inside an iframe and its container is commonly
+    zero-height until it loads, so a challenge element can be present but
+    never 'visible'. Detection has to key on presence, not visibility -
+    the first version of this waited for visibility and so sat through the
+    full timeout on exactly the pages it was written to catch.
+    """
+    import time
+
+    page = await page_factory(
+        '<html><body><div class="g-recaptcha" style="display:none"></div></body></html>')
+
+    started = time.perf_counter()
+    with pytest.raises(GoogleScholarBlockedException):
+        await _extract_google_scholar_results(page, 10, timeout_ms=5000)
+    elapsed = time.perf_counter() - started
+
+    # Asserting the timing, not just the exception: the fallback checks
+    # below catch a hidden element anyway, so a version that waits out the
+    # whole timeout first still raises the right error - it just does it
+    # slowly, which is precisely the bug this is guarding against.
+    assert elapsed < 2, f"took {elapsed:.1f}s to notice a hidden challenge element"
+
+
+async def test_a_sorry_redirect_url_counts_as_a_block(page_factory):
+    """Google redirects blocked clients to /sorry/index. The final URL is
+    markup-independent evidence, unlike any selector we can guess at."""
+    page = await page_factory("<html><body><p>nothing recognisable here</p></body></html>")
+    page.url_override = "https://scholar.google.com/sorry/index?continue=..."
+
+    with pytest.raises(GoogleScholarBlockedException):
+        await _extract_google_scholar_results(
+            page, 10, timeout_ms=1000, final_url=page.url_override)
+
 
 async def test_normal_scholar_results_are_unaffected(page_factory):
     """The block check must not cost the happy path anything."""