From 07f0e57b064188a16c6d30a849f73d1e4419ba5e Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:42:58 +0000 Subject: [PATCH 1/2] One paid solve per page, parser failures reported, atomic CSV-safe output - A page buys at most one CAPTCHA solve across every attempt, relaunch and proxy rotation (was 2 with no pool, one per exit with a pool). The suite's `solves <= 2` pinned the old behaviour; it now asserts 1. - A served page whose own total says it holds rows, parsed to none, is `parser_found_nothing`: exit 5 on page 1 (was 4), exit 6 later (was 0, complete). The canary names it. - JSON, CSV and sidecar writes are atomic, with the mode open() would give. - CSV cells starting with = + - @ TAB CR LF get a leading apostrophe; `csv_cells_escaped` in the sidecar, merged under the caller's extra. - Counted and timed flags refuse values below their floor in every entry point (exit 2 naming the flag). - Rules are stated in place instead of pointing at private notes; a suite guard keeps it that way. Captcha module notes describe this site. - The expired-endpoint hint no longer claims profiles last about a day. Co-Authored-By: Claude Opus 5.5 --- .env.example | 4 +- .github/ci_checks.py | 6 +- .github/workflows/canary.yml | 13 +- .gitignore | 14 +- CHANGELOG.md | 56 +++++- CONTRIBUTING.md | 4 +- README.md | 10 +- TROUBLESHOOTING.md | 5 + captcha_solver.py | 61 ++---- env_config.py | 2 +- fingerprint_client.py | 7 +- make_fixtures.py | 10 +- output_writer.py | 147 ++++++++++++--- page_flow.py | 210 ++++++++++++++++----- playwright_scraper.py | 36 ++-- product_parser.py | 18 +- puppeteer_scraper.py | 31 ++- scraper_api_client.py | 9 +- selenium_scraper.py | 28 +-- smoke_test.py | 355 +++++++++++++++++++++++++++++++---- 20 files changed, 783 insertions(+), 243 deletions(-) diff --git a/.env.example b/.env.example index 043300b..e189f0d 100644 --- a/.env.example +++ b/.env.example @@ -60,8 +60,8 @@ TWOCAPTCHA_KEY=your_2captcha_api_key_here # Selenium cannot use this: chromedriver's `debuggerAddress` takes a bare # host:port with nowhere to put a password. Playwright and pyppeteer can. # -# The credentials in a profile expire in about a day, so never paste a -# working one anywhere that is committed. +# The credentials in a profile expire (an HTTP 401 says so), so never paste +# a working one anywhere that is committed. GOODREADS_CDP_ENDPOINT=ws://{login}-zone-scraping_browser-country-de-pid-{profileId}:{password}@cb.2captcha.com:9222 # --- Proxy ---------------------------------------------------------------- diff --git a/.github/ci_checks.py b/.github/ci_checks.py index a638eab..8ad8d1d 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -92,8 +92,8 @@ # 32-hex shape in its markup is an asset's build fingerprint # (`loading-trans-.gif`), which no row carries and `make_fixtures.py` # takes out of every fixture. So a bare 32-hex anywhere in the tree fails, -# with no exception to reason about (§24: take a value out rather than -# exempt it). +# with no exception to reason about: a value nothing reads is taken out +# rather than exempted. SITE_PUBLIC_IDS = () def _without_site_ids(line): @@ -280,7 +280,7 @@ def tracked_files(): # into pip's vendored code and flags a 32-hex string in `_elffile.py` as # key-shaped. Correct about the string, wrong about the file — and it is the # FIRST thing a new user sees from `python3 smoke_test.py`. A guard people -# have to argue with is one they learn to suppress (CLAUDE.md §22). +# have to argue with is one they learn to suppress. # # Deliberately NOT a narrowing of what gets scanned: an untracked file is # still read, because a key pasted into a scratch file beside the scripts is diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 1c10397..38ebc02 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -3,7 +3,7 @@ # # No secrets are needed, and that is the point of running it: the README's # central claim is "no key, no proxy, no account", and an ungated scheduled -# canary is that sentence under test every morning (CLAUDE.md §21, §24). +# canary is that sentence under test every morning. # Every mode ran from a datacentre VPS when this repo was written # (2026-09-27), AWS WAF's challenge included, which the browser passes by # itself; a GitHub runner is a datacentre address too. Its first dispatch @@ -73,7 +73,7 @@ jobs: pip install -r requirements.txt -r requirements-playwright.txt playwright install --with-deps chromium - # >= 3 pages, because with one page pagination is never exercised (§11). + # >= 3 pages, because with one page pagination is never exercised. - name: Run id: run run: | @@ -87,11 +87,16 @@ jobs: run: | code="${{ steps.run.outputs.exit_code }}" echo "exit code: $code" + # A served page that stated rows and parsed to none is named on its + # own: it is this repo's parser that broke, not the site's listing. + if [ -f canary.meta.json ] && grep -q '"stop_reason": "parser_found_nothing"' canary.meta.json; then + echo "::error::parser_found_nothing: a served page stated rows and the parser read none. The page shape has moved; see canary_page*_debug.html."; exit 1 + fi case "$code" in 0) ;; 3) echo "::error::Blocked. If the log names aws-waf, the WAF's challenge did not clear by itself: it showed its CAPTCHA, or refused the runner. If it names http-403, the runner's address was refused. Either way, set the GOODREADS_PROXY secret (see the header)."; exit 1 ;; 4) echo "::error::Zero rows from a listing that is never empty. The page shape has moved; see canary_dump.html."; exit 1 ;; - 5) echo "::error::The data never arrived, or the API REFUSED the parameters (the log names its own complaint)."; exit 1 ;; + 5) echo "::error::The data never arrived, the API REFUSED the parameters, or page 1 stated rows and parsed to none (parser_found_nothing). The log names which."; exit 1 ;; 6) echo "::error::Partial: a page or a book failed after others succeeded. See the sidecar's pages_failed."; exit 1 ;; *) echo "::error::Unexpected exit $code."; exit 1 ;; esac @@ -104,6 +109,8 @@ jobs: rows = json.load(open("canary.json")) mode, floor, pages = "${{ matrix.mode }}", ${{ matrix.floor }}, ${{ matrix.pages }} problems = [] + if meta["stop_reason"] == "parser_found_nothing": + problems.append("parser_found_nothing: a served page stated rows and parsed to none") if meta["status"] != "complete": problems.append("status %r (%s)" % (meta["status"], meta["stop_reason"])) if meta["pages_completed"] != pages: diff --git a/.gitignore b/.gitignore index c88c8db..13f8386 100644 --- a/.gitignore +++ b/.gitignore @@ -23,11 +23,11 @@ goodreads_rows_*.* live_results/ live_results.zip # ANY working directory a live run writes into, by KIND rather than by the one -# name an older repo used (CLAUDE.md §24). Measured here: `live/` was not -# listed, and a `--dump-html` of a 3-page run wrote `canary_dump.json.page3`, -# which `*.json` does not cover. It sat untracked and NOT ignored, one +# name an older repo used. In a sibling repo `live/` was not listed, and a +# `--dump-html` of a 3-page run wrote `canary_dump.json.page3`, which +# `*.json` does not cover. It sat untracked and NOT ignored, one # `git add -A` from a commit, until the credential scan flagged the 32-hex -# announcement codes inside it. +# ids inside it. live/ out/ output/ @@ -47,7 +47,7 @@ dumps/ *_debug.png *.html.page* # A single-page `--dump-html page.html` lands at the repo root with no -# suffix any rule above knows (CLAUDE.md §24). Every HTML file this repo +# suffix any rule above knows. Every HTML file this repo # needs lives in a subdirectory or inside fixtures_generated.json. /*.html *.json.page* @@ -101,7 +101,7 @@ secrets* # recoverable only because that repo was private at the time. On 2026-09-17 # a sibling had an untracked `.env.bak` holding a 32-hex key in a PUBLIC # repository, one `git add -A` from publication. Craigslist was fixed then -# and the fix never reached anyone else, which is §16's rule unapplied: when -# you fix one, check the siblings. +# and the fix never reached anyone else. When you fix one, check the +# siblings. .env* !.env.example diff --git a/CHANGELOG.md b/CHANGELOG.md index effc3ef..a3e10c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,12 +9,25 @@ said so at the top of its release notes. ## [Unreleased] -> **Behaviour change:** `--retries 0` is now a usage error (exit 2). It used -> to be accepted and made the page-load loop unreachable, so nothing was -> fetched and the engine reported the browser's own blank page as a result: -> exit 3 or 4 with a 39-byte "capture", in tens of milliseconds. If a -> script of yours passes 0, pass **1**: the flag counts ATTEMPTS, so 1 means -> "try once and do not retry". +> **Behaviour changes:** +> +> * **A page buys at most one paid CAPTCHA solve, however many times it is +> fetched.** A page re-fetched after a refusal used to start a new +> allowance: 2 solves per page with no proxy pool, and one per exit with a +> pool (4 on a pool of four). With `TWOCAPTCHA_KEY` set, that was money. +> * **A served page that states rows and parses to none is now a parser +> failure** (`stop_reason: parser_found_nothing`): exit 5 on page 1 +> (was 4, "the listing is empty") and exit 6 on a later page (was 0 and +> `status: complete`). +> * **CSV cells that start with `=`, `+`, `-`, `@`, a tab or a line break +> get a leading `'`**, so a spreadsheet does not run them. JSON is +> unchanged; the sidecar's `csv_cells_escaped` counts the difference. +> * **New usage errors (exit 2):** `--retries 0`; `--pages`, +> `--concurrency` and `--max-books` below 1; a negative `--delay`, +> `--retry-delay` or `--proxy-block-retries`; a negative `--retries`, +> `--pages` below 1 or a negative delay in `scraper_api_client.py`. If a +> script of yours passes `--retries 0`, pass **1**: the flag counts +> ATTEMPTS, so 1 means "try once and do not retry". ### Fixed @@ -22,9 +35,38 @@ said so at the top of its release notes. `range(1, retries + 1)`, so 0 never runs it and the engine judged `about:blank`. Three third-party audits of this family ran exactly that and reported a working scraper as broken. `--retries` now goes through - `page_flow.attempts` in every entry point; a check reads the source of each + `page_flow.attempts` in every engine; a check reads the source of each one and calls the validator, so a new engine declaring the flag with a plain int fails the suite. +- **The paid-solve limit was per call, not per page.** `SOLVES_PER_PAGE = 1` + was applied inside each navigation, and every re-fetch after a refusal (a + fresh browser, or the next exit in a pool) navigated again. One + allowance now lives for the whole page, is checked before each purchase + and is not reset by a retry or a rotation; the language check that lands + on the book before page 1 of a reviews run spends from page 1's. The + suite's old assertion `solves <= 2` pinned the bug; it now asserts 1, with + no pool, with a pool of four, and on a reviews landing. +- **A page whose own count says it holds rows, parsed to none, was reported + as an empty listing or as the end of one.** It is now + `parser_found_nothing`, the page is saved for an issue, and the canary + names it. +- **Output writes are atomic.** JSON, CSV and the sidecar are written to a + temporary file in the same directory and renamed over the target, so a + crash or a full disk mid-write leaves the previous file whole (before: a + 539-byte `out.json` came back as 10 bytes). A new file gets the mode + `open()` would give it under the current umask; an existing file keeps its + own. +- **`--delay`, `--retry-delay`, `--pages`, `--concurrency`, + `--proxy-block-retries` and `--max-books` are validated** in every engine, + and `--pages`, `--delay`, `--retries` and `--retry-delay` in + `scraper_api_client.py` (where `--retries` counts EXTRA attempts, so 0 + stays legal there). +- Comments, docstrings and one `--concurrency` error message pointed at + notes that are not public; each rule is now stated where it is used. The + captcha module's notes described another site; they describe this one. +- The expired-endpoint hint no longer says a Scraping Browser profile lasts + "about a day": one was measured working after ten days, so the HTTP 401 is + the signal, not an age. ## [0.1.1] — 2026-09-28 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 72a5728..b084ad3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,8 +86,8 @@ Then the rest of the presentation, in the order that matters: the claim the README makes. If a run is refused for the runner's address, the canary's `GOODREADS_PROXY` secret (an exit elsewhere) is the fix, with no workflow edit. -2. The repo description, homepage and topics set (see the family notes on - what those should say). +2. The repo description, homepage and topics set: the description names + the site and what it really publishes, the topics are what people search for. 3. Only then the row in the org profile README — and check it with an ANONYMOUS request rather than your own logged-in browser. A row pointing at a private repo is a 404 for every visitor, which costs more trust than diff --git a/README.md b/README.md index 91ed449..9277308 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,12 @@ each mode is committed: [list](sample_output.json), [book](sample_output_book.js [reviews](sample_output_reviews.json) (the reviews sample's reviewer names, ids and words are placeholders; everything else is as the run wrote it). +In the CSV, a text cell that starts with `=`, `+`, `-`, `@`, a tab or a +line break gets a leading `'`, so a spreadsheet shows it as text instead of +running it as a formula (review text and shelf names are written by anyone). +The JSON keeps the site's text unchanged, and the sidecar's +`csv_cells_escaped` says how many cells differ. + --- ## Six things about Goodreads that will look like bugs @@ -221,8 +227,8 @@ Surveyed 2026-09-27: | 2 | bad usage, including a `--review-language` the book has no reviews in | | 3 | blocked: AWS WAF's challenge did not clear, or a 403; distinct from an empty listing | | 4 | zero rows: the listing has nothing in it, or the book id names no book | -| 5 | the data never arrived: a timeout, a dead proxy, a refused API parameter, a remote API error | -| 6 | partial: some pages (or, from a listing, some books) came back and some did not | +| 5 | the data never arrived: a timeout, a dead proxy, a refused API parameter, a remote API error, or page 1 stated rows and the parser read none (`stop_reason: parser_found_nothing`) | +| 6 | partial: some pages (or, from a listing, some books) came back and some did not, including a later page that stated rows and parsed to none | **A run that finds nothing writes nothing**, so a failure never replaces last night's good output with `[]`. `--allow-empty` is the opt-out. diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md index ce36ed1..f392e1d 100644 --- a/TROUBLESHOOTING.md +++ b/TROUBLESHOOTING.md @@ -50,6 +50,11 @@ writes the empty file. * **"The book page carries no API key or work id"** — the page shape moved; same issue template. * **"Gave up on page N"** — a timeout or a dead proxy. The log names which. +* **`stop_reason: parser_found_nothing`** ("Page N was served and states ... + match(es) ... but the parser read NO rows from it") — the page said it + holds rows and none were read, so the page shape has moved. The page is + saved as `_pageN_debug.html`; open a "Site changed" issue with it. + On page 2 or later this is exit 6 and the earlier pages are written. ## Exit 6 — partial diff --git a/captcha_solver.py b/captcha_solver.py index e60d28e..6b7a668 100644 --- a/captcha_solver.py +++ b/captcha_solver.py @@ -6,12 +6,10 @@ Detection runs after EVERY page navigation in the main loop of all three scrapers, regardless of what URL was requested (category hub, product page, sign-in, checkout, anything) — this is deliberate, not scoped to any one -page. If Transfermarkt renders a reCAPTCHA/Turnstile challenge anywhere — -this fires. Live research for this repo (2026-09-10, one proxied fetch each -of a ranking page, a player profile, a squad page and a transfer list, all -served without any challenge or 403) found none, but that is a much smaller -sample than the family's other members ran before writing this note — see -the closing section of this file for what that does and does not license. +page. If goodreads.com renders a reCAPTCHA/Turnstile challenge anywhere, +this fires. None has been observed on this site: what it serves is AWS WAF, +whose challenge action a browser passes by itself and whose CAPTCHA action +is read by `detect_aws_waf` below. Flow: 1. Both detectors run and are reconciled (see reconcile_detections) to decide @@ -99,7 +97,7 @@ class CaptchaChallenge: page_url: str = "" # AWS WAF only. Its task needs three values that all live together in # `window.gokuProps` on the challenge page, plus the two script URLs the - # page loads. Unlike Cloudflare Turnstile (§19 of the family notes), + # page loads. Unlike Cloudflare Turnstile, # nothing has to be intercepted at runtime to get them: AWS WAF prints # all of it into the served HTML, so a static read is enough. iv: Optional[str] = None @@ -140,8 +138,8 @@ def aws_waf_action(self) -> str: running it. No widget, nothing to show a human. Both carry `window.gokuProps`, which is why reading the props alone - cannot tell them apart. `captcha.js` can. Measured on - www.transfermarkt.com 2026-09-17: challenge-action pages came back + cannot tell them apart. `captcha.js` can. Measured on another AWS + WAF site, 2026-09-17: challenge-action pages came back with challengeScript present and captchaScript absent, and the captcha-action pages had both. """ @@ -151,7 +149,7 @@ def aws_waf_action(self) -> str: def has_captcha_widget(self) -> bool: """Is there a widget on this page for a solver to work on? - Section 19's rule: "unsolvable" is a property of a PAGE, not of a + "Unsolvable" is a property of a PAGE, not of a vendor, and it means the page carries no widget. A challenge-action page carries none, so sending it to a solver buys a token for a puzzle that was never rendered -- measured at $0.00145 a time, since @@ -182,10 +180,11 @@ def has_captcha_widget(self) -> bool: def detect_aws_waf(html: str, page_url: str = "") -> Optional[CaptchaChallenge]: """An AWS WAF CAPTCHA challenge read out of served HTML, or None. - Measured on www.transfermarkt.com 2026-09-16: the site sits behind AWS - WAF on CloudFront, and a plain datacentre-IP request is answered with - this challenge -- HTTP 405, `x-amzn-waf-action: captcha`, a 2331-byte - body -- on most attempts. + Measured on another site behind AWS WAF on CloudFront, 2026-09-16: a + plain datacentre-IP request was answered with this challenge -- HTTP + 405, `x-amzn-waf-action: captcha`, a 2331-byte body -- on most + attempts. goodreads.com has served its challenge action, not this one, + in every run of this repo so far. Everything the solver needs is in the served markup, so unlike the Cloudflare Turnstile case this family documents, no init script and no @@ -917,36 +916,16 @@ def solve_recaptcha(challenge: CaptchaChallenge, twocaptcha_api_key: Optional[st # =========================================================================== # What is deliberately NOT here # =========================================================================== -# No first-party image captcha or custom widget is ported here: nothing in -# this family's other members' bespoke solvers (a JPEG-of-distorted-text -# form is one example) was ever observed on Transfermarkt, and porting one -# on spec would be dead code with no fixture to test it against. -# -# What was measured, on 2026-09-10 from this repo's own build/test -# environment: neither a direct `requests` call nor a local Playwright -# browser could reach www.transfermarkt.com at all — both failed at the -# TCP/TLS layer, and a plain `requests.get` through the sandbox's own egress -# proxy came back with the proxy's OWN "403 Forbidden" on the CONNECT -# tunnel, never reaching the site. That is this sandbox's outbound -# allowlist refusing the destination, not Transfermarkt refusing the -# request — confirmed by ruling it out, not assumed. A PROXIED fetch of the -# same URLs (through 2captcha's own fetch infrastructure, which sits -# outside this sandbox) returned normal 200 pages every time with no -# challenge and no captcha markers. So the only live evidence this repo has -# is: Transfermarkt served every page it was asked for, without a captcha, -# over a working proxied path. What was NOT established is how the site -# treats a bare datacentre IP with no proxy at all, or what a real user's -# browser sees on a residential connection — this repo's build sandbox -# cannot test either. Do not port the OTHER family members' "datacentre -# addresses get refused" conclusion onto Transfermarkt without measuring it -# here; it was true for them and is simply unknown for this site. See +# No first-party image captcha or custom widget is ported here: none has +# been observed on goodreads.com, and porting one on spec would be dead code +# with no fixture to test it against. What this site has served in every run +# so far is AWS WAF; see detect_aws_waf above and # product_parser.detect_bot_challenge for how a real captcha, once observed, # would be told apart from a network-level refusal. # -# The reCAPTCHA / hCaptcha / Turnstile machinery above IS kept, and that is a +# The reCAPTCHA detection and solving above IS kept, and that is a # deliberate asymmetry rather than an inconsistency. Detection stays broad # because which challenge a visitor meets depends on the exit country and on # what the address has been doing — a narrow list is how a challenge gets -# reported as an empty page months later. A solver for a challenge this site -# has never been observed to serve is dead code; a DETECTOR for one is cheap -# insurance. See the family note in CLAUDE.md. +# reported as an empty page months later. A DETECTOR for a challenge this +# site has never been observed to serve is cheap insurance. diff --git a/env_config.py b/env_config.py index 158f294..7343036 100644 --- a/env_config.py +++ b/env_config.py @@ -66,7 +66,7 @@ # A literal-only check reported both of those as CONFIGURED, so `cp # .env.example .env` followed by a run connected to cb.2captcha.com with the # string `{login}-zone-…` as its username and got a 401 — a confusing failure -# a long way from its cause, which is precisely what §3's rule exists to +# a long way from its cause, which is precisely what this check exists to # prevent. Any `{…}` left in a value means the line was never filled in. _BRACED_PLACEHOLDER_RE = re.compile(r"\{[A-Za-z_][A-Za-z0-9_]*\}") diff --git a/fingerprint_client.py b/fingerprint_client.py index e84ce1e..a3be9ca 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -249,9 +249,8 @@ def playwright_context_kwargs(fp: dict) -> dict: # stating `deviceScaleFactor: 1.25` produced a browser reporting # `window.devicePixelRatio === 1`, so the identity contradicted itself # on an axis any fingerprinter reads for free -- and a contradiction is - # exactly what this flag exists to avoid. Same shape as the three - # defects the family notes already list for this file: a key the API - # returns that nothing applies. Found by comparing a live browser + # exactly what this flag exists to avoid. Same shape as three earlier + # defects in this file: a key the API returns that nothing applies. Found by comparing a live browser # against the fingerprint rather than by reading the code. scale = (fp.get("screen") or {}).get("deviceScaleFactor") if isinstance(scale, (int, float)) and not isinstance(scale, bool) and scale > 0: @@ -331,7 +330,7 @@ def main() -> int: # loader. It used to read `os.environ` alone, which meant a key put in # `.env` exactly as the README and .env.example instruct worked for every # engine and failed HERE with "No API key" — a documented mechanism not - # applied on one path, which is the shape of half the defects §16 lists. + # applied on one path. # # `.env` has to be LOADED before it can be read: `env_value` looks at # os.environ, and `load_env` is what fills that from the file. Calling it diff --git a/make_fixtures.py b/make_fixtures.py index 8f9bd6f..9e0e9c9 100644 --- a/make_fixtures.py +++ b/make_fixtures.py @@ -31,10 +31,10 @@ * the scrub runs BEFORE anything is embedded: - the AppSync key the book page hands every visitor (`pageProps.apiKey`) and every CSRF `authenticity_token`. Neither grants anything to a - reader of this repo, and neither belongs in it (§10); + reader of this repo, and neither belongs in it; - a reviewer's name, profile id and url, and the review's own text and - id. The checks need the STRUCTURE of a review, not a person's words - (§10). Book, author and series data is the site's catalogue and is + id. The checks need the STRUCTURE of a review, not a person's words. + Book, author and series data is the site's catalogue and is kept verbatim; * a fixture that still holds a 32-hex string, a `da2-` key or a CSRF token after the scrub is refused. @@ -97,7 +97,7 @@ "waf_challenge": ("waf_challenge.html", GR + "/book/show/5907", 0, "waf", None), # A book page fetched over --cdp-endpoint (2026-09-28, a country-us # profile). The Scraping Browser's auto-solve extension injects the same - # captcha hunters into EVERY page it loads (§24): 16 scripts here, + # captcha hunters into EVERY page it loads: 16 scripts here, # amazon_waf among them, plus a `` mount. Cut like any # book page, with the injected tags kept verbatim; the markers must score # zero on it WITHOUT any strip. @@ -123,7 +123,7 @@ def _strip_forms(fragment: str) -> str: another. The parser reads neither, so both go. Asset urls carry a 32-hex build fingerprint (`loading-trans-.gif`), which is the shape of an API key and would teach the credential scan an exception, so the - fingerprint goes too (§24: take it out rather than exempt it).""" + fingerprint goes too: taken out rather than exempted.""" fragment = re.sub(r"-[0-9a-f]{32}(\.[a-z]+)", r"\1", fragment) fragment = re.sub(r"", "", fragment, flags=re.S) fragment = re.sub(r"", "", fragment, flags=re.S) diff --git a/output_writer.py b/output_writer.py index f30e670..66ebb28 100644 --- a/output_writer.py +++ b/output_writer.py @@ -14,9 +14,8 @@ A listing tile and a book page describe the same thing at two depths, and a tile has perhaps a third of the columns, so they are two dataclasses rather -than one wide row that is two-thirds null on every listing line (CLAUDE.md -§9: a column that is null on every row of a mode should not exist in that -mode's file). A review is a different kind of thing altogether. +than one wide row that is two-thirds null on every listing line. A column +that is null on every row of a mode should not exist in that mode's file. A review is a different kind of thing altogether. What they DO share, byte-identical and in order, is the family prefix: `source`, `scraped_at`, `url`, `sku`, `title`. One column name then works @@ -31,8 +30,11 @@ an empty CSV still gets the right header for the mode that produced it. """ +import contextlib import csv import json +import os +import tempfile from dataclasses import dataclass, asdict, field, fields from datetime import datetime, timezone from typing import Optional, List, Set, Sequence, Any, Type @@ -63,7 +65,7 @@ class ListedBook: author_id: Optional[str] = None author_url: Optional[str] = None # Average of the WORK's ratings, 0-5, and how many there are. A book - # nobody has rated is null on both, never 0.00 (§21). + # nobody has rated is null on both, never 0.00. avg_rating: Optional[float] = None ratings_count: Optional[int] = None # The ORIGINAL publication year, as the listing prints it @@ -92,7 +94,7 @@ class ListedBook: mode: Optional[str] = None # Which of the site's structures the row was read from: "microdata" # (schema.org/Book on the page), "rsc" (the search page's own React - # payload), "shelf-html", "series-props". Provenance in a column (§8). + # payload), "shelf-html", "series-props". Provenance in a column. data_source: Optional[str] = None @@ -175,7 +177,7 @@ class Review: # The review's own page, /review/show/{id}. url: str = "" # The review's numeric id, from that url. Unique per review, which - # `book_id` is not: many rows share one book (§9). + # `book_id` is not: many rows share one book. sku: Optional[str] = None # The BOOK's title, so the family's `title` column says what the row is # about in every repo. @@ -206,7 +208,7 @@ class Review: reviewer_reviews: Optional[int] = None # The ordering the run asked for (default, newest, oldest). It decides # WHICH reviews a capped run holds, so it is part of what the file - # means and diff_runs.py refuses to compare two different ones (§21). + # means and diff_runs.py refuses to compare two different ones. sort: Optional[str] = None page: Optional[int] = None @@ -267,6 +269,78 @@ def dedupe_by_sku(rows: Sequence[Any], seen: Set[str]) -> List[Any]: LIST_CSV_SEPARATOR = " | " +@contextlib.contextmanager +def _atomic(path: str, newline: Optional[str] = None): + """Write to a temporary file beside `path`, then rename over it. + + `open(path, "w")` truncates before a single byte is written, so a crash, + a kill, a full disk or a Ctrl-C halfway through `json.dump` leaves a + SHORTER file where a complete one was: the previous good run destroyed + by the ATTEMPT to replace it. Measured here before this was added: a + 539-byte `out.json` came back as 10 bytes of invalid JSON when the dump + failed part-way. + + * The temp file goes in the target's OWN directory, because `os.replace` + is atomic only within one filesystem. + * `fsync` before the rename makes the bytes durable, not merely visible. + * `newline` is passed through because the csv module needs `newline=""`. + * The mode is what `open()` would have given: an existing file keeps its + own mode, a new one gets `0o666 & ~umask`. `NamedTemporaryFile` + creates at 0600 and `os.replace` keeps that, so without this every + output would be readable by its owner alone. + """ + directory = os.path.dirname(os.path.abspath(path)) or "." + handle = tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", newline=newline, dir=directory, + prefix=os.path.basename(path) + ".", suffix=".tmp", delete=False) + try: + with handle: + yield handle + handle.flush() + os.fsync(handle.fileno()) + try: + mode = os.stat(path).st_mode & 0o777 + except OSError: + umask = os.umask(0) + os.umask(umask) + mode = 0o666 & ~umask + os.chmod(handle.name, mode) + os.replace(handle.name, path) + except BaseException: + # KeyboardInterrupt and SystemExit included: an interrupted run must + # not leave `out.json.ab12cd.tmp` beside the good file. + try: + os.unlink(handle.name) + except OSError: + pass + raise + + +# A spreadsheet treats a cell beginning with one of these as a FORMULA, and +# review text, shelf names, tags and reviewer names on this site are written +# by whoever posted them. The tab and the two newlines are here because some +# readers strip a leading one and then execute whatever followed it. +CSV_FORMULA_LEADS = ("=", "+", "-", "@", "\t", "\r", "\n") + +# Excel, Google Sheets and LibreOffice read a leading apostrophe as "the +# rest of this cell is text". A consumer parsing the file with the `csv` +# module DOES see it, which is why the count goes into the sidecar. +CSV_FORMULA_ESCAPE = "'" + + +def _csv_escape(v: Any): + """Neutralise a formula-shaped cell. Returns (value, was_escaped). + + Only STRINGS are touched: escaping a number would turn it into text and + break every sum over the column. CSV only: the JSON output keeps the + site's text exactly as served, and `csv_cells_escaped` in the sidecar + declares the difference between the two files. + """ + if isinstance(v, str) and v.startswith(CSV_FORMULA_LEADS): + return CSV_FORMULA_ESCAPE + v, True + return v, False + + def _csv_value(v: Any) -> Any: if isinstance(v, (list, tuple)): return LIST_CSV_SEPARATOR.join(str(x) for x in v) @@ -274,24 +348,33 @@ def _csv_value(v: Any) -> Any: def write_json(rows: Sequence[Any], path: str) -> None: - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump([asdict(r) for r in rows], f, ensure_ascii=False, indent=2) -def write_csv(rows: Sequence[Any], path: str, row_cls: Type = ListedBook) -> None: - # An empty result still gets the header row. A zero-byte file makes a - # consumer fail on read (no columns to parse) instead of reading a valid - # table with zero rows — and "an empty result is still a well-formed - # result" is the same principle as `save` refusing to overwrite good data. - # - # The header comes from `row_cls`, not from the first row, so an empty - # run still writes the columns of the mode that produced it. +def write_csv(rows: Sequence[Any], path: str, row_cls: Type = ListedBook) -> int: + """Write the CSV and return how many cells were formula-escaped. + + An empty result still gets the header row: a zero-byte file makes a + consumer fail on read instead of reading a valid table with zero rows. + The header comes from `row_cls`, not from the first row, so an empty run + still writes the columns of the mode that produced it. + """ fieldnames = [f.name for f in fields(row_cls)] - with open(path, "w", encoding="utf-8", newline="") as f: + escaped = 0 + with _atomic(path, newline="") as f: writer = csv.DictWriter(f, fieldnames=fieldnames) writer.writeheader() for r in rows: - writer.writerow({k: _csv_value(v) for k, v in asdict(r).items()}) + row = {} + for k, v in asdict(r).items(): + # AFTER the list join: a joined list is text too, and its + # first element can be formula-shaped. + value, was_escaped = _csv_escape(_csv_value(v)) + row[k] = value + escaped += was_escaped + writer.writerow(row) + return escaped # Exit code used when a run completes but produced nothing. Distinct from 1 @@ -299,7 +382,7 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = ListedBook) -> Non EXIT_NO_PRODUCTS = 4 # Exit code for a run blocked before any data arrived: AWS WAF's CAPTCHA or -# challenge, a 403, or a 451 refusing the exit's country. Distinct from +# challenge, or a 403. Distinct from # EXIT_NO_PRODUCTS so a caller can tell "the listing genuinely has nothing in # it" from "something stood between us and the listing". # @@ -366,7 +449,7 @@ def write_run_meta(out_prefix: str, meta: dict) -> str: both complete, and between runs of different `mode`. """ path = f"{out_prefix}.meta.json" - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump(meta, f, ensure_ascii=False, indent=2) print(f"[+] Wrote run metadata -> {path} (status={meta.get('status')})") return path @@ -423,14 +506,15 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, } if extra: # Merged rather than nested under a key, so a consumer reads - # `shop_rating` at the top level beside `products`. Run fields win a + # `total_results` at the top level beside `products`. Run fields win a # name collision: a caller cannot accidentally overwrite `status`. meta.update({k: v for k, v in extra.items() if k not in meta}) return meta def save(rows: Sequence[Any], out_prefix: str, fmt: str, - allow_empty: bool = False, row_cls: Type = ListedBook) -> int: + allow_empty: bool = False, row_cls: Type = ListedBook, + stats: Optional[dict] = None) -> int: """Write JSON/CSV and return a process exit code. Returns 0 when rows were written, EXIT_NO_PRODUCTS when there were none. @@ -457,8 +541,17 @@ def save(rows: Sequence[Any], out_prefix: str, fmt: str, write_json(rows, f"{out_prefix}.json") print(f"[+] Saved {len(rows)} rows -> {out_prefix}.json") if fmt in ("csv", "both"): - write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) + escaped = write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) print(f"[+] Saved {len(rows)} rows -> {out_prefix}.csv") + # Through `stats` rather than the return value, which IS the exit + # code. finish_run puts it in the sidecar. + if stats is not None: + stats["csv_cells_escaped"] = escaped + if escaped: + print(f"[!] {escaped} CSV cell(s) began with a formula character " + f"and were prefixed with {CSV_FORMULA_ESCAPE!r} so a " + f"spreadsheet reads them as text. The JSON output is " + f"unchanged; see csv_cells_escaped in the sidecar.") return 0 if rows else EXIT_NO_PRODUCTS @@ -515,12 +608,18 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, # names has a hole for every name nobody added to it. complete = stop_reason in COMPLETE_STOP_REASONS and not pages_failed row_cls = ROW_CLASS_BY_MODE.get(mode, ListedBook) - rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls) + write_stats: dict = {} + rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls, + stats=write_stats) wrote_output = bool(rows) or allow_empty if wrote_output: status = "complete" if (rows and complete) else ( "partial" if rows else "failed") + # UNDER the caller's `extra`, never over it: on a name collision what + # the engine measured about the site wins over housekeeping. + if write_stats: + extra = {**write_stats, **(extra or {})} write_run_meta(out_prefix, run_meta( status=status, stop_reason=stop_reason, pages_requested=pages_requested, pages_completed=pages_completed, diff --git a/page_flow.py b/page_flow.py index ac96c19..6be0170 100644 --- a/page_flow.py +++ b/page_flow.py @@ -2,7 +2,7 @@ page_flow.py ------------ The retry / solve / blocked decision, as DATA rather than as three copies of -an if-chain (CLAUDE.md §1), and the fetch loop the three engines share. +an if-chain, and the fetch loop the three engines share. goodreads.com answers this repo's requests in nine ways, and they want seven different responses: @@ -25,8 +25,8 @@ would be silent: one engine reporting exit 3 where its twin reports exit 0 on the same response. -Nothing here imports a browser, and **no JavaScript crosses this boundary** -(§1). Each engine spells its fetch() in its own driver's dialect. +Nothing here imports a browser, and **no JavaScript crosses this boundary**. +Each engine spells its fetch() in its own driver's dialect. """ import copy @@ -52,8 +52,8 @@ # How long one fetch() may take before the engine gives up on it. A 100- # review GraphQL page arrived in well under a second (2026-09-27). The bound -# exists because a browser fetch() has no timeout of its own, and CLAUDE.md -# §8 requires every remote call to have one. +# exists because a browser fetch() has no timeout of its own, and every remote +# call here has to be bounded. FETCH_TIMEOUT_MS = 30_000 # How long to wait at the SAME exit after a 429/503 before trying again. No @@ -80,7 +80,7 @@ def classify(html: Optional[str], status: Optional[int] = None, The argument ORDER is the contract: every engine calls `classify(html, status, url, waf_action)`. A sibling repo shipped `classify(html, url=...)` in two of three engines against a callee that - took `status` second, and both crashed on their first fetch (§17). + took `status` second, and both crashed on their first fetch. `smoke_test.py` binds every engine's call against this signature for that reason. """ @@ -109,7 +109,7 @@ def classify(html: Optional[str], status: Optional[int] = None, "challenge": {"retry": True, "solve": True, "blocked": True, "parse": False}, # Rate limited. The retry happens at the same exit after a wait # (THROTTLE_*). NOT counted as blocked: calling a throttle a block - # reports exit 3 for a page that was about to come back (§24). + # reports exit 3 for a page that was about to come back. "throttled": {"retry": True, "solve": False, "blocked": False, "parse": False}, # HTTP 403. Headless Chromium on /search got this with a 39-byte body, # 3 of 3, while headful was served 3 of 3 (2026-09-27). @@ -137,7 +137,7 @@ def should_parse(state: str) -> bool: # Whether a blocked page is worth re-fetching at all. CONSULTED by every -# engine, so setting it False really does stop the retry loop (§17). +# engine, so setting it False really does stop the retry loop. RETRY_ON_BLOCKED = True # How many times to re-fetch a blocked page when there is no proxy pool to @@ -152,6 +152,38 @@ def should_parse(state: str) -> bool: SOLVES_PER_PAGE = 1 +class SolveBudget: + """The paid-solve allowance for ONE page, shared by every solve call site. + + A page can reach the solver more than once: on its first navigation, on + each re-fetch after a refusal (from a fresh browser or a different exit), + and, on a reviews run, on the landing that reads the API key. Each of + those used to start a fresh `range(SOLVES_PER_PAGE)`, so with no proxy + pool one page bought two solves and with a pool of four it bought four. + One object per page, checked BEFORE each purchase and never reset by a + retry or a rotation, is what makes the limit a limit. A fresh exit is a + reason to fetch again, not a fresh allowance. + """ + + def __init__(self, limit: int = None): + self.limit = SOLVES_PER_PAGE if limit is None else limit + self.spent = 0 + + def allows(self) -> bool: + return self.spent < self.limit + + def spend(self, solve: Callable[[], bool]) -> bool: + """Call `solve` if the allowance has room; False when it has none. + Counted before the call, because a solver can charge for an answer + and still report failure.""" + if not self.allows(): + log.info("This page's paid-solve allowance (%d) is spent; not " + "buying another.", self.limit) + return False + self.spent += 1 + return bool(solve()) + + # --------------------------------------------------------------------------- # Pagination # --------------------------------------------------------------------------- @@ -174,7 +206,7 @@ def concurrency_limit(cdp_endpoint: Optional[str]) -> Optional[int]: The Scraping Browser API allows ONE live connection per profile, so N workers sharing a `pid` collide with `profile_locked`. Several `pid`s, - one run each, is the way to parallelise that path (§7). + one run each, is the way to parallelise that path. """ return 1 if cdp_endpoint else None @@ -220,6 +252,8 @@ def refusal_advice(state: str, url: str = "", headless: bool = False) -> str: def stop_reason_for(outcome) -> str: """The run's stop_reason when `outcome` is the page that ended it.""" + if getattr(outcome, "parse_failed", False): + return "parser_found_nothing" if getattr(outcome, "rejected", None): return "api_rejected" if getattr(outcome, "blocked_by", None): @@ -284,7 +318,7 @@ def build_query(args, error: Callable[[str], None]) -> Query: if args.concurrency > 1 and query.mode == "reviews": error("--concurrency cannot apply to --mode reviews: the feed is " "walked by the cursor each page returns, so page N's address " - "does not exist until page N-1 has been read (§18).") + "does not exist until page N-1 has been read.") return query @@ -303,7 +337,7 @@ def query_summary(query: Query) -> dict: def merge_outcomes(outcomes: List, key: str = "sku") -> List: - """Rows of `outcomes` in PAGE order, duplicates dropped (§8).""" + """Rows of `outcomes` in PAGE order, duplicates dropped.""" rows, seen = [], set() for oc in sorted(outcomes, key=lambda o: o.page_num): fresh = dedupe_by_key(oc.products, seen, key=key) @@ -320,7 +354,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, """Merge the pages in PAGE order, write the output, return the exit code. One implementation for the three engines, so the merge order, the - dedupe and the sidecar cannot differ between them (§6). `listing` is + dedupe and the sidecar cannot differ between them. `listing` is phase A of a book run started from a listing: its pages are what `pages_requested` describes, and a listing page that failed is a failed page of the run. @@ -339,7 +373,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, # A listing states far more than it serves: a list says 79,607 # books and serves 100 pages of 100. "complete" then means "every # page the site will serve", which is not the whole listing, and the - # sidecar says which (§21). + # sidecar says which. per = {"list": 100, "author": 30, "search": 20, "shelf": 50, "series": 100}[query.kind] reachable = (available or 1) * per @@ -352,7 +386,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, # identically on two runs; sorted by title, 739 and 52. The union of # the two orderings was exactly 791. So a repeat is not a listing # moving under the run: the site's paging over tied sort keys shows - # some books twice and the same number of others NEVER (§21: + # some books twice and the same number of others NEVER: # "complete" is not "exhaustive"). log.warning("%d book(s) appeared on more than one page, so the site " "never served %d others under this ordering. Another " @@ -429,7 +463,7 @@ def cookie_domain(host: Optional[str], html: Optional[str] = None) -> str: # waiting out a throttle, rotating on a refusal, reading the API key off the # book page, and parsing what came back. The three engines differ only in # HOW they ask their driver, so each passes in an object with these -# operations and no JavaScript crosses this boundary (§1): +# operations and no JavaScript crosses this boundary: # # ops.goto(url) -> (status, waf_header). Raises TransportError. # ops.document_text() -> the current document's markup @@ -442,7 +476,7 @@ def cookie_domain(host: Optional[str], html: Optional[str] = None) -> str: # ops.proxy_failure(text) -> the driver's proxy-error name in text, or "" # # One copy of the loop is how the three engines agree on exit codes and on -# whether a run spends money by construction rather than by discipline (§6). +# whether a run spends money by construction rather than by discipline. class TransportError(Exception): @@ -455,7 +489,7 @@ class PageOutcome: Collected per page and merged afterwards, in page order, rather than folded into shared state as the loop goes, so the output cannot depend - on which page happened to finish first (§8). + on which page happened to finish first. """ page_num: int url: str @@ -477,11 +511,15 @@ class PageOutcome: past_end: bool = False # A reviews page with no cursor after it: the feed ended here. last_page: bool = False + # A served page that states rows exist at this position (its own total + # and page count) and parsed to none: this repo's parser, not the + # listing, is what came up empty. + parse_failed: bool = False @property def ok(self) -> bool: return (not self.load_failed and self.blocked_by is None - and self.rejected is None) + and self.rejected is None and not self.parse_failed) # The columns the site filled on EVERY record of every capture, per mode. @@ -497,13 +535,17 @@ def ok(self) -> bool: } -def settle(ops, url: str, status: Optional[int], waf: Optional[str]) -> str: +def settle(ops, url: str, status: Optional[int], waf: Optional[str], + budget: Optional[SolveBudget] = None) -> str: """Classify the document `ops` is on, letting a WAF challenge run first. - Solves at most SOLVES_PER_PAGE CAPTCHAs, because a CAPTCHA that survives - a solved token is not one this run can pass, and a second solve is a - second charge for the same answer (§23). + Any solve goes through `budget`, the page's one allowance, so however + many times a page is navigated it buys at most SOLVES_PER_PAGE. A + CAPTCHA that survives a solved token is not one this run can pass, and a + second solve is a second charge for the same answer. """ + if budget is None: + budget = SolveBudget() text = ops.document_text() state = classify(text, status, url, waf) if status is None and state == "unknown" and len(text.strip()) < 2_000: @@ -511,15 +553,15 @@ def settle(ops, url: str, status: Optional[int], waf: Optional[str]) -> str: # headless browser both reach it as an EMPTY document (the 403 is # 39 bytes: ``), so an # empty document is waited on like the challenge it usually is, - # rather than retried as a parse failure (§24: read the ambiguous - # answer as the recoverable one). + # rather than retried as a parse failure: the ambiguous answer is + # read as the recoverable one. state = "challenge" if state != "challenge": return state # The WAF's challenge action computes a token and reloads the page by # itself, so let it run before judging it or paying for anything. The # document AFTER the reload is judged by what it holds: the status and - # header belonged to the interstitial it replaced (§26). + # header belonged to the interstitial it replaced. # # "unknown" keeps the wait going too. The first poll after the WAF's # reload often lands on the NEW page before it has finished arriving @@ -532,8 +574,8 @@ def settle(ops, url: str, status: Optional[int], waf: Optional[str]) -> str: waited += CHALLENGE_POLL_MS state = _unsettled_state(ops.document_text(), url) if state == "challenge" and should_solve(state): - for _ in range(SOLVES_PER_PAGE): - if not ops.solve_captcha(): + while budget.allows(): + if not budget.spend(ops.solve_captcha): break state = classify(ops.document_text(), None, url, None) if state != "challenge": @@ -558,17 +600,19 @@ def _unsettled_state(text: str, url: str) -> str: return state -def navigate(ops, url: str) -> Tuple[str, str, Optional[str]]: +def navigate(ops, url: str, budget: Optional[SolveBudget] = None + ) -> Tuple[str, str, Optional[str]]: """Go to `url` and judge the document: (state, text, transport_error).""" try: status, waf = ops.goto(url) except TransportError as e: return "load_failed", "", str(e) - state = settle(ops, url, status, waf) + state = settle(ops, url, status, waf, budget) return state, ops.document_text(), None -def land_for_reviews(ops, query: Query) -> Tuple[str, Optional[str]]: +def land_for_reviews(ops, query: Query, budget: Optional[SolveBudget] = None + ) -> Tuple[str, Optional[str]]: """Put the page on the book, and read the API key and ids off it. Returns (state, error). The key is the one the page hands every visitor @@ -576,7 +620,7 @@ def land_for_reviews(ops, query: Query) -> Tuple[str, Optional[str]]: AppSync key expires; it is never stored anywhere but in memory. """ url = landing_url(query) - state, text, err = navigate(ops, url) + state, text, err = navigate(ops, url, budget) if err: return "load_failed", err if state != "content": @@ -596,6 +640,26 @@ def land_for_reviews(ops, query: Query) -> Tuple[str, Optional[str]]: return "content", None +def page_states_rows(query: Query, page_num: int, total: Optional[int], + pages: Optional[int]) -> bool: + """Whether the page's OWN arithmetic says page `page_num` holds rows. + + The site's total is read from the page title or header, and its page + count from the pagination control, neither of which goes through the + row parser, so they can contradict it. A stated total of 0 (a reviews + filter that matches nothing) is an answer, not a statement of rows. + Shelves and series are one page to an anonymous visitor and their page + count of 1 is this repo's constant, not something the page said, so for + them only a stated total counts. + """ + if total is not None: + return total > 0 and (pages is None or page_num <= pages) + if (query.mode == "list" and query.kind in ADDRESSABLE_KINDS + and pages is not None): + return page_num <= pages + return False + + def _core_field_warnings(rows: List, mode: str, page_num: int) -> None: for name in CORE_FIELDS.get(mode, ()): if not rows: @@ -610,7 +674,7 @@ def _core_field_warnings(rows: List, mode: str, page_num: int) -> None: def _dump(args, page_num: int, text: str, pages: int) -> None: - """Write the exact response the parser was given, on success too (§9).""" + """Write the exact response the parser was given, on success too.""" if not args.dump_html: return path = args.dump_html if pages == 1 else f"{args.dump_html}.page{page_num}" @@ -627,12 +691,13 @@ def _save_debug(args, page_num: int, text: str) -> str: return path -def _fetch_attempt(ops, query: Query, req, page_num: int) -> Tuple[str, str, Optional[str]]: +def _fetch_attempt(ops, query: Query, req, page_num: int, + budget: Optional[SolveBudget] = None) -> Tuple[str, str, Optional[str]]: """One try at `req`: (state, text, transport_error_or_None).""" if req.navigate: - return navigate(ops, req.url) + return navigate(ops, req.url, budget) if not ops.landed: - state, err = land_for_reviews(ops, query) + state, err = land_for_reviews(ops, query, budget) if err: return "load_failed", "", err if not ops.landed: @@ -652,7 +717,8 @@ def _fetch_attempt(ops, query: Query, req, page_num: int) -> Tuple[str, str, Opt def fetch_one_page(ops, args, pool, query: Query, page_num: int, mask: Callable[[str], str] = lambda s: s, - pages: Optional[int] = None) -> PageOutcome: + pages: Optional[int] = None, + budget: Optional[SolveBudget] = None) -> PageOutcome: """Fetch and parse one page. Retries, rotations and debug dumps live here. Never raises for an EXPECTED failure. A timeout, a refusal, a WAF @@ -663,6 +729,8 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, pages = pages or args.pages req = request_for(query, page_num) outcome = PageOutcome(page_num=page_num, url=req.label) + # ONE allowance for every attempt, relaunch and rotation below. + budget = budget if budget is not None else SolveBudget() has_pool = bool(pool and len(pool) > 1) block_retries = 0 if not RETRY_ON_BLOCKED else ( @@ -678,14 +746,15 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, attempt = 0 while attempt < args.retries: attempt += 1 - state, text, last_error = _fetch_attempt(ops, query, req, page_num) + state, text, last_error = _fetch_attempt(ops, query, req, page_num, + budget) if last_error: exit_failed = ops.proxy_failure(last_error) or None if exit_failed: break # a different exit is the only thing that helps if state == "throttled" and throttles < THROTTLE_RETRIES: throttles += 1 - attempt -= 1 # a throttle wait spends its own budget (§24) + attempt -= 1 # a throttle wait spends its own budget pause = THROTTLE_WAIT_S * throttles log.warning("Rate-limited on page %d — waiting %.0fs at the " "same exit (%d/%d).", page_num, pause, throttles, @@ -778,6 +847,22 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, query.cursors[page_num + 1] = cursor outcome.last_page = not cursor log.info("Parsed %d row(s) from page %d.", len(rows), page_num) + if (state == "content" and not rows + and page_states_rows(query, page_num, outcome.total_available, + outcome.pages_available)): + # Not an empty listing (exit 4) and not the end of one (complete): + # the page says it holds rows and none were read, so the page shape + # has moved. Through the not-complete rule in finish_run this is + # exit 5 with nothing gathered, exit 6 after earlier pages. + outcome.parse_failed = True + debug = _save_debug(args, page_num, text) + log.error("Page %d was served and states %s match(es) over %s " + "page(s), but the parser read NO rows from it. That is a " + "parser failure, not an empty listing: the page shape has " + "moved. Saved to %s; please open an issue with it.", + page_num, outcome.total_available, outcome.pages_available, + debug) + return outcome if page_num == 1 and outcome.total_available is not None: log.info("The site reports %d match(es) — %s page(s) at this page " "size.", outcome.total_available, outcome.pages_available) @@ -785,7 +870,8 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, return outcome -def check_language(ops, args, query: Query) -> Optional[str]: +def check_language(ops, args, query: Query, + budget: Optional[SolveBudget] = None) -> Optional[str]: """Refuse a --review-language the book has no reviews in, before the first review request, with the list it does have. The API answers one with an EMPTY result, not an error, so without this a typo is an exit-4 @@ -793,7 +879,7 @@ def check_language(ops, args, query: Query) -> Optional[str]: if query.mode != "reviews" or not query.review_language: return None if not ops.landed: - land_for_reviews(ops, query) + land_for_reviews(ops, query, budget) if not ops.landed: return None # the first page reports why if query.languages is None: @@ -901,8 +987,11 @@ def run_pages(open_ops, close_ops, run_concurrently, args, pool, """ ops_box = [open_ops()] listing: Optional[List[PageOutcome]] = None + # The language check lands on the book page 1 is read from, so a solve + # there is page 1's solve and comes out of page 1's allowance. + first_budget = SolveBudget() try: - refusal = check_language(ops_box[0], args, query) + refusal = check_language(ops_box[0], args, query, first_budget) if refusal: log.error("%s", refusal) return 2 @@ -930,7 +1019,8 @@ def run_pages(open_ops, close_ops, run_concurrently, args, pool, ops_box[0] = open_ops() n = len(query.book_urls) if query.book_urls else args.pages - first = fetch_one_page(ops_box[0], args, pool, query, 1, mask, pages=n) + first = fetch_one_page(ops_box[0], args, pool, query, 1, mask, pages=n, + budget=first_budget) outcomes = [first] if not first.ok and query.mode != "book": return finish(args, query, outcomes, stop_reason_for(first), @@ -1015,7 +1105,7 @@ def worker_pool(pool, worker_index: int): """A private ProxyPool for one worker, starting at a different exit. Workers start on distinct exits and share no mutable state, so rotation - needs no lock (§7). + needs no lock. """ if not pool: return None @@ -1030,16 +1120,17 @@ def cdp_connect_hint(error_text: str) -> str: Two answers that want opposite fixes. Measured 2026-09-24 against four Scraping Browser endpoints left in sibling repos' .env files: all four - answered 401 Unauthorized, because a profile's credentials last about a - day. The message used to explain a 500 (a pid another run still holds) + answered 401 Unauthorized, because their credentials had expired. How + long a profile lasts varies; one endpoint was measured still working + after ten days, so the status, not an age, is the signal. The message used to explain a 500 (a pid another run still holds) whatever the status was, which sent the reader to wait for a run that did not exist. """ if "401" in (error_text or ""): return ("HTTP 401: the endpoint's credentials were refused. A Scraping " - "Browser profile's credentials last about a day, so an " - "endpoint copied from an older .env has usually expired. " - "Get a fresh one from your 2Captcha dashboard.") + "Browser profile's credentials expire, so an endpoint " + "copied from an older .env may have expired. Get a fresh " + "one from your 2Captcha dashboard.") return ("A Scraping Browser profile allows ONE live connection at a time, " "so an HTTP 500 here usually means another run still holds this " "`pid`. Wait for it to finish, or use a different pid.") @@ -1115,3 +1206,26 @@ def attempts(value) -> int: f"With 0 the load loop never runs, nothing is fetched, and the " f"engine reports the browser's blank page as a result.") return number + + +def bounded(kind, minimum, flag: str): + """argparse `type=` for a numeric flag that has a floor. + + A negative --delay reached `wait_ms` as a negative sleep, a negative + --proxy-block-retries silently meant "never rotate", and --pages 0 or + --concurrency 0 asked for nothing. Each is refused at parse time, as a + usage error (exit 2) that names the flag, in every entry point. + """ + def check(value): + try: + number = kind(value) + except (TypeError, ValueError): + raise _argparse.ArgumentTypeError( + f"{flag} must be a{'n integer' if kind is int else ' number'}, " + f"not {value!r}") + if number < minimum: + raise _argparse.ArgumentTypeError( + f"{flag} must be at least {minimum}, not {value}") + return number + check.__name__ = kind.__name__ # argparse names the type in its errors + return check diff --git a/playwright_scraper.py b/playwright_scraper.py index a2703ac..1a4695c 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -40,7 +40,7 @@ * **A listing answers a page past its end with ANOTHER page, HTTP 200**: a list with its last, a search with "Page 100 of 100", a shelf with its first. Every page is asked which page it served, and a mismatch ends the - listing (§23). + listing. Usage ----- @@ -85,7 +85,7 @@ # Chromium's own names for a proxy that could not be used. Distinguished -# from a timeout because the two want opposite responses (CLAUDE.md §8). +# from a timeout because the two want opposite responses. _PROXY_ERROR_MARKERS = ( "ERR_PROXY_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED", @@ -100,7 +100,7 @@ # The fetch() the GraphQL API goes through. It is handed to `page.evaluate` # as a FUNCTION, which Playwright sends through `Runtime.callFunctionOn` # rather than evaluating a string, so it works under any Content-Security- -# Policy (§18). It returns the status, the body and the WAF's own header, +# Policy. It returns the status, the body and the WAF's own header, # because a 202 challenge has an empty body and the header is then the only # signal. # @@ -108,7 +108,7 @@ # admit credentials, so cookies ride along only on a same-origin request; # the key goes in `x-api-key`, exactly as the site's own front end sends it. # The AbortController is the timeout: a browser fetch() has none of its own, -# and §8 requires every remote call to be bounded. +# and every remote call here has to be bounded. FETCH_JS = """ async ([url, method, body, timeoutMs, extra]) => { const ctl = new AbortController(); @@ -136,7 +136,7 @@ def _chrome_ua(chromium_version: str) -> str: - """A desktop-Chrome UA naming the browser's OWN real version (§8).""" + """A desktop-Chrome UA naming the browser's OWN real version.""" return (f"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " f"(KHTML, like Gecko) Chrome/{chromium_version} Safari/537.36") @@ -155,7 +155,7 @@ def _launch_local(pw, args, pool): A proxy rotation tears the whole browser down and calls this again. Cookies a bot manager issued against one exit, replayed from another, - are a stronger signal than either address alone (§8), and on this site + are a stronger signal than either address alone, and on this site that includes a solved `aws-waf-token`. """ launch_kwargs = {"headless": args.headless} @@ -193,7 +193,7 @@ class _Ops: A rotation means a genuinely FRESH browser: cookies a bot manager issued against one exit, replayed from another, are a stronger signal than - either address alone (§8), and here that includes a solved + either address alone, and here that includes a solved `aws-waf-token`. """ @@ -309,7 +309,7 @@ def _connect_remote(pw, args): break if browser is None: # The endpoint carries a password, and Playwright repeats it five - # times in its error text (§8). Rewritten with it masked, keeping + # times in its error text. Rewritten with it masked, keeping # host and port, which are the useful half. raise PWError( f"could not connect to --cdp-endpoint " @@ -338,7 +338,7 @@ def _connect_remote(pw, args): # Every `scheme://user:pass@` in a string, however many times it occurs. # Matching GLOBALLY is the point: a Playwright connection error repeats the -# endpoint five times (§8). +# endpoint five times. _CREDENTIALS_IN_URL_RE = re.compile(r"([a-z][a-z0-9+.\-]*://)[^\s/@]+:[^\s/@]+@", re.IGNORECASE) @@ -367,7 +367,7 @@ def handle_captcha_if_present(page, args, proxy: Optional[str] = None) -> bool: Nothing is paid for a page that holds no puzzle. The WAF's CHALLENGE action carries only `challenge.js`, and there is no answer to buy for that. A browser that runs it passes by itself, which is what the settle - wait in `_land` is for (§19: "unsolvable" is a property of a page). + wait in `_land` is for: "unsolvable" is a property of a page. """ try: html = page.content() @@ -392,7 +392,7 @@ def handle_captcha_if_present(page, args, proxy: Optional[str] = None) -> bool: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, min_score=args.min_score, proxy=proxy) - except Exception as e: # noqa: BLE001 — a solver error is a warning (§8) + except Exception as e: # noqa: BLE001 — a solver error is a warning logger.error("Solving the AWS WAF CAPTCHA failed (%s) — continuing.", _mask_credentials(str(e))) return False @@ -415,7 +415,7 @@ def _fetch_pages_concurrently(args, pool, query: Query, page_nums, concurrency: Each worker owns its own Playwright instance, browser and exit: with the sync API a browser belongs to the thread that made it, so sharing one is - not an option even in principle (§7). The page loop itself is + not an option even in principle. The page loop itself is page_flow.worker_loop, shared by all three engines. """ work = queue.Queue() @@ -490,7 +490,7 @@ def parse_args(argv=None): "/series/{id} or /book/show/{id}. Default: the Best " "Books Ever list. Also read from GOODREADS_URL.") g = p.add_argument_group("book") - g.add_argument("--max-books", type=int, default=None, metavar="N", + g.add_argument("--max-books", type=page_flow.bounded(int, 1, "--max-books"), default=None, metavar="N", help="With --mode book and a listing url: read at most N of " "the books the listing named (default: all of them).") g = p.add_argument_group("reviews") @@ -512,14 +512,14 @@ def parse_args(argv=None): help="Reviews per request, 1-100 (default 30, the site's " "own). 200 is answered with ZERO reviews, so it is " "refused.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.bounded(int, 1, "--pages"), default=1, help="Pages to fetch (100 books on a list, 30 on an " "author's, 20 on a search, 30 reviews). Planned " "against what page 1 states, so asking for more than " "exist fetches all of them.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=page_flow.bounded(float, 0, "--delay"), default=1.0, help="Delay between pages, seconds (default %(default)s)") - p.add_argument("--concurrency", type=int, default=1, metavar="N", + p.add_argument("--concurrency", type=page_flow.bounded(int, 1, "--concurrency"), default=1, metavar="N", help="Fetch pages through N parallel workers (default 1). " "Each worker runs its own browser and holds its own " "proxy exit. Ignored with --cdp-endpoint.") @@ -528,7 +528,7 @@ def parse_args(argv=None): "The pause doubles each time. A request the API " "REFUSED is not retried: its parameters would be " "refused again.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.bounded(float, 0, "--retry-delay"), default=2.0, help="Seconds before the first retry, doubling thereafter.") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="goodreads_rows", help="Output file prefix") @@ -547,7 +547,7 @@ def parse_args(argv=None): "per-page: a new exit, and a fresh browser, per page.") p.add_argument("--proxy-shuffle", action="store_true", help="Shuffle the pool at startup.") - p.add_argument("--proxy-block-retries", type=int, default=2, + p.add_argument("--proxy-block-retries", type=page_flow.bounded(int, 0, "--proxy-block-retries"), default=2, help="When a page is refused (403, AWS WAF), retry it " "from this many OTHER exits (default 2). Needs a pool " "of more than one.") diff --git a/product_parser.py b/product_parser.py index 30eb58b..0db369c 100644 --- a/product_parser.py +++ b/product_parser.py @@ -1,7 +1,7 @@ """ product_parser.py ----------------- -Everything this repo knows about goodreads.com lives here (CLAUDE.md §1). +Everything this repo knows about goodreads.com lives here. Three modes ----------- @@ -40,7 +40,7 @@ Every listing page is server-rendered, and three of them carry schema.org microdata (`itemtype="http://schema.org/Book"`), which is the anchor this -parser uses: a standard, not a build-generated class (§4). None of the +parser uses: a standard, not a build-generated class. None of the listing pages carries a JSON-LD block. The book page carries exactly one (`@type: Book`), and a much richer Apollo cache beside it. @@ -69,7 +69,7 @@ /shelf/show/fantasy?page=2 -> page 1 again, to an anonymous visitor /author/list/{id}?page=999 -> HTTP 404 -So every listing page is asked which page it SERVED (§23), and an answer +So every listing page is asked which page it SERVED, and an answer that differs from the page requested is the end of the listing, not more data. """ @@ -160,7 +160,7 @@ class Query: # The page the url itself asks for (?page=N). A run started on page 3 # calls it page 1 of the RUN and asks the site for page 3; the check # that the site served what was asked compares against the page the - # URL names, never the loop counter (§24). + # URL names, never the loop counter. start_page: int = 1 # book mode from a listing: at most this many books (None: all found). max_books: Optional[int] = None @@ -294,7 +294,7 @@ def kind_of(url: str) -> Optional[str]: def query_from_url(url: str) -> Tuple[Optional[Query], Optional[str]]: """The Query a url describes, or (None, why) for one this repo cannot read. The reason names what the url IS, never "not a Goodreads url" - when it is one (§5).""" + when it is one.""" u = urlparse(url if "://" in url else "https://" + url) host = (u.hostname or "").lower() if host not in HOSTS: @@ -377,11 +377,11 @@ def _clean_url(url: Optional[str]) -> Optional[str]: # AWS WAF's own vocabulary. Counted on every served capture (0 each) and # present on the challenge. `awswaf` alone is NOT here: the site's `_app` # bundle names a WAF challenge script in its config, and a marker that -# matches a served page is worse than none (§18). +# matches a served page is worse than none. AWS_WAF_MARKERS = ("gokuProps", "token.awswaf.com", "challenge.js") WAF_STATUSES = (202, 405) # A page the site built references its own asset host; an interstitial or -# Chromium's own error page does not (§8, §18). 9 to 298 occurrences on +# Chromium's own error page does not. 9 to 298 occurrences on # every served capture, the 404 and the missing-book page included. OWN_ASSET = "gr-assets.com" @@ -407,7 +407,7 @@ def _json_or_none(text: Optional[str]) -> Any: def detect_page_state(text: Optional[str], status: Optional[int] = None, url: str = "", waf_action: Optional[str] = None) -> str: """Name what the site answered with. The names are page_flow's policy - states; order is by how much each signal PROVES (§17): a WAF header or + states; order is by how much each signal PROVES: a WAF header or status first, then the page's own content, then heuristics.""" text = text or "" if waf_action or (status in WAF_STATUSES and len(text) < 20_000): @@ -540,7 +540,7 @@ def html_to_text(html: Optional[str]) -> Optional[str]: def _rating_pair(avg: Any, count: Any) -> Tuple[Optional[float], Optional[int]]: """A book nobody has rated shows "0.00 avg rating — 0 ratings". Both go - null together, so the zero does not drag an average (§21).""" + null together, so the zero does not drag an average.""" c = _int(count) if not c: return None, None diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index e0eefbd..c2439b8 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -52,8 +52,7 @@ # At module level, deliberately, and not inside the launch path. The offline # suite guards `import puppeteer_scraper` behind try/except ImportError and # REPORTS the skip, and CI's engine-smoke job fails on any reported skip. -# That only works if importing this module actually requires the driver -# (CLAUDE.md §10). +# That only works if importing this module actually requires the driver. from pyppeteer import launch, connect from captcha_solver import detect_aws_waf, solve_recaptcha, AWS_WAF_COOKIE @@ -69,11 +68,11 @@ # Every await in this file goes through the bridge below with a timeout, so a # hung remote call ends the operation instead of the run. pyppeteer provides -# no connect timeout of its own (§8). +# no connect timeout of its own. DEFAULT_OP_TIMEOUT = 120 CONNECT_TIMEOUT = 30 -# Chromium's own names for a proxy that could not be used (§8). +# Chromium's own names for a proxy that could not be used. _PROXY_ERROR_MARKERS = ( "ERR_PROXY_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED", @@ -87,7 +86,7 @@ # pyppeteer's dialect of the fetch() in playwright_scraper.FETCH_JS: the # arguments arrive POSITIONALLY rather than as one array. Same body, same -# return shape, same AbortController timeout (§8), same credentials rule +# return shape, same AbortController timeout, same credentials rule # (cookies on a same-origin request only; the API's CORS admits none). FETCH_JS = """ async (url, method, body, timeoutMs, extra) => { @@ -181,7 +180,7 @@ def _cancel_and_stop(): self._thread.join(timeout=5) -# Every `scheme://user:pass@` in a string, however many times it occurs (§8). +# Every `scheme://user:pass@` in a string, however many times it occurs. _CREDENTIALS_IN_URL_RE = re.compile(r"([a-z][a-z0-9+.\-]*://)[^\s/@]+:[^\s/@]+@", re.IGNORECASE) @@ -192,7 +191,7 @@ def _mask_credentials(text: str) -> str: def _chrome_ua(version: str) -> str: - """A desktop-Chrome UA naming the browser's OWN real version (§8). + """A desktop-Chrome UA naming the browser's OWN real version. pyppeteer's `browser.version()` returns "HeadlessChrome/115.0.0.0".""" number = version.split("/")[-1] if "/" in version else version return (f"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " @@ -218,7 +217,7 @@ class _Ops: """One pyppeteer browser + page, exposed as page_flow's named operations. Same contract as playwright_scraper._Ops, including the rule that a - rotation means a genuinely FRESH browser (§8). + rotation means a genuinely FRESH browser. """ def __init__(self, bridge: _AsyncBridge, args, pool): @@ -279,7 +278,7 @@ def open(self): exit_url = self.pool.current # Credentials go through page.authenticate(), never onto the # command line: --proxy-server= is part of the browser's argv, - # readable by anything that can run `ps` (§8). + # readable by anything that can run `ps`. scrubbed, credentials = split_credentials(exit_url) launch_args.append(f"--proxy-server={scrubbed}") logger.info("Using proxy exit %s", mask(exit_url)) @@ -443,7 +442,7 @@ def handle_captcha_if_present(ops, args, proxy: Optional[str] = None) -> bool: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, min_score=args.min_score, proxy=proxy) - except Exception as e: # noqa: BLE001 — a solver error is a warning (§8) + except Exception as e: # noqa: BLE001 — a solver error is a warning logger.error("Solving the AWS WAF CAPTCHA failed (%s) — continuing.", _mask_credentials(str(e))) return False @@ -538,7 +537,7 @@ def parse_args(argv=None): "/series/{id} or /book/show/{id}. Default: the Best " "Books Ever list. Also read from GOODREADS_URL.") g = p.add_argument_group("book") - g.add_argument("--max-books", type=int, default=None, metavar="N", + g.add_argument("--max-books", type=page_flow.bounded(int, 1, "--max-books"), default=None, metavar="N", help="With --mode book and a listing url: read at most N of " "the books the listing named (default: all of them).") g = p.add_argument_group("reviews") @@ -560,14 +559,14 @@ def parse_args(argv=None): help="Reviews per request, 1-100 (default 30, the site's " "own). 200 is answered with ZERO reviews, so it is " "refused.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.bounded(int, 1, "--pages"), default=1, help="Pages to fetch (100 books on a list, 30 on an " "author's, 20 on a search, 30 reviews). Planned " "against what page 1 states, so asking for more than " "exist fetches all of them.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=page_flow.bounded(float, 0, "--delay"), default=1.0, help="Delay between pages, seconds (default %(default)s)") - p.add_argument("--concurrency", type=int, default=1, metavar="N", + p.add_argument("--concurrency", type=page_flow.bounded(int, 1, "--concurrency"), default=1, metavar="N", help="Fetch pages through N parallel workers (default 1). " "Each worker runs its own browser and holds its own " "proxy exit. Ignored with --cdp-endpoint.") @@ -576,7 +575,7 @@ def parse_args(argv=None): "The pause doubles each time. A request the API " "REFUSED is not retried: its parameters would be " "refused again.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.bounded(float, 0, "--retry-delay"), default=2.0, help="Seconds before the first retry, doubling thereafter.") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="goodreads_rows", help="Output file prefix") @@ -595,7 +594,7 @@ def parse_args(argv=None): "per-page: a new exit, and a fresh browser, per page.") p.add_argument("--proxy-shuffle", action="store_true", help="Shuffle the pool at startup.") - p.add_argument("--proxy-block-retries", type=int, default=2, + p.add_argument("--proxy-block-retries", type=page_flow.bounded(int, 0, "--proxy-block-retries"), default=2, help="When a page is refused (403, AWS WAF), retry it " "from this many OTHER exits (default 2). Needs a pool " "of more than one.") diff --git a/scraper_api_client.py b/scraper_api_client.py index 373bfe3..00944d2 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -53,6 +53,7 @@ requested_page, served_page) from output_writer import dedupe_by_key, finish_run, SOURCE_DEFAULT import env_config +import page_flow logging.basicConfig(level=logging.INFO, format="%(asctime)s [%(levelname)s] %(message)s") logger = logging.getLogger("scraper_api_client") @@ -321,10 +322,10 @@ def parse_args(): p.add_argument("--mode", choices=["list", "book", "reviews"], default=None, help="Inferred from --url. reviews is refused with the " "reason: its API answers POST only.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.bounded(int, 1, "--pages"), default=1, help="Listing pages to fetch (default 1). Each is a " "separate billable task.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=page_flow.bounded(float, 0, "--delay"), default=1.0, help="Seconds between pages (default 1.0)") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="goodreads_rows_scraperapi", help="Output file prefix") @@ -342,10 +343,10 @@ def parse_args(): help="Wait for a page load state instead of specific content") p.add_argument("--allow-empty", action="store_true", help="Write output files even when 0 rows were parsed.") - p.add_argument("--retries", type=int, default=1, + p.add_argument("--retries", type=page_flow.bounded(int, 0, "--retries"), default=1, help="Extra attempts if a WAF page comes back. Each attempt is " "a separate billable task, so this defaults to 1.") - p.add_argument("--retry-delay", type=int, default=10, + p.add_argument("--retry-delay", type=page_flow.bounded(int, 0, "--retry-delay"), default=10, help="Seconds between retries (default 10)") p.add_argument("--dump-html", default=None, help="Also write the page to this path, even on success") diff --git a/selenium_scraper.py b/selenium_scraper.py index 621f6bf..e4308e0 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -75,7 +75,7 @@ # script timeout cuts the call off with an exception. SCRIPT_TIMEOUT = page_flow.FETCH_TIMEOUT_MS // 1000 + 15 -# Chromium's own names for "the proxy is the problem, not the site" (§8). +# Chromium's own names for "the proxy is the problem, not the site". _PROXY_ERROR_MARKERS = ( "ERR_PROXY_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED", "ERR_PROXY_AUTH_UNSUPPORTED", "ERR_PROXY_AUTH_REQUESTED", @@ -86,7 +86,7 @@ # Selenium's dialect of the fetch() in playwright_scraper.FETCH_JS: a # function BODY run by execute_async_script, with the arguments in # `arguments` and the result handed to the callback Selenium appends last. -# Same request, same return shape, same AbortController timeout (§8). +# Same request, same return shape, same AbortController timeout. FETCH_JS = """ var done = arguments[arguments.length - 1]; var url = arguments[0], method = arguments[1], body = arguments[2]; @@ -113,7 +113,7 @@ }); """ -# Every `scheme://user:pass@` in a string, however many times it occurs (§8). +# Every `scheme://user:pass@` in a string, however many times it occurs. _CREDENTIALS_IN_URL_RE = re.compile(r"([a-z][a-z0-9+.\-]*://)[^\s/@]+:[^\s/@]+@", re.IGNORECASE) @@ -124,7 +124,7 @@ def _mask_credentials(text: str) -> str: def _chrome_ua(version: str) -> str: - """A desktop-Chrome UA naming the installed Chrome's own version (§8).""" + """A desktop-Chrome UA naming the installed Chrome's own version.""" return (f"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " f"(KHTML, like Gecko) Chrome/{version} Safari/537.36") @@ -166,7 +166,7 @@ class _Ops: """One Chrome driver, exposed as page_flow's named operations. Same contract as playwright_scraper._Ops, including the rule that a - rotation means a genuinely FRESH browser (§8). + rotation means a genuinely FRESH browser. """ def __init__(self, args, pool): @@ -183,7 +183,7 @@ def open(self): logger.info("Attaching to an existing browser at %s.", options.debugger_address) # No UA, no proxy, no fingerprint on this path: the remote browser - # brings its own (§8). + # brings its own. self.driver = webdriver.Chrome(options=options) self._apply_timeouts() return self @@ -223,7 +223,7 @@ def open(self): def _apply_timeouts(self): # Explicit, because a driver that stops answering otherwise hangs - # the run (§8). + # the run. self.driver.set_page_load_timeout(PAGE_LOAD_TIMEOUT) self.driver.set_script_timeout(SCRIPT_TIMEOUT) @@ -329,7 +329,7 @@ def handle_captcha_if_present(ops, args, proxy: Optional[str] = None) -> bool: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, min_score=args.min_score, proxy=proxy) - except Exception as e: # noqa: BLE001 — a solver error is a warning (§8) + except Exception as e: # noqa: BLE001 — a solver error is a warning logger.error("Solving the AWS WAF CAPTCHA failed (%s) — continuing.", _mask_credentials(str(e))) return False @@ -421,7 +421,7 @@ def parse_args(argv=None): "/series/{id} or /book/show/{id}. Default: the Best " "Books Ever list. Also read from GOODREADS_URL.") g = p.add_argument_group("book") - g.add_argument("--max-books", type=int, default=None, metavar="N", + g.add_argument("--max-books", type=page_flow.bounded(int, 1, "--max-books"), default=None, metavar="N", help="With --mode book and a listing url: read at most N of " "the books the listing named (default: all of them).") g = p.add_argument_group("reviews") @@ -443,14 +443,14 @@ def parse_args(argv=None): help="Reviews per request, 1-100 (default 30, the site's " "own). 200 is answered with ZERO reviews, so it is " "refused.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.bounded(int, 1, "--pages"), default=1, help="Pages to fetch (100 books on a list, 30 on an " "author's, 20 on a search, 30 reviews). Planned " "against what page 1 states, so asking for more than " "exist fetches all of them.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=page_flow.bounded(float, 0, "--delay"), default=1.0, help="Delay between pages, seconds (default %(default)s)") - p.add_argument("--concurrency", type=int, default=1, metavar="N", + p.add_argument("--concurrency", type=page_flow.bounded(int, 1, "--concurrency"), default=1, metavar="N", help="Fetch pages through N parallel workers (default 1). " "Each worker runs its own browser and holds its own " "proxy exit. Ignored with --cdp-endpoint.") @@ -459,7 +459,7 @@ def parse_args(argv=None): "The pause doubles each time. A request the API " "REFUSED is not retried: its parameters would be " "refused again.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.bounded(float, 0, "--retry-delay"), default=2.0, help="Seconds before the first retry, doubling thereafter.") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="goodreads_rows", help="Output file prefix") @@ -478,7 +478,7 @@ def parse_args(argv=None): "per-page: a new exit, and a fresh browser, per page.") p.add_argument("--proxy-shuffle", action="store_true", help="Shuffle the pool at startup.") - p.add_argument("--proxy-block-retries", type=int, default=2, + p.add_argument("--proxy-block-retries", type=page_flow.bounded(int, 0, "--proxy-block-retries"), default=2, help="When a page is refused (403, AWS WAF), retry it " "from this many OTHER exits (default 2). Needs a pool " "of more than one.") diff --git a/smoke_test.py b/smoke_test.py index 9e6ddd1..df00df3 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -26,9 +26,11 @@ import argparse import ast +import contextlib import copy import csv import inspect +import io import json import os import re @@ -141,7 +143,7 @@ def check_fixture_corpus_is_real_and_scrubbed(): # --------------------------------------------------------------------------- -# The parser, asserted on VALUES rather than on coverage (§10) +# The parser, asserted on VALUES rather than on coverage # --------------------------------------------------------------------------- def check_a_list_page_parses_to_the_captured_values(): @@ -168,7 +170,7 @@ def check_a_list_page_parses_to_the_captured_values(): def check_out_of_range_pages_are_the_site_serving_another_page(): - """§23: /list/show/1?page=800 answers 200 with page 100, and a search + """/list/show/1?page=800 answers 200 with page 100, and a search ?page=999 with "Page 100 of 100". A run that trusted its request would re-collect the last page for as long as it was asked to.""" import product_parser as P @@ -307,7 +309,7 @@ def check_reviews_parse_to_the_captured_values(): equal("...and is the last one: no cursor after it", P.next_cursor(fx("reviews_fr_1star")), None) payload = json.loads(fx("reviews_p1")) payload["data"]["getReviews"]["edges"][0]["node"]["rating"] = 0 - equal("a review with no stars is null, not 0 (§21)", + equal("a review with no stars is null, not 0", P.parse_reviews(json.dumps(payload), q, 1)[0].rating, None) @@ -333,7 +335,7 @@ def check_the_reviews_request_is_the_one_measured(): def check_the_api_silent_fallbacks_are_refused_up_front(): - """§26: each of these was answered by the live API with HTTP 200 and + """Each of these was answered by the live API with HTTP 200 and nothing, or refused only after a request.""" q = _q("https://www.goodreads.com/book/show/5907", "reviews") q.page_size = 200 @@ -354,7 +356,7 @@ def check_zero_ratings_are_null_not_zero(): "0.00 avg rating — 0 ratings") r = P.parse_listing(html, _q(fx_url("author_p1")), 1)[0] check("the edit applied (a control that did nothing proves nothing)", "0 ratings" in html) - equal("an unrated book is null on both, never 0.00 (§21)", (r.avg_rating, r.ratings_count), + equal("an unrated book is null on both, never 0.00", (r.avg_rating, r.ratings_count), (None, None)) @@ -418,7 +420,7 @@ def check_page_states_on_real_captures(): def check_markers_do_not_match_a_good_page_or_the_extension(): - """§18: count a marker on pages you know are good. §24: the Scraping + """Count a marker on pages you know are good. The Scraping Browser's auto-solve extension injects captcha hunters (amazon_waf among them) into every page, and the set must score zero on that WITHOUT any strip.""" @@ -432,7 +434,7 @@ def check_markers_do_not_match_a_good_page_or_the_extension(): # The line above would pass on the own-asset guard alone (a served page # references gr-assets.com), whatever the markers were: a planted # "amazon_waf" marker left it green. So the SET is checked directly - # too, as a raw substring count, which is what §24 asks for. + # too, as a raw substring count, which is what that check needs. equal("...and not one marker even OCCURS in it (the set, not the guard)", [m for m in P.AWS_WAF_MARKERS if m in html], []) equal("...and it classifies as the book it is", P.detect_page_state( @@ -447,7 +449,7 @@ def check_markers_do_not_match_a_good_page_or_the_extension(): def check_the_waf_page_is_a_challenge_not_a_captcha(): - """§19: "unsolvable" is a property of a PAGE. This is the CHALLENGE + """"unsolvable" is a property of a PAGE. This is the CHALLENGE action: no widget, nothing to buy, and a browser passes it for free.""" from captcha_solver import detect_aws_waf c = detect_aws_waf(fx("waf_challenge"), fx_url("waf_challenge")) @@ -475,7 +477,7 @@ def check_state_policy(): F.should_retry("unauthorized") and not F.counts_as_blocked("unauthorized")) check("challenge: retried, solved, blocked", F.should_retry("challenge") and F.should_solve("challenge") and F.counts_as_blocked("challenge")) - check("throttled: retried, NOT blocked (§24)", + check("throttled: retried, NOT blocked", F.should_retry("throttled") and not F.counts_as_blocked("throttled")) equal("at most one solve per page", F.SOLVES_PER_PAGE, 1) check("headless /search is told to go headful", @@ -580,7 +582,7 @@ def check_the_shared_loop_end_to_end(): equal("a served list page: exit 0", rc, 0) equal("...complete", meta and meta["status"], "complete") equal("...the site's total in the sidecar", meta and meta["total_results"], 79607) - equal("...and that it caps what it serves (§21)", + equal("...and that it caps what it serves", meta and (meta["reachable_max"], meta["capped_by_site"]), (10000, True)) equal("...rows written", len(rows or []), 5) @@ -591,7 +593,7 @@ def check_the_shared_loop_end_to_end(): (meta["status"], meta["stop_reason"]), ("complete", "end_of_listing")) equal("...pages 1-3 fetched, not 4-5", len(ops.gotos), 3) equal("...and page 100's rows were NOT collected", len(rows), 8) - check("page+position unique across the run (§18)", + check("page+position unique across the run", len({(r["page"], r["position"]) for r in rows}) == len(rows)) rc, meta, rows, ops = _run(_FakeOps(lambda u: (404, fx("author_p999"))), @@ -606,8 +608,8 @@ def check_the_shared_loop_end_to_end(): rc, meta, rows, ops = _run(_FakeOps(lambda u: (202, "")), url="https://www.goodreads.com/book/show/5907") equal("a WAF challenge that never clears: exit 3, not 5", rc, 3) - equal("...and nothing was sent to the solver? (no widget: one attempt, returned False)", - ops.solves <= 2, True) + equal("...the solver was reached ONCE for the page, though it was " + "navigated twice (once, then from a fresh browser)", ops.solves, 1) class Clearing(_FakeOps): """The WAF's own script reloads the page: the first reads see the @@ -681,11 +683,103 @@ def document_text(self): throttled = [(429, ""), (200, fx("reviews_p1"))] rc, meta, rows, ops = _run(_FakeOps(book, throttled), url="https://www.goodreads.com/book/show/5907", mode="reviews", retries=1) - equal("a throttle wait spends its OWN budget, not --retries (§24, §26: " + equal("a throttle wait spends its OWN budget, not --retries (" "with --retries 1 the page still arrives)", rc, 0) equal("...at the SAME exit", ops.relaunches, 0) +class _PaidButStillChallenged(_FakeOps): + """A solver that reports success while the page stays challenged: the + worst case for a budget, since every call site believes it may buy.""" + + def solve_captcha(self): + self.solves += 1 + return True + + +def _run_with_pool(ops, proxies, **kw): + import page_flow + from proxy_pool import ProxyPool + with tempfile.TemporaryDirectory() as tmp: + args = _args(tmp, **kw) + errors = [] + q = page_flow.build_query(args, errors.append) + assert not errors, errors + pool = ProxyPool(proxies) + return page_flow.run_pages(lambda: ops, lambda o: None, + lambda pages, query: ([], [], False), + args, pool, q, 1) + + +def check_one_paid_solve_per_page_on_every_path(): + """SOLVES_PER_PAGE is one allowance for the PAGE, not for each call into + the solver. Before, a page re-fetched after a refusal started a fresh + allowance: 2 solves with no pool, 4 with a pool of four exits.""" + import page_flow + waf = lambda u: (202, fx("waf_challenge")) + book = "https://www.goodreads.com/book/show/5907" + for retries in (1, 3): + rc, meta, rows, ops = _run(_PaidButStillChallenged(waf), url=book, retries=retries) + equal("no pool, --retries %d: ONE solve for the page" % retries, ops.solves, 1) + equal("...and it was still re-fetched from a fresh browser", ops.relaunches, 1) + equal("...and the run is blocked (exit 3)", rc, 3) + ops = _PaidButStillChallenged(waf) + rc = _run_with_pool(ops, ["http://10.0.0.%d:8080" % i for i in range(1, 5)], + url=book, proxy_block_retries=3) + equal("a pool of 4, --proxy-block-retries 3: rotated three times", ops.relaunches, 3) + equal("...and still ONE solve: a fresh exit is not a fresh allowance", ops.solves, 1) + rc, meta, rows, ops = _run(_PaidButStillChallenged(waf), url=book, mode="reviews") + equal("a reviews run whose LANDING is challenged: one solve", ops.solves, 1) + rc, meta, rows, ops = _run(_PaidButStillChallenged(waf), url=book, mode="reviews", + review_language="en") + equal("...also when the language check lands first: one solve for page 1", + ops.solves, 1) + budget = page_flow.SolveBudget() + calls = [] + check("the allowance is checked BEFORE a purchase", + budget.spend(lambda: calls.append(1) or True) and not budget.spend( + lambda: calls.append(2) or True) and calls == [1], repr(calls)) + + +def check_a_page_that_states_rows_and_parses_none_is_a_parser_failure(): + """A served page whose own total and page count say it holds rows, and + from which the parser read none, is this repo's parser failing. Before, + page 1 reported exit 4 ("the listing is empty") and page 2 ended the run + as complete with exit 0.""" + import page_flow + real = page_flow.parse_page + try: + page_flow.parse_page = lambda text, query, page=1: [] + rc, meta, rows, ops = _run(_FakeOps(_listing(lambda n: "list_p1"))) + equal("page 1 states 79,607 books and parses to none: exit 5, not 4", rc, 5) + page_flow.parse_page = lambda text, query, page=1: ( + real(text, query, page) if page == 1 else []) + rc, meta, rows, ops = _run(_FakeOps(_listing( + lambda n: {1: "list_p1"}.get(n, "list_p2"))), pages=3) + equal("page 2 served and parsed to none: exit 6, partial", + (rc, meta and meta["status"]), (6, "partial")) + equal("...named parser_found_nothing, with page 2 failed", + meta and (meta["stop_reason"], meta["pages_failed"]), + ("parser_found_nothing", [2])) + equal("...and page 3 was not fetched", len(ops.gotos), 2) + finally: + page_flow.parse_page = real + check("parser_found_nothing is NOT a complete stop reason", + "parser_found_nothing" not in __import__("output_writer").COMPLETE_STOP_REASONS) + rc, meta, rows, ops = _run(_FakeOps(lambda u: (404, fx("author_p999"))), + url=fx_url("author_p999")) + equal("an author list past its end is still exit 4 (the site said so)", rc, 4) + rc, meta, rows, ops = _run(_FakeOps(lambda u: (200, fx("search_none"))), + url=fx_url("search_none")) + equal("a search with no results is still exit 4", rc, 4) + q = _q("https://www.goodreads.com/shelf/show/fantasy") + check("a shelf's constant page count of 1 is not a statement of rows", + not page_flow.page_states_rows(q, 1, None, 1)) + q = _q(book_url := "https://www.goodreads.com/book/show/5907", "reviews") + check("a reviews filter whose total is 0 is an answer, not a parser failure", + not page_flow.page_states_rows(q, 1, 0, 0), book_url) + + def check_a_book_run_from_a_listing_reads_every_book(): pages = lambda u: (200, fx("series") if "/series/" in u else fx("book_hobbit")) rc, meta, rows, ops = _run(_FakeOps(pages), url=fx_url("series"), mode="book", max_books=3) @@ -713,7 +807,7 @@ def check_usage_errors_are_refused_before_a_request(): rc, *_ = _run(ops, url="https://www.goodreads.com/list/show/1", sort="newest") check("--sort on a list run: refused (it would do nothing)", rc[0] == "usage") rc, *_ = _run(ops, url="https://www.goodreads.com/book/show/1", mode="reviews", concurrency=3) - check("--concurrency with reviews: refused, the cursor chains the pages (§18)", + check("--concurrency with reviews: refused, the cursor chains the pages", rc[0] == "usage" and "cursor" in rc[1][0]) rc, *_ = _run(ops, url="https://www.goodreads.com/list/show/1", max_books=5) check("--max-books without --mode book: refused", rc[0] == "usage") @@ -761,7 +855,7 @@ def check_row_schema(): UNIQUE_BY_SKU_MODES) for cls in (ListedBook, Book, Review): names = [f.name for f in fields(cls)] - equal("%s: the family prefix is byte-identical and in order (§9)" % cls.__name__, + equal("%s: the family prefix is byte-identical and in order" % cls.__name__, names[:5], ["source", "scraped_at", "url", "sku", "title"]) equal("%s: the run-describing tail closes the row" % cls.__name__, names[-4:], ["page", "position", "mode", "data_source"]) @@ -795,12 +889,175 @@ def check_csv_and_json_writers(): check("a list column stays a real list in JSON", isinstance(loaded[0]["genres"], list)) +def check_a_failed_write_keeps_the_previous_file(): + """A write that fails part-way must leave the previous good file as it + was, and leave no temp file behind. Before, `open(path, "w")` truncated + first, and a failed dump left 10 bytes of a 539-byte file.""" + import output_writer as O + rows = _rows("list_p1") + real_dump = O.json.dump + + def broken_dump(obj, f, **kw): + f.write("[{") + raise OSError("disk full") + with tempfile.TemporaryDirectory() as tmp: + for name, write in (("out.json", lambda p: O.write_json(rows, p)), + ("out.meta.json", lambda p: O.write_run_meta(p[:-10], {"status": "complete"}))): + path = os.path.join(tmp, name) + with open(path, "w", encoding="utf-8") as f: + f.write('{"good": true}') + O.json.dump = broken_dump + try: + write(path) + raised = False + except OSError: + raised = True + finally: + O.json.dump = real_dump + check("%s: the failure is reported, not swallowed" % name, raised) + equal("%s: the previous good file is intact" % name, + open(path, encoding="utf-8").read(), '{"good": true}') + equal("no temp file is left behind", sorted(os.listdir(tmp)), + ["out.json", "out.meta.json"]) + + +def check_written_files_get_the_mode_open_would_give(): + """A temp file is created 0600 and the rename keeps that, so the writer + sets the mode itself: a new file `0o666 & ~umask`, an existing file its + own mode. Checked under a NON-default umask too, where a flat 0o644 + would differ.""" + import output_writer as O + rows = _rows("list_p1") + with tempfile.TemporaryDirectory() as tmp: + for umask in (0o022, 0o002, 0o077): + old = os.umask(umask) + try: + path = os.path.join(tmp, "new-%o.json" % umask) + O.write_json(rows, path) + equal("umask %03o: a new file is %03o" % (umask, 0o666 & ~umask), + os.stat(path).st_mode & 0o777, 0o666 & ~umask) + finally: + os.umask(old) + path = os.path.join(tmp, "kept.csv") + open(path, "w").close() + os.chmod(path, 0o640) + O.write_csv(rows, path) + equal("an existing file keeps its own mode", os.stat(path).st_mode & 0o777, 0o640) + + +def check_csv_cells_that_start_a_formula_are_neutralised(): + """Review text, shelf names, tags and reviewer names are written by + whoever posted them. A cell starting with = + - @ or a tab/CR/LF is a + formula to a spreadsheet: CSV gets a leading apostrophe, JSON keeps the + site's text, and the sidecar counts the difference.""" + import output_writer as O + rows = _rows("reviews_p1", mode="reviews") + r = rows[0] + r.text = '=HYPERLINK("http://x")' + r.tags = ["@sum", "fine"] + r.reviewer = "-dash" + r.likes = -5 + with tempfile.TemporaryDirectory() as tmp: + prefix = os.path.join(tmp, "out") + rc = O.finish_run(rows, prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", + mode="reviews", extra={"total_results": 9}) + equal("the run still exits 0", rc, 0) + got = next(csv.DictReader(open(prefix + ".csv", encoding="utf-8", newline=""))) + equal("a formula cell is escaped in CSV", got["text"], '\'=HYPERLINK("http://x")') + equal("a joined list is escaped AFTER the join", got["tags"], "'@sum | fine") + equal("a leading minus in TEXT is escaped", got["reviewer"], "'-dash") + equal("a negative NUMBER is left a number", got["likes"], "-5") + loaded = json.load(open(prefix + ".json", encoding="utf-8")) + equal("JSON keeps the site's text", loaded[0]["text"], '=HYPERLINK("http://x")') + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("the sidecar counts the escaped cells", meta.get("csv_cells_escaped"), 3) + O.finish_run(rows, prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", + mode="reviews", extra={"csv_cells_escaped": "engine"}) + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("...merged UNDER the caller's extra, never over it", + meta.get("csv_cells_escaped"), "engine") + + +NUMERIC_FLAG_FLOORS = { + "--pages": 1, "--delay": 0, "--retry-delay": 0, "--concurrency": 1, + "--proxy-block-retries": 0, "--max-books": 1, +} +# scraper_api_client.py counts --retries as EXTRA attempts, so 0 is legal +# there and only a negative is refused. +API_CLIENT_FLAG_FLOORS = {"--pages": 1, "--delay": 0, "--retries": 0, + "--retry-delay": 0} + + +def _bounded_flag_problems(): + """Every counted or timed flag is declared with page_flow.bounded and the + floor this repo means, read from the SOURCE so an engine whose driver is + absent is checked too.""" + import argparse as _ap + import page_flow + problems = [] + targets = {m + ".py": NUMERIC_FLAG_FLOORS for m in ENGINES} + targets["scraper_api_client.py"] = API_CLIENT_FLAG_FLOORS + for filename, floors in targets.items(): + tree = ast.parse(open(os.path.join(HERE, filename), encoding="utf-8").read()) + found = {} + for node in ast.walk(tree): + if (isinstance(node, ast.Call) and getattr(node.func, "attr", "") == "add_argument" + and node.args and isinstance(node.args[0], ast.Constant) + and node.args[0].value in floors): + kind = next((k.value for k in node.keywords if k.arg == "type"), None) + found[node.args[0].value] = kind + for flag, floor in floors.items(): + kind = found.get(flag) + if kind is None: + problems.append("%s declares no %s" % (filename, flag)) + continue + text = ast.unparse(kind) if hasattr(ast, "unparse") else "" + if text and not (text.startswith("page_flow.bounded(") + and ", %d, %r)" % (floor, flag) in text.replace('"', "'")): + problems.append("%s: %s has type=%s, expected page_flow.bounded(..., %d, %r)" + % (filename, flag, text, floor, flag)) + for flag, floor in NUMERIC_FLAG_FLOORS.items(): + check_fn = page_flow.bounded(float if "delay" in flag else int, floor, flag) + try: + check_fn(str(floor - 1)) + problems.append("bounded accepted %s %d" % (flag, floor - 1)) + except _ap.ArgumentTypeError as e: + if flag not in str(e): + problems.append("the error for %s does not name the flag: %s" % (flag, e)) + if check_fn(str(floor)) != floor: + problems.append("bounded refused %s %d" % (flag, floor)) + return problems + + +def check_numeric_flags_refuse_values_below_their_floor(): + problems = _bounded_flag_problems() + check("--pages/--delay/--retry-delay/--concurrency/--proxy-block-retries/" + "--max-books refuse values below their floor in every entry point", + not problems, "; ".join(problems)) + import page_flow + p = __import__("argparse").ArgumentParser(prog="x") + p.add_argument("--delay", type=page_flow.bounded(float, 0, "--delay")) + err = io.StringIO() + try: + with contextlib.redirect_stderr(err): + p.parse_args(["--delay", "-1"]) + code = 0 + except SystemExit as e: + code = e.code + equal("argparse turns it into exit 2 naming the flag", + (code, "--delay must be at least 0" in err.getvalue()), (2, True)) + + def check_exit_codes(): import output_writer as O equal("3 blocked / 4 empty / 5 never obtained / 6 partial", (O.EXIT_BLOCKED, O.EXIT_NO_PRODUCTS, O.EXIT_FETCH_FAILED, O.EXIT_PARTIAL), (3, 4, 5, 6)) - check("end_of_listing is a COMPLETE stop reason (§24)", + check("end_of_listing is a COMPLETE stop reason", "end_of_listing" in O.COMPLETE_STOP_REASONS) check("api_rejected is NOT complete", "api_rejected" not in O.COMPLETE_STOP_REASONS) @@ -849,11 +1106,11 @@ def check_sidecar_shape(): # --------------------------------------------------------------------------- -# The family's own checks (CLAUDE.md §10, §17, §22, §24, §26) +# The family's own checks # --------------------------------------------------------------------------- def check_policy_constants_have_a_consumer(): - """§17: a policy constant nothing reads is the same defect as dead code.""" + """A policy constant nothing reads is the same defect as dead code.""" src = open(os.path.join(HERE, "page_flow.py"), encoding="utf-8").read() for constant in ("RETRY_ON_BLOCKED", "BLOCK_RETRIES_WITHOUT_POOL", "SOLVES_PER_PAGE", "THROTTLE_RETRIES", "THROTTLE_WAIT_S", @@ -898,9 +1155,9 @@ def check_engines_import_their_driver_at_module_level(): def check_shared_calls_bind_against_the_real_signature(): - """§17's check #1. Every call from an engine (and the Scraper API client, + """Every call from an engine (and the Scraper API client, diff_runs and page_flow itself) into a shared module is bound against the - callee's real signature. A name that does not exist FAILS (§22). A name + callee's real signature. A name that does not exist FAILS. A name bound in the calling file shadows a same-named module.""" import captcha_solver import output_writer @@ -975,8 +1232,7 @@ def _argparse_flags(module_name): return flags -# The family's flag contract (CLAUDE.md §9, including the five it omitted -# for months), plus this repo's own query flags. +# The family's flag contract, plus this repo's own query flags. CONTRACT_FLAGS = { "--url", "--pages", "--category", "--format", "--out", "--delay", "--retries", "--retry-delay", "--concurrency", "--proxy", "--proxy-file", @@ -993,7 +1249,7 @@ def _argparse_flags(module_name): def check_engine_flag_sets(): - """§17's check #2: against the contract AND against each other, both + """Engine flags, against the contract AND against each other, both ways. The exception list IS the documentation.""" sets = {m: _argparse_flags(m) for m in ENGINES} for module, flags in sets.items(): @@ -1022,7 +1278,7 @@ def check_banned_and_removed_flags(): def check_undefined_names_in_every_module(): - """§10: compileall proves a file PARSES, not that its names RESOLVE. + """`compileall` proves a file PARSES, not that its names RESOLVE. Kept COARSE (pooled bindings) so it under-reports rather than invents.""" import builtins for filename in sorted(f for f in os.listdir(HERE) if f.endswith(".py")): @@ -1136,7 +1392,7 @@ def check_credential_scan_is_one_implementation_invoked_from_both(): script = os.path.join(HERE, ".github", "ci_checks.py") if not os.path.isdir(os.path.join(HERE, ".github")): # Inside the Docker image, which copies no .github at all. Triggered - # by the WHOLE directory being absent, never by one file in it (§22). + # by the WHOLE directory being absent, never by one file in it. skip("ci_checks", "no .github directory (the image)") return check("the credential scan exists as a script", os.path.exists(script)) @@ -1184,8 +1440,8 @@ def check_the_hex_exemption_is_one_context_only(): # Assembled from pieces, so this file can be scanned like every other rather -# than exempted (§22: the file most likely to acquire a stray phrase is the -# one a wholesale exemption never reads). +# than exempted: the file most likely to acquire a stray phrase is the +# one a wholesale exemption never reads. BANNED_WORDING = ( "cloud" + " browser", "anti" + "detect browser", "2scraper " + "Anti" + "detect Browser", "gate." + "2prx.com", "ANTI" + "DETECT_LOCAL_API", @@ -1193,7 +1449,7 @@ def check_the_hex_exemption_is_one_context_only(): def check_banned_wording(): - """§12, enforced by this test rather than by review.""" + """Banned product wording, enforced by this test rather than by review.""" scanned = 0 for root, dirs, files in os.walk(HERE): dirs[:] = [d for d in dirs if d not in (".git", "__pycache__", ".pytest_cache", @@ -1212,8 +1468,41 @@ def check_banned_wording(): check("the banned-wording scan read the repo (%d files)" % scanned, scanned > 20) +# The family's own working notes are private, so a pointer into them from a +# public file reads as a reference nobody outside can follow. Assembled from +# pieces so this file does not match itself. +_SECTION_SIGN = chr(0xA7) +PRIVATE_CITATIONS = re.compile("|".join([ + re.escape("CLAUDE" + ".md"), re.escape("claude_" + "2scraper"), + re.escape("2scraper_" + "repo_rules"), "play" + "book", + re.escape(_SECTION_SIGN) + r"\s*\d", +]), re.IGNORECASE) + + +def check_no_public_file_cites_the_private_notes(): + """Every rule a file relies on is stated in that file's own words.""" + scanned, hits = 0, [] + for root, dirs, files in os.walk(HERE): + dirs[:] = [d for d in dirs + if d not in (".git", "__pycache__", ".pytest_cache", "live", + "captures", ".claude", "node_modules") + and not os.path.exists(os.path.join(root, d, "pyvenv.cfg"))] + for filename in files: + path = os.path.join(root, filename) + try: + text = open(path, encoding="utf-8").read() + except (UnicodeDecodeError, OSError): + continue + scanned += 1 + for n, line in enumerate(text.splitlines(), 1): + if PRIVATE_CITATIONS.search(line): + hits.append("%s:%d" % (os.path.relpath(path, HERE), n)) + check("no shipped file cites the private notes", not hits, ", ".join(hits[:10])) + check("the citation scan read the repo (%d files)" % scanned, scanned > 20) + + def check_concurrency_with_the_browser_stubbed(): - """§10: a live run cannot always reach this machinery. Driven through the + """A live run cannot always reach this machinery. Driven through the SHARED worker loop with the fetch replaced.""" import page_flow import queue as queue_mod @@ -1322,7 +1611,7 @@ def check_fingerprint_kwargs_are_ones_the_driver_accepts(): def check_engines_do_not_evaluate_a_string_in_the_browser(): - """§18: wait_for_function evaluates a string, which a CSP without + """wait_for_function evaluates a string, which a CSP without unsafe-eval kills. page.evaluate with a real function is fine.""" for module in ENGINES: tree = ast.parse(open(os.path.join(HERE, module + ".py"), encoding="utf-8").read()) @@ -1471,7 +1760,7 @@ def check_x_debug_header_is_redacted(): def check_captcha_capability_claims_match_the_code(): - """§19: the most expensive bug this family can ship is a SENTENCE.""" + """The most expensive bug this family can ship is a SENTENCE.""" readme = open(os.path.join(HERE, "README.md"), encoding="utf-8").read() solver = open(os.path.join(HERE, "captcha_solver.py"), encoding="utf-8").read() low = readme.lower() @@ -1484,7 +1773,7 @@ def check_captcha_capability_claims_match_the_code(): def check_readme_numbers_are_not_stale(): - """§17's check #4: a column count claimed in the README is a class's.""" + """A column count claimed in the README is a class's.""" readme = open(os.path.join(HERE, "README.md"), encoding="utf-8").read() from output_writer import ROW_CLASS_BY_MODE sizes = {len(fields(c)) for c in ROW_CLASS_BY_MODE.values()} From e1ee222841a18c67c539e098cb280307eb6d7e51 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:46:02 +0000 Subject: [PATCH 2/2] Canary fails when a core column is empty on any row A row count alone passes when the parser fills rows with nulls. sku, title and author (list, book; plus work_id on books), and sku, reviewer and created_at (reviews), are filled on every row of every capture, so one null fails the canary. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/canary.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 38ebc02..fd43b67 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -121,6 +121,15 @@ jobs: problems.append("no total_results in the sidecar") if {r["mode"] for r in rows} != {mode}: problems.append("rows name modes %r" % {r["mode"] for r in rows}) + # Core columns the parser fills on every row of every capture: a + # null here means the parser broke, whatever the row count says. + core = {"list": ("sku", "title", "author"), + "book": ("sku", "title", "author", "work_id"), + "reviews": ("sku", "reviewer", "created_at")}[mode] + for col in core: + missing = sum(1 for r in rows if r.get(col) in (None, "", [])) + if missing: + problems.append("%d of %d rows have no %s" % (missing, len(rows), col)) pairs = [(r["page"], r["position"]) for r in rows] if len(set(pairs)) != len(pairs): problems.append("page+position is not unique")