From feb48222bef5030b0f634e1a3c67929e445ca475 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:50:51 +0000 Subject: [PATCH] Keep good output on a failed write, escape CSV formulas, let typed flags win over WEBMOTORS_URL - JSON, CSV, the sidecar and diff_runs --out are written atomically (temp file beside the target, fsync, rename; new files 0o666 & ~umask, existing files keep their mode). - CSV string cells starting with = + - @ tab CR LF get a leading apostrophe; numbers and JSON untouched; csv_cells_escaped in the sidecar under the caller's extra. - .env.example ships WEBMOTORS_URL commented out, and an environment URL gives way to typed search flags, --ads-file or a different --mode. - Negative delays/counts and --concurrency/--pages below 1 are usage errors naming the flag, in every engine and the Scraper API client. - The CDP 401 hint no longer claims credentials last about a day. - The canary fails by name without WEBMOTORS_PROXY/WEBMOTORS_CDP_ENDPOINT instead of skipping green, accepts either, and covers ad mode. - Bug template example uses this scraper's flags; banned-wording scan skips virtualenvs; comments state rules instead of citing private notes, with a suite guard against their return. Co-Authored-By: Claude Opus 5.5 --- .env.example | 12 +- .github/ISSUE_TEMPLATE/bug_report.yml | 7 +- .github/ci_checks.py | 6 +- .github/workflows/canary.yml | 113 ++++++--- .gitignore | 6 +- CHANGELOG.md | 55 +++++ CONTRIBUTING.md | 12 +- README.md | 17 +- TROUBLESHOOTING.md | 9 +- diff_runs.py | 6 +- env_config.py | 4 +- fingerprint_client.py | 8 +- make_fixtures.py | 2 +- output_writer.py | 123 +++++++++- page_flow.py | 96 +++++--- playwright_scraper.py | 29 ++- product_parser.py | 22 +- puppeteer_scraper.py | 29 ++- scraper_api_client.py | 14 +- selenium_scraper.py | 25 +- smoke_test.py | 338 +++++++++++++++++++++++--- 21 files changed, 740 insertions(+), 193 deletions(-) diff --git a/.env.example b/.env.example index b191e3b..14c4dd8 100644 --- a/.env.example +++ b/.env.example @@ -54,8 +54,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 endpoint carries a password: never paste a working one anywhere +# that is committed. WEBMOTORS_CDP_ENDPOINT=ws://{login}-zone-scraping_browser-country-br-pid-{profileId}:{password}@cb.2captcha.com:9222 # --- Proxy ---------------------------------------------------------------- @@ -78,10 +78,10 @@ WEBMOTORS_PROXY=http://{user}-zone-custom-region-br:{password}@na.proxy.2captcha # https://www.webmotors.com.br/carros/sp/toyota?anode=2020 # https://www.webmotors.com.br/motos/estoque/honda # -# While it is set, the search flags (--make, --model, --state ...) are -# refused, since they would have to agree with it and nothing checks that -# they do. Leave it unset if you use the flags. -WEBMOTORS_URL=https://www.webmotors.com.br/carros/estoque/volkswagen/gol +# It is only a default: a --url, search flags (--make, --model, --state +# ...) or an --ads-file typed on the command line replace it. Shipped +# commented out, so a copied .env does not pick a search for you. +# WEBMOTORS_URL=https://www.webmotors.com.br/carros/estoque/volkswagen/gol # Deliberately NOT here: an output prefix. `--out` already carries a # non-empty default, so the loader, which only fills UNSET values, would diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index b34ba77..4282438 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -70,8 +70,8 @@ body: render: shell placeholder: | python3 playwright_scraper.py \ - --asset USDT --fiat EUR --side buy \ - --pages 3 --out usdt_eur + --make volkswagen --model gol \ + --pages 3 --out gol validations: required: true @@ -91,7 +91,8 @@ body: attributes: label: What you expected instead placeholder: >- - 60 adverts over 3 pages, as the site shows ~200 for USDT/EUR. + About 141 listings over 3 pages of 47, as the site shows over a + thousand Gol listings. validations: required: true diff --git a/.github/ci_checks.py b/.github/ci_checks.py index e4e8092..d2ec433 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -89,8 +89,8 @@ # This site publishes no 32-hex identifier at all: an advert's id is a short # integer and its photo file names are `WMIMAGEM` plus digits, checked in # every capture rather than assumed. So a bare 32-hex string anywhere in the -# repo fails, with no exception to reason about (CLAUDE.md §24: before -# adding an exemption, check whether the value is needed at all). +# repo fails, with no exception to reason about (before adding an +# exemption, check whether the value is needed at all). SITE_PUBLIC_IDS = () def _without_site_ids(line): @@ -279,7 +279,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 817ef09..64c4e65 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -1,22 +1,25 @@ -# One real run of each mode against webmotors.com.br, daily. tests.yml -# (deliberately) never touches the site. +# A real run of each mode against webmotors.com.br, daily: a car search, a +# motorcycle search, and the first adverts of the car search read in ad mode. +# tests.yml (deliberately) never touches the site. # -# THIS CANARY NEEDS A SECRET, and without one it SKIPS rather than fails -# (CLAUDE.md §11). CloudFront refused every datacentre address measured on -# 2026-09-28 — a VPS, headful Chromium included, and the Scraper API's own -# exits — and a GitHub-hosted runner is a datacentre address. A canary that -# cannot pass is red every day, and a check that is always red teaches -# everyone to ignore checks. So the live steps run only when a RESIDENTIAL -# proxy is configured: +# THIS CANARY NEEDS A SECRET, and without one it FAILS. CloudFront refused +# every datacentre address measured on 2026-09-28 (a VPS, headful Chromium +# included, and the Scraper API's own exits), and a GitHub-hosted runner is a +# datacentre address. So a run from the bare runner cannot reach the +# listings, and a canary that skipped green in that case reported a scraper +# as working when nothing had been tested. Set ONE of: # # gh secret set WEBMOTORS_PROXY --repo 2scraper/webmotors-scraper +# a residential exit, http://user:pass@host:port +# gh secret set WEBMOTORS_CDP_ENDPOINT --repo 2scraper/webmotors-scraper +# a 2Captcha Scraping Browser API endpoint (country-br) # -# It goes through the ENVIRONMENT, not a --proxy line: env_config.py reads -# exactly that variable, an unset secret arrives as an empty string which the -# loader treats as unset, and the credential never reaches a command line. -# -# Not a Scraping Browser endpoint: a profile's credentials expire in about a -# day, so a secret holding one would rot between runs (§19). +# They go through the ENVIRONMENT, not a command line: env_config.py reads +# exactly those variables, an unset secret arrives as an empty string which +# the loader treats as unset, and the credential never reaches a command +# line. A Scraping Browser endpoint stops working when its credentials are +# refused; how long that takes is not a fixed figure, so a 401 here means +# "copy the endpoint again", not a broken scraper. name: canary @@ -44,40 +47,39 @@ jobs: - name: search args: "--make volkswagen --model gol --pages 3" floor: 120 + ads: 3 - name: motorcycles args: "--category motos --make honda --pages 3" floor: 120 + ads: 0 env: WEBMOTORS_PROXY: ${{ secrets.WEBMOTORS_PROXY }} + WEBMOTORS_CDP_ENDPOINT: ${{ secrets.WEBMOTORS_CDP_ENDPOINT }} steps: - - name: Skip without a residential proxy - if: env.WEBMOTORS_PROXY == '' + # No skip: a run that never contacted the site must not end green. + - name: Require a residential exit run: | - echo "::notice::No WEBMOTORS_PROXY secret. CloudFront refuses datacentre addresses (a GitHub runner is one), so a live run here could only fail. Set a residential proxy secret to enable this canary (see the header)." - # A green run that tested nothing must SAY so where a reader looks. - # The badge cannot: GitHub colours it by the run's conclusion - # (third-party audit, 2026-10-08). - echo "## ${{ matrix.name }}: SKIPPED — the site was NOT contacted" >> "$GITHUB_STEP_SUMMARY" - echo "No residential proxy secret is set, so no live step ran. This run proves the workflow is intact, not that the scraper works today." >> "$GITHUB_STEP_SUMMARY" + if [ -z "$WEBMOTORS_PROXY" ] && [ -z "$WEBMOTORS_CDP_ENDPOINT" ]; then + echo "::error::Neither the WEBMOTORS_PROXY nor the WEBMOTORS_CDP_ENDPOINT secret is set. CloudFront refuses datacentre addresses (a GitHub runner is one), so this canary cannot reach the site without one. See the workflow header." + echo "## ${{ matrix.name }}: FAILED — the site was NOT contacted" >> "$GITHUB_STEP_SUMMARY" + echo "Set the WEBMOTORS_PROXY or WEBMOTORS_CDP_ENDPOINT secret." >> "$GITHUB_STEP_SUMMARY" + exit 1 + fi - uses: actions/checkout@v4 - if: env.WEBMOTORS_PROXY != '' - uses: actions/setup-python@v5 - if: env.WEBMOTORS_PROXY != '' with: python-version: "3.12" - name: Install - if: env.WEBMOTORS_PROXY != '' run: | python -m pip install --upgrade pip 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 - if: env.WEBMOTORS_PROXY != '' run: | set +e # `run:` is `bash -e`; without this a non-zero exit aborts # the step before the echo below records it. @@ -86,7 +88,6 @@ jobs: echo "exit_code=$?" >> "$GITHUB_OUTPUT" - name: Interpret - if: env.WEBMOTORS_PROXY != '' run: | code="${{ steps.run.outputs.exit_code }}" echo "exit code: $code" @@ -101,7 +102,6 @@ jobs: esac - name: Assert - if: env.WEBMOTORS_PROXY != '' run: | python - <<'EOF' import json, sys @@ -141,8 +141,56 @@ jobs: print("canary OK: %d rows of %s, complete" % (len(rows), meta["total_results"])) EOF + # Ad mode: the first adverts the search returned, each read from its + # own detail endpoint. + - name: Run ad mode + id: ads + if: matrix.ads > 0 + run: | + python - <<'EOF' + import json + rows = json.load(open("canary.json")) + with open("canary_ads.txt", "w") as f: + f.write("\n".join(r["url"] for r in rows[:${{ matrix.ads }}]) + "\n") + EOF + set +e + python playwright_scraper.py --ads-file canary_ads.txt \ + --out canary_ad --format both --dump-html canary_ad_dump.json + code=$? + echo "exit code: $code" + if [ "$code" != 0 ]; then + echo "::error::Ad mode exited $code (3 blocked, 5 never arrived, 6 some adverts failed). See canary_ad.meta.json." + exit 1 + fi + + - name: Assert ad mode + if: matrix.ads > 0 + run: | + python - <<'EOF' + import json, sys + meta = json.load(open("canary_ad.meta.json")) + rows = json.load(open("canary_ad.json")) + want = ${{ matrix.ads }} + problems = [] + if meta["status"] != "complete": + problems.append("status %r (%s)" % (meta["status"], meta["stop_reason"])) + if len(rows) != want: + problems.append("%d adverts, asked for %d" % (len(rows), want)) + for r in rows: + if not (isinstance(r["price"], float) and r["price"] > 0): + problems.append("advert %s has no price" % r["sku"]) + if not r.get("make") or not r.get("year_model"): + problems.append("advert %s lacks make/year_model" % r["sku"]) + if r["seller_type"] == "private" and (r["seller_name"] or r["description"]): + problems.append("private advert %s carries a name or text" % r["sku"]) + if problems: + print("ad canary FAILED: " + "; ".join(problems)) + sys.exit(1) + print("ad canary OK: %d adverts" % len(rows)) + EOF + - uses: actions/upload-artifact@v4 - if: always() && env.WEBMOTORS_PROXY != '' + if: always() with: name: canary-${{ matrix.name }} path: | @@ -151,4 +199,7 @@ jobs: canary.meta.json canary_dump.json* canary_page*_debug.html + canary_ad.json + canary_ad.meta.json + canary_ad_dump.json* if-no-files-found: ignore diff --git a/.gitignore b/.gitignore index dd526b1..9cb3c1e 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,7 @@ webmotors_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 in a sibling: `live/` was not +# name an older repo used. Measured in a sibling: `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 @@ -95,7 +95,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 a24f41f..7c9a7e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,56 @@ said so at the top of its release notes. > script of yours passes 0, pass **1**: the flag counts ATTEMPTS, so 1 means > "try once and do not retry". +> **Behaviour change:** negative values of `--delay`, `--retry-delay` and +> `--proxy-block-retries`, `--concurrency` below 1 and `--pages` below 1 are +> now usage errors (exit 2) naming the flag, in every browser engine. In +> `scraper_api_client.py`, `--retries`, `--delay` and `--retry-delay` below 0 +> and `--pages` below 1 are refused the same way (`--retries 0` stays legal +> there: it counts EXTRA attempts). + +> **Behaviour change:** CSV cells that begin with `=`, `+`, `-`, `@`, a tab +> or a line break now get a leading `'`, so a spreadsheet reads them as text. +> Numbers and the JSON output are unchanged; the sidecar's +> `csv_cells_escaped` says how many cells were changed. + +> **Behaviour change:** the canary now FAILS, naming the missing secret, when +> neither `WEBMOTORS_PROXY` nor `WEBMOTORS_CDP_ENDPOINT` is set. It used to +> skip and end green without contacting the site. + ### Fixed +- **A failed write destroyed the previous good output.** JSON, CSV, the + `.meta.json` sidecar and `diff_runs.py --out` were opened with truncation, + so a full disk or an interrupt mid-write left a short, invalid file where a + good one had been. Each file is now written to a temporary file in the same + directory, flushed, synced and renamed over the target; the temporary file + is removed on failure. A new file gets the mode `open()` would have given + it (`0o666` minus the umask); an existing file keeps its own mode. The + three files are replaced one by one, not as a single transaction. +- **CSV cells were not protected against formula injection.** Titles, + versions, attributes, optionals and dealer names and descriptions are + third-party text, and a cell starting with `=` ran as a formula when the + CSV was opened in a spreadsheet. Escaping runs after list columns are + joined. +- **A copied `.env.example` broke the README's flag examples.** It shipped an + active `WEBMOTORS_URL`, and a URL from `.env` was treated as a typed + `--url`, so `--make volkswagen --model gol` exited 2. The example line now + ships commented out, and a `WEBMOTORS_URL` from the environment is only a + default: typed search flags, `--ads-file` or a different `--mode` replace + it. A typed `--url` with filter flags is still refused. +- **The Scraping Browser 401 hint claimed credentials "last about a day".** + That figure is not a constant (an endpoint was seen working after ten + days). The hint, README, TROUBLESHOOTING and `.env.example` now say the + credentials were refused (expired, revoked or mistyped) without guessing a + lifetime. +- **The bug-report template's example command used flags this scraper does + not have.** It now shows a Volkswagen Gol search. +- **The banned-wording check read virtualenvs.** A `.venv` inside the repo, + as the README suggests, had its site-packages scanned. Directories holding + a `pyvenv.cfg` are now skipped. +- **Comments cited private notes by section number.** They now state the + reason in place, and a suite check fails if such a citation returns. + - **`--retries 0` fetched nothing and called it a result.** The load loop is `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 @@ -26,6 +74,13 @@ said so at the top of its release notes. one and calls the validator, so a new engine declaring the flag with a plain int fails the suite. +### Changed + +- **The canary covers ad mode** (the first three adverts of the car search) + and accepts a 2Captcha Scraping Browser API endpoint + (`WEBMOTORS_CDP_ENDPOINT`) as well as a residential proxy. Without either + secret it fails rather than skipping. + ## [0.1.1] — 2026-10-08 > **Exit codes change for a page the parser cannot read.** v0.1.0 treated a diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3c619f0..e2a6b6b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -90,12 +90,12 @@ history needs a decision, not a red check on every push. Then the rest of the presentation, in the order that matters: 1. `python3 smoke_test.py` green, and the canary dispatched at least once. - Without a `WEBMOTORS_PROXY` secret it SKIPS with a notice, because - CloudFront refuses the runner's datacentre address; dispatch it once to - see the skip branch run, and again with the secret set to see the real - one. -2. The repo description, homepage and topics set (see the family notes on - what those should say). + It needs a `WEBMOTORS_PROXY` (residential) or `WEBMOTORS_CDP_ENDPOINT` + secret, because CloudFront refuses the runner's datacentre address; + without either it FAILS and names the missing secret. +2. The repo description, homepage and topics set (what the scraper reads, + which engines and paid products it supports, and nothing it does not + use). 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 b97b1ea..d77aa69 100644 --- a/README.md +++ b/README.md @@ -182,8 +182,7 @@ One key, four separately-billed products: the top for the acceptance rates measured. - **The Scraping Browser API** (`--cdp-endpoint`) — a remote browser on a residential exit, nothing to install; served on every attempt measured. - One live connection per profile, so no `--concurrency` through it; a - profile's credentials last about a day. + One live connection per profile, so no `--concurrency` through it. - **The Scraper API** (`scraper_api_client.py`) — no local browser at all, but its own exits were refused, so on this site it works only with `--cdp-url` (a Scraping Browser session), measured at $0.0005 a page. @@ -227,12 +226,14 @@ captured on 2026-09-28 (`tools/capture.py`), cut and scrubbed by `make_fixtures.py`, which proves each one parses exactly as its original. CI runs the suite on Python 3.9 and 3.12, once per engine in its own virtualenv, builds the Docker image, and scans for committed credentials. -The canary runs a real search daily when a residential proxy secret is set, -and skips with a notice when it is not. **This repository has no such secret -set, so the canary badge above is green because it skipped:** it says the -workflow is intact, not that the site was scraped today. Each skipped run -says so at the top of its summary page. Measure the scraper yourself with -the commands under "Run". +The canary runs a car search, a motorcycle search and ad mode daily +through a residential exit: the `WEBMOTORS_PROXY` or +`WEBMOTORS_CDP_ENDPOINT` secret, because the site refuses the runner's +datacentre address. Without either secret the canary **fails** and names +the missing secret, rather than reporting green for a run that never +contacted the site. A red canary badge may therefore mean that no secret is +set; its summary page says which. Measure the scraper yourself with the +commands under "Run". ## Legal diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md index 082eaf6..dae10b2 100644 --- a/TROUBLESHOOTING.md +++ b/TROUBLESHOOTING.md @@ -17,7 +17,8 @@ sidecar's `stop_reason` in `.meta.json` if one was written. `estoque` for all of Brazil. * **"--url already carries the search"** — pass a URL or the filter flags, not both. Merging them could scrape something neither named. A - `WEBMOTORS_URL` in `.env` counts as a `--url`. + `WEBMOTORS_URL` in `.env` is only a default: typed filter flags or an + `--ads-file` replace it. * **"--condition is only offered for cars"** — the used/new listings were measured for cars only. @@ -61,9 +62,9 @@ falls in. Nothing is written, so an earlier good file is left alone; be refused again, so it is not retried. If the address looks right, the API changed: open a "Site changed" issue with `--dump-html` output. * **"Gave up on page N"** — a timeout or a dead proxy. The log names which. -* **`could not connect to --cdp-endpoint`** — a 401 is an expired Scraping - Browser profile (they last about a day); a 500 is a profile another run - still holds. +* **`could not connect to --cdp-endpoint`** — a 401 means the profile's + credentials were refused (expired, revoked or mistyped): copy the endpoint + again from the dashboard. A 500 is a profile another run still holds. ## `stop_reason: parser_found_nothing` (exit 5, or 6 after earlier pages) diff --git a/diff_runs.py b/diff_runs.py index a2673bb..d1c4e92 100644 --- a/diff_runs.py +++ b/diff_runs.py @@ -47,7 +47,7 @@ from dataclasses import fields from typing import Dict, List, Optional, Tuple -from output_writer import ROW_CLASS_BY_MODE, UNIQUE_BY_SKU_MODES +from output_writer import ROW_CLASS_BY_MODE, UNIQUE_BY_SKU_MODES, _atomic # Columns that describe the RUN, or that restate the key, rather than the # vehicle. `page` and `position` are here because a live listing reorders @@ -225,7 +225,9 @@ def main() -> int: result = diff_products(old, new) _print_summary(result) if args.out: - with open(args.out, "w", encoding="utf-8") as f: + # Atomically, like the scrapers' own output: a failed write must not + # leave a truncated diff where a previous one was. + with _atomic(args.out) as f: json.dump(result, f, ensure_ascii=False, indent=2) print(f"[+] Full diff written to {args.out}") if args.fail_on_change and (result["added"] or result["removed"] or result["changed"]): diff --git a/env_config.py b/env_config.py index ef17d6a..3d30a1e 100644 --- a/env_config.py +++ b/env_config.py @@ -66,8 +66,8 @@ # 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 -# prevent. Any `{…}` left in a value means the line was never filled in. +# a long way from its cause, which is precisely what reading .env strictly +# 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_]*\}") _loaded_from = None diff --git a/fingerprint_client.py b/fingerprint_client.py index e84ce1e..5444a7f 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -249,9 +249,9 @@ 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 +331,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, a common shape of defect in this code. # # `.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 ae2cace..42bac8a 100644 --- a/make_fixtures.py +++ b/make_fixtures.py @@ -95,7 +95,7 @@ "cloudfront_403": ("cloudfront_403.html", None, None, "text"), # A page fetched over --cdp-endpoint: the Scraping Browser's auto-solve # extension injects its hunter scripts into every page it loads, and the - # marker set must score zero against them (CLAUDE.md §24). + # marker set must score zero against them. "cdp_landing": ("cdp_landing.html", None, None, "text"), } diff --git a/output_writer.py b/output_writer.py index 0004b65..6e9c670 100644 --- a/output_writer.py +++ b/output_writer.py @@ -23,8 +23,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 @@ -237,12 +240,91 @@ def _csv_value(v: Any) -> Any: return v +@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. Reproduced here before this + existed: a good 994-byte `out.json` came back as 16 bytes of invalid + JSON after a failure inside `json.dump`. That destroys the previous good + run by the ATTEMPT to replace it, which is exactly what `save` refusing + to write an empty result exists to prevent. + + * 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=""`. + + Each file is replaced atomically on its own. JSON, CSV and the sidecar + are three replacements, not one transaction. + """ + 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()) + # `NamedTemporaryFile` creates at 0600 and `os.replace` keeps that + # mode. An existing target keeps its OWN mode (someone may have + # tightened it on purpose); a new one gets what `open()` would have + # given it, `0o666 & ~umask`. Never a flat 0o644, which narrows + # 0664 to 0644 under umask 002. + 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: + # Including KeyboardInterrupt: an interrupted run must not leave + # `out.json.ab12cd.tmp` behind 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 +# rather than as text, and every string in these rows was written by a third +# party: a listing's title and version, the seller's ticked attributes and +# optionals, a dealer's trading name and description. 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 `-5` into text + and break every sum over the column. CSV only: the JSON output keeps the site's bytes, and + `csv_cells_escaped` in the sidecar declares the divergence. + """ + if isinstance(v, str) and v.startswith(CSV_FORMULA_LEADS): + return CSV_FORMULA_ESCAPE + v, True + return v, False + + 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 = Listing) -> None: +def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Listing) -> 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 (no columns to parse) instead of reading a valid # table with zero rows — and "an empty result is still a well-formed @@ -251,11 +333,20 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Listing) -> None: # 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 serialiser: 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 @@ -330,7 +421,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 @@ -387,14 +478,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 = Listing) -> int: + allow_empty: bool = False, row_cls: Type = Listing, + 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. @@ -421,8 +513,16 @@ 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` because this function returns an EXIT CODE. + 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 @@ -479,8 +579,13 @@ 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, Listing) - rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls) + stats: dict = {} + rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls, + stats=stats) wrote_output = bool(rows) or allow_empty + # Housekeeping goes UNDER the caller's `extra`: a collision must never + # let a count this module made up drop a fact about the site. + extra = {**stats, **(extra or {})} if wrote_output: status = "complete" if (rows and complete) else ( diff --git a/page_flow.py b/page_flow.py index a545e64..0b2952f 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 one fetch loop all three engines share. +an if-chain, and the one fetch loop all three engines share. webmotors.com.br answers one of this repo's requests in seven ways, and they want five 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 hashlib @@ -56,8 +56,8 @@ # How long one fetch() may take before the engine gives up on it. The # largest response measured was a 100-row search page, ~410 KB, which # arrived in about a second through a residential exit. The bound exists -# because a browser fetch() has no timeout of its own, and CLAUDE.md §8 -# requires every remote call to have one. +# because a browser fetch() has no timeout of its own, and every remote +# call here has to have one. FETCH_TIMEOUT_MS = 30_000 # How long to wait at the SAME exit after a 429 before trying again. NOT @@ -79,7 +79,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. """ @@ -108,7 +108,7 @@ def classify(html: Optional[str], status: Optional[int] = None, "blocked": {"retry": True, "solve": False, "blocked": True, "parse": False}, # Rate limited: the retry happens at the same exit after a wait # (THROTTLE_*), and it is NOT counted as blocked. Calling a throttle a - # block reports exit 3 for a page that was about to come back (§24). + # block reports exit 3 for a page that was about to come back. "throttled": {"retry": True, "solve": False, "blocked": False, "parse": False}, # Not JSON and not a refusal. Worth one more try. "unknown": {"retry": True, "solve": False, "blocked": False, "parse": False}, @@ -132,7 +132,7 @@ def should_parse(state: str) -> bool: # Whether a blocked page is worth re-fetching at all. CONSULTED by the loop -# below, so setting it False really does stop the retry (§17). +# below, so setting it False really does stop the retry. RETRY_ON_BLOCKED = True # How many times to re-fetch a refused page, each from a FRESH browser, when @@ -168,7 +168,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 @@ -236,7 +236,7 @@ def build_query(args, error: Callable[[str], None]) -> Query: --url and the filter flags are two ways to say the same thing. A flag the user typed that disagrees with the URL would silently scrape something neither of them named, so the combination is refused rather - than merged (the family's --country rule, §10). `error` is argparse's + than merged (the family's --country rule). `error` is argparse's `p.error`, so a refusal is exit 2 with the usage line, as in every engine. """ @@ -245,6 +245,19 @@ def build_query(args, error: Callable[[str], None]) -> Query: typed = [flag for dest, flag in FILTER_FLAGS if getattr(args, dest, None) is not None] ads_file = getattr(args, "ads_file", None) + typed_url = getattr(args, "typed_url", args.url) + if args.url and not typed_url: + # The URL came from WEBMOTORS_URL, not from the command line. It is a + # default for a run given nothing else, so search filters, an + # --ads-file or a --mode the user typed win instead of being refused + # as a conflict with a value they never typed. + env_query, _ = query_from_url(args.url, sort=sort, per_page=per_page) + mode_differs = bool(args.mode and env_query is not None + and env_query.mode != args.mode) + if typed or ads_file or mode_differs: + log.info("Ignoring WEBMOTORS_URL from the environment: the " + "search was given on the command line.") + args.url = None if ads_file: if args.url or typed: error("--ads-file is a list of adverts; it cannot be combined " @@ -319,7 +332,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). + dedupe and the sidecar cannot differ between them. """ rows, seen = [], set() for oc in sorted(outcomes, key=lambda o: o.page_num): @@ -386,7 +399,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, # origin, retrying a transport failure, waiting out a throttle, rotating on a # refusal, 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): +# and no JavaScript crosses this boundary: # # ops.goto(url) -> (status, waf_header). Raises TransportError. # ops.document_text() -> the landing document, JSON text if it is JSON @@ -397,7 +410,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, # ops.proxy_failure(text) -> the driver's proxy-error name in text, or "" # # One copy of the loop is how the family rule that the three engines agree -# on exit codes and on whether a run spends money (§6) holds by +# on exit codes and on whether a run spends money holds by # construction rather than by discipline. @@ -411,7 +424,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 @@ -496,7 +509,7 @@ def _core_field_warnings(rows: List, mode: str, page_num: int) -> List[str]: def _dump(args, page_num: int, text: str) -> 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 args.pages == 1 else f"{args.dump_html}.page{page_num}" @@ -559,7 +572,7 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, ops.landed = False 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 (HTTP 429) — waiting " "%.0fs at the same exit (%d/%d).", page_num, pause, @@ -739,7 +752,7 @@ def run_pages(open_ops, close_ops, run_concurrently, args, pool, ops = open_ops() try: # Page 1 is always fetched alone: its total decides how many pages - # there are to address (§7), and its echo decides whether the site + # there are to address, and its echo decides whether the site # searched for what was asked at all. first = fetch_one_page(ops, args, pool, query, 1, mask) if first.filter_problem: @@ -876,7 +889,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 @@ -896,9 +909,9 @@ def ua_override(chromium_version: str, locale: str) -> dict: which no real Chrome 153 reports. Measured 2026-09-28 against an echo service, then on the site: PerimeterX refused pyppeteer that way 2 of 2, while Playwright (whose override keeps the hints) was served on the same - exit. Half an identity is refused; a whole one is not (§24). + exit. Half an identity is refused; a whole one is not. - The version is the browser's OWN (§8), the platform agrees with the + The version is the browser's OWN, the platform agrees with the Windows UA string, and the brand list is the one Chromium itself sends, minus the `HeadlessChrome` token. """ @@ -931,15 +944,16 @@ def ua_override(chromium_version: str, locale: str) -> dict: def cdp_connect_hint(error_text: str) -> str: """What a failed --cdp-endpoint connection means, from its status. - Two answers that want opposite fixes. A Scraping Browser profile's - credentials last about a day, so a 401 is usually an endpoint copied from - an older .env; a 500 is usually a pid another run still holds. + Two answers that want opposite fixes. A 401 is the service refusing the + credentials in the endpoint: expired, mistyped or revoked. How long a + profile's credentials live is not a constant (one endpoint was measured + still working after ten days), so the message does not guess which. A + 500 is usually a pid another run still holds. """ 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.") + return ("HTTP 401: the endpoint's credentials were refused, for " + "example an expired, revoked or mistyped profile login. " + "Copy the endpoint again 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.") @@ -961,7 +975,7 @@ def cdp_connect_hint(error_text: str) -> str: def cdp_should_retry(error_text: str) -> bool: """Whether a failed --cdp-endpoint connection is worth another attempt: a locked profile (500) or a connect that never answered. A 401 is not: - expired credentials stay expired.""" + refused credentials stay refused.""" text = error_text or "" if "401" in text: return False @@ -972,7 +986,7 @@ def cdp_should_retry(error_text: str) -> bool: # --------------------------------------------------------------------------- # --retries # --------------------------------------------------------------------------- -import argparse as _argparse # noqa: E402 (kept beside its only user) +import argparse as _argparse # noqa: E402 (kept beside its users) def attempts(value) -> int: @@ -1012,3 +1026,27 @@ 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 at_least(flag: str, minimum, kind=int): + """argparse `type=` for a count or a delay: rejects values below `minimum`. + + A negative delay reached `time.sleep` (or a negative wait in the + browser) and crashed or was silently treated as zero, and a negative + retry or concurrency count was quietly read as "none" or "one". Each is + a typo the run should name, not absorb, so it is a usage error (exit 2) + naming the flag. + """ + def parse(value): + try: + number = kind(value) + except (TypeError, ValueError): + raise _argparse.ArgumentTypeError( + f"{flag} must be {'an integer' if kind is int else 'a number'}" + f", not {value!r}") + if number < minimum: + raise _argparse.ArgumentTypeError( + f"{flag} must be at least {minimum}, not {value}") + return number + parse.__name__ = flag.lstrip("-").replace("-", "_") + return parse diff --git a/playwright_scraper.py b/playwright_scraper.py index 9c710f1..4a05cef 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -77,7 +77,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", @@ -92,12 +92,12 @@ # The fetch() every page 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). No headers are set: the site's own front end sends none that the +#. No headers are set: the site's own front end sends none that the # endpoints need, and the one HTML page fetched per run (for its currency, # page_flow.read_currency) must be asked for as a browser asks for a page. # # 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]) => { const ctl = new AbortController(); @@ -125,7 +125,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. LOAD-BEARING on this site, not cosmetic. Headless Chromium's default UA carries the token `HeadlessChrome`, and PerimeterX refused that 0 of 3 @@ -151,7 +151,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). + are a stronger signal than either address alone. """ launch_kwargs = {"headless": args.headless} proxy = to_playwright(pool.current) if pool else None @@ -292,7 +292,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 " @@ -321,7 +321,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) @@ -336,7 +336,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() @@ -442,20 +442,20 @@ def parse_args(argv=None): g.add_argument("--ads-file", default=None, metavar="PATH", help="Adverts to fetch: a search run's JSON output (its " "`url` column) or one advert address per line.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.at_least("--pages", 1), default=1, help="Search pages to fetch. Planned against the page count " "the site states on page 1, so asking for more than " "exist fetches all of them. Ignored in ad mode.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=page_flow.at_least("--delay", 0, float), 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.at_least("--concurrency", 1), 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.") p.add_argument("--retries", type=page_flow.attempts, default=3, help="Attempts per page on a transport failure (default 3). " "The pause doubles each time.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.at_least("--retry-delay", 0, float), 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="webmotors_rows", help="Output file prefix") @@ -474,7 +474,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.at_least("--proxy-block-retries", 0), default=2, help="When a page is refused (CloudFront, PerimeterX), " "retry it from this many OTHER exits (default 2). " "Needs a pool of more than one.") @@ -518,6 +518,9 @@ def parse_args(argv=None): p.add_argument("--headless", action="store_true", default=True) p.add_argument("--headful", dest="headless", action="store_false") args = p.parse_args(argv) + # Before .env fills it: a URL from the environment is a default, and + # must not refuse a search filter or --ads-file the user typed. + args.typed_url = args.url env_config.apply(args) args.query = page_flow.build_query(args, p.error) args.mode = args.query.mode diff --git a/product_parser.py b/product_parser.py index fa36317..0a32fc9 100644 --- a/product_parser.py +++ b/product_parser.py @@ -1,7 +1,7 @@ """ product_parser.py ----------------- -Everything this repo knows about webmotors.com.br lives here (CLAUDE.md §1). +Everything this repo knows about webmotors.com.br lives here. Two modes, four of the site's own JSON endpoints ------------------------------------------------ @@ -41,7 +41,7 @@ The same answers hold for the JSON endpoints: the refusal is about the CLIENT and the ADDRESS, never about the route, so the endpoints are not an -ungated side door (§21). They are simply the cheapest thing a served browser +ungated side door. They are simply the cheapest thing a served browser can ask for. A browser lands on a small endpoint (ORIGIN_URL), which gives it a www.webmotors.com.br origin, and issues every page as a same-origin `fetch()`. Nothing is rendered, so a 3 MB listing page and its third-party @@ -238,7 +238,7 @@ class Query: # The currency the site states, read ONCE per run (page_flow) from the # JSON-LD of a page the site renders. No endpoint states one, and a # constant compiled in here would be a guess wearing a fact's clothes - # (§4). None until read, and None for good if it could not be. + #. None until read, and None for good if it could not be. currency: Optional[str] = None def validate(self) -> Optional[str]: @@ -548,10 +548,10 @@ def ads_from_text(text: str) -> List[str]: # "Access to this page has been denied" 1 0 # `px-captcha` (42 on the refusal) is deliberately NOT a marker: it is the # id of the element the challenge renders into, and a bare element id is the -# shape §24 warns about, a thing another script could carry. The bare words +# weak kind of marker, a thing another script could carry. The bare words # "captcha" and "perimeterx" are not markers either: the Scraping Browser's # auto-solve extension injects hunter scripts that say "captcha" on every -# page it loads (§24); smoke_test scores this set against a page fetched +# page it loads; smoke_test scores this set against a page fetched # that way. PERIMETERX_MARKERS = ("_pxAppId", "captcha.px-cloud.net", "/captcha/captcha.js", "Access to this page has been denied") @@ -566,7 +566,7 @@ def detect_bot_challenge(html: Optional[str], url: str = "") -> Optional[str]: Entities are unescaped over a bounded prefix first, so a marker matches the raw bytes an HTTP client gets and the DOM a browser serialises - alike (§20). Both refusal pages are under 12 KB. + alike. Both refusal pages are under 12 KB. """ head = html_lib.unescape((html or "")[:60_000]) if any(m in head for m in PERIMETERX_MARKERS): @@ -616,7 +616,7 @@ def detect_page_state(text: Optional[str], status: Optional[int] = None, because a 429 means the same thing wherever it appears unknown anything else - Signals are ordered by what they PROVE, not by what they cost (§17): the + Signals are ordered by what they PROVE, not by what they cost: the site's own JSON envelope first, because no refusal page carries it. `waf_action` is accepted for the family's signature and ignored: this site has no AWS WAF. @@ -658,7 +658,7 @@ def is_sponsored(rec: Dict[str, Any]) -> bool: `mediaZeroKm=true`, each with `UniqueId: 0`, `MediaZeroKm: true`, no `Seller` and an `AdvertisementLink` to a dealer's lead form. They are not listings of this search, they have no id to key on, and counting them - would shift every position after them (§24). + would shift every position after them. Only a POSITIVE advertising marker makes a record sponsored. v0.1.0 also called a record sponsored when it lacked a usable `UniqueId` or a @@ -756,7 +756,7 @@ def filter_mismatch(payload: Any, query: Query) -> Optional[str]: re.S | re.I) # ISO 4217 codes a Brazilian marketplace could plausibly state. An # allowlist, never a bare [A-Z]{3}, so nothing that merely looks like a code -# is read as one (§4). +# is read as one. _ISO_CODES = ("BRL", "USD", "EUR", "ARS", "UYU", "PYG", "CLP") @@ -767,7 +767,7 @@ def currency_from_html(html: Optional[str]) -> Optional[str]: OfferCatalog (48 of 48 on the page read), an advert page once. No endpoint states a currency at all, so this is read once per run and threaded into the rows (page_flow), and a run that could not read it - writes null rather than a default (§4). + writes null rather than a default. """ found = [] for m in _JSONLD_RE.finditer(html or ""): @@ -1038,7 +1038,7 @@ def parse_page(payload: Any, query: Query, page: int = 1) -> List[Any]: """Every row on one page of one mode's response, in the site's order. `position` counts the rows EMITTED, not the slots in the payload, so a - sponsored tile the parser drops cannot shift every later position (§24). + sponsored tile the parser drops cannot shift every later position. """ if query.mode == "ad": row = parse_ad(payload, query, page) diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index a4fa34b..421bb9c 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -51,7 +51,7 @@ # 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). +#. from pyppeteer import launch, connect from product_parser import (CONDITION_SUFFIX, DEFAULT_PER_PAGE, DEFAULT_SORT, @@ -67,11 +67,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", @@ -85,7 +85,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), and no headers, for the +# return shape, same AbortController timeout, and no headers, for the # reason given there. FETCH_JS = """ async (url, method, body, timeoutMs) => { @@ -177,7 +177,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) @@ -199,7 +199,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): @@ -265,7 +265,7 @@ def open(self): exit_url = self.pool.current # Credentials are answered over CDP (_authenticate_proxy), never # put on the command line: --proxy-server= is part of the - # browser's argv, readable by anything that can run `ps` (§8). + # browser's argv, 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)) @@ -313,7 +313,7 @@ def _authenticate_proxy(self, username: str, password: str): Every request is paused and continued unchanged; only an auth challenge FROM THE PROXY is answered with its credentials, which - never leave this process's memory (§8). A site's own challenge gets + never leave this process's memory. A site's own challenge gets the browser's default answer. """ async def _enable(): @@ -559,20 +559,20 @@ def parse_args(argv=None): g.add_argument("--ads-file", default=None, metavar="PATH", help="Adverts to fetch: a search run's JSON output (its " "`url` column) or one advert address per line.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.at_least("--pages", 1), default=1, help="Search pages to fetch. Planned against the page count " "the site states on page 1, so asking for more than " "exist fetches all of them. Ignored in ad mode.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=page_flow.at_least("--delay", 0, float), 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.at_least("--concurrency", 1), 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.") p.add_argument("--retries", type=page_flow.attempts, default=3, help="Attempts per page on a transport failure (default 3). " "The pause doubles each time.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.at_least("--retry-delay", 0, float), 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="webmotors_rows", help="Output file prefix") @@ -591,7 +591,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.at_least("--proxy-block-retries", 0), default=2, help="When a page is refused (CloudFront, PerimeterX), " "retry it from this many OTHER exits (default 2). " "Needs a pool of more than one.") @@ -643,6 +643,9 @@ def parse_args(argv=None): p.add_argument("--headless", action="store_true", default=True) p.add_argument("--headful", dest="headless", action="store_false") args = p.parse_args(argv) + # Before .env fills it: a URL from the environment is a default, and + # must not refuse a search filter or --ads-file the user typed. + args.typed_url = args.url env_config.apply(args) args.query = page_flow.build_query(args, p.error) args.mode = args.query.mode diff --git a/scraper_api_client.py b/scraper_api_client.py index a0d121a..6192a63 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -60,6 +60,7 @@ request_for, total_results) from output_writer import dedupe_by_key, finish_run, SOURCE_DEFAULT import env_config +from page_flow import at_least logging.basicConfig(level=logging.INFO, format="%(asctime)s [%(levelname)s] %(message)s") logger = logging.getLogger("scraper_api_client") @@ -407,9 +408,9 @@ def parse_args(): help="The site's own orderings (default %s)." % DEFAULT_SORT) p.add_argument("--per-page", type=int, default=None, metavar="N", help="Listings per page (default %d)." % DEFAULT_PER_PAGE) - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=at_least("--pages", 1), default=1, help="Search pages to fetch (default 1). Each is a billable task.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=at_least("--delay", 0, float), 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="webmotors_rows_scraperapi", help="Output file prefix") @@ -427,14 +428,17 @@ 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=at_least("--retries", 0), default=1, help="Extra attempts if a refusal 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=at_least("--retry-delay", 0), default=10, help="Seconds between retries (default 10)") p.add_argument("--dump-html", default=None, help="Also write the response (JSON) to this path, even on success") args = p.parse_args() + # Before .env fills it: a WEBMOTORS_URL is a default, and an --ads-file + # typed on the command line replaces it rather than conflicting with it. + typed_url = args.url # This client uses --key and --cdp-url rather than --twocaptcha-key and # --cdp-endpoint, so the env mapping is spelled out instead of defaulted. env_config.apply(args, keys={ @@ -442,6 +446,8 @@ def parse_args(): "WEBMOTORS_CDP_ENDPOINT": "cdp_url", "WEBMOTORS_URL": "url", }) + if args.url and not typed_url and args.ads_file: + args.url = None if not args.url and not args.ads_file: p.error("--url (a search page or an advert) or --ads-file is required") if args.url and args.ads_file: diff --git a/selenium_scraper.py b/selenium_scraper.py index 77130b7..183de57 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -67,7 +67,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", @@ -78,7 +78,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), and +# Same request, same return shape, same AbortController timeout, and # no headers, for the reason given there. FETCH_JS = """ var done = arguments[arguments.length - 1]; @@ -104,7 +104,7 @@ BODY_TEXT_JS = "return document.body ? document.body.innerText : '';" -# 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) @@ -143,7 +143,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): @@ -160,7 +160,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 @@ -200,7 +200,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) @@ -385,20 +385,20 @@ def parse_args(argv=None): g.add_argument("--ads-file", default=None, metavar="PATH", help="Adverts to fetch: a search run's JSON output (its " "`url` column) or one advert address per line.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.at_least("--pages", 1), default=1, help="Search pages to fetch. Planned against the page count " "the site states on page 1, so asking for more than " "exist fetches all of them. Ignored in ad mode.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=page_flow.at_least("--delay", 0, float), 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.at_least("--concurrency", 1), 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.") p.add_argument("--retries", type=page_flow.attempts, default=3, help="Attempts per page on a transport failure (default 3). " "The pause doubles each time.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.at_least("--retry-delay", 0, float), 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="webmotors_rows", help="Output file prefix") @@ -417,7 +417,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.at_least("--proxy-block-retries", 0), default=2, help="When a page is refused (CloudFront, PerimeterX), " "retry it from this many OTHER exits (default 2). " "Needs a pool of more than one.") @@ -461,6 +461,9 @@ def parse_args(argv=None): p.add_argument("--headless", action="store_true", default=True) p.add_argument("--headful", dest="headless", action="store_false") args = p.parse_args(argv) + # Before .env fills it: a URL from the environment is a default, and + # must not refuse a search filter or --ads-file the user typed. + args.typed_url = args.url env_config.apply(args) args.query = page_flow.build_query(args, p.error) args.mode = args.query.mode diff --git a/smoke_test.py b/smoke_test.py index 1f478ff..198c7f4 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -152,7 +152,7 @@ def check_fixture_corpus_is_real_and_scrubbed(): # --------------------------------------------------------------------------- -# Parsing: VALUES on real fixtures, not coverage (§10) +# Parsing: VALUES on real fixtures, not coverage # --------------------------------------------------------------------------- def check_a_search_page_parses_to_the_captured_values(): @@ -237,9 +237,9 @@ def check_sponsored_tiles_are_skipped_and_do_not_shift_positions(): P.sponsored_count(payload), 3) rows = P.parse_page(payload, _q("https://www.webmotors.com.br/carros/estoque"), 1) equal("only the listing becomes a row", [r.sku for r in rows], ["80269940"]) - equal("...at position 1, not 4 (§24)", rows[0].position, 1) + equal("...at position 1, not 4", rows[0].position, 1) # The FILTER is what is tested, not id recovery: a tile with a real-looking - # id but MediaZeroKm set must still be skipped (§24's weak-check lesson). + # id but MediaZeroKm set must still be skipped (a check on one field alone is weak). tile = copy.deepcopy(json.loads(payload)) tile["SearchResults"][0]["UniqueId"] = 12345 tile["SearchResults"][0]["Seller"] = {"SellerType": "PJ"} @@ -408,7 +408,7 @@ def check_page_states_on_real_captures(): def check_markers_do_not_match_a_page_the_scraping_browser_served(): - """§24: the Scraping Browser's auto-solve extension injects captcha + """The Scraping Browser's auto-solve extension injects captcha hunters into EVERY page. The marker set must score zero against a page fetched that way, WITHOUT any strip.""" import product_parser as P @@ -437,7 +437,7 @@ def check_state_policy(): all(F.should_retry(s) and F.counts_as_blocked(s) for s in ("challenge", "blocked"))) check("nothing is ever solved: this repo implements no solve for this site", not any(F.should_solve(s) for s in F.STATE_POLICY)) - check("throttled: retried, NOT blocked (§24)", + check("throttled: retried, NOT blocked", F.should_retry("throttled") and not F.counts_as_blocked("throttled")) check("the advice for PerimeterX names what was measured", "HeadlessChrome" in F.refusal_advice("challenge")) @@ -446,8 +446,14 @@ def check_state_policy(): and "cannot" not in F.refusal_advice("challenge").lower()) check("the advice for CloudFront names the ADDRESS", "ADDRESS" in F.refusal_advice("blocked", fx("cloudfront_403"))) - check("a CDP 401 is explained as expired credentials, not a held pid", - "expired" in F.cdp_connect_hint("WebSocket error: 401 Unauthorized")) + # Reworded: the hint used to promise that credentials "last about a + # day", which a ten-day-old working endpoint disproved. It now says the + # credentials were refused without guessing why. + hint = F.cdp_connect_hint("WebSocket error: 401 Unauthorized") + check("a CDP 401 is explained as refused credentials, not a held pid", + "refused" in hint and "pid" not in hint) + check("...without claiming how long credentials last", + "a day" not in hint) def check_the_identity_override_is_complete(): @@ -474,7 +480,7 @@ def check_the_identity_override_is_complete(): 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() engines = "".join(open(os.path.join(HERE, m + ".py"), encoding="utf-8").read() for m in ENGINES) @@ -621,7 +627,7 @@ def check_the_shared_loop_end_to_end(): rc, meta, rows, ops = _run({("search", 1): [(429, ""), (200, fx("search_gol_p1"))]}, pages=1, retries=1) - equal("a throttle wait spends its OWN budget, not --retries (§24): " + equal("a throttle wait spends its OWN budget, not --retries: " "with --retries 1 the page still arrives", rc, 0) equal("...at the SAME exit (no relaunch)", ops.relaunches, 0) @@ -759,7 +765,7 @@ def check_an_ad_runs_identity_is_its_set_of_adverts(): def check_every_engine_implements_the_operations_page_flow_uses(): """The fetch loop is shared, so an engine missing ONE operation fails only when a live run reaches it. The set is DERIVED from page_flow's own - source (every `ops.`), not listed by hand (§26).""" + source (every `ops.`), not listed by hand.""" tree = ast.parse(open(os.path.join(HERE, "page_flow.py"), encoding="utf-8").read()) used = {n.attr for n in ast.walk(tree) if isinstance(n, ast.Attribute) and isinstance(n.value, ast.Name) @@ -794,7 +800,7 @@ def check_row_schema(): from output_writer import Ad, Listing, Product, ROW_CLASS_BY_MODE, UNIQUE_BY_SKU_MODES for cls in (Listing, Ad): 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[:7], ["source", "scraped_at", "url", "sku", "title", "price", "currency"]) check("%s: the run-describing tail is present" % cls.__name__, {"page", "position", "mode", "data_source"} <= set(names)) @@ -841,7 +847,7 @@ def check_exit_codes(): 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) @@ -909,7 +915,7 @@ def check_sidecar_shape(): # --------------------------------------------------------------------------- -# The engines — the checks CLAUDE.md §17 says to steal +# The engines: checks worth copying to any sibling # --------------------------------------------------------------------------- def check_engines_import_their_driver_at_module_level(): @@ -926,10 +932,10 @@ 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, page_flow and the fixture tools) into a shared module is bound against the callee's real signature. A name that does not exist FAILS - (§22). A name bound in the calling file shadows a same-named module.""" + . A name bound in the calling file shadows a same-named module.""" import output_writer import page_flow import product_parser @@ -1004,8 +1010,8 @@ 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 (including the five it once omitted for +# months), plus this repo's own query flags. CONTRACT_FLAGS = { "--url", "--pages", "--category", "--format", "--out", "--delay", "--retries", "--retry-delay", "--concurrency", "--proxy", "--proxy-file", @@ -1019,7 +1025,7 @@ def _argparse_flags(module_name): def check_engine_flag_sets(): - """§17's check #2: against the contract AND against each other, both + """Flag sets: 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(): @@ -1093,7 +1099,7 @@ def _py_files(): 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 path in _py_files(): @@ -1176,10 +1182,15 @@ def check_dockerfile_copies_everything_the_entrypoint_imports(): def check_env_example_documents_exactly_what_the_loader_reads(): import env_config text = open(os.path.join(HERE, ".env.example"), encoding="utf-8").read() - documented = set(re.findall(r"^([A-Z][A-Z0-9_]+)=", text, re.M)) + # A variable may be documented as a commented-out example line. + documented = set(re.findall(r"^(?:# )?([A-Z][A-Z0-9_]+)=", text, re.M)) read = set(env_config.ENV_KEYS) equal("the example and the loader name the same variables", sorted(documented), sorted(read)) + # An active WEBMOTORS_URL in a copied .env picked a search for every run + # and made the README's own flag examples exit 2. + check("the example ships WEBMOTORS_URL commented out, not active", + not re.search(r"^WEBMOTORS_URL=", text, re.M)) check("the per-site variables carry the WEBMOTORS_ prefix", {"WEBMOTORS_CDP_ENDPOINT", "WEBMOTORS_PROXY", "WEBMOTORS_URL"} <= read) @@ -1207,7 +1218,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)) @@ -1222,7 +1233,7 @@ def check_credential_scan_is_one_implementation_invoked_from_both(): def check_no_workflow_imports_the_code_inline(): """A workflow calls ci_checks.py or the CLIs; it does not carry its own - copy of a check that imports the code (§26).""" + copy of a check that imports the code.""" wf_dir = os.path.join(HERE, ".github", "workflows") if not os.path.isdir(wf_dir): skip("workflows", "no .github directory (the image)") @@ -1235,7 +1246,7 @@ def check_no_workflow_imports_the_code_inline(): # 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 +# 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", @@ -1244,11 +1255,15 @@ def check_no_workflow_imports_the_code_inline(): 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", - "live", "captures", ".claude")] + # A virtualenv is someone else's text (site-packages), recognised by + # its pyvenv.cfg rather than by a name like ".venv". + dirs[:] = [d for d in dirs + if d not in (".git", "__pycache__", ".pytest_cache", + "live", "captures", ".claude") + and not os.path.exists(os.path.join(root, d, "pyvenv.cfg"))] for filename in files: if not filename.endswith((".py", ".md", ".yml", ".yaml", ".txt", ".toml", ".html", ".example", ".json")): @@ -1263,8 +1278,45 @@ def check_banned_wording(): check("the banned-wording scan read the repo (%d files)" % scanned, scanned > 20) +# References to private notes this public repo cannot link to: a reader +# can open none of them, so a rule is stated in this repo's own words instead. +# Assembled from pieces so this file is scanned like every other. +PRIVATE_CITATIONS = ( + "CLAUDE" + ".md", "claude_" + "2scraper", "2scraper_" + "repo_rules", + "play" + "book", "template " + "\u00a7", +) +SECTION_SIGN_REF = re.compile("\u00a7" + r"\s*\d") + + +def check_no_private_citations_in_shipped_files(): + scanned = 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") + and not os.path.exists(os.path.join(root, d, "pyvenv.cfg"))] + for filename in files: + if not filename.endswith((".py", ".md", ".yml", ".yaml", ".txt", + ".toml", ".cfg", ".example", ".json", + ".gitignore", ".dockerignore")) \ + and filename != "Dockerfile": + continue + path = os.path.join(root, filename) + text = open(path, encoding="utf-8", errors="replace").read() + scanned += 1 + rel = os.path.relpath(path, HERE) + for token in PRIVATE_CITATIONS: + if token.lower() in text.lower(): + check("%s cites no private note (%r)" % (rel, token), False) + m = SECTION_SIGN_REF.search(text) + if m: + line = text.count("\n", 0, m.start()) + 1 + check("%s:%d carries no bare section-number reference" % (rel, line), False) + check("the private-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 @@ -1375,7 +1427,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()) @@ -1477,7 +1529,7 @@ def check_x_debug_header_is_redacted(): def check_captcha_capability_claims_are_honest(): - """§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() low = readme.lower() for phrase in ("cannot be solved", "can't be solved", "is not solvable", @@ -1491,7 +1543,7 @@ def check_captcha_capability_claims_are_honest(): 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()} @@ -1578,6 +1630,232 @@ def check_retries_is_validated_in_every_entry_point(): check("--retries is validated in every entry point, so 0 is a usage error", not problems, "; ".join(problems)) +def _one_listing(**kw): + from output_writer import Listing + base = dict(url="https://www.webmotors.com.br/comprar/x/1", sku="1", + title="VOLKSWAGEN GOL", price=50000.0, currency="BRL", + page=1, position=1, mode="search") + base.update(kw) + return Listing(**base) + + +def check_output_writes_are_atomic_and_keep_modes(): + """A failed write must leave the previous good file intact, with no + temporary file beside it; a new file gets `0o666 & ~umask` and an + existing file keeps its own mode.""" + import output_writer as O + rows = [_one_listing()] + with tempfile.TemporaryDirectory() as tmp: + prefix = os.path.join(tmp, "out") + O.write_json(rows, prefix + ".json") + O.write_csv(rows, prefix + ".csv") + O.write_run_meta(prefix, {"status": "complete"}) + before = {ext: open(prefix + ext, encoding="utf-8").read() + for ext in (".json", ".csv", ".meta.json")} + + real_dump, real_writerow = O.json.dump, O.csv.DictWriter.writerow + + def half_dump(obj, f, **kw): + f.write('[{"sku": "half') + raise OSError(28, "No space left on device") + + def half_row(self, row): + self.writer.writerow(["half"]) + raise OSError(28, "No space left on device") + + O.json.dump, O.csv.DictWriter.writerow = half_dump, half_row + try: + for ext, call in ((".json", lambda: O.write_json(rows, prefix + ".json")), + (".csv", lambda: O.write_csv(rows, prefix + ".csv")), + (".meta.json", lambda: O.write_run_meta(prefix, {"status": "x"}))): + try: + call() + raised = False + except OSError: + raised = True + check("a failed %s write raises" % ext, raised) + equal("a failed %s write leaves the previous file intact" % ext, + open(prefix + ext, encoding="utf-8").read(), before[ext]) + finally: + O.json.dump, O.csv.DictWriter.writerow = real_dump, real_writerow + equal("no temporary file is left behind", + sorted(os.listdir(tmp)), ["out.csv", "out.json", "out.meta.json"]) + + 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) + mode = os.stat(path).st_mode & 0o777 + finally: + os.umask(old) + equal("a new file under umask %03o is %03o" % (umask, 0o666 & ~umask), + oct(mode), oct(0o666 & ~umask)) + os.chmod(prefix + ".csv", 0o640) + O.write_csv(rows, prefix + ".csv") + equal("an existing file keeps its own mode", + oct(os.stat(prefix + ".csv").st_mode & 0o777), oct(0o640)) + + +def check_csv_cells_are_formula_escaped(): + import output_writer as O + rows = [_one_listing(title='=HYPERLINK("http://x","y")', city="+SP", + seller_name="@me", attributes=["-cmd", "ok"], + version="\tTAB", odometer_km=-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="search", extra={"total_results": 7}) + equal("the run still exits 0", rc, 0) + cells = list(csv.DictReader(open(prefix + ".csv", encoding="utf-8")))[0] + equal("a formula title is prefixed", cells["title"], "'" + rows[0].title) + equal("a '+' cell is prefixed", cells["city"], "'+SP") + equal("an '@' cell is prefixed", cells["seller_name"], "'@me") + equal("a joined list is escaped AFTER joining", cells["attributes"], "'-cmd | ok") + equal("a tab-led cell is prefixed", cells["version"], "'\tTAB") + equal("a negative NUMBER is left alone", cells["odometer_km"], "-5") + data = json.load(open(prefix + ".json", encoding="utf-8"))[0] + equal("JSON keeps the site's bytes", data["title"], rows[0].title) + equal("JSON keeps a negative number", data["odometer_km"], -5) + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("the sidecar counts the escaped cells", meta.get("csv_cells_escaped"), 5) + equal("the caller's extra is still merged", meta.get("total_results"), 7) + O.finish_run(rows, prefix, "csv", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", + mode="search", extra={"csv_cells_escaped": "caller"}) + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("housekeeping goes UNDER the caller's extra", + meta.get("csv_cells_escaped"), "caller") + + +def _query_args(**kw): + base = dict(url=None, typed_url=None, sort=None, per_page=None, + category=None, condition=None, state=None, make=None, + model=None, ads_file=None, mode=None, pages=1) + base.update(kw) + return types.SimpleNamespace(**base) + + +class _UsageError(Exception): + pass + + +def _raise_usage(message): + raise _UsageError(message) + + +def check_an_env_url_gives_way_to_typed_flags(): + """WEBMOTORS_URL is a default. Search flags typed on the command line + replace it; a TYPED --url with flags is still refused.""" + import page_flow + env_url = "https://www.webmotors.com.br/carros/estoque/toyota" + try: + q = page_flow.build_query(_query_args(url=env_url, make="volkswagen", + model="gol"), _raise_usage) + got = q.listing_url + except _UsageError as e: + got = "refused: %s" % e + check("an environment URL gives way to typed --make/--model", + "volkswagen/gol" in got, got) + try: + q = page_flow.build_query(_query_args(url=env_url), _raise_usage) + got = q.listing_url + except _UsageError as e: + got = "refused: %s" % e + check("an environment URL alone is still the default search", + "toyota" in got, got) + try: + page_flow.build_query(_query_args(url=env_url, typed_url=env_url, + make="volkswagen"), _raise_usage) + refused = False + except _UsageError: + refused = True + check("a TYPED --url plus a filter flag is still refused", refused) + + +def _flag_types(module_name): + tree = ast.parse(open(os.path.join(HERE, module_name + ".py"), encoding="utf-8").read()) + out = {} + 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)): + kind = next((ast.unparse(k.value) for k in node.keywords if k.arg == "type"), None) + out[node.args[0].value] = kind + return out + + +def check_counts_and_delays_reject_negative_values(): + """A negative delay crashed `time.sleep` or was read as zero, and a + negative count was read as 'none'. Each is now a usage error naming the + flag, in every entry point (read off disk, so it runs without drivers).""" + import page_flow + want = {"--delay": 0, "--retry-delay": 0, "--concurrency": 1, + "--proxy-block-retries": 0, "--pages": 1} + for module in ENGINES: + types_ = _flag_types(module) + for flag, minimum in want.items(): + check("%s %s is validated (at least %d)" % (module, flag, minimum), + (types_.get(flag) or "").startswith( + "page_flow.at_least('%s', %d" % (flag, minimum)), + types_.get(flag)) + api = _flag_types("scraper_api_client") + for flag, minimum in {"--delay": 0, "--retry-delay": 0, "--retries": 0, + "--pages": 1}.items(): + check("scraper_api_client %s is validated (at least %d)" % (flag, minimum), + (api.get(flag) or "").startswith("at_least('%s', %d" % (flag, minimum)), + api.get(flag)) + for flag, minimum, kind, bad in (("--delay", 0, float, "-1"), + ("--concurrency", 1, int, "0"), + ("--pages", 1, int, "x")): + try: + page_flow.at_least(flag, minimum, kind)(bad) + message = None + except argparse.ArgumentTypeError as e: + message = str(e) + check("at_least rejects %s %s and names the flag" % (flag, bad), + message is not None and flag in message, message) + equal("at_least accepts the minimum", page_flow.at_least("--delay", 0, float)("0"), 0.0) + # The Scraper API client imports no driver, so its parser runs here. + import scraper_api_client + import contextlib + import io + saved = sys.argv + try: + for argv in (["--retries", "-1"], ["--delay", "-2"]): + sys.argv = ["scraper_api_client.py", "--url", GOL] + argv + try: + with contextlib.redirect_stderr(io.StringIO()) as err: + scraper_api_client.parse_args() + code = 0 + except SystemExit as e: + code = e.code + check("scraper_api_client %s exits 2" % " ".join(argv), + code == 2 and argv[0] in err.getvalue(), err.getvalue()[-200:]) + finally: + sys.argv = saved + + +def check_the_canary_fails_without_a_secret(): + """A canary that skipped green reported a scraper as tested when the site + was never contacted. Without a secret it must fail, naming the secret.""" + text = open(os.path.join(HERE, ".github", "workflows", "canary.yml"), + encoding="utf-8").read() + check("the canary has no green skip branch", + "::notice::" not in text and "SKIPPED" not in text) + check("the canary fails by name without a secret", + "::error::" in text and "WEBMOTORS_PROXY" in text + and "WEBMOTORS_CDP_ENDPOINT" in text and "exit 1" in text) + check("both secrets are mapped into the job's environment", + "WEBMOTORS_PROXY: ${{ secrets.WEBMOTORS_PROXY }}" in text + and "WEBMOTORS_CDP_ENDPOINT: ${{ secrets.WEBMOTORS_CDP_ENDPOINT }}" in text) + check("live steps are not gated on a secret (no silent skip)", + "env.WEBMOTORS_PROXY != ''" not in text) + + + CHECKS = [v for k, v in sorted(globals().items()) if k.startswith("check_")]