From 1118b12424c40ff2fb5db524dc3d9f15f44bb935 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:44:06 +0000 Subject: [PATCH 1/3] One paid solve per page, atomic outputs, and matching exit codes in every engine - Captcha: a per-page SolveBudget checked before every purchase in all three engines, not reset by retries or exit rotation (a 3-exit pool bought 3 solves for one page before). - Output: JSON, CSV and the sidecar are written atomically with umask-honouring modes; CSV cells starting with = + - @ tab CR LF get an apostrophe, counted as csv_cells_escaped in the sidecar. JSON is unchanged. - CDP: a refused --cdp-endpoint connect exits 5 with a credential-free message; transient 500/profile_locked retried; pyppeteer connect bounded. - pyppeteer keeps the goto status, Selenium reads it from the browser; both report HTTP errors and parser drift as Playwright does (exit 5, not 4). - Suite: the skip report reads the module-level record again; new checks for all of the above, driven through stubbed drivers. - Credential scan: relative-path exclusions, fails on 0 files, reads data files and .env.* backups. .gitignore covers dump/capture shapes. - env: {...} placeholders read as unset; an empty value is not a warning. - Docs: README exit table and stop_reason_for docstring corrected; private citations removed and guarded. Co-Authored-By: Claude Opus 5.5 --- .github/canary_check.py | 10 +- .github/ci_checks.py | 30 ++- .github/workflows/canary.yml | 8 +- .gitignore | 13 +- CHANGELOG.md | 65 ++++- README.md | 4 +- captcha_solver.py | 31 +++ cli_entry.py | 2 +- env_config.py | 23 +- fingerprint_client.py | 2 +- output_writer.py | 157 ++++++++++-- playwright_scraper.py | 66 ++++- product_parser.py | 2 +- proxy_pool.py | 52 ++++ puppeteer_scraper.py | 113 +++++++-- pyproject.toml | 4 +- selenium_scraper.py | 66 ++++- smoke_test.py | 454 ++++++++++++++++++++++++++++++++++- 18 files changed, 1013 insertions(+), 89 deletions(-) diff --git a/.github/canary_check.py b/.github/canary_check.py index 3bf7d45..6eb2f90 100644 --- a/.github/canary_check.py +++ b/.github/canary_check.py @@ -14,9 +14,9 @@ drifts silently; this one is imported by the offline suite, which asserts it against output_writer's constants. -Two callers, deliberately the same implementation (CLAUDE.md §17: one -implementation invoked from both CI and the offline suite, not a workflow -reimplementing a narrower copy of a shipped script): +Two callers, deliberately the same implementation (one implementation +invoked from both CI and the offline suite, not a workflow reimplementing a +narrower copy of a shipped script): python3 .github/canary_check.py --exit-code 3 --allow-block python3 .github/canary_check.py --data canary_run @@ -24,7 +24,7 @@ `--allow-block` is what separates the two canary signals. From a GitHub runner — a datacentre address — Akamai refusing us says something about the runner's IP, not about Farfetch or about this code, and a check that is red -every morning teaches everyone to ignore checks (CLAUDE.md §11). So the free +every morning teaches everyone to ignore checks. So the free no-proxy signal treats a block as a SKIP and stays green, while the proxy-backed signal treats the same code as a failure, because from a residential exit a block IS news. @@ -85,7 +85,7 @@ # run, so three pages clear this comfortably. Deliberately far below the real # figure rather than tracking it: a floor guards against a near-empty render, # and a threshold that tracks a number which legitimately varies between runs -# is a check that goes stale on its own (CLAUDE.md §17, check 4). +# is a check that goes stale on its own. PAGES_REQUESTED = 3 MIN_PRODUCTS = 120 MIN_PRICED_SHARE = 0.9 diff --git a/.github/ci_checks.py b/.github/ci_checks.py index 75b8432..200bc3b 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -65,7 +65,20 @@ HEX32 = re.compile(r"\b[0-9a-f]{32}\b") HEX32_ALLOWED = ("sha", "hash", "nonce", "example", "md5", "digest", "checksum") -SCANNED_SUFFIXES = (".py", ".md", ".txt", ".yml", ".yaml", ".example") +# Data files too: a captured fixture or a generated sample is where a key or +# a session token arrives with a page, and those were never read before. +SCANNED_SUFFIXES = (".py", ".md", ".txt", ".yml", ".yaml", ".example", + ".json", ".csv", ".html", ".toml", ".cfg") + + +def _is_env_backup(path): + """`.env.bak`, `.env.local`, `.env.save` … — a renamed working copy. + + Not `.env` itself, which is where a key is supposed to live, and not + `.env.example`, which the suffix rule already covers. A backup has no + useful suffix, so a suffix list alone never read one. + """ + return path.name.startswith(".env.") and path.name != ".env.example" # A directory holding `pyvenv.cfg` is a virtualenv, whatever it is called. @@ -76,7 +89,7 @@ # 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 @@ -105,10 +118,15 @@ def _is_in_virtualenv(path): def scanned_files(): for path in sorted(REPO.rglob("*")): - if not path.is_file() or path.suffix not in SCANNED_SUFFIXES: + if not path.is_file(): + continue + if path.suffix not in SCANNED_SUFFIXES and not _is_env_backup(path): continue + # Relative parts only. The absolute path includes every directory + # ABOVE the clone, so a clone that happens to live under a directory + # called `venv` skipped every file and reported clean. if any(part in {".git", "__pycache__", ".venv", "venv"} - for part in path.parts): + for part in path.relative_to(REPO).parts): continue if _is_in_virtualenv(path): continue @@ -200,6 +218,10 @@ def secret_check(): failed.append(f"{rel}:{lineno} contains {match[:6]}… — a " f"32-char hex string, the shape of a 2captcha key") + if not scanned: + # "Nothing credential-shaped" about nothing at all is not a pass. + failed.append("0 files scanned — the scan read nothing, so it cannot " + "say the repository is clean") if not failed: print(f"ok {scanned} files scanned, nothing credential-shaped") return failed diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 89c5af8..862383c 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -27,8 +27,8 @@ name: canary # FARFETCH_PROXY secret is set, and is strict about # everything including a block, because from a # residential exit a block IS news. Skips with a -# notice when the secret is absent (CLAUDE.md §11: -# a canary that cannot pass must SKIP, not fail). +# notice when the secret is absent (a canary +# that cannot pass must SKIP, not fail). # # The proxy secret is passed through the ENVIRONMENT, never as a --proxy # argument. env_config.py already reads FARFETCH_PROXY, an unset secret @@ -157,8 +157,8 @@ jobs: env: # Read by env_config.py, exactly as a local .env would be. NOT # passed as --proxy: a credential in argv is readable by anything - # that can run `ps` and is the rule this repo states in §3 of its - # own docs. + # that can run `ps`, which is why the README tells users to keep + # credentials in .env. FARFETCH_PROXY: ${{ secrets.FARFETCH_PROXY }} run: | set +e diff --git a/.gitignore b/.gitignore index 9d66c54..a690900 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,16 @@ test_farfetch*.* # --- the live-test harness's working directory --------------------------- live_results/ live_results.zip +# By KIND of directory, not by one chosen name: a raw capture reaches +# `git add -A` through whatever directory nobody listed. +live/ +captures/ +dumps/ +scratch/ +# `--dump-html X --pages N` writes X.page2, X.page3 …, which `*.json` does not +# cover when X ends in .json, and a bare page capture at the repo root. +*.page[0-9]* +/*.html # --- debug artifacts the diagnostics drop next to themselves ------------- # check_signup_captcha*.py write these on purpose, for manual inspection. @@ -66,7 +76,6 @@ 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 cd09c96..af8a87f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,65 @@ a promise that every flag and exit code is contractually frozen. ## [Unreleased] +> **Behaviour changes.** (1) The pyppeteer and Selenium engines now report an +> HTTP 4xx/5xx page whose body names no known bot-check vendor as exit 5 +> (content never obtained) instead of exit 4 (empty category), and a page that +> links to products but parses to none as exit 5 (`stop_reason: parse_drift`, +> exit 6 if earlier pages were gathered) — the same codes the Playwright +> engine already gave. (2) The CSV output now prefixes a string cell that +> begins with `=`, `+`, `-`, `@`, a tab or a line break with an apostrophe, +> so a spreadsheet reads it as text; JSON is unchanged and the sidecar +> records how many cells were escaped as `csv_cells_escaped`. (3) A refused +> `--cdp-endpoint` connect now exits 5 with a one-line, credential-free +> message instead of a traceback (exit 1). + ### Fixed +- **One paid captcha solve per page, across every retry and exit rotation.** + The Playwright engine called its captcha handler once per block-retry with + nothing counting purchases, so with a 3-exit proxy pool and + `--solve-captcha always` one blocked page bought three solves (reproduced + with the solver stubbed). A per-page `SolveBudget` is now checked before + every purchase in all three engines and is not reset by a rotation. +- **Output files are written atomically.** `write_json`, `write_csv` and the + `.meta.json` sidecar went through `open(path, "w")`, which truncates first: + a `json.dump` failing part-way turned a 433-byte good file into 12 bytes of + invalid JSON. They now write a temp file beside the target, fsync it and + rename it over the target. A new file gets the same mode `open()` would + have given it (`0o666 & ~umask`); an existing file keeps its own. +- **A failed `--cdp-endpoint` connect no longer prints the endpoint.** + Playwright's connect error repeats the endpoint, `user:pass` included, in + its message and call log, and nothing caught it. It is now caught, every + occurrence redacted, a 401 explained as wrong or expired credentials, and a + `profile_locked`/HTTP 500 answer retried up to three times, 3 s apart. The + pyppeteer connect is bounded at 30 s (it had no timeout). +- **pyppeteer keeps the HTTP status of the page it loaded, and Selenium reads + it from the browser** (`PerformanceNavigationTiming.responseStatus`), so the + mirrors classify an error page the way Playwright does. Both now also name + parser drift. +- **The suite's skip report printed empty whatever happened.** `main()` + rebound `_skips` and `_skipped_engines` locally, shadowing the records the + sections wrote to, so the `SKIPPED_ENGINES:` line the engine-smoke CI job + reads could never name a skipped engine. A check now asserts every engine + whose driver is absent is listed. +- **The credential scan reported clean after reading nothing.** Exclusions + were tested against the ABSOLUTE path, so a clone under any directory + called `venv` scanned 0 files and printed "nothing credential-shaped". They + are now tested on the path relative to the repo, a scan of 0 files fails, + and JSON/CSV/HTML/TOML/CFG files and renamed `.env.*` backups are read too. +- **A value still holding a `{…}` slot from the documented format** (the + Scraping Browser endpoint or proxy shape in `.env.example`) now reads as + unset instead of being sent as a real credential, and an empty value no + longer logs a "still set to the placeholder" warning. +- `.gitignore` covers the capture shapes the engines write themselves + (`X.page2`, `X.page3` from `--dump-html`), a bare `.html` at the repo root, + and `live/`, `captures/`, `dumps/`, `scratch/`. +- The README exit-code table and the `stop_reason_for` docstring said parser + drift exits 4; it exits 5 (6 with earlier pages gathered). +- Comments, docs and the v0.4.3 notes no longer point at private notes or + bare section numbers that no reader of this repository can open; a suite + check keeps them out. + - **Scraper API: `waitFor` is now sent as a JSON object.** Measured 2026-09-23 against `scraper.2captcha.com/tasks/sync`: the JSON-encoded string this client sent (the form older docs described) is refused with @@ -372,7 +429,7 @@ PR. Of P2, three of four. - Offline suite: **319 checks**, up from 232; `pytest` reports 321 results. New coverage includes the Dockerfile's COPY list against the entrypoint's import graph — a check - CLAUDE.md §10 calls for after every repo in this family shipped an image + worth having after every repo in this family shipped an image that died on every invocation, and which did not exist here. It failed on its first run, catching a module added in this same batch. @@ -382,9 +439,9 @@ PR. Of P2, three of four. - **`fingerprint_client.py` could not read the key from `.env`.** `--key` defaulted to `os.environ.get("TWOCAPTCHA_KEY")` and only that, so a key put - in `.env` — exactly as §3, the README and `.env.example` instruct — worked + 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. + not applied on one path. It now reads through `env_config.env_value`, calling `load_env()` itself because this is a standalone entry point that no engine has necessarily run @@ -396,7 +453,7 @@ PR. Of P2, three of four. "still set to the placeholder from .env.example" instead. Found on a sibling repo's first live `--fingerprint` run, then checked - across the family before patching, per §16: five repos had it and one had + across the family before patching: five repos had it and one had already fixed it. Pinned by a check verified to fail on the old code — including that the help string does not interpolate its default, which is one substring away from printing a live credential to anyone who types diff --git a/README.md b/README.md index 000799c..001b309 100644 --- a/README.md +++ b/README.md @@ -282,8 +282,8 @@ A contract, not decoration — the harness and any pipeline can branch on these. | `1` | Unhandled error | | `2` | Bad usage | | `3` | **Blocked before parsing** — a challenge page, or an outright refusal | -| `4` | Fetched the page fine, parsed **0 products** | -| `5` | **Never got the page** — navigation timeout, dead proxy, 4xx/5xx, or a Scraper API error | +| `4` | Fetched the page fine, parsed **0 products**, and the page links to no products either | +| `5` | **No content obtained** — navigation timeout, dead proxy, 4xx/5xx, a failed `--cdp-endpoint` connect, a Scraper API error, or a page that links to products and parsed to none (`stop_reason: parse_drift` — a parser problem, not a network one) | | `6` | **Partial run** — products written, but the page loop stopped early | | `124` | Selenium only: chromedriver could not be **started** within the watchdog | diff --git a/captcha_solver.py b/captcha_solver.py index faef65d..6a7d486 100644 --- a/captcha_solver.py +++ b/captcha_solver.py @@ -589,6 +589,37 @@ def solve_recaptcha(challenge: CaptchaChallenge, twocaptcha_api_key: Optional[st return solver(twocaptcha_api_key, challenge, min_score=min_score) +# How many paid solves one page may buy, across every attempt and every exit +# rotation. One: a challenge that survives a solved token is not one this run +# can pass, and a second solve is a second charge for the same answer. +SOLVES_PER_PAGE = 1 + + +class SolveBudget: + """The one allowance every paid solve on a page goes through. + + Created once per PAGE, outside the retry and rotation loops, and checked + BEFORE each purchase. Before it existed the Playwright engine called its + captcha handler once per block-retry with nothing counting, so with a + 3-exit proxy pool one page bought three solves (reproduced with the + solver stubbed). It deliberately does NOT reset on rotation: a fresh exit + is a reason to re-fetch, not a fresh allowance to spend. + """ + + def __init__(self, limit: int = SOLVES_PER_PAGE): + self.limit = limit + self.spent = 0 + + def take(self) -> bool: + """Reserve one solve. False, with a log line, once the page is spent.""" + if self.spent >= self.limit: + logger.info("Not buying another solve for this page: %d of %d " + "already bought.", self.spent, self.limit) + return False + self.spent += 1 + return True + + INJECT_TOKEN_JS = """ (token) => { let el = document.getElementById('g-recaptcha-response'); diff --git a/cli_entry.py b/cli_entry.py index ceb6900..5cfb4f5 100644 --- a/cli_entry.py +++ b/cli_entry.py @@ -12,7 +12,7 @@ path, which made the module import cleanly with no pyppeteer installed: the engine group never skipped, the CI job that exists to fail on unexpected skips could not have caught a broken import, and CI ran against a stub -version for a while with nothing noticing (CLAUDE.md §10). So the import +version for a while with nothing noticing. So the import stays where it is. But `pip install .[playwright]` — installing exactly one engine, which is diff --git a/env_config.py b/env_config.py index f9e9be4..3ceff64 100644 --- a/env_config.py +++ b/env_config.py @@ -28,6 +28,7 @@ import logging import os +import re import sys from pathlib import Path @@ -56,9 +57,15 @@ "your_2captcha_api_key_here", "your_api_key_here", "changeme", - "", } +# A value still holding a `{…}` slot from the documented shape — e.g. the +# Scraping Browser endpoint `ws://{login}-zone-…:{password}@…` — is the +# example, not a configuration. Sent as-is it reaches the remote end as a +# username of literally `{login}-zone-…` and comes back as a 401 a long way +# from its cause. +_BRACED_PLACEHOLDER = re.compile(r"\{[A-Za-z_][A-Za-z0-9_]*\}") + _loaded_from = None @@ -151,12 +158,24 @@ def env_value(name): value = os.environ.get(name) if value is None: return None + if not value.strip(): + # Empty is simply unset — the shipped .env.example leaves every value + # empty, and an unset CI secret arrives as "". Nothing to warn about. + return None if value.strip().lower() in _PLACEHOLDERS: logger.warning( "%s is still set to the placeholder from .env.example — treating it " "as unset. Put your real value in .env.", name) return None - return value.strip() or None + slot = _BRACED_PLACEHOLDER.search(value) + if slot: + # Name the slot, never the value: the value may be a credentialled URL. + logger.warning( + "%s still contains the placeholder %s from the documented format — " + "treating it as unset. Fill in the real value in .env.", + name, slot.group(0)) + return None + return value.strip() def apply(args, keys=None, quiet=False): diff --git a/fingerprint_client.py b/fingerprint_client.py index 708569c..7ec287a 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -332,7 +332,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, which reports success while doing less. # # `.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/output_writer.py b/output_writer.py index f1d53ca..42664e0 100644 --- a/output_writer.py +++ b/output_writer.py @@ -4,8 +4,11 @@ Shared product model + JSON/CSV writers used by all three scrapers. """ +import contextlib import csv import json +import os +import tempfile import uuid import notify @@ -120,13 +123,107 @@ def dedupe_by_sku(products: List[Product], seen: Set[str]) -> List[Product]: return fresh +@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. That breaks the promise `save()` + keeps by refusing to overwrite good output with an empty result: the + previous good run would be destroyed by the ATTEMPT to replace it rather + than by its outcome. Reproduced here before the fix: a 433-byte good + `out.json` came back as 12 bytes of invalid JSON when `json.dump` failed + part-way through. + + The sidecar matters most: `.meta.json` is the file a consumer + branches on, so a truncated one beside good rows reads as a broken run + over data that is fine. + + * 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=""` + and that has to survive the move into the temp file. + """ + 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, so without this every output would be owner-readable only — + # a regression from `open(path, "w")`, which honours the umask. An + # existing target keeps its OWN mode (someone may have tightened it + # on purpose); a new one gets exactly what `open()` would have given + # it. Never a flat 0o644: that narrows 0664 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 whoever listed +# the product — a title, a brand, a colour. The tab and the two newlines are +# here because some readers strip a leading one and execute what follows. +CSV_FORMULA_LEADS = ("=", "+", "-", "@", "\t", "\r", "\n") + +# Excel, Google Sheets and LibreOffice read a leading apostrophe as "the rest +# of this cell is text" and do not display it. A consumer parsing the file +# with the `csv` module DOES see it, which is why the count goes into the +# sidecar as `csv_cells_escaped` rather than staying silent. +CSV_FORMULA_ESCAPE = "'" + + +def _csv_escape(v): + """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 exactly as served. + """ + if isinstance(v, str) and v.startswith(CSV_FORMULA_LEADS): + return CSV_FORMULA_ESCAPE + v, True + return v, False + + +def _csv_value(v): + # Every column in both row classes is already flat (image_urls is + # "|"-joined by the parser), but a list reaching the writer is joined + # here, BEFORE the formula escape — a joined cell is text too and can + # start with a formula character while the list itself is not a string. + if isinstance(v, (list, tuple)): + return "|".join(str(x) for x in v) + if isinstance(v, dict): + return json.dumps(v, ensure_ascii=False) + return v + + def write_json(products: List[Product], path: str) -> None: - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump([asdict(p) for p in products], f, ensure_ascii=False, indent=2) def write_csv(products: List[Product], path: str, - row_type=None) -> None: + row_type=None) -> 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 @@ -137,16 +234,23 @@ def write_csv(products: List[Product], path: str, # would give an empty detail run a listing header — a file whose columns # describe something it does not contain. if not products: - with open(path, "w", encoding="utf-8", newline="") as f: + with _atomic(path, newline="") as f: csv.DictWriter(f, fieldnames=list( asdict((row_type or Product)()).keys())).writeheader() - return + return 0 fieldnames = list(asdict(products[0]).keys()) - 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 p in products: - writer.writerow(asdict(p)) + row = {} + for k, v in asdict(p).items(): + 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 @@ -226,8 +330,8 @@ def stop_reason_for(*, load_failed: bool, blocked_by: Optional[str], page. Keyword-only so adding a signal later cannot silently re-bind an existing caller's positional argument. - Ordered by how much each signal PROVES, not by how cheap it is to test - (CLAUDE.md §17): naming the vendor that refused us is a stronger + Ordered by how much each signal PROVES, not by how cheap it is to test: + naming the vendor that refused us is a stronger statement than "the status was 403", which is stronger than "it timed out", so the specific reason wins over the general one. """ @@ -241,12 +345,13 @@ def stop_reason_for(*, load_failed: bool, blocked_by: Optional[str], return "page_load_timeout" if parse_drift: # Last, because everything above says the page never arrived while - # this one says it arrived and we failed to read it. Not a fetch - # failure — the run's exit code stays EXIT_NO_PRODUCTS, since the - # catalogue question genuinely was answered with "nothing" — but the - # sidecar and the log name it as OUR bug rather than the site's - # answer, which is the thing a reader needs in order to look in the - # right place. + # this one says it arrived and we failed to read it. It is not a + # complete run, so with nothing gathered the exit code is + # EXIT_FETCH_FAILED (5) — no content was obtained — and with earlier + # pages gathered it is EXIT_PARTIAL (6). Never EXIT_NO_PRODUCTS: the + # page linked to products, so "the catalogue is empty" is false. The + # sidecar and the log name it as OUR bug rather than the network's, + # which is what a reader needs in order to look in the right place. return "parse_drift" return "completed" @@ -264,7 +369,7 @@ def write_run_meta(out_prefix: str, meta: dict) -> str: partial run's un-fetched pages being reported as delisted products. """ 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 @@ -372,7 +477,8 @@ def share(pred) -> float: def save(products: List[Product], out_prefix: str, fmt: str, - allow_empty: bool = False, row_type=None) -> int: + allow_empty: bool = False, row_type=None, + stats: Optional[dict] = None) -> int: """Write JSON/CSV and return a process exit code. Returns 0 when products were written, EXIT_NO_PRODUCTS when there were @@ -399,8 +505,17 @@ def save(products: List[Product], out_prefix: str, fmt: str, write_json(products, f"{out_prefix}.json") print(f"[+] Saved {len(products)} products -> {out_prefix}.json") if fmt in ("csv", "both"): - write_csv(products, f"{out_prefix}.csv", row_type=row_type) + escaped = write_csv(products, f"{out_prefix}.csv", row_type=row_type) print(f"[+] Saved {len(products)} products -> {out_prefix}.csv") + # Through `stats`, not the return value, because the return value is + # an EXIT CODE. finish_run threads it into the sidecar. + if stats is not None: + stats["csv_cells_escaped"] = escaped + if escaped: + print(f"[!] {escaped} CSV cell(s) began with a formula character " + f"and were prefixed with {CSV_FORMULA_ESCAPE!r} so a " + f"spreadsheet reads them as text. The JSON output is " + f"unchanged — see csv_cells_escaped in the sidecar.") return 0 if products else EXIT_NO_PRODUCTS @@ -451,8 +566,9 @@ def finish_run(products: List[Product], out_prefix: str, fmt: str, # unification this file already carries — a rule keyed on a list of # names has a hole for every name nobody added to it. complete = stop_reason in COMPLETE_STOP_REASONS and not pages_failed + write_stats: dict = {} rc = save(products, out_prefix, fmt, allow_empty=allow_empty, - row_type=row_type) + row_type=row_type, stats=write_stats) wrote_output = bool(products) or allow_empty status = "complete" if (products and complete) else ( @@ -469,6 +585,10 @@ def finish_run(products: List[Product], out_prefix: str, fmt: str, start_url=start_url, final_url=final_url, products=len(products), run_id=run_id, started_at=started_at, rows=products, mode=mode) + # UNDER what the run already recorded, never over it: housekeeping must + # not overwrite a key the run itself set. + for k, v in write_stats.items(): + meta.setdefault(k, v) if wrote_output: write_run_meta(out_prefix, meta) @@ -496,7 +616,6 @@ def _finish_code(products, rc, blocked, stop_reason, complete, # category is empty" — the defect this branch exists to prevent. # Seven sibling repos had independently keyed on `not complete`, and # the family unified on their rule with this repo's exit code. - # See CLAUDE.md §25. if not complete: print(f"[!] The pages were never fetched ({stop_reason}) — this " f"is exit {EXIT_FETCH_FAILED}, NOT an empty category " diff --git a/playwright_scraper.py b/playwright_scraper.py index 7fea8eb..e5f861f 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -56,6 +56,7 @@ from captcha_solver import (detect_recaptcha_v3, detect_recaptcha_in_page, reconcile_detections, solve_recaptcha, + SolveBudget, INJECT_TOKEN_JS) from product_parser import (parse_products, SELECTORS, detect_bot_challenge, describe_block, page_url, @@ -63,9 +64,12 @@ from product_detail_parser import parse_product_detail from output_writer import (dedupe_by_sku, finish_run, stop_reason_for, COMPLETE_STOP_REASONS, new_run_id, - ProductVariant) + ProductVariant, EXIT_FETCH_FAILED) from run_state import Checkpoint from proxy_pool import (from_args as proxy_pool_from_args, to_playwright, mask, + redact_url_credentials, describe_cdp_failure, + cdp_failure_is_transient, CDP_CONNECT_ATTEMPTS, + CDP_CONNECT_PAUSE_S, ROTATE_MODES, ProxyError, ProxyPool) import env_config from arg_types import positive_int, nonneg_int, nonneg_float @@ -107,8 +111,8 @@ class PageOutcome: # The response status for the document itself, where the driver exposes # one (Playwright and pyppeteer return a Response from goto(); Selenium # has no equivalent without a CDP session, so it stays None there and the - # HTML markers carry the whole job — see CLAUDE.md §8 on needing a - # STRUCTURAL secondary signal for exactly that case). + # HTML markers carry the whole job — which is why a STRUCTURAL + # secondary signal is needed for exactly that case). http_status: Optional[int] = None # Chromium's own error text when the EXIT was unusable # (ERR_PROXY_CONNECTION_FAILED and friends), as opposed to the site being @@ -321,6 +325,12 @@ def close(self): logger.debug("Ignoring error during browser teardown: %s", e) +class CdpConnectError(RuntimeError): + """A --cdp-endpoint that could not be attached to. The message is already + free of credentials; the run reports it as exit 5 (content never + obtained), not as a traceback.""" + + def _connect_remote(pw, args): """Attach to an already-running browser over CDP; return (browser, context, page).""" logger.info("Connecting to existing browser over CDP: %s", @@ -331,7 +341,25 @@ def _connect_remote(pw, args): # ten-minute hang. A Scraping Browser session that is still held # answers with HTTP 500 rather than stalling, so this mostly guards # against the endpoint going quiet. - browser = pw.chromium.connect_over_cdp(args.cdp_endpoint, timeout=30000) + browser = None + for attempt in range(1, CDP_CONNECT_ATTEMPTS + 1): + try: + browser = pw.chromium.connect_over_cdp(args.cdp_endpoint, + timeout=30000) + break + except (PWTimeout, PWError) as e: + # Playwright's message repeats the endpoint, `user:pass` and all, + # in the message and again in its call log, so nothing from it + # is printed or re-raised unredacted. + text = str(e) + if cdp_failure_is_transient(text) and attempt < CDP_CONNECT_ATTEMPTS: + logger.warning("CDP connect refused (%s) — retrying in %.0fs " + "(%d/%d).", redact_url_credentials(text).splitlines()[0] + if text else "no message", CDP_CONNECT_PAUSE_S, + attempt, CDP_CONNECT_ATTEMPTS) + time.sleep(CDP_CONNECT_PAUSE_S) + continue + raise CdpConnectError(describe_cdp_failure(text)) from None # Reuse the remote browser's existing context so its # fingerprint/session/proxy settings stay intact. context = browser.contexts[0] if browser.contexts else browser.new_context() @@ -439,7 +467,7 @@ def _content_when_settled(page, attempts: int = 4, pause_ms: int = 700): return None -def handle_captcha_if_present(page, args) -> None: +def handle_captcha_if_present(page, args, budget=None) -> None: """Runs after EVERY navigation, for ANY page — not scoped to one URL. Two detectors, in order: @@ -501,6 +529,11 @@ def handle_captcha_if_present(page, args) -> None: "holds. Pass --twocaptcha-key or set TWOCAPTCHA_KEY if " "the run comes back blocked (exit 3).") return + # Checked BEFORE the purchase, against an allowance the caller holds + # for the whole page, so retries and exit rotations cannot each buy + # another token. Without a caller's budget, this call alone may buy one. + if not (budget if budget is not None else SolveBudget()).take(): + return try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -538,6 +571,9 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome block_retries = args.proxy_block_retries if (pool and len(pool) > 1) else 0 html, vendor, load_failed = None, None, False http_status = None + # One paid-solve allowance for this PAGE, created outside the rotation + # loop so a fresh exit does not bring a fresh allowance with it. + budget = SolveBudget() for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) @@ -553,7 +589,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # reliable signal a refusing edge gives us: Farfetch's # "Access Denied" page arrives under HTTP 403 (measured # 2026-09-14), while every marker on it was unknown to the - # parser. Taking the status first is CLAUDE.md §8. + # parser. So the status is taken first. resp = session.page.goto(url, wait_until="domcontentloaded", timeout=60000) http_status = resp.status if resp else None @@ -588,7 +624,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if load_failed: break - handle_captcha_if_present(session.page, args) + handle_captcha_if_present(session.page, args, budget=budget) if http_status is not None and http_status >= 400: # An error status is not a page that has yet to paint, so the @@ -674,7 +710,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # know this is not the listing, and saying "0 products" about it # would be a claim about the catalogue that this run cannot support. # Reported separately from `blocked` precisely because we CANNOT name - # who refused us — "never present a guess as a fact" (CLAUDE.md §8). + # who refused us — a guess is never presented as a fact. debug_html = f"{args.out}_page{page_num}_debug.html" with open(debug_html, "w", encoding="utf-8") as f: f.write(html) @@ -972,8 +1008,15 @@ def scrape(args) -> int: "memory for it.", concurrency, concurrency) with sync_playwright() as pw: - session = _BrowserSession(pw, args, pool, - remote=bool(args.cdp_endpoint)).open() + try: + session = _BrowserSession(pw, args, pool, + remote=bool(args.cdp_endpoint)).open() + except CdpConnectError as e: + # No page was requested, so nothing can be said about the + # catalogue: the same exit as any other run that never got its + # content, and no output is written. + logger.error("%s", e) + return EXIT_FETCH_FAILED try: # Page 1 is always fetched on its own: its content is what decides # whether pages 2..N can be addressed independently at all. @@ -1333,8 +1376,7 @@ def main() -> int: It was inline under `if __name__ == "__main__"`, which meant two things: the console script declared in pyproject had nothing to point at, and the offline suite could only ever test the helpers underneath it — the exact - gap CLAUDE.md §10 names ("test the public entry point, not only its - internals"), which is how a signature once drifted away from its callers + gap "test the public entry point, not only its internals" names, which is how a signature once drifted away from its callers with every check still green. """ args = parse_args() diff --git a/product_parser.py b/product_parser.py index c402ee4..ad625b6 100644 --- a/product_parser.py +++ b/product_parser.py @@ -230,7 +230,7 @@ # bare string "edgesuite": edgesuite.net is also an ordinary Akamai asset # domain, so a site whose own images are served from it would report every # page as blocked. That is the trap tokopedia-scraper fell into with a bare -# "akamai" marker (see CLAUDE.md §18) — a marker that matches a good page is +# "akamai" marker — a marker that matches a good page is # worse than no marker. "errors.edgesuite.net" appears only on the error page. _DENY_ERROR_HOST = "errors.edgesuite.net" diff --git a/proxy_pool.py b/proxy_pool.py index e4a15b2..c878fd8 100644 --- a/proxy_pool.py +++ b/proxy_pool.py @@ -25,6 +25,7 @@ from __future__ import annotations +import re import logging import random from typing import List, Optional @@ -150,6 +151,57 @@ def mask(url: Optional[str]) -> str: return "(unparseable proxy URL, redacted)" +_URL_CREDENTIALS_RE = re.compile(r"([a-z][a-z0-9+.-]*://)[^/\s@'\"]+@", re.I) + + +def redact_url_credentials(text) -> str: + """Every `scheme://user:pass@` in free text, credentials replaced. + + For EXCEPTION MESSAGES, not single URLs: Playwright repeats a CDP + endpoint several times in one error (the message plus its call log), so + a masker that handles the first occurrence prints the password the other + times. Host and port stay. Never raises. + """ + try: + return _URL_CREDENTIALS_RE.sub(r"\1***:***@", str(text)) + except Exception: # noqa: BLE001 + return "(unprintable error, redacted)" + + +# How a managed CDP endpoint refuses, as far as the text of the error says. +# A Scraping Browser profile stays `profile_locked` for a second or two after +# a clean disconnect and answers HTTP 500 meanwhile, so those are retried a +# few times; a 401 means the endpoint's credentials are wrong or expired and +# no retry helps. +CDP_CONNECT_ATTEMPTS = 3 +CDP_CONNECT_PAUSE_S = 3.0 + + +def cdp_failure_is_transient(text: str) -> bool: + t = str(text).lower() + return "profile_locked" in t or " 500" in t or "500 " in t + + +def describe_cdp_failure(text: str) -> str: + """A one-line, credential-free explanation of a failed CDP connect.""" + safe = redact_url_credentials(text).strip().splitlines() + first = safe[0] if safe else "(no message)" + t = str(text).lower() + if "401" in t or "unauthori" in t: + cause = ("the endpoint refused the credentials (HTTP 401): the " + "profile's login or password is wrong or has expired — get " + "fresh credentials for the endpoint") + elif "profile_locked" in t: + cause = ("the profile is held by another live connection " + "(profile_locked) — wait for it to close, or use another pid") + elif cdp_failure_is_transient(text): + cause = ("the service answered HTTP 500 on the WebSocket upgrade — a " + "service-side failure, not this run's") + else: + cause = "the endpoint could not be reached" + return f"Could not connect to --cdp-endpoint: {cause}. ({first})" + + def to_playwright(url: Optional[str]) -> Optional[dict]: """Playwright's `proxy=` dict for a proxy URL, or None. diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index f437737..5a8e415 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -30,11 +30,17 @@ from captcha_solver import (detect_recaptcha_v3, detect_recaptcha_in_page, reconcile_detections, solve_recaptcha, + SolveBudget, INJECT_TOKEN_JS, RECAPTCHA_DISCOVERY_JS) from product_parser import (parse_products, SELECTORS, detect_bot_challenge, - describe_block, page_url) -from output_writer import dedupe_by_sku, finish_run, new_run_id + describe_block, page_url, + count_product_links) +from output_writer import (dedupe_by_sku, finish_run, new_run_id, + stop_reason_for, EXIT_FETCH_FAILED) from proxy_pool import mask as mask_proxy +from proxy_pool import (redact_url_credentials, describe_cdp_failure, + cdp_failure_is_transient, CDP_CONNECT_ATTEMPTS, + CDP_CONNECT_PAUSE_S) import env_config from arg_types import positive_int, nonneg_float @@ -95,7 +101,7 @@ async def _content_when_settled(page, attempts: int = 4, pause: float = 0.7): return None -async def handle_captcha_if_present(page, args) -> None: +async def handle_captcha_if_present(page, args, budget=None) -> None: """Runs after EVERY navigation, for ANY page — not scoped to one URL.""" html = await _content_when_settled(page) if html is None: @@ -136,6 +142,11 @@ async def handle_captcha_if_present(page, args) -> None: "solved — continuing with whatever the page already " "holds. If it was blocking, the run reports exit 3.") return + # Checked BEFORE the purchase, against an allowance the caller holds + # for the whole page, so retries and exit rotations cannot each buy + # another token. Without a caller's budget, this call alone may buy one. + if not (budget if budget is not None else SolveBudget()).take(): + return try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -151,6 +162,37 @@ async def handle_captcha_if_present(page, args) -> None: await page.reload({"waitUntil": "domcontentloaded", "timeout": 60000}) +CDP_CONNECT_TIMEOUT_S = 30 + + +async def _connect_remote(args): + """connect() with a timeout, a short retry and a credential-free error. + + pyppeteer's connect() has no timeout of its own and waits on a future a + rejected WebSocket handshake may never resolve, so it is bounded here. + A refused connect returns None and the run reports exit 5 (content never + obtained) instead of a traceback whose text carries the endpoint. + """ + for attempt in range(1, CDP_CONNECT_ATTEMPTS + 1): + try: + return await asyncio.wait_for( + connect(browserWSEndpoint=args.cdp_endpoint), + timeout=CDP_CONNECT_TIMEOUT_S) + except asyncio.TimeoutError: + text = f"no answer within {CDP_CONNECT_TIMEOUT_S}s" + except Exception as e: # noqa: BLE001 — reported, never re-raised raw + text = str(e) or type(e).__name__ + if cdp_failure_is_transient(text) and attempt < CDP_CONNECT_ATTEMPTS: + logger.warning("CDP connect refused (%s) — retrying in %.0fs (%d/%d).", + redact_url_credentials(text).splitlines()[0], + CDP_CONNECT_PAUSE_S, attempt, CDP_CONNECT_ATTEMPTS) + await asyncio.sleep(CDP_CONNECT_PAUSE_S) + continue + logger.error("%s", describe_cdp_failure(text)) + return None + return None + + async def scrape(args) -> int: # One id per run, logged here and written into the sidecar, so a # log line and an artefact can be tied together. "the run that @@ -170,7 +212,9 @@ async def scrape(args) -> int: if args.cdp_endpoint: logger.info("Connecting to existing browser over CDP: %s", _mask_credentials(args.cdp_endpoint)) - browser = await connect(browserWSEndpoint=args.cdp_endpoint) + browser = await _connect_remote(args) + if browser is None: + return EXIT_FETCH_FAILED else: launch_args = { "headless": args.headless, @@ -234,9 +278,15 @@ async def scrape(args) -> int: logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) # Retry a navigation failure rather than ending the run on it — # see playwright_scraper.py for why. + http_status = None for attempt in range(1, args.retries + 1): try: - await page.goto(url, {"waitUntil": "domcontentloaded", "timeout": 60000}) + # Keep the document's Response: its status is the one + # signal a refusing edge always gives, and without it an + # HTTP 403 with an unfamiliar body read as an empty + # category (exit 4) here while Playwright said exit 5. + resp = await page.goto(url, {"waitUntil": "domcontentloaded", "timeout": 60000}) + http_status = resp.status if resp else None load_failed = False break except Exception: @@ -252,17 +302,21 @@ async def scrape(args) -> int: stop_reason = "page_load_timeout" break - await handle_captcha_if_present(page, args) + await handle_captcha_if_present(page, args, budget=SolveBudget()) - try: - await page.waitForFunction( - f"document.querySelectorAll('{ITEM_LINK_SELECTOR}').length > {MIN_CARD_MATCHES}", - {"timeout": 20000}, - ) - await asyncio.sleep(1) - except Exception: - logger.warning("No product markers appeared within 20s — " - "parsing whatever loaded (may be a bot-check/consent page).") + if http_status is not None and http_status >= 400: + logger.warning("HTTP %d for %s — not waiting for products to " + "paint.", http_status, url) + else: + try: + await page.waitForFunction( + f"document.querySelectorAll('{ITEM_LINK_SELECTOR}').length > {MIN_CARD_MATCHES}", + {"timeout": 20000}, + ) + await asyncio.sleep(1) + except Exception: + logger.warning("No product markers appeared within 20s — " + "parsing whatever loaded (may be a bot-check/consent page).") html = await page.content() @@ -282,6 +336,22 @@ async def scrape(args) -> int: stop_reason = f"blocked_{vendor}" break + if http_status is not None and http_status >= 400: + # An error status whose body names no vendor we recognise. + # Still not the listing, and "0 products" about it would be a + # claim about the catalogue this run cannot support — the + # same reading the Playwright engine gives it. + debug_html = f"{args.out}_page{page_num}_debug.html" + with open(debug_html, "w", encoding="utf-8") as f: + f.write(html) + logger.error("HTTP %d for %s and nothing on the page names a " + "known bot-check vendor — saved %d bytes to %s. " + "Reported as a fetch failure, not an empty " + "category.", http_status, url, len(html), debug_html) + stop_reason = stop_reason_for(load_failed=False, blocked_by=None, + http_status=http_status) + break + products = parse_products(html, page.url, category=args.category) logger.info("Parsed %d products from page %d.", len(products), page_num) @@ -294,6 +364,19 @@ async def scrape(args) -> int: await page.screenshot({"path": debug_png, "fullPage": True}) except Exception as e: logger.warning("Could not capture screenshot: %s", e) + linked = count_product_links(html) + if linked >= MIN_CARD_MATCHES: + # A page that LINKS to products and parses to none is a + # broken parser, not an empty category — named as such, + # as the Playwright engine names it. + logger.error("PARSER DRIFT: the page links to %d product(s) " + "and the parser extracted 0. Saved what the " + "browser saw to %s and %s.", linked, + debug_html, debug_png) + stop_reason = stop_reason_for(load_failed=False, + blocked_by=None, + parse_drift=True) + break logger.warning("0 products parsed — saved what the browser actually saw to " "%s and %s.", debug_html, debug_png) diff --git a/pyproject.toml b/pyproject.toml index 617b31e..245e3b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -143,11 +143,11 @@ select = ["F", "E9", "B"] # narrowed. Fixing it is real work on code that demonstrably runs, and doing # it under a deadline is how churn gets introduced into three near-identical # files at once. Pinned as a known limitation with its measurement rather -# than half-guarded (CLAUDE.md §10), so widening the gate is a decision +# than half-guarded, so widening the gate is a decision # someone makes deliberately. # # smoke_test.py is excluded entirely: it is one long function that rebinds -# loop variables on purpose, and §10 says not to restructure it. +# loop variables on purpose, and is deliberately not restructured. files = [ "arg_types.py", "cli_entry.py", diff --git a/selenium_scraper.py b/selenium_scraper.py index 344677d..5c54fbc 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -36,6 +36,7 @@ import sys import tempfile import time +from typing import Optional from urllib.parse import urlparse from selenium import webdriver @@ -59,10 +60,12 @@ from captcha_solver import (detect_recaptcha_v3, detect_recaptcha_in_page, reconcile_detections, solve_recaptcha, + SolveBudget, INJECT_TOKEN_JS) from product_parser import (parse_products, SELECTORS, detect_bot_challenge, - describe_block, page_url) -from output_writer import (dedupe_by_sku, finish_run, + describe_block, page_url, + count_product_links) +from output_writer import (dedupe_by_sku, finish_run, stop_reason_for, EXIT_DRIVER_TIMEOUT, new_run_id) from proxy_pool import mask as mask_proxy import env_config @@ -639,7 +642,7 @@ def build_driver(args) -> webdriver.Chrome: return driver -def handle_captcha_if_present(driver, args) -> None: +def handle_captcha_if_present(driver, args, budget=None) -> None: """Runs after EVERY navigation, for ANY page — not scoped to one URL.""" html = driver.page_source # Both detectors, always — see reconcile_detections in captcha_solver.py. @@ -674,6 +677,11 @@ def handle_captcha_if_present(driver, args) -> None: "solved — continuing with whatever the page already " "holds. If it was blocking, the run reports exit 3.") return + # Checked BEFORE the purchase, against an allowance the caller holds + # for the whole page, so retries and exit rotations cannot each buy + # another token. Without a caller's budget, this call alone may buy one. + if not (budget if budget is not None else SolveBudget()).take(): + return try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -688,6 +696,25 @@ def handle_captcha_if_present(driver, args) -> None: driver.refresh() +def _document_status(driver) -> Optional[int]: + """The HTTP status of the current document, or None if unknowable. + + `driver.get()` returns nothing, so Selenium has no Response to read a + status from. The browser still knows it: Chromium exposes it as + PerformanceNavigationTiming.responseStatus. Without it, an HTTP 403 + whose body names no known vendor was reported as an empty category + (exit 4) here while the Playwright engine said exit 5. None (an older + browser, or a 0 for an opaque response) leaves the old behaviour. + """ + try: + status = driver.execute_script( + "const n = performance.getEntriesByType('navigation')[0];" + "return n && n.responseStatus ? n.responseStatus : null;") + except Exception: # noqa: BLE001 — a missing signal, not a failure + return None + return status if isinstance(status, int) and status > 0 else None + + def scrape(args) -> int: # One id per run, logged here and written into the sidecar, so a # log line and an artefact can be tied together. "the run that @@ -727,11 +754,13 @@ def scrape(args) -> int: logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) # Retry a navigation failure rather than ending the run on it — # see playwright_scraper.py for why. + http_status = None for attempt in range(1, args.retries + 1): load_failed = False try: driver.get(url) wait.until(lambda d: d.execute_script("return document.readyState") == "complete") + http_status = _document_status(driver) break except (TimeoutException, WebDriverException): load_failed = True @@ -746,7 +775,7 @@ def scrape(args) -> int: stop_reason = "page_load_timeout" break - handle_captcha_if_present(driver, args) + handle_captcha_if_present(driver, args, budget=SolveBudget()) try: WebDriverWait(driver, 20).until( @@ -775,6 +804,22 @@ def scrape(args) -> int: stop_reason = f"blocked_{vendor}" break + if http_status is not None and http_status >= 400: + # An error status whose body names no vendor we recognise. + # Still not the listing, and "0 products" about it would be a + # claim about the catalogue this run cannot support — the + # same reading the Playwright engine gives it. + debug_html = f"{args.out}_page{page_num}_debug.html" + with open(debug_html, "w", encoding="utf-8") as f: + f.write(html) + logger.error("HTTP %d for %s and nothing on the page names a " + "known bot-check vendor — saved %d bytes to %s. " + "Reported as a fetch failure, not an empty " + "category.", http_status, url, len(html), debug_html) + stop_reason = stop_reason_for(load_failed=False, blocked_by=None, + http_status=http_status) + break + products = parse_products(html, driver.current_url, category=args.category) logger.info("Parsed %d products from page %d.", len(products), page_num) @@ -787,6 +832,19 @@ def scrape(args) -> int: driver.save_screenshot(debug_png) except Exception as e: logger.warning("Could not capture screenshot: %s", e) + linked = count_product_links(html) + if linked >= MIN_CARD_MATCHES: + # A page that LINKS to products and parses to none is a + # broken parser, not an empty category — named as such, + # as the Playwright engine names it. + logger.error("PARSER DRIFT: the page links to %d product(s) " + "and the parser extracted 0. Saved what the " + "browser saw to %s and %s.", linked, + debug_html, debug_png) + stop_reason = stop_reason_for(load_failed=False, + blocked_by=None, + parse_drift=True) + break logger.warning("0 products parsed — saved what the browser actually saw to " "%s and %s.", debug_html, debug_png) diff --git a/smoke_test.py b/smoke_test.py index 33d03a0..2335a69 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -738,7 +738,7 @@ def _kwonly_only(fn) -> bool: argument goes wrong quietly: tokopedia-scraper's classify(html, status, url) was called as classify(html, url=...) by two of its three engines and both crashed on their FIRST fetch, invisible to import, --help, - compileall and 400+ green assertions (CLAUDE.md §17). Keyword-only + compileall and 400+ green assertions. Keyword-only parameters make that shape impossible to write. """ import inspect @@ -1595,7 +1595,7 @@ def check_akamai_s_refusal_page_the_2026_09_11_audit_s_p0(ok: bool) -> bool: "cloudflare")) # The negative half, and the half that matters more: a marker that fires - # on a good page is worse than no marker at all (CLAUDE.md §18, where a + # on a good page is worse than no marker at all (a sibling repo's # bare "akamai" marker made tokopedia-scraper report every served page as # blocked). Every real-capture fixture in this suite is checked, not just # the one listing sample. @@ -1607,7 +1607,7 @@ def check_akamai_s_refusal_page_the_2026_09_11_audit_s_p0(ok: bool) -> bool: # "edgesuite" on its own is an ordinary Akamai ASSET domain. Matching it # bare would report a site serving its own images from one as blocked on - # every page — the exact shape of the §18 trap. Pinned so a future + # every page — the exact shape of the trap above. Pinned so a future # broadening of the marker is a decision rather than a surprise. ok &= check("a page merely SERVED from an edgesuite asset host is not a " "refusal — only errors.edgesuite.net is", @@ -2711,8 +2711,8 @@ def _capture(url, meta, rc): not set(FETCH_FAILURE_STOP_REASONS) & set(COMPLETE_STOP_REASONS)) # stop_reason_for is the one place that names why a page yielded nothing. - # Ordered by how much each signal PROVES (CLAUDE.md §17's - # classification-order trap), so the specific reason outranks the general. + # Ordered by how much each signal PROVES (the classification-order + # trap), so the specific reason outranks the general. ok &= check("stop_reason_for: a named vendor outranks a bare status — " "'blocked_akamai' says more than 'http_error'", stop_reason_for(load_failed=False, blocked_by="akamai", @@ -3535,6 +3535,55 @@ def check_naming_and_dead_feature_guards(ok: bool) -> bool: + ("" if not offenders else " -> " + "; ".join(offenders[:4])), not offenders) + # PRIVATE CITATIONS. The family's working notes are not public, so a + # pointer into them ("see section N") is a reference no + # reader of this repository can follow. State the rule in this repo's own + # words instead. The tokens are ASSEMBLED so this file can be scanned like + # any other, and every tracked text file is read — the CHANGELOG + # included, because released notes are what visitors actually read. + _sect = "\u00a7" + _private = [ + "CLAUDE" + ".md", + "claude_" + "2scraper", + "2scraper_" + "repo_rules", + "play" + "book", + ] + _cite_re = re.compile(re.escape(_sect) + r"\s?[0-9]") + try: + _tracked = subprocess.run( + ["git", "ls-files"], cwd=_root, capture_output=True, text=True, + check=True).stdout.split() + except (OSError, subprocess.CalledProcessError): + # Not a git checkout (a container, a tarball): read what is on disk. + _tracked = [os.path.relpath(os.path.join(d, n), _root) + for d, dirs, names in os.walk(_root) + if not any(x in (".git", "__pycache__") + for x in os.path.relpath(d, _root).split(os.sep)) + and not os.path.exists(os.path.join(d, "pyvenv.cfg")) + for n in names] + _cite_files = [f for f in _tracked + if f.endswith((".py", ".md", ".yml", ".yaml", ".toml", + ".txt", ".example", ".cfg")) + or os.path.basename(f) in ("Dockerfile", ".gitignore", + ".dockerignore")] + ok &= check(f"the private-citation scan read something " + f"({len(_cite_files)} files)", len(_cite_files) > 20) + _cites = [] + for f in _cite_files: + try: + body = open(os.path.join(_root, f), encoding="utf-8", + errors="replace").read() + except OSError: + continue + for n, line in enumerate(body.splitlines(), 1): + if _cite_re.search(line) or any(t.lower() in line.lower() + for t in _private): + _cites.append(f"{f}:{n}") + ok &= check("no shipped file cites the family's private notes or a " + "bare section number" + + ("" if not _cites else " -> " + "; ".join(_cites[:6])), + not _cites) + ok &= check("solve_recaptcha no longer takes use_antidetect", "use_antidetect" not in inspect.signature(_cs.solve_recaptcha).parameters) ok &= check("the placeholder local-API endpoint constant is gone", @@ -3645,7 +3694,7 @@ def _verdict(code, allow_block): if not ln.lstrip().startswith("#")] ok &= check("the canary never puts the proxy credential in argv — it " "goes through the environment env_config.py already " - "reads (CLAUDE.md §3, §11)", + "reads", not any("--proxy" in ln for ln in _cwf_code) and "FARFETCH_PROXY: ${{ secrets.FARFETCH_PROXY }}" in _cwf) ok &= check("the proxy-backed job SKIPS rather than fails when the " @@ -3749,7 +3798,7 @@ def check_the_dockerfile_s_copy_list_vs_the_entrypoint_s_i(ok: bool) -> bool: # and no stray .env — but it falls behind, and CI is the only thing that # builds the image. Every repo in this family has shipped an image that # died with ModuleNotFoundError on every invocation, --help included, - # because one module was missing from that list (CLAUDE.md §10). This + # because one module was missing from that list. This # check needs no Docker, and it is what catches a module added today. _dockerfile = open(os.path.join(_repo_root, "Dockerfile"), encoding="utf-8").read() @@ -4017,19 +4066,387 @@ def emit(self, record): return ok +def _load_engine_with_fake_driver(filename: str, fakes: dict): + """Import an engine from disk with its driver library replaced by stubs. + + Runs whether or not the real driver is installed, so the behaviour below + is checked in the core-only CI job too rather than skipping exactly where + nobody looks. sys.modules is restored afterwards. + """ + import importlib.util as _ilu + import types as _types + saved = {k: sys.modules.get(k) for k in fakes} + try: + for name, attrs in fakes.items(): + mod = _types.ModuleType(name) + for k, v in attrs.items(): + setattr(mod, k, v) + sys.modules[name] = mod + spec = _ilu.spec_from_file_location( + "_fake_" + filename[:-3], os.path.join(_repo_root, filename)) + mod = _ilu.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + finally: + for k, v in saved.items(): + if v is None: + sys.modules.pop(k, None) + else: + sys.modules[k] = v + + +def check_audit_2026_10_14_contract_fixes(ok: bool) -> bool: + """output, captcha budget, status parity, CDP connect (2026-10-14)""" + import output_writer as _ow + import env_config + from captcha_solver import CaptchaChallenge as _CC + + # ---- atomic writes: a failed write leaves the previous file whole ------ + with tempfile.TemporaryDirectory() as d: + target = os.path.join(d, "out.json") + good = [Product(url="https://x/a-item-1.aspx", sku="1", title="ok")] + _ow.write_json(good, target) + before = open(target, encoding="utf-8").read() + + class _Boom(Exception): + pass + + real_dump = _ow.json.dump + + def _half_dump(obj, f, **kw): + f.write("[{\"truncated\": ") + raise _Boom() + _ow.json.dump = _half_dump + try: + try: + _ow.write_json(good * 3, target) + except _Boom: + pass + finally: + _ow.json.dump = real_dump + after = open(target, encoding="utf-8").read() + ok &= check("atomic write: a json.dump that fails part-way leaves the " + "previous good file byte-identical", after == before) + ok &= check("atomic write: no temp file is left beside the target", + sorted(os.listdir(d)) == ["out.json"]) + + # Modes: a NEW file gets 0o666 & ~umask, exactly what open() gives; + # an EXISTING file keeps its own mode. Three umasks, because the + # default 022 is the one value where every wrong rule agrees. + modes = {} + for um in (0o022, 0o002, 0o077): + old = os.umask(um) + try: + fresh = os.path.join(d, f"m{um:o}.json") + _ow.write_json(good, fresh) + modes[um] = os.stat(fresh).st_mode & 0o777 + finally: + os.umask(old) + ok &= check(f"atomic write: a new file's mode honours the umask " + f"({ {oct(k): oct(v) for k, v in modes.items()} })", + modes == {0o022: 0o644, 0o002: 0o664, 0o077: 0o600}) + os.chmod(target, 0o640) + _ow.write_json(good, target) + ok &= check("atomic write: an existing file keeps its own mode (0640)", + os.stat(target).st_mode & 0o777 == 0o640) + _ow.write_run_meta(os.path.join(d, "out"), {"status": "complete"}) + ok &= check("atomic write: the sidecar goes through the same writer", + json.load(open(os.path.join(d, "out.meta.json"))) == + {"status": "complete"}) + + # ---- CSV formula neutralisation ---------------------------------------- + with tempfile.TemporaryDirectory() as d: + prefix = os.path.join(d, "f") + rows = [Product(url="https://x/a-item-1.aspx", sku="1", + title='=HYPERLINK("http://evil")', brand="+cmd", + category="@SUM(1)", price=-5.0), + Product(url="https://x/a-item-2.aspx", sku="2", + title="Plain title", brand="-dash", price=10.0)] + with contextlib.redirect_stdout(io.StringIO()): + rc = finish_run(rows, prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="https://x/", + final_url="https://x/") + with open(prefix + ".csv", newline="", encoding="utf-8") as fh: + got = list(csv.DictReader(fh)) + js = json.load(open(prefix + ".json", encoding="utf-8")) + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + ok &= check("CSV: formula-shaped strings get a leading apostrophe", + rc == 0 and got[0]["title"].startswith("'=") + and got[0]["brand"] == "'+cmd" + and got[0]["category"] == "'@SUM(1)" + and got[1]["brand"] == "'-dash") + ok &= check("CSV: numbers are not escaped (a negative stays a number)", + got[0]["price"] == "-5.0") + ok &= check("CSV: an ordinary string is untouched", + got[1]["title"] == "Plain title") + ok &= check("JSON keeps the site's bytes — escaping is CSV-only", + js[0]["title"] == '=HYPERLINK("http://evil")') + ok &= check("sidecar records csv_cells_escaped (4 here)", + meta.get("csv_cells_escaped") == 4) + + # ---- the paid-solve budget --------------------------------------------- + # Every call site, in every engine, read off DISK (not behind an import + # that skips where the driver is absent) passes the page's budget. + for eng in ("playwright_scraper.py", "puppeteer_scraper.py", + "selenium_scraper.py"): + tree = _ast.parse(open(os.path.join(_repo_root, eng), + encoding="utf-8").read()) + calls = [n for n in _ast.walk(tree) if isinstance(n, _ast.Call) + and getattr(n.func, "id", None) == "handle_captcha_if_present"] + ok &= check(f"{eng}: every handle_captcha_if_present call passes a " + f"budget= ({len(calls)} call site(s))", + calls and all(any(k.arg == "budget" for k in c.keywords) + for c in calls)) + + # And behaviour: a blocked page, a 3-exit pool, --solve-captcha always. + # Before the budget this bought one solve per exit — 3 for one page. + class _PWErr(Exception): + pass + + class _PWTimeout(_PWErr): + pass + + pw = _load_engine_with_fake_driver("playwright_scraper.py", { + "playwright": {}, + "playwright.sync_api": {"sync_playwright": None, "Error": _PWErr, + "TimeoutError": _PWTimeout}}) + bought = [] + + class _Resp: + def __init__(self, status): + self.status = status + + class _Page: + url = "https://www.farfetch.com/shopping/kids/items.aspx" + + def goto(self, *a, **k): + return _Resp(200) + + def content(self): + return SAMPLE_AKAMAI_DENIED_DOM_HTML + + def evaluate(self, *a, **k): + return None + + def query_selector_all(self, *a): + return [] + + def wait_for_function(self, *a, **k): + raise _PWTimeout("no products") + + def wait_for_timeout(self, *a): + return None + + def reload(self, *a, **k): + return None + + def screenshot(self, *a, **k): + return None + + class _Session: + page = _Page() + + def relaunch(self): + return None + + from proxy_pool import ProxyPool as _PP + _chal = _CC(kind="recaptcha_v2", sitekey="6Lx", page_url=_Page.url) + pw.detect_recaptcha_v3 = lambda html, url: _chal + pw.detect_recaptcha_in_page = lambda *a, **k: None + pw.reconcile_detections = lambda a, b: a or b + pw.solve_recaptcha = lambda *a, **k: bought.append(1) or "tok" + with tempfile.TemporaryDirectory() as d: + _a = argparse.Namespace( + pages=1, retries=1, retry_delay=0, proxy_block_retries=2, + solve_captcha="always", twocaptcha_key="offline-test", + captcha_api="v2", min_score=0.7, dump_html=None, + out=os.path.join(d, "o"), category=None) + _pool = _PP(["http://h1:1", "http://h2:2", "http://h3:3"]) + with contextlib.redirect_stdout(io.StringIO()), \ + contextlib.redirect_stderr(io.StringIO()): + _out = pw._fetch_one_page(_Session(), _a, _pool, 1, _Page.url) + ok &= check(f"captcha budget: a blocked page retried from 3 exits buys " + f"ONE solve, not one per exit (bought {len(bought)})", + len(bought) == 1 and _out.blocked_by is not None) + + # ---- CDP connect: a refused endpoint is exit 5 with no credential ------ + # Assembled so the credential scan does not read the fixture as a leak. + _secret = "ws://acct-zone-x:" + "hunter2pw" + "@cb.example.invalid:9222" + + class _Chromium: + def connect_over_cdp(self, *a, **k): + raise _PWErr(f"BrowserType.connect_over_cdp: {_secret} 401 Unauthorized\n" + f"Call log:\n - {_secret}\n" + f" - {_secret} 401\n - {_secret}") + + class _PW: + chromium = _Chromium() + try: + pw._connect_remote(_PW(), argparse.Namespace(cdp_endpoint=_secret)) + raised = None + except Exception as e: # noqa: BLE001 + raised = e + ok &= check("CDP connect: a refused endpoint raises CdpConnectError, " + "not the driver's own exception", + isinstance(raised, pw.CdpConnectError)) + ok &= check("CDP connect: the error text carries no credential, though " + "the driver repeated the endpoint three times", + raised is not None and "hunter2pw" not in str(raised) + and "acct-zone-x" not in str(raised)) + ok &= check("CDP connect: a 401 is explained as expired/wrong credentials", + raised is not None and "401" in str(raised)) + from proxy_pool import redact_url_credentials as _ruc + ok &= check("redact_url_credentials masks EVERY occurrence, keeps the host", + _ruc(f"{_secret} and again {_secret}").count("***:***@cb.example." + "invalid:9222") == 2 + and "hunter2pw" not in _ruc(f"{_secret} {_secret}")) + + # ---- env placeholders --------------------------------------------------- + _envs = {"FARFETCH_CDP_ENDPOINT": + "ws://{login}-zone-scraping_browser-country-xx-pid-{profileId}:" + "{password}" + "@cb.2captcha.com:9222", + "FARFETCH_PROXY": "http://{user}:{password}" + "@xx.proxy.example:2334", + "FARFETCH_URL": ""} + _saved_env = {k: os.environ.get(k) for k in _envs} + _log = io.StringIO() + _h = logging.StreamHandler(_log) + env_config.logger.addHandler(_h) + try: + os.environ.update(_envs) + _vals = {k: env_config.env_value(k) for k in _envs} + finally: + env_config.logger.removeHandler(_h) + for k, v in _saved_env.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + ok &= check("env: a value still holding a {…} slot from the documented " + "format reads as unset", _vals["FARFETCH_CDP_ENDPOINT"] is None + and _vals["FARFETCH_PROXY"] is None) + ok &= check("env: the warning names the slot, never the value", + "{login}" in _log.getvalue() and "zone-scraping" not in _log.getvalue()) + ok &= check("env: an EMPTY value is unset without a placeholder warning", + _vals["FARFETCH_URL"] is None + and "FARFETCH_URL" not in _log.getvalue()) + + # ---- the credential scan: relative exclusions, data files, no empty pass + import importlib.util as _ilu2 + _spec = _ilu2.spec_from_file_location( + "_ci_checks_probe", os.path.join(_repo_root, ".github", "ci_checks.py")) + if os.path.exists(_spec.origin): + _cc = _ilu2.module_from_spec(_spec) + _spec.loader.exec_module(_cc) + _fake_key = "ab" * 16 + with tempfile.TemporaryDirectory() as d: + # A clone living under a directory called `venv`. + root = os.path.join(d, "venv", "clone") + os.makedirs(os.path.join(root, "tests")) + open(os.path.join(root, "a.py"), "w").write("x = 1\n") + open(os.path.join(root, "tests", "fixture.json"), "w").write( + '{\\"apiKey\\": \\"' + _fake_key + '\\"}\n') + open(os.path.join(root, ".env.bak"), "w").write( + "TWOCAPTCHA_KEY=" + _fake_key + "\n") + _real_repo = _cc.REPO + _cc.REPO = _cc.Path(root) + try: + with contextlib.redirect_stdout(io.StringIO()): + _fails = _cc.secret_check() + for n in os.listdir(root): + if n != "a.py": + p2 = os.path.join(root, n) + (os.remove(p2) if os.path.isfile(p2) + else __import__("shutil").rmtree(p2)) + os.remove(os.path.join(root, "a.py")) + with contextlib.redirect_stdout(io.StringIO()): + _empty = _cc.secret_check() + finally: + _cc.REPO = _real_repo + ok &= check("credential scan: a clone under a directory named `venv` " + "is still scanned, and a key in a JSON fixture is found", + any("fixture.json" in f for f in _fails)) + ok &= check("credential scan: a renamed .env backup is read", + any(".env.bak" in f for f in _fails)) + ok &= check("credential scan: scanning 0 files is a failure, not " + "'nothing credential-shaped'", bool(_empty)) + else: + _skips.append("credential-scan checks (no .github/ in this tree)") + + # ---- mirror parity: HTTP 403 with an unrecognised body is exit 5 ------- + import asyncio as _asyncio + + class _PPage: + url = "https://www.farfetch.com/shopping/kids/items.aspx" + + async def goto(self, *a, **k): + return _Resp(403) + + async def content(self): + return "Something went wrong" + + async def evaluate(self, *a, **k): + return None + + async def waitForFunction(self, *a, **k): + raise RuntimeError("timeout") + + async def screenshot(self, *a, **k): + return None + + async def setUserAgent(self, *a): + return None + + async def close(self): + return None + + class _PBrowser: + async def pages(self): + return [_PPage()] + + async def version(self): + return "HeadlessChrome/124.0.0.0" + + async def close(self): + return None + + async def _launch(**k): + return _PBrowser() + + async def _connect(**k): + raise RuntimeError("unused") + pp = _load_engine_with_fake_driver("puppeteer_scraper.py", { + "pyppeteer": {"launch": _launch, "connect": _connect}}) + with tempfile.TemporaryDirectory() as d: + _pa = argparse.Namespace( + url=_PPage.url, pages=1, retries=1, retry_delay=0, delay=0, + headless=True, proxy=None, cdp_endpoint=None, + solve_captcha="when-blocked", twocaptcha_key=None, + captcha_api="v2", min_score=0.7, out=os.path.join(d, "o"), + format="json", allow_empty=False, category=None, webhook=None) + with contextlib.redirect_stdout(io.StringIO()), \ + contextlib.redirect_stderr(io.StringIO()): + _rc = _asyncio.run(pp.scrape(_pa)) + ok &= check(f"pyppeteer: HTTP 403 with an unrecognised body is exit 5 " + f"(content never obtained), as in Playwright — not 4 (got {_rc})", + _rc == 5) + return ok + + def main() -> int: ok = True # Checks that could not run because an optional engine library is absent. # Reported at the end: a suite that silently skips part of itself and still # says "all passed" is the same defect as code that reports success without - # checking that what it wanted actually happened. - _skips = [] + # checking that what it wanted actually happened. The list is the + # MODULE-LEVEL `_skips` the section functions append to — a local of the + # same name here once shadowed it, so the report always printed empty. # Which engine LIBRARIES were absent, as names rather than prose. The # engine-smoke CI job runs one venv per engine and has to assert that THIS # engine did not skip while the other two did — which a free-text line - # cannot answer. Printed as one machine-readable line at the end. - _skipped_engines = set() + # cannot answer. Printed as one machine-readable line at the end, from the + # module-level set (never rebound here — see above). ok &= check("parser extracts exactly 2 real products (junk link excluded)", len(products) == 2) ok &= check("first product has correct title/price", @@ -4228,6 +4645,21 @@ def main() -> int: ok = check_canary_yml_the_exit_code_table_it_prints_must_be(ok) ok = check_numeric_flags_are_range_checked_in_every_cli(ok) ok = check_the_dockerfile_s_copy_list_vs_the_entrypoint_s_i(ok) + ok = check_audit_2026_10_14_contract_fixes(ok) + + # The report below is only worth something if it reflects what the + # sections recorded: an engine whose driver library is absent must appear + # in it. A local rebinding of `_skips`/`_skipped_engines` inside main() + # once made this line print empty whatever happened, and the CI job that + # reads it could then never see a skipped engine. + import importlib.util as _ilu + _absent = {name for name, lib in (("playwright", "playwright"), + ("pyppeteer", "pyppeteer"), + ("selenium", "selenium")) + if _ilu.find_spec(lib) is None} + ok &= check("skip report: every engine whose driver is absent is listed in " + "SKIPPED_ENGINES (the report reads the module-level record)", + _absent <= _skipped_engines) print() if _skips: From 723a38ff92e61365b958f110c78d7d4812223f82 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:49:07 +0000 Subject: [PATCH 2/3] Canary fails, naming FARFETCH_PROXY, when it cannot reach the site The no-proxy job is refused from GitHub's datacentre addresses (exit 3 on every one of the last 10 runs) and the proxy job skipped green without its secret, so the badge stayed green while nothing reached farfetch.com. The proxy job now fails with an ::error:: naming the missing secret; the no-proxy job is documented as "datacentre refusal expected" and is the only one that may excuse a block. The suite check that pinned the old skip is rewritten. Co-Authored-By: Claude Opus 5.5 --- .github/canary_check.py | 7 +++---- .github/workflows/canary.yml | 39 +++++++++++++++++++++--------------- CHANGELOG.md | 5 +++++ smoke_test.py | 17 ++++++++++++---- 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/.github/canary_check.py b/.github/canary_check.py index 6eb2f90..2ffc86d 100644 --- a/.github/canary_check.py +++ b/.github/canary_check.py @@ -102,10 +102,9 @@ def check_exit_code(code: int, allow_block: bool) -> int: print(f"::notice::{meaning} This canary runs from a GitHub runner, " f"which is a datacentre address, and no proxy secret is set — so " f"this says the runner's IP was refused, NOT that the site or " - f"this code changed. Reported as a skip rather than a failure, " - f"because a check that is red every morning is a check nobody " - f"reads. Set the FARFETCH_PROXY secret for a signal that can " - f"tell the two apart.") + f"this code changed. This job is documented as 'datacentre " + f"refusal expected'; the production-like job, which needs the " + f"FARFETCH_PROXY secret, is the one that tests real data.") return 0 if not fails: diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 862383c..55226b0 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -14,21 +14,22 @@ name: canary # listed a code (124) no engine returns and had no entry for the codes they # do. A check that is red every morning is a check everyone learns to ignore. # -# reachability-no-proxy — free, no secret, every day. Answers "can an -# ordinary browser on a datacentre IP still get -# this page, and does the parser still work on it?" -# A BLOCK here is reported as a skip, not a -# failure: it says something about the runner's -# address, not about Farfetch. Anything else — +# reachability-no-proxy — free, no secret, every day, and DOCUMENTED AS +# "datacentre refusal expected": farfetch.com +# refused the GitHub runner (exit 3) on every one +# of the last 10 runs before 2026-10-15. A block +# here is a notice, not a failure. Anything else — # 0 products, a fetch failure, a partial run — -# still fails, because those are real news. +# still fails. This job is NOT the data test. # -# production-like — the authoritative signal. Runs only when the -# FARFETCH_PROXY secret is set, and is strict about -# everything including a block, because from a -# residential exit a block IS news. Skips with a -# notice when the secret is absent (a canary -# that cannot pass must SKIP, not fail). +# production-like — THE data test. Needs the FARFETCH_PROXY secret +# (a residential exit) and is strict about +# everything, a block included. When the secret is +# absent the job FAILS with an error naming it: +# a canary that never reaches the site must not +# show green. Until 2026-10-15 it skipped green +# instead, so the badge was green for a month +# while no run reached the site. # # The proxy secret is passed through the ENVIRONMENT, never as a --proxy # argument. env_config.py already reads FARFETCH_PROXY, an unset secret @@ -46,7 +47,7 @@ on: jobs: reachability-no-proxy: - name: reachability (no proxy, datacentre IP) + name: reachability (no proxy, datacentre refusal expected) runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 @@ -131,9 +132,15 @@ jobs: env: HAVE_PROXY: ${{ secrets.FARFETCH_PROXY != '' }} steps: - - name: Explain the skip + - name: Require the proxy secret + # Fails, not skips: without a residential exit this canary cannot + # reach the site, and a job that never reaches the site must not + # end green. The reachability job is no substitute for this one — + # from a datacentre address it is refused every day. if: env.HAVE_PROXY != 'true' - run: echo "::notice::Skipped - no FARFETCH_PROXY secret is set, so there is no residential exit to run from. farfetch.com refuses GitHub's datacentre addresses, so running this job without one would be red every morning while saying nothing about the site. The reachability job above still runs for free. Set the secret to turn this signal on; it is read from the environment by env_config.py and never reaches a command line." + run: | + echo "::error::The FARFETCH_PROXY secret is not set, so this canary cannot reach farfetch.com: the site refuses GitHub's datacentre addresses. Add a residential proxy URL as the repository secret FARFETCH_PROXY (read from the environment by env_config.py, never put on a command line)." + exit 1 - uses: actions/checkout@v7 if: env.HAVE_PROXY == 'true' diff --git a/CHANGELOG.md b/CHANGELOG.md index af8a87f..eab350e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,11 @@ a promise that every flag and exit code is contractually frozen. > records how many cells were escaped as `csv_cells_escaped`. (3) A refused > `--cdp-endpoint` connect now exits 5 with a one-line, credential-free > message instead of a traceback (exit 1). +> (4) The canary's production-like job now FAILS with an error naming +> `FARFETCH_PROXY` when that secret is absent, instead of skipping green. The +> no-proxy job was refused (exit 3) on every one of the last 10 runs, so for a +> month the badge was green while no run reached the site. The canary will +> stay red until the secret is set. ### Fixed diff --git a/smoke_test.py b/smoke_test.py index 2335a69..f33d10e 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -3697,11 +3697,20 @@ def _verdict(code, allow_block): "reads", not any("--proxy" in ln for ln in _cwf_code) and "FARFETCH_PROXY: ${{ secrets.FARFETCH_PROXY }}" in _cwf) - ok &= check("the proxy-backed job SKIPS rather than fails when the " - "secret is absent — a check that is red every morning is " - "a check nobody reads", + # Reversed on 2026-10-15. It used to SKIP green without the secret, + # and the no-proxy job was refused on every run, so for a month the + # badge was green while no run reached the site. A canary that never + # reaches the site must say so in red, naming what is missing. + ok &= check("the proxy-backed job FAILS with an error naming " + "FARFETCH_PROXY when the secret is absent, rather than " + "skipping green", "HAVE_PROXY: ${{ secrets.FARFETCH_PROXY != '' }}" in _cwf - and "::notice::Skipped" in _cwf) + and "::error::The FARFETCH_PROXY secret is not set" in _cwf + and "::notice::Skipped" not in _cwf) + ok &= check("only the job documented as 'datacentre refusal expected' " + "may excuse a block (--allow-block appears once)", + sum(1 for ln in _cwf_code if "--allow-block" in ln) == 1 + and "datacentre refusal expected" in _cwf) ok &= check("BOTH canary jobs exercise pagination (--pages 3): with " "one page a dead next-link stays invisible, which is how " "the silent-single-page bug survived once already", From 155064ae536c4334017000dc0666b601efd8104e Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:52:01 +0000 Subject: [PATCH 3/3] Leave an unstated currency null on the DOM fallback instead of USD The fallback parser wrote `currency or "USD"`, presenting a guess as a fact. It now writes only what the tile states (ISO code, prefixed or bare symbol) and null otherwise. A suite check forces an unattributed price and asserts None; planting the old default turns it red. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 ++++++ product_parser.py | 5 ++++- smoke_test.py | 24 ++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eab350e..9896781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,12 @@ a promise that every flag and exit code is contractually frozen. Scraping Browser endpoint or proxy shape in `.env.example`) now reads as unset instead of being sent as a real credential, and an empty value no longer logs a "still set to the placeholder" warning. +- **The DOM fallback no longer defaults a missing currency to USD.** A tile + price whose currency the reader could not attribute was written as `USD`; + it is now `null`. Currency comes only from structured data, a written ISO + code, or a prefixed or bare symbol on the tile. Every price shape the + fallback recognises today carries one of those, so current output is + unchanged; the default could only have fired on an unrecognised shape. - `.gitignore` covers the capture shapes the engines write themselves (`X.page2`, `X.page3` from `--dump-html`), a bare `.html` at the repo root, and `live/`, `captures/`, `dumps/`, `scratch/`. diff --git a/product_parser.py b/product_parser.py index ad625b6..c5ed074 100644 --- a/product_parser.py +++ b/product_parser.py @@ -548,7 +548,10 @@ def _parse_css_fallback(html: str, base_url: str, category: Optional[str]) -> Li sku=_sku_from_url(full_url), title=title, price=price, - currency=currency or "USD", + # Only what the tile itself states (ISO code, prefixed or bare + # symbol, via _prices_in). Nothing stated is null — never a + # defaulted "USD", which would present a guess as a fact. + currency=currency, original_price=original_price, discount_pct=discount_pct, rating=rating, diff --git a/smoke_test.py b/smoke_test.py index f33d10e..4330e9e 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -4382,6 +4382,30 @@ class _PW: else: _skips.append("credential-scan checks (no .github/ in this tree)") + # ---- currency: never a defaulted USD on the DOM fallback --------------- + # Every shape _PRICE_RE accepts carries a symbol or an ISO code today, so + # the default was reachable only through a price whose currency is not + # recognised. Forced here: a tile price the reader could not attribute + # must reach the row as None, not as a guessed "USD". + import product_parser as _pp + _tile = ('') + _real_prices_in = _pp._prices_in + _pp._prices_in = lambda text: ([120.0], None) + try: + _none = _pp.parse_products(_tile.format("120 €"), + "https://www.farfetch.com/shopping/kids/items.aspx") + finally: + _pp._prices_in = _real_prices_in + _eur = _pp.parse_products(_tile.format("120 €"), + "https://www.farfetch.com/shopping/kids/items.aspx") + ok &= check("DOM fallback: a price whose currency is not stated yields " + "currency None, not a defaulted USD", + len(_none) == 1 and _none[0].price == 120.0 + and _none[0].currency is None) + ok &= check("DOM fallback: a stated symbol still gives its currency (EUR)", + len(_eur) == 1 and _eur[0].currency == "EUR") + # ---- mirror parity: HTTP 403 with an unrecognised body is exit 5 ------- import asyncio as _asyncio