From eaf1834f1146b3926db22b21e9d9aeb90f938e3c Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:55:38 +0000 Subject: [PATCH 1/3] Exit 5 for content never obtained, exit 2 for usage errors, CSV formula guard, umask-respecting outputs - finish_run: zero rows exit 4 only for stop reasons meaning the listing was read to its end; any other reason (failed page, item with no item, profile never parsed, a reason added later) exits 5. - URL/host/robots/--mode/--from refusals are argparse errors (exit 2), checked in parse_args before a browser starts; run() raises a SystemExit(2) subclass. - Atomic writer: new files 0o666 & ~umask, existing files keep their mode. - CSV: text cells starting = + - @ TAB CR LF get a leading apostrophe after list/dict serialisation; csv_cells_escaped recorded in the sidecar. - Selenium: typed --cdp-endpoint exits 2; one from .env is ignored with a notice. - scan_secrets --worktree reads untracked, non-ignored files, skips pyvenv.cfg directories, fails when it read nothing. - pyppeteer CDP connect bounded at 30 s. - Private section references and sibling-repo pointers removed; suite guard added. requirements-playwright aligned with the extra; .gitignore covers non-.html dump names. Co-Authored-By: Claude Opus 5.5 --- .github/ci_checks.py | 3 +- .github/workflows/tests.yml | 2 +- .gitignore | 3 + CHANGELOG.md | 52 +++++++++ CONTRIBUTING.md | 3 +- README.md | 20 ++-- browser_bridge.py | 70 ++++++++++-- env_config.py | 8 +- output_writer.py | 106 ++++++++++++------ puppeteer_scraper.py | 11 +- requirements-playwright.txt | 2 +- selenium_scraper.py | 5 +- smoke_test.py | 215 +++++++++++++++++++++++++++++++++++- tools/scan_secrets.py | 49 +++++++- 14 files changed, 480 insertions(+), 69 deletions(-) diff --git a/.github/ci_checks.py b/.github/ci_checks.py index 2e4522a..9b9b726 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -103,7 +103,8 @@ def sample_check(prefix="sample_output"): def secret_check(): - """Delegate to tools/scan_secrets.py — ONE scanner, not two (§21).""" + """Delegate to tools/scan_secrets.py — ONE scanner, not two, so CI and + the git hooks cannot disagree about what is a secret.""" result = subprocess.run( [sys.executable, str(REPO / "tools" / "scan_secrets.py"), "--worktree"], capture_output=True, text=True, cwd=REPO) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 328235b..b1db0a7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -64,7 +64,7 @@ jobs: run: python3 .github/ci_checks.py --wheel-check - name: No credentials committed - # THE SHIPPED CHECK — one scanner, shared with the git hooks (§21). + # THE SHIPPED CHECK — one scanner, shared with the git hooks. run: python3 .github/ci_checks.py --secret-check engine-smoke: diff --git a/.gitignore b/.gitignore index 305f40e..fffb7a5 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,9 @@ canary*.meta.json *_debug.png *.html.page* *_p[0-9]*.html +# --dump-html with any extension writes _p +*_p[0-9]*.txt +*_p[0-9]*.htm # --- raw research captures --------------------------------------------------- recon/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a9f2b3..3971d26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,58 @@ follow SemVer as closely as a CLI toolkit can: a **patch** means fixes, and a fix may change a default when the old one was wrong — such a change leads its entry in a blockquote. +## [Unreleased] + +> **Exit codes and file modes changed.** A run that ends with zero rows for +> any reason other than a listing read to its end now exits **5** (content +> never obtained) instead of **4** — an item page with no item on it, a +> seller profile that never parsed, a failed page. A wrong host, a +> robots-disallowed URL, the wrong `--mode`, two URLs outside item mode and +> an unreadable `--from` file now exit **2** instead of **1**. Output files +> were created `0600`; they now follow the umask, and an existing file keeps +> its mode. Selenium with a typed `--cdp-endpoint` now exits **2** instead of +> **5**. + +### Fixed + +* **Zero rows read as an empty catalogue.** `finish_run` returned 4 for + every zero-row run that was not blocked, so a removed item or a page that + failed looked like "nothing listed". Only the stop reasons that mean the + listing was read to its end keep 4; any other one, including one added + later, is 5. +* **Usage errors exited 1**, which the exit table calls a crash, and a + missing `--from` file ended in a traceback. They are now argparse errors + (exit 2), checked before any browser starts. +* **Outputs were owner-only.** The atomic writer's temp file is created + `0600` and the rename kept it. New files now get `0o666 & ~umask`; an + existing file keeps its own mode. Applies to the rows, the sidecar and the + attempt sidecar. +* **Selenium and the Scraping Browser.** An endpoint from `.env` (shared by + all three engines) made every Selenium run exit 5 "transport_error". It is + now ignored with a notice; a typed `--cdp-endpoint` is a usage error. +* **The secret scan read tracked files only.** `tools/scan_secrets.py + --worktree` (and `ci_checks.py --secret-check`) now also reads untracked + files that are not ignored, skips any directory holding `pyvenv.cfg`, prints + how many files it read, and fails when it read none instead of reporting + "clean". +* **pyppeteer's Scraping Browser connect is bounded** (30 s); a rejected + handshake could leave it waiting forever. + +### Added + +* **CSV formula guard.** Text cells starting with `=`, `+`, `-`, `@`, tab, CR + or LF get a leading `'` in the CSV only (after lists and dicts are joined); + numbers are untouched and the JSON keeps the site's text. The sidecar + records `csv_cells_escaped`. + +### Changed + +* `requirements-playwright.txt` asks for `playwright>=1.44`, as the + `[playwright]` extra already did. +* `.gitignore` covers `--dump-html` names with a `.txt` or `.htm` extension. +* Comments and docstrings no longer point at internal documents or at other + repositories; each states its reason here. + ## [1.0.2] — 2026-10-10 Fixes from a review of v1.0.1, each reproduced against v1.0.1 before it diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b72f18a..154c85f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -71,7 +71,8 @@ wrong: - **The wall is recognised by its `
`**, not by the class name, which the wall's own script also mentions. - **Exit codes are a contract**: `0` ok, `1` crash, `2` bad usage, `3` - blocked, `4` zero rows, `5` transport error, `6` partial. A transport fault + blocked, `4` zero rows from a listing read to its end, `5` content never obtained + (transport, or zero rows for any other reason), `6` partial. A transport fault is never `3`. - **A run that finds nothing writes nothing.** `--allow-empty` is the opt-out. - **No phone, no IMEI, no private seller's placeholder name** in any row. diff --git a/README.md b/README.md index 9c1b0ce..db46eed 100644 --- a/README.md +++ b/README.md @@ -252,7 +252,7 @@ you operate applies to that data — in Russia, Federal Law 152-FZ. |---|---|---| | `playwright_scraper.py` | yes — **recommended** | yes | | `puppeteer_scraper.py` | yes | yes | -| `selenium_scraper.py` | **no** — chromedriver's `debuggerAddress` has nowhere to put the endpoint's password | yes | +| `selenium_scraper.py` | **no** — chromedriver's `debuggerAddress` has nowhere to put the endpoint's password (a typed `--cdp-endpoint` exits 2; `AVITO_CDP_ENDPOINT` from `.env` is ignored with a notice) | yes | All three share one CLI and one run loop (`browser_bridge.py`), so they agree on exit codes and on when to wait, solve or rotate. Live-verified on @@ -277,17 +277,23 @@ pyppeteer is effectively unmaintained; it is here for parity. |---|---| | `0` | Complete | | `1` | Crash — an unexpected exception: a bug, not the site and not the transport (traceback printed, credentials masked) | -| `2` | Bad usage — wrong host, wrong `--mode` for the URL, a robots-disallowed URL | +| `2` | Bad usage — wrong host, wrong `--mode` for the URL, a robots-disallowed URL, an unreadable `--from` file | | `3` | Blocked — the firewall stayed up after every rung | -| `4` | Zero results — including a search with no matches | -| `5` | Transport error — **our** plumbing: a navigation timeout, a dead exit, `profile_locked`, `proxy_error` | +| `4` | Zero results — the listing was read to its end and is empty, including a search with no matches | +| `5` | Content never obtained — **our** plumbing (a navigation timeout, a dead exit, `profile_locked`, `proxy_error`), or zero rows for any reason other than a listing read to its end: a failed page, an item page with no item on it, a seller profile that never parsed | | `6` | Partial — some pages fetched, then stopped | -`3` and `5` mean different things: `5` says the site was never reached, so -the answer is "run it again" or "rebuild the endpoint", not the firewall. +`3` and `5` mean different things: `5` says the content was never obtained, +so the answer is "run it again", "rebuild the endpoint" or "check the URL", +not the firewall. `4` is only for a run that read the listing to its end. A run that finds nothing writes nothing — last night's output is not replaced -with `[]` (`--allow-empty` is the opt-out). `.meta.json` describes the +with `[]` (`--allow-empty` is the opt-out). Output files get the mode +`open()` would give them under your umask, and an existing file keeps its +own. In the CSV, a text cell starting with `=`, `+`, `-`, `@`, a tab or a +line break is prefixed with `'` so a spreadsheet does not run it as a +formula; the JSON keeps the site's text, and `csv_cells_escaped` in the +sidecar counts the cells that differ. `.meta.json` describes the data beside it; `.latest_attempt.meta.json` is written on EVERY ending — success, block, transport error or crash, with `exit_code` and `error_class` — and records `transport_facts`: walls met, which rung cleared each, every solve with its time and cost, rotations, and auto-solve events. diff --git a/browser_bridge.py b/browser_bridge.py index 3d367df..b172064 100644 --- a/browser_bridge.py +++ b/browser_bridge.py @@ -440,12 +440,31 @@ def _new_facts(driver, args) -> Dict[str, Any]: } +class UsageError(SystemExit): + """A run refused before it fetched anything: exit 2, like argparse. + + Raised as `SystemExit("message")` these used to exit 1, which the exit + table calls a crash. `str()` is the message; the exit code is 2. + """ + + def __init__(self, message: str): + super().__init__(2) + self.message = message + + def __str__(self) -> str: + return self.message + + def _input_urls(args) -> List[str]: # `--url` is repeatable; AVITO_URL from .env arrives as one string. urls = [args.url] if isinstance(args.url, str) else list(args.url or []) if getattr(args, "from_file", None): - with open(args.from_file, encoding="utf-8") as handle: - data = json.load(handle) + try: + with open(args.from_file, encoding="utf-8") as handle: + data = json.load(handle) + except (OSError, ValueError) as exc: + raise UsageError("cannot read --from %s: %s" + % (args.from_file, exc)) from None for row in data if isinstance(data, list) else []: if isinstance(row, dict) and row.get("url"): urls.append(row["url"]) @@ -459,31 +478,40 @@ def _input_urls(args) -> List[str]: def _check_urls(urls: List[str], mode: str) -> None: if not urls: - raise SystemExit("nothing to fetch: pass --url (repeatable) or --from FILE.json") + raise UsageError("nothing to fetch: pass --url (repeatable) or --from FILE.json") for u in urls: ok, reason = P.supported_host(u) if not ok: - raise SystemExit("refusing %s: %s" % (u, reason)) + raise UsageError("refusing %s: %s" % (u, reason)) allowed, rule = P.robots_verdict(u) if not allowed: - raise SystemExit("refusing %s: avito.ru's robots.txt says %s. Narrow a " + raise UsageError("refusing %s: avito.ru's robots.txt says %s. Narrow a " "listing by its PATH (city / category), not by " "sort or filter parameters." % (u, rule)) kind = P.page_kind(u) if kind != mode: - raise SystemExit("refusing %s: that is %s page, not a %s page — use " + raise UsageError("refusing %s: that is %s page, not a %s page — use " "--mode %s" % (u, "an item" if kind == "item" else "a " + kind, mode, kind)) +def _validated_urls(args) -> List[str]: + urls = _input_urls(args) + _check_urls(urls, args.mode) + if args.mode != "item" and len(urls) > 1: + raise UsageError("--mode %s takes one --url per run" % args.mode) + return urls + + def run(args, driver) -> int: """Drive one browser engine end to end and return its exit code.""" log = (lambda m: print(m, file=sys.stderr)) if getattr(args, "verbose", False) \ else (lambda m: None) - urls = _input_urls(args) - _check_urls(urls, args.mode) - if args.mode != "item" and len(urls) > 1: - raise SystemExit("--mode %s takes one --url per run" % args.mode) + try: + urls = _validated_urls(args) + except UsageError as exc: + print("error: %s" % exc, file=sys.stderr) + raise facts = _new_facts(driver, args) rows: List[Any] = [] @@ -933,9 +961,23 @@ def build_parser(description: str): return p -def parse_args(parser, argv=None): +def parse_args(parser, argv=None, *, cdp_supported: bool = True): args = parser.parse_args(argv) + typed_cdp = args.cdp_endpoint env_config.apply(args) + if not cdp_supported and args.cdp_endpoint: + if typed_cdp: + parser.error( + "this engine cannot use --cdp-endpoint: chromedriver's " + "debuggerAddress takes a bare host:port and has nowhere to put " + "the endpoint's password. Use playwright_scraper.py or " + "puppeteer_scraper.py for the Scraping Browser API.") + # From .env: the file is shared by all three engines, so it is not a + # request for THIS one. Say so and launch a local browser. + print("[i] ignoring AVITO_CDP_ENDPOINT from the environment: this " + "engine cannot use the Scraping Browser API; launching a local " + "browser.", file=sys.stderr) + args.cdp_endpoint = None if args.concurrency > 1: parser.error( "--concurrency above 1 is refused on avito.ru: its firewall is a " @@ -945,6 +987,12 @@ def parse_args(parser, argv=None): "processes on different exits instead.") if args.from_file and args.mode != "item": parser.error("--from is for --mode item") + # The URL checks run here too, so a wrong host, a robots-disallowed URL + # or the wrong --mode is an argparse error (exit 2), before any browser. + try: + _validated_urls(args) + except UsageError as exc: + parser.error(str(exc)) if args.cdp_endpoint: # Never applied to the remote browser — it brings its own exit. diff --git a/env_config.py b/env_config.py index 3e4d7ee..630623f 100644 --- a/env_config.py +++ b/env_config.py @@ -66,12 +66,8 @@ # a copied example reads as CONFIGURED: `cp .env.example .env` followed by a # run connects to cb.2captcha.com with the string `{login}-zone-...` as its # username and gets a 401 -- a confusing auth error a long way from its -# cause. This exact defect shipped in this family's etsy-scraper and -# a sibling repo (both of which use braced placeholders) and was absent -# from amazon-scraper/a sibling repo only because neither of those uses -# them -- see 2scraper family rule. Any `{...}` left in a value is unset here, -# named rather than quoted in the warning, since the value can be a -# credentialled URL. +# cause. Any `{...}` left in a value is unset here, named rather than +# quoted in the warning, since the value can be a credentialled URL. _BRACED_PLACEHOLDER_RE = re.compile(r"\{[^{}]+\}") _loaded_from = None diff --git a/output_writer.py b/output_writer.py index f62f2e1..9b5746e 100644 --- a/output_writer.py +++ b/output_writer.py @@ -35,7 +35,7 @@ import json from dataclasses import dataclass, asdict, field, fields from datetime import datetime, timezone -from typing import Optional, List, Set, Sequence, Any, Dict, Type +from typing import Optional, List, Set, Sequence, Any, Dict, Tuple, Type SOURCE_DEFAULT = "avito.ru" @@ -207,6 +207,16 @@ def _atomic_write(path: str, text: str) -> None: handle_fd, tmp = tempfile.mkstemp(dir=directory, prefix=".tmp-", suffix=os.path.basename(path)) try: + # mkstemp creates 0600 and os.replace keeps it, which made every + # output owner-only. A new file gets what open() would have given it + # (0o666 under the umask); an existing file keeps its own mode. + try: + mode = os.stat(path).st_mode & 0o7777 + except FileNotFoundError: + umask = os.umask(0) + os.umask(umask) + mode = 0o666 & ~umask + os.chmod(tmp, mode) with os.fdopen(handle_fd, "w", encoding="utf-8", newline="") as handle: handle.write(text) handle.flush() @@ -225,8 +235,28 @@ def write_json(rows: Sequence[Any], path: str) -> None: _atomic_write(path, payload + "\n") -def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> None: - """CSV with the same field order as JSON. +# A spreadsheet runs a cell that starts with one of these as a formula, and +# titles, locations and seller names are written by whoever posted the ad. +FORMULA_PREFIXES = ("=", "+", "-", "@", "\t", "\r", "\n") + + +def _csv_cell(v: Any) -> Tuple[Any, bool]: + """Serialise one value for CSV; return it and whether it was escaped. + + Only STRINGS are escaped, and only after a list or dict has been joined + into one: `-5` stays a number so sums over the column keep working, while + a joined list whose first element looks like a formula is text and is + caught. JSON output is never touched. + """ + v = _csv_value(v) + if isinstance(v, str) and v.startswith(FORMULA_PREFIXES): + return "'" + v, True + return v, False + + +def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> int: + """CSV with the same field order as JSON; return how many cells were + prefixed with an apostrophe to stop a spreadsheet running them. An EMPTY result still carries its header, so a consumer reads a table with no rows instead of failing on a zero-byte file. @@ -236,9 +266,15 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> None: buffer = io.StringIO() writer = csv.DictWriter(buffer, fieldnames=names, extrasaction="ignore") writer.writeheader() + escaped = 0 for row in rows: - writer.writerow({k: _csv_value(v) for k, v in asdict(row).items()}) + out = {} + for k, v in asdict(row).items(): + out[k], hit = _csv_cell(v) + escaped += hit + writer.writerow(out) _atomic_write(path, buffer.getvalue()) + return escaped EXIT_NO_PRODUCTS = 4 @@ -253,34 +289,20 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> None: # Exit code for a run that gathered SOME rows and then stopped early. EXIT_PARTIAL = 6 -# Exit code for a failure in one of THIS PROJECT's own 2Captcha-product calls -# -- the Fingerprint API rejecting a request (bad key, bad --tags, rate -# limit), or a Scraping Browser CDP connection failing (e.g. profile_locked) -# -- as opposed to EXIT_BLOCKED (the TARGET SITE refusing a page) or an -# uncaught crash (1). Per the 2scraper family exit-code contract ("5" = -# "remote API error"). Deliberately NOT what a captcha-solve failure gets: -# per that same document's captcha section, a solver error is a WARNING that -# lets the run continue (see captcha_solver.py and each engine's -# handle_captcha_if_present) -- exit 5 is for calls the user explicitly -# opted into (--fingerprint, --cdp-endpoint) where silently continuing -# without them would hide a billing/plan/profile-lock problem rather than a -# page the site declined to serve. +# Exit code for a run that holds NO rows because the content was never +# obtained: a Scraping Browser connection that failed (e.g. profile_locked), +# a browser that never started, a navigation that timed out on every try, a +# dead or unauthenticated proxy, a page that failed, an item or profile that +# never parsed. As opposed to EXIT_BLOCKED (the site refused the page), +# EXIT_NO_PRODUCTS (the listing was read and is empty) and an uncaught crash +# (1). A captcha-solve failure is not this: it is a warning, and the run +# continues to report what the page really was. EXIT_REMOTE_API_ERROR = 5 class RemoteAPIError(RuntimeError): - """A 2Captcha product call (Fingerprint API, Scraping Browser CDP - connect) failed on its own terms, not the target site blocking a page. - - Raised by fingerprint_client.get_fingerprint and by each engine's - --cdp-endpoint connect path; caught once at each engine's entry point - and mapped to EXIT_REMOTE_API_ERROR, so the three engines cannot drift - on which of 1 (crash) / 3 (blocked) / 5 (remote API error) a given - failure gets -- before this, both paths raised a bare RuntimeError with - no engine catching it, so either failure reached the interpreter as an - unhandled exception and exited 1 (a raw traceback, no run-metadata - sidecar) regardless of which one it actually was. - """ + """A 2Captcha product call failed on its own terms, as opposed to the + target site blocking a page. Mapped to EXIT_REMOTE_API_ERROR.""" def write_run_meta(out_prefix: str, meta: dict, *, suffix: str = ".meta.json") -> str: @@ -379,7 +401,8 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, def save(rows: Sequence[Any], out_prefix: str, fmt: str, - allow_empty: bool = False, row_cls: Type = Product) -> int: + allow_empty: bool = False, row_cls: Type = Product, + stats: Optional[dict] = None) -> int: """Write JSON/CSV and return a process exit code. On zero rows, nothing is written at all unless `allow_empty` — see the @@ -396,7 +419,9 @@ def save(rows: Sequence[Any], out_prefix: str, fmt: str, write_json(rows, f"{out_prefix}.json") print(f"[+] Saved {len(rows)} row(s) -> {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) + if stats is not None: + stats["csv_cells_escaped"] = escaped print(f"[+] Saved {len(rows)} row(s) -> {out_prefix}.csv") return 0 if rows else EXIT_NO_PRODUCTS @@ -417,14 +442,22 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, """Write output + the run-metadata sidecar; return the exit code. Shared by all three browser engines so the status/exit-code mapping - cannot drift between them. See a sibling repo's output_writer.py for - the full rationale; the logic here is unchanged. + cannot drift between them. + + Zero rows means one of three things, and each gets its own code: the + site refused (3), the listing was read to its end and really is empty + (4, only for the stop reasons in COMPLETE_STOP_REASONS), or the content + was never obtained (5) — a page that failed, a profile that never + parsed, any stop reason not known to be complete. An unknown reason + must not read as an empty catalogue. """ import uuid complete = stop_reason in COMPLETE_STOP_REASONS row_cls = ROW_CLASS_BY_MODE.get(mode, Product) run_id = uuid.uuid4().hex[:12] - 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 status = "complete" if (rows and complete) else ("partial" if rows else "failed") @@ -435,6 +468,9 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, start_url=start_url, final_url=final_url, products=len(rows)) base["run_id"] = run_id base["blocked"] = bool(blocked) + if "csv_cells_escaped" in stats: + # Declared, so CSV and JSON differing in those cells is not a surprise. + base["csv_cells_escaped"] = stats["csv_cells_escaped"] if extra: base["transport_facts"] = extra @@ -447,7 +483,9 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, write_attempt_meta(out_prefix, dict(base, data_updated=bool(wrote_output))) if not rows: - return EXIT_BLOCKED if blocked else rc + if blocked: + return EXIT_BLOCKED + return rc if complete else EXIT_REMOTE_API_ERROR if not complete: print(f"[!] Partial run: stopped after {pages_completed} of " f"{pages_requested} page(s) ({stop_reason}). The output holds " diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index 1f6b435..ac42fd7 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -23,10 +23,13 @@ import browser_bridge # Module level, so the offline suite can skip this engine when pyppeteer is -# absent instead of importing cleanly and failing on first launch (§10). +# absent instead of importing cleanly and failing on first launch. from pyppeteer import connect, launch +CDP_CONNECT_TIMEOUT_S = 30 + + class PuppeteerDriver: name = "pyppeteer" @@ -48,7 +51,11 @@ def start(self): self._loop.set_exception_handler(_quiet_detach) if self.args.cdp_endpoint: try: - self._browser = self._run(connect(browserWSEndpoint=self.args.cdp_endpoint)) + # Bounded: a handshake the service rejects never resolves pyppeteer's + # connect future, so without a timeout the run hangs. + self._browser = self._run(asyncio.wait_for( + connect(browserWSEndpoint=self.args.cdp_endpoint), + timeout=CDP_CONNECT_TIMEOUT_S)) except Exception as exc: raise browser_bridge.BridgeError( "could not connect over CDP: %s: %s" diff --git a/requirements-playwright.txt b/requirements-playwright.txt index 457d54c..a1f2e8f 100644 --- a/requirements-playwright.txt +++ b/requirements-playwright.txt @@ -3,4 +3,4 @@ # pip install -r requirements.txt -r requirements-playwright.txt # playwright install chromium # -playwright>=1.40.0 +playwright>=1.44 diff --git a/selenium_scraper.py b/selenium_scraper.py index df822a0..7328f16 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -9,7 +9,8 @@ * **Selenium cannot use an authenticated remote CDP endpoint.** chromedriver's `debuggerAddress` takes a bare host:port with nowhere to put the Scraping - Browser's password, so `--cdp-endpoint` is refused here, not ignored. + Browser's password, so a typed `--cdp-endpoint` is refused as a usage error (exit + 2); one that only comes from `.env` is ignored with a notice. * **It CAN use an authenticated proxy — through a loopback forwarder.** Chrome's `--proxy-server` flag carries no credential; sibling repos stripped it and warned. On avito.ru that would make Selenium useless: the @@ -120,7 +121,7 @@ def stop(self): def main(argv=None) -> int: parser = browser_bridge.build_parser( "Scrape avito.ru listings, items and seller profiles with Selenium.") - args = browser_bridge.parse_args(parser, argv) + args = browser_bridge.parse_args(parser, argv, cdp_supported=False) return browser_bridge.run(args, SeleniumDriver(args)) diff --git a/smoke_test.py b/smoke_test.py index 2bd0574..35d9fe4 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -19,6 +19,7 @@ import argparse import ast import contextlib +import csv import dataclasses import inspect import io @@ -976,6 +977,7 @@ def test_wrong_mode_and_robots_are_refused_before_fetching(): check("refused: %s" % why, False) except SystemExit as exc: check("refused: %s" % why, "refusing" in str(exc)) + eq("refused with exit 2: %s" % why, exc.code, 2) def test_cli_contract(): @@ -1262,7 +1264,7 @@ def test_gitignore_covers_every_env_variant_and_working_files(): def test_fixtures_carry_no_session_or_personal_material(): - """Guarded with PATTERNS, so the NEXT capture is caught too (§10).""" + """Guarded with PATTERNS, so the NEXT capture is caught too.""" scan = _scanner() patterns = [ ("a context token", re.compile(r"context=H4sI")), @@ -1695,6 +1697,217 @@ def test_schema_version_is_per_mode(): versions["item"]) +def test_zero_rows_exit_by_stop_reason(): + """0 rows is 4 only when the run is known to have read the listing to its + end; any other reason means the content was never obtained (5). An item + that no longer exists, or a profile that never parsed, used to exit 4.""" + import output_writer as O + with tempfile.TemporaryDirectory() as tmp: + for reason, want in (("completed", 4), ("pagination_exhausted", 4), + ("no_new_products", 4), ("single_page_mode", 4), + ("pages_failed", 5), ("no_profile", 5), + ("transport_error", 5), ("a_reason_added_later", 5)): + rc = _quiet(O.finish_run, [], os.path.join(tmp, reason), "json", False, + blocked=False, stop_reason=reason, pages_requested=1, + pages_completed=0, start_url=LISTING, final_url=LISTING) + eq("0 rows, stop_reason=%s -> exit %d" % (reason, want), rc, want) + rc = _quiet(O.finish_run, [], os.path.join(tmp, "b"), "json", False, + blocked=True, stop_reason="blocked", pages_requested=1, + pages_completed=0, start_url=LISTING, final_url=LISTING) + eq("0 rows, blocked -> exit 3", rc, 3) + import browser_bridge + gone = "Ошибка 404Такой страницы нет" + with tempfile.TemporaryDirectory() as tmp: + rc = _quiet(browser_bridge.run, _args(mode="item", url=[ITEM_COMPANY], + out=os.path.join(tmp, "o")), + FakeDriver([gone])) + eq("an item page with no item on it exits 5, not 4", rc, 5) + + +def test_usage_errors_exit_2(): + """README: exit 2 for a wrong host, a wrong --mode or a robots-disallowed + URL. They were SystemExit("message"), which exits 1.""" + import browser_bridge + for argv, why in ((["--url", LISTING + "?s=104"], "robots-disallowed"), + (["--url", "https://m.avito.ru/moskva"], "foreign host"), + (["--url", ITEM_COMPANY], "item url in listing mode"), + (["--mode", "item", "--from", "/nonexistent/rows.json"], "missing --from"), + (["--url", LISTING, "--url", LISTING + "/x"], "two urls in listing mode")): + try: + _quiet(browser_bridge.parse_args, browser_bridge.build_parser("x"), argv) + check("parse_args refuses: %s" % why, False) + except SystemExit as exc: + eq("parse_args refuses with exit 2: %s" % why, exc.code, 2) + try: + _quiet(browser_bridge.run, _args(mode="item", url=None, from_file="/nonexistent.json"), + FakeDriver([])) + check("run() refuses an unreadable --from", False) + except SystemExit as exc: + eq("run() refuses an unreadable --from with exit 2", exc.code, 2) + + +def test_output_file_modes(): + """mkstemp creates 0600 and os.replace kept it: every output was + owner-only. New files follow the umask; an existing file keeps its mode.""" + import output_writer as O + old = os.umask(0o022) + try: + with tempfile.TemporaryDirectory() as tmp: + for umask, want in ((0o022, 0o644), (0o002, 0o664), (0o077, 0o600)): + os.umask(umask) + path = os.path.join(tmp, "new_%o.json" % umask) + O.write_json([O.Product(sku="1")], path) + eq("new file under umask %03o is %03o" % (umask, want), + oct(os.stat(path).st_mode & 0o777), oct(want)) + os.umask(0o022) + path = os.path.join(tmp, "kept.json") + open(path, "w").close() + os.chmod(path, 0o664) + O.write_json([O.Product(sku="1")], path) + eq("an existing 664 file stays 664", oct(os.stat(path).st_mode & 0o777), oct(0o664)) + _quiet(O.write_run_meta, os.path.join(tmp, "m"), {"status": "x"}) + eq("the sidecar follows the umask too", + oct(os.stat(os.path.join(tmp, "m.meta.json")).st_mode & 0o777), oct(0o644)) + finally: + os.umask(old) + + +def test_csv_formula_cells_are_neutralised(): + """Titles, locations and seller names are written by whoever posted the + ad. A CSV cell starting = + - @ TAB CR LF runs as a formula in a + spreadsheet; JSON keeps the site's bytes.""" + import output_writer as O + row = O.Product(sku="1", title="=HYPERLINK(\"http://x\")", price=-5.0, + currency="RUB", location="+79001234567") + with tempfile.TemporaryDirectory() as tmp: + prefix = os.path.join(tmp, "o") + _quiet(O.finish_run, [row], prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, pages_completed=1, + start_url=LISTING, final_url=LISTING, extra={"csv_cells_escaped": "caller"}) + with open(prefix + ".csv", newline="", encoding="utf-8") as handle: + got = list(csv.DictReader(handle))[0] + data = json.load(open(prefix + ".json", encoding="utf-8"))[0] + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + eq("title escaped in CSV", got["title"], "'=HYPERLINK(\"http://x\")") + eq("location escaped in CSV", got["location"], "'+79001234567") + eq("a negative NUMBER stays a number", got["price"], "-5.0") + eq("JSON keeps the site's bytes", data["title"], "=HYPERLINK(\"http://x\")") + eq("sidecar declares the count", meta.get("csv_cells_escaped"), 2) + eq("the caller's own extra is not overwritten", + meta["transport_facts"].get("csv_cells_escaped"), "caller") + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "i.csv") + n = O.write_csv([O.Item(sku="1", images=["=1+1", "ok"], params={"a": "b"})], + path, row_cls=O.Item) + with open(path, newline="", encoding="utf-8") as handle: + got = list(csv.DictReader(handle))[0] + eq("a joined list is escaped after joining", got["images"], "'=1+1 | ok") + eq("and counted", n, 1) + + +def test_worktree_scan_reads_untracked_files_and_skips_venvs(): + """--worktree read tracked files only, so an untracked key-holding file + was 'clean', and a tree with no git scanned 0 files and said 'clean'.""" + import shutil + import subprocess + if not shutil.which("git"): + skip("worktree scan (git absent)") + return + scanner = os.path.join(HERE, "tools", "scan_secrets.py") + key = "".join("%x" % (i % 16) for i in range(7, 39)) # 32 hex, assembled + with tempfile.TemporaryDirectory() as tmp: + repo = os.path.join(tmp, "build", "out", "r") + os.makedirs(os.path.join(repo, "myenv", "lib")) + subprocess.run(["git", "init", "-q", repo], check=True) + open(os.path.join(repo, "ok.py"), "w").write("x = 1\n") + subprocess.run(["git", "-C", repo, "add", "ok.py"], check=True) + open(os.path.join(repo, "myenv", "pyvenv.cfg"), "w").write("home = /usr\n") + open(os.path.join(repo, "myenv", "lib", "vendored.py"), "w").write("# %s\n" % key) + + def scan(cwd): + r = subprocess.run([sys.executable, scanner, "--worktree"], cwd=cwd, + capture_output=True, text=True) + return r.returncode, r.stdout + r.stderr + + rc, out = scan(repo) + eq("a venv (found by pyvenv.cfg) is not scanned", rc, 0) + open(os.path.join(repo, "notes.py"), "w").write("k = '%s'\n" % key) + rc, out = scan(repo) + check("an UNTRACKED file holding a key fails the scan", + rc == 1 and "notes.py" in out, "(rc=%s)" % rc) + os.remove(os.path.join(repo, "notes.py")) + open(os.path.join(repo, ".env.bak"), "w").write("TWOCAPTCHA_KEY=x\n") + rc, out = scan(repo) + check("an untracked .env backup fails the scan", + rc == 1 and ".env.bak" in out, "(rc=%s)" % rc) + bare = os.path.join(tmp, "nogit") + os.makedirs(bare) + rc, out = scan(bare) + check("a tree with no git fails loudly instead of reading as clean", + rc == 1 and "0 files" in out, "(rc=%s)" % rc) + + +def test_selenium_cdp_endpoint_is_a_usage_question(): + """Selenium cannot use the Scraping Browser. A typed --cdp-endpoint is a + usage error (2); the one in .env is shared by every engine and is ignored + with a notice. It used to reach start() and exit 5 'transport_error'.""" + import browser_bridge + endpoint = "ws://u-zone-scraping_browser-country-ru:p@h:9222" + try: + _quiet(browser_bridge.parse_args, browser_bridge.build_parser("x"), + ["--url", LISTING, "--cdp-endpoint", endpoint], cdp_supported=False) + check("a typed --cdp-endpoint is refused", False) + except SystemExit as exc: + eq("a typed --cdp-endpoint is refused with exit 2", exc.code, 2) + saved = os.environ.get("AVITO_CDP_ENDPOINT") + os.environ["AVITO_CDP_ENDPOINT"] = endpoint + try: + args = _quiet(browser_bridge.parse_args, browser_bridge.build_parser("x"), + ["--url", LISTING], cdp_supported=False) + eq("an endpoint from the environment is dropped", args.cdp_endpoint, None) + args = _quiet(browser_bridge.parse_args, browser_bridge.build_parser("x"), + ["--url", LISTING]) + check("and still used by an engine that can", bool(args.cdp_endpoint)) + finally: + if saved is None: + os.environ.pop("AVITO_CDP_ENDPOINT", None) + else: + os.environ["AVITO_CDP_ENDPOINT"] = saved + body = open(os.path.join(HERE, "selenium_scraper.py"), encoding="utf-8").read() + check("selenium_scraper declares it cannot use CDP", "cdp_supported=False" in body) + + +def test_no_private_citations_in_shipped_files(): + """Public files must state their reasons in their own words. A pointer to + an internal document or a bare section number is unreachable to a reader. + The tokens are assembled so this file does not match itself.""" + import re + tokens = ["CLAUDE" + ".md", "claude_" + "2scraper", "2scraper_repo" + "_rules", + "play" + "book " + "§", "template " + "§"] + section = re.compile("§" + r"\s*\d") + hits = [] + for root, dirs, files in os.walk(HERE): + dirs[:] = [d for d in dirs if d not in (".git", "__pycache__", "node_modules") + and not os.path.isfile(os.path.join(root, d, "pyvenv.cfg"))] + for name in files: + if not name.endswith((".py", ".md", ".yml", ".yaml", ".toml", ".txt", + ".cfg", ".example", ".json", ".csv", ".html")) \ + and name not in ("Dockerfile", ".gitignore", ".dockerignore", + "pre-commit", "pre-push"): + continue + path = os.path.join(root, name) + rel = os.path.relpath(path, HERE) + if rel.split(os.sep)[0] in ("live", "build", "dist"): + continue + try: + text = open(path, encoding="utf-8", errors="replace").read() + except OSError: + continue + if any(t in text for t in tokens) or section.search(text): + hits.append(rel) + eq("no private citations in shipped files", sorted(hits), []) + + def main(): tests = [v for k, v in sorted(globals().items()) if k.startswith("test_") and callable(v)] for test in tests: diff --git a/tools/scan_secrets.py b/tools/scan_secrets.py index 528566e..3c40e19 100644 --- a/tools/scan_secrets.py +++ b/tools/scan_secrets.py @@ -17,7 +17,7 @@ machine, before the commit object exists. python3 tools/scan_secrets.py --staged # what is about to be committed - python3 tools/scan_secrets.py --worktree # every tracked file + python3 tools/scan_secrets.py --worktree # tracked + untracked, not ignored python3 tools/scan_secrets.py --range A..B # commits about to be pushed python3 tools/scan_secrets.py --history # every blob that ever existed @@ -141,8 +141,53 @@ def scan_staged() -> int: return _report(findings) +def _in_virtualenv(name: str, cache: dict) -> bool: + """True when any parent directory of `name` holds pyvenv.cfg. + + A virtualenv is recognised by the file that defines one, not by its name: + a venv called anything but `venv`/`.venv` is full of vendored hex, and + walking into it makes the scan fail on pip's code. + """ + import os + parts = name.split("/")[:-1] + for i in range(1, len(parts) + 1): + directory = "/".join(parts[:i]) + if directory not in cache: + cache[directory] = os.path.isfile(os.path.join(directory, "pyvenv.cfg")) + if cache[directory]: + return True + return False + + +def worktree_names(): + """Tracked files AND untracked files that are not ignored. + + Tracked-only missed exactly the file this scanner exists for: an + untracked `.env.bak` or a scratch script with a key in it, one + `git add -A` away from a commit. Ignored files are left out: `.env` + itself lives there by design and cannot be committed without `-f`. + """ + tracked = [n for n in _git("ls-files", "-z").split("\0") if n] + untracked = [n for n in _git("ls-files", "-z", "--others", + "--exclude-standard").split("\0") if n] + cache: dict = {} + untracked = [n for n in untracked if not _in_virtualenv(n, cache)] + return tracked + untracked + + def scan_worktree() -> int: - names = [n for n in _git("ls-files", "-z").split("\0") if n] + import os + top = _git("rev-parse", "--show-toplevel").strip() + if top: + os.chdir(top) # names are repo-relative, as SKIP expects + names = worktree_names() + if not names: + # No git, or not run from the repository: "clean" would be a claim + # about files nobody read. + print("secret scan: 0 files scanned — run it from inside the git " + "checkout", file=sys.stderr) + return 1 + print("secret scan: %d file(s) in the working tree" % len(names)) findings = [] for name in names: try: From 60e72a2f1bdf5bbf71e57578bd1b121ec0b40fdc Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:43:36 +0000 Subject: [PATCH 2/3] Cap paid captcha solves at one per page, across retries and exit rotations A page that stayed walled bought one solve per exit (2 by default, N+1 with --proxy-block-retries N). solve_wall, the only purchase point shared by all three engines, now draws from a per-URL allowance (page_flow.SOLVES_PER_PAGE) checked before buying; rotation re-fetches the page without resetting it. Refusals are counted in transport_facts.solves_refused_by_budget. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 +++++++++- README.md | 4 ++++ browser_bridge.py | 20 ++++++++++++++++++++ page_flow.py | 7 +++++++ smoke_test.py | 38 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3971d26..ad1c0b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,10 @@ entry in a blockquote. > an unreadable `--from` file now exit **2** instead of **1**. Output files > were created `0600`; they now follow the umask, and an existing file keeps > its mode. Selenium with a typed `--cdp-endpoint` now exits **2** instead of -> **5**. +> **5**. **At most one paid captcha solve per page per run:** a page that +> stays walled used to buy one solve per exit (2 with the default +> `--proxy-block-retries 1`, 4 with 3); rotating to a fresh exit still +> re-fetches the page, but no longer buys a second token for it. ### Fixed @@ -40,6 +43,11 @@ entry in a blockquote. files that are not ignored, skips any directory holding `pyvenv.cfg`, prints how many files it read, and fails when it read none instead of reporting "clean". +* **Paid solves are capped per page.** The allowance + (`page_flow.SOLVES_PER_PAGE = 1`) is checked before each purchase, keyed by + the page's URL, and not reset by a retry or by rotating to a fresh exit. A + refused purchase is logged and counted as `solves_refused_by_budget` in + `transport_facts`. The Scraping Browser's own auto-solve is not counted. * **pyppeteer's Scraping Browser connect is bounded** (30 s); a rejected handshake could leave it waiting forever. diff --git a/README.md b/README.md index db46eed..ebcdb0a 100644 --- a/README.md +++ b/README.md @@ -283,6 +283,10 @@ pyppeteer is effectively unmaintained; it is here for parity. | `5` | Content never obtained — **our** plumbing (a navigation timeout, a dead exit, `profile_locked`, `proxy_error`), or zero rows for any reason other than a listing read to its end: a failed page, an item page with no item on it, a seller profile that never parsed | | `6` | Partial — some pages fetched, then stopped | +A page buys at most one paid captcha solve per run +(`page_flow.SOLVES_PER_PAGE`): a fresh exit re-fetches a page that stayed +walled, but does not buy a second token for it. + `3` and `5` mean different things: `5` says the content was never obtained, so the answer is "run it again", "rebuild the endpoint" or "check the URL", not the firewall. `4` is only for a run that read the listing to its end. diff --git a/browser_bridge.py b/browser_bridge.py index b172064..119e099 100644 --- a/browser_bridge.py +++ b/browser_bridge.py @@ -202,6 +202,11 @@ def solve_wall(driver, args, url: str, html: str, facts: Dict[str, Any], log=Non print("[!] firewall captcha (%s) and no TWOCAPTCHA_KEY — continuing without " "solving." % variant, file=sys.stderr) return None + if not _spend_solve(facts, url): + print("[!] firewall captcha (%s) on %s: the paid-solve allowance for this " + "page (%d) is spent — not buying another." + % (variant, url, page_flow.SOLVES_PER_PAGE), file=sys.stderr) + return None import captcha_solver t0 = time.monotonic() try: @@ -244,6 +249,21 @@ def solve_wall(driver, args, url: str, html: str, facts: Dict[str, Any], log=Non return _after_submit(driver, url, record, facts) +def _spend_solve(facts: Dict[str, Any], url: str) -> bool: + """Take one paid solve from this page's allowance; False when none is left. + + Called before the purchase, so a refusal costs nothing. Keyed by URL and + kept in the run's facts, so a retry or a rotation to a fresh exit (which + re-fetches the same URL) draws on the same allowance. + """ + spent = facts.setdefault("paid_solves_by_page", {}) + if spent.get(url, 0) >= page_flow.SOLVES_PER_PAGE: + facts["solves_refused_by_budget"] = facts.get("solves_refused_by_budget", 0) + 1 + return False + spent[url] = spent.get(url, 0) + 1 + return True + + def _after_submit(driver, url: str, record: Dict[str, Any], facts: Dict[str, Any]) -> Optional[str]: """What the site served after a submitted solution. diff --git a/page_flow.py b/page_flow.py index c3a942f..1294335 100644 --- a/page_flow.py +++ b/page_flow.py @@ -53,6 +53,13 @@ # failures and a success that would arrive in the same window. AUTOSOLVE_BUDGET_MS = 25000 +# Paid 2Captcha solves allowed for ONE page (one URL) per run. Checked BEFORE +# each purchase in browser_bridge.solve_wall, the only place a solve is +# bought, and never reset by a navigation retry or by rotating to a fresh +# exit: a fresh exit is a reason to re-fetch, not a fresh allowance. The +# browser's own auto-solve over the Scraping Browser is not counted. +SOLVES_PER_PAGE = 1 + # Readiness anchors per mode — what "the page has painted" means. # A listing is ready when MORE than one card is present: one match can # resolve on an unrelated card-shaped node long before the grid paints diff --git a/smoke_test.py b/smoke_test.py index 35d9fe4..5624e1b 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -1908,6 +1908,44 @@ def test_no_private_citations_in_shipped_files(): eq("no private citations in shipped files", sorted(hits), []) +def test_paid_solves_are_capped_per_page_across_rotations(): + """One page that stays walled used to buy one solve PER EXIT: 2 with the + default --proxy-block-retries 1, 4 with 3. The allowance is per page, + checked before each purchase, and a rotation does not reset it.""" + import browser_bridge + import page_flow + import captcha_solver + real = captcha_solver.solve_geetest_v4 + calls = [] + + def fake_solve(key, cid, url, proxy=None, **kw): + calls.append(proxy) + return {"captcha_id": cid, "lot_number": "l", "pass_token": "p", "gen_time": "1", + "captcha_output": "o", "_cost": "0.00299"} + captcha_solver.solve_geetest_v4 = fake_solve + try: + d = FakeDriver([fixture("wall_captcha.html")] * 10) + args = _args(proxy=GATEWAY_PROXY.replace("region-RU", "region-RU-session-abcdefghi"), + proxy_block_retries=3, twocaptcha_key="k") + args.solver_proxy = args.proxy + with tempfile.TemporaryDirectory() as tmp: + args.out = os.path.join(tmp, "o") + rc = _quiet(browser_bridge.run, args, d) + facts = json.load(open(os.path.join(tmp, "o.latest_attempt.meta.json")))[ + "transport_facts"] + finally: + captcha_solver.solve_geetest_v4 = real + eq("a walled page exits 3", rc, 3) + check("the run did rotate exits", facts["rotations"] >= 1, "(%s)" % facts["rotations"]) + eq("one paid solve for one page, whatever the rotations", len(calls), + page_flow.SOLVES_PER_PAGE) + check("the refused purchases are counted", facts.get("solves_refused_by_budget", 0) >= 1) + f = {} + check("another page has its own allowance", + browser_bridge._spend_solve(f, LISTING) and browser_bridge._spend_solve(f, LISTING + "?p=2")) + check("and the same page does not get a second one", not browser_bridge._spend_solve(f, LISTING)) + + def main(): tests = [v for k, v in sorted(globals().items()) if k.startswith("test_") and callable(v)] for test in tests: From 39856b2965f0e5e38754d149b670ea59791c359c Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:46:44 +0000 Subject: [PATCH 3/3] Canary fails when it could not scrape avito.ru instead of going green Missing TWOCAPTCHA_KEY, AVITO_PROXY or AVITO_BROWSER_ACCOUNT_ID now fails the job with an error naming each one; a live run that does not exit 0 is red. The data check requires sku, url and title on every row instead of a seller-name share, which depends on the category's private/company mix. Co-Authored-By: Claude Opus 5.5 --- .github/ci_checks.py | 12 ++++++++---- .github/workflows/canary.yml | 35 ++++++++++++++++++++--------------- CHANGELOG.md | 6 ++++++ README.md | 14 ++++++++------ smoke_test.py | 23 +++++++++++++++++++++++ 5 files changed, 65 insertions(+), 25 deletions(-) diff --git a/.github/ci_checks.py b/.github/ci_checks.py index 9b9b726..e6fb5ab 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -156,15 +156,19 @@ def run_check(prefix="sample_output"): # each fail quietly in their own way on this site: # price — the grid painted before its prices did # region_slug — every item URL states it; empty means the URL was lost - # seller_name — 49 of 50 cards named a seller when measured + # sku/url/title — the row's identity; a card read without them is a + # parser fault, whatever the category is selling priced = sum(1 for r in rows if r.get("price") is not None) if priced < 0.9 * len(rows): failed.append(f"only {priced}/{len(rows)} rows carry a price") if not all(r.get("region_slug") for r in rows): failed.append("a row has no region_slug") - named = sum(1 for r in rows if r.get("seller_name")) - if named < 0.5 * len(rows): - failed.append(f"only {named}/{len(rows)} rows name a seller") + # Not a seller-name share: how many private vs. company sellers a category + # shows is merchandising, not the parser. + for column in ("sku", "url", "title"): + empty = sum(1 for r in rows if not r.get(column)) + if empty: + failed.append(f"{empty}/{len(rows)} rows have no {column}") inverted = [r for r in rows if r.get("original_price") is not None and r.get("price") is not None and r["original_price"] <= r["price"]] if inverted: diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index ce73334..9ec70b8 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -18,9 +18,11 @@ name: canary # different exit is rejected (measured). # # Needs: secrets TWOCAPTCHA_KEY and AVITO_PROXY (the base gateway login), and -# repository variables AVITO_BROWSER_ACCOUNT_ID / AVITO_BROWSER_PROFILE_ID. -# Without them the job goes GREEN with a ::notice:: — a check that is always -# red teaches everyone to ignore checks. +# repository variable AVITO_BROWSER_ACCOUNT_ID (AVITO_BROWSER_PROFILE_ID is +# optional). Without them the job FAILS, naming what is missing: a green +# canary must mean avito.ru was scraped, and a skip that reads green said +# nothing about the site for weeks. A blocked run (exit 3), a run that never +# reached the site (5) or zero rows is red too. on: schedule: - cron: "23 5 * * *" @@ -33,30 +35,31 @@ jobs: env: # `secrets` is not available in a job-level `if`; presence goes through # job env and is read in each step's `if`. - READY: ${{ secrets.TWOCAPTCHA_KEY != '' && secrets.AVITO_PROXY != '' && vars.AVITO_BROWSER_ACCOUNT_ID != '' }} + HAS_TWOCAPTCHA_KEY: ${{ secrets.TWOCAPTCHA_KEY != '' }} + HAS_AVITO_PROXY: ${{ secrets.AVITO_PROXY != '' }} + HAS_ACCOUNT_ID: ${{ vars.AVITO_BROWSER_ACCOUNT_ID != '' }} steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: python-version: "3.12" - - name: Explain the skip - if: env.READY != 'true' + - name: Require the live-run credentials + # A GitHub runner is not a Russian residential address, so there is no + # live path without these. Missing ones fail the job by name. run: | - echo "::warning::LIVE RUN SKIPPED — this green run proves the workflow is healthy, NOT that avito.ru was scraped." - echo "::notice::Needs secrets TWOCAPTCHA_KEY and AVITO_PROXY and the" - echo "::notice::variable AVITO_BROWSER_ACCOUNT_ID. A GitHub runner is not a" - echo "::notice::Russian residential address; a live run from here without the" - echo "::notice::Scraping Browser would be red every day regardless of the code." + missing=0 + [ "$HAS_TWOCAPTCHA_KEY" = "true" ] || { echo "::error::secret TWOCAPTCHA_KEY is not set — the canary cannot reach avito.ru"; missing=1; } + [ "$HAS_AVITO_PROXY" = "true" ] || { echo "::error::secret AVITO_PROXY (the base Russian gateway login) is not set"; missing=1; } + [ "$HAS_ACCOUNT_ID" = "true" ] || { echo "::error::repository variable AVITO_BROWSER_ACCOUNT_ID is not set"; missing=1; } + exit $missing - name: Install - if: env.READY == 'true' run: | python -m pip install --upgrade pip pip install -r requirements.txt -r requirements-playwright.txt - name: Build a fresh endpoint and its matching solver proxy - if: env.READY == 'true' # Credentials through the ENVIRONMENT, never argv. The tool writes # AVITO_CDP_ENDPOINT and the minted AVITO_PROXY into a 0600 .env and # prints both masked. @@ -71,7 +74,6 @@ jobs: --mint-session --session-minutes 60 --write-env - name: Live run - if: env.READY == 'true' # AVITO_PROXY is deliberately NOT exported here: an exported variable # outranks .env, and the base login would replace the minted session # the endpoint was built with. @@ -87,10 +89,13 @@ jobs: set -e - name: Assert the data, not just the exit code - if: env.READY == 'true' # THE SHIPPED CHECK, not an inline copy (a 2scraper family rule). run: | echo "exit code from the run: $rc" + if [ "$rc" != "0" ]; then + echo "::error::the live run exited $rc (3 blocked, 4 no rows, 5 content never obtained, 6 partial) — the canary did not get a complete scrape" + exit 1 + fi python3 .github/ci_checks.py --run-check --sample-check --output-prefix canary - name: Remove the minted credentials diff --git a/CHANGELOG.md b/CHANGELOG.md index ad1c0b9..353ccb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,12 @@ entry in a blockquote. ### Changed +* **The canary no longer goes green without scraping.** Without + `TWOCAPTCHA_KEY`, `AVITO_PROXY` or `AVITO_BROWSER_ACCOUNT_ID` it fails, + naming the missing one, instead of taking a green skip branch; a live run + that does not exit 0 is red. Its data check no longer requires half the rows + to name a seller (the private/company mix is the category's, not the + parser's) and instead requires `sku`, `url` and `title` on every row. * `requirements-playwright.txt` asks for `playwright>=1.44`, as the `[playwright]` extra already did. * `.gitignore` covers `--dump-html` names with a `.txt` or `.htm` extension. diff --git a/README.md b/README.md index ebcdb0a..65fb9bc 100644 --- a/README.md +++ b/README.md @@ -18,12 +18,14 @@ actually needs. [![engines](https://img.shields.io/badge/engines-Playwright%20%7C%20Selenium%20%7C%20pyppeteer%20%7C%20CDP-informational)](#engines) [![needs a Russian exit](https://img.shields.io/badge/needs-a%20Russian%20exit%20%2B%20a%20browser-orange)](#the-one-thing-you-actually-need) -> **What the canary badge means.** The daily canary scrapes avito.ru only when -> the repository holds the secrets for a Russian exit and a captcha key. This -> public repository does not, so the canary takes its documented SKIP branch: -> a green badge here says the workflow is healthy, **not** that avito.ru was -> scraped today. Each skipped run says so in a warning. The live measurements -> are in [Measured results](#measured-results), with their dates. +> **What the canary badge means.** The daily canary scrapes three pages of +> avito.ru through the Scraping Browser API on a Russian exit and is green only +> when that run is complete and its rows pass the data checks. It needs the +> secrets `TWOCAPTCHA_KEY` and `AVITO_PROXY` and the variable +> `AVITO_BROWSER_ACCOUNT_ID`; without them it is **red**, naming what is +> missing, rather than green without having scraped anything. A blocked run, +> one that never reached the site, or zero rows is red too. The live +> measurements are in [Measured results](#measured-results), with their dates. --- diff --git a/smoke_test.py b/smoke_test.py index 5624e1b..f5f4cac 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -1946,6 +1946,29 @@ def fake_solve(key, cid, url, proxy=None, **kw): check("and the same page does not get a second one", not browser_bridge._spend_solve(f, LISTING)) +def test_canary_cannot_go_green_without_scraping(): + """The canary took a green SKIP branch every day for a week: a badge that + said nothing about the site. Missing credentials and a non-zero live run + must both fail the job, and the data check must not lean on a + merchandising share.""" + path = os.path.join(HERE, ".github", "workflows", "canary.yml") + if not os.path.isdir(os.path.join(HERE, ".github")): + skip("canary shape (.github absent)") + return + body = open(path, encoding="utf-8").read() + for name in ("TWOCAPTCHA_KEY", "AVITO_PROXY", "AVITO_BROWSER_ACCOUNT_ID"): + check("a missing %s fails the canary by name" % name, + re.search(r"::error::[^\n]*" + name, body) is not None) + check("no live step is skipped on missing credentials", + "env.READY" not in body and "SKIPPED" not in body) + check("a non-zero live run fails the canary", + 'if [ "$rc" != "0" ]' in body and "exit 1" in body) + checks = open(os.path.join(HERE, ".github", "ci_checks.py"), encoding="utf-8").read() + check("the run check requires sku/url/title on every row", + '("sku", "url", "title")' in checks) + check("and no longer a seller-name share", "rows name a seller" not in checks) + + def main(): tests = [v for k, v in sorted(globals().items()) if k.startswith("test_") and callable(v)] for test in tests: