diff --git a/.env.example b/.env.example index 5f5fdb2..6de8104 100644 --- a/.env.example +++ b/.env.example @@ -130,8 +130,10 @@ BBB_PROXY=http://{user}:{password}@na.proxy.2captcha.com:2334 # only one that is Cloudflare-gated. # # `--text` and `--location` build a search URL for you, so this is optional -# even for a search. -BBB_URL=https://www.bbb.org/search?find_country=USA&find_text=restaurants&find_loc=New+York%2C+NY +# even for a search, and it is commented out so that a copied example does +# not become a default nobody chose. A URL set here is a default only: query +# flags typed on the command line replace it. +# BBB_URL=https://www.bbb.org/search?find_country=USA&find_text=restaurants&find_loc=New+York%2C+NY # Deliberately NOT here: an output prefix. `--out` already carries a # non-empty default, so the loader — which only fills UNSET values — would diff --git a/.github/ci_checks.py b/.github/ci_checks.py index 0d34b70..2df6809 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -81,14 +81,27 @@ HEX32 = re.compile(r"\b[0-9a-f]{32}\b") # Contexts in which a 32-hex string is plainly not a key. Site-specific # entries belong here only once a committed fixture actually carries one — -# CLAUDE.md §16: a family-wide entry copied without checking is how dead -# code spreads. Kept as a CONTEXT allowlist rather than by loosening the +# a family-wide entry copied without checking is how dead code spreads. Kept as a CONTEXT allowlist rather than by loosening the # pattern: a bare 32-hex string anywhere else still fails, which is the # point of the check. HEX32_ALLOWED = ("sha", "hash", "nonce", "example", "md5", "digest", "checksum") -SCANNED_SUFFIXES = (".py", ".md", ".txt", ".yml", ".yaml", ".example") +SCANNED_SUFFIXES = (".py", ".md", ".txt", ".yml", ".yaml", ".example", + ".json", ".csv", ".html", ".toml", ".cfg", ".ini", ".sh") + +# Directories never worth reading, matched against the path RELATIVE to the +# repository root. Matching the absolute path skipped everything once the +# clone itself lived under a directory with one of these names: a clone +# under `~/venv/…` scanned 0 files and reported "nothing credential-shaped". +SKIPPED_DIRS = {".git", "__pycache__", ".venv", "venv"} + + +def _is_env_file(path): + """`.env`, `.env.bak`, `.env.local`, ... — the copies people make of the + one file that holds a key. Their suffix is anything at all, so they are + recognised by name rather than by suffix.""" + return path.name == ".env" or path.name.startswith(".env.") # A directory holding `pyvenv.cfg` is a virtualenv, whatever it is called. @@ -99,7 +112,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 @@ -128,10 +141,12 @@ 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_file(path): continue - if any(part in {".git", "__pycache__", ".venv", "venv"} - for part in path.parts): + if any(part in SKIPPED_DIRS + for part in path.relative_to(REPO).parts[:-1]): continue if _is_in_virtualenv(path): continue @@ -197,9 +212,8 @@ def sample_check(): failed.append("sample_output.csv header differs from output_writer.Business") if not failed: - discounted = sum(1 for r in rows if r.get("original_price")) print(f"ok {len(rows)} rows, {len(expected)} columns, " - f"{discounted} discounted, schema matches") + f"schema matches") return failed @@ -223,6 +237,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 scanned == 0: + # A scan that read nothing has proved nothing, and must not say "ok". + failed.append("the credential scan read 0 files — the exclusions " + "matched the whole repository") 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 59df16a..13aa907 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -12,9 +12,11 @@ # exactly the case this proves. # # The PROFILE path is different — it fetches a Cloudflare-gated page — so that -# half is guarded on a secret and SKIPS with a ::notice:: when the secret is -# absent. A check that is always red teaches everyone to ignore checks -# (CLAUDE.md §11). +# half needs a Scraping Browser endpoint. Without the secret it FAILS with an +# ::error:: naming the secret rather than going green: a skipped job did not +# test the site, and a green badge over an untested path is a claim nobody +# measured. Exit 3 (blocked), exit 5 (never reached the page) and zero rows +# are failures too. # # gh secret set BBB_CDP_ENDPOINT --repo 2scraper/bbb-scraper # @@ -52,7 +54,7 @@ jobs: # >= 3 pages, because with one page pagination is never exercised — # which is exactly how a silent single-page bug survived in a sibling - # repo (§11). + # repo. - name: Run id: run run: | @@ -87,6 +89,9 @@ jobs: meta = json.load(open("canary.meta.json")) rows = json.load(open("canary.json")) + if not rows: + print("::error::The listing run wrote zero rows.") + sys.exit(1) print(json.dumps(meta, indent=1)) failures = [] @@ -173,20 +178,19 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Explain the skip + - name: Require the secret if: env.HAVE_CDP != 'true' run: | - echo "::notice::Skipping the profile run: no BBB_CDP_ENDPOINT secret is set." + echo "::error::Missing secret BBB_CDP_ENDPOINT — the profile canary cannot test the site." echo "A business profile page is Cloudflare-gated, and a GitHub runner is a" - echo "datacenter address, so this half cannot pass without a residential exit." - echo "The LISTING job above needs no credentials and is the real canary." + echo "datacenter address, so this job needs a Scraping Browser endpoint:" echo "" echo " gh secret set BBB_CDP_ENDPOINT --repo 2scraper/bbb-scraper" { - echo "### Profile canary skipped" - echo "No \`BBB_CDP_ENDPOINT\` secret. The listing canary covers the parser;" - echo "this job covers the gated page only." + echo "### Profile canary did not run" + echo "Missing secret \`BBB_CDP_ENDPOINT\`. Nothing about the gated page was tested." } >> "$GITHUB_STEP_SUMMARY" + exit 1 - uses: actions/setup-python@v5 if: env.HAVE_CDP == 'true' @@ -207,7 +211,7 @@ jobs: # Through the ENVIRONMENT, not a --flag: env_config.py already reads # exactly this name, an unset secret arrives as an empty string # which the loader treats as unset, and the credential never reaches - # a command line (§3, §11). + # a command line. BBB_CDP_ENDPOINT: ${{ secrets.BBB_CDP_ENDPOINT }} run: | set +e @@ -224,22 +228,26 @@ jobs: python - <<'PY' import json, os, sys code = int(os.environ["EXIT_CODE"]) + # Every non-zero exit is red: a run that was blocked or never + # reached the page tested nothing, and a green job would say it had. if code == 3: - # Access, not a regression — and not a failure of this repo. - print("::warning::Blocked (exit 3). The Scraping Browser profile " - "was refused or has expired. Nothing was tested; this is " - "access rather than a parser regression.") - sys.exit(0) + print("::error::Blocked (exit 3). The page was refused through " + "the Scraping Browser endpoint; nothing was tested.") + sys.exit(1) if code == 5: - print("::warning::Remote API error (exit 5). If the log says " - "'profile_locked', another run still holds this pid — give " - "CI its own. Otherwise the secret has expired.") - sys.exit(0) + print("::error::Nothing was obtained (exit 5). Read the log for " + "the endpoint's own answer: 'profile_locked' means another " + "run holds this pid; a 401 means the credentials were " + "refused.") + sys.exit(1) if code != 0: print(f"::error::The profile run exited {code}.") sys.exit(1) rows = json.load(open("canary_profile.json")) + if not rows: + print("::error::The profile run wrote zero rows.") + sys.exit(1) row = rows[0] failures = [] if len(rows) != 1: diff --git a/.gitignore b/.gitignore index 2b35944..7c60cb8 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,24 @@ live_results.zip *_debug.html *_debug.png *.html.page* +# `--dump-html X --pages 3` writes X, X.page2, X.page3 whatever X is called, +# and `*.json` does not cover `canary_dump.json.page3`. +*.page[0-9]* +# A raw page saved at the top of the repo while debugging. The landing page +# is the one HTML file that is committed. +/*.html +!/landing.html +# Working directories for live runs, by KIND rather than by one name: a +# 1.5 MB dump reached `git add -A` in a sibling through `live/`, which no +# pattern listed. +live/ +out/ +output/ +runs/ +results/ +tmp/ +scratch/ +dumps/ # --- captures kept for parser work --------------------------------------- # The smoke suite's fixtures are committed (trimmed, inside smoke_test.py); @@ -72,8 +90,6 @@ secrets* # a `.env.hold` made while testing that was committed WITH A LIVE KEY in it, # 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. +# repository, one `git add -A` from publication. .env* !.env.example diff --git a/CHANGELOG.md b/CHANGELOG.md index 520c2fb..a730e2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,8 +24,59 @@ When it does, the release notes lead with it. > in `pages_failed` with `stop_reason: "unreadable_response"`. A pipeline > that branched on exit 0 was reading an unread page as the end of a listing. +> **Behaviour change:** a URL BBB answers with its own 404 page now stops at +> once with `stop_reason: "not_found"` (exit 5 when nothing was gathered). +> It used to wait 30 s, re-fetch, and end as `unreadable_response`. + +> **Behaviour change:** negative values for `--pages`, `--delay`, +> `--retry-delay`, `--proxy-block-retries` and `--concurrency` (and 0 for +> `--pages` and `--concurrency`) are now usage errors (exit 2) in every +> engine, and so is a negative `--retries`/`--retry-delay` in +> `scraper_api_client.py`. + +> **Behaviour change:** CSV cells that begin with `=`, `+`, `-`, `@`, a tab +> or a line break are now prefixed with an apostrophe so a spreadsheet does +> not run them as formulas. The JSON output is unchanged; the sidecar says +> how many cells differ in `csv_cells_escaped`. Output files are now written +> through a temporary file and renamed, keeping the mode `open()` would give +> a new file and the existing mode of a file being replaced. + ### Fixed +- **The Selenium and pyppeteer engines crashed on a block with no proxy.** + With no `--proxy`, a blocked page (and, in pyppeteer, a dead exit) reached + `pool.current`/`pool.advance` on None: a traceback, exit 1, and every row + from earlier pages lost. Both now re-fetch on the same path, as the + Playwright engine always did, and report exit 3. +- **An interrupted write destroyed the previous good output.** `out.json`, + `out.csv` and `out.meta.json` were truncated before being written, so a + failure halfway left a short or empty file. They are now written to a + temporary file beside the target, synced and renamed over it. +- **The HTTP status never reached the classifier.** Playwright and + pyppeteer now pass the response status, so BBB's 404 is terminal and a + marker-less 403 is a block. Selenium has no status to pass; BBB's own 404 + title is recognised in the body for it. +- **A `BBB_URL` from `.env` blocked `--text`/`--location`.** A URL from the + environment is now a default that the query flags replace; a TYPED + `--url` with query flags is still refused. `.env.example` ships `BBB_URL` + commented out. +- **The credential scan judged exclusions on the absolute path**, so a + clone under a directory named `venv` scanned 0 files and reported clean. + Exclusions are now relative to the repository, a scan of 0 files fails, + and `.env*` copies plus `.json`, `.csv` and `.html` files are read too. +- **Dump and capture files were not ignored**: `X.page3` from + `--dump-html`, a raw `.html` at the repository root, and `live/`, + `out/`-style working directories. +- **The profile canary went green without testing anything.** With no + `BBB_CDP_ENDPOINT` secret it skipped with a notice, and with one it + passed on exit 3 or 5. It now fails with an error naming the missing + secret, and every non-zero exit or empty result is red. The listing + canary needs no secret and stays scheduled; it now also fails explicitly + on zero rows. +- `--solve-captcha` help, a docstring and the README's exit-code table + described refusals and exit 5 inaccurately; corrected. Comments no longer + point at documents a reader of this repository cannot open. + - **`--retries 0` fetched nothing and called it a result.** The load loop is `range(1, retries + 1)`, so 0 never runs it and the engine judged `about:blank`. Three third-party audits of this family ran exactly that and diff --git a/README.md b/README.md index 3c358a3..7be0903 100644 --- a/README.md +++ b/README.md @@ -240,7 +240,7 @@ Nothing here integrates a competitor. | 2 | bad usage | | 3 | blocked — Cloudflare, distinct from an empty result | | 4 | zero businesses — the query matched nothing | -| 5 | remote API error (the Scraping Browser or Scraper API) | +| 5 | nothing was obtained — a remote API error (the Scraping Browser or Scraper API), a navigation timeout, a dead proxy, a page that could not be read, or a URL BBB answers with its own 404 | | 6 | partial — some pages came back and some did not | **A run that finds nothing writes nothing**, so a failure never replaces last diff --git a/captcha_solver.py b/captcha_solver.py index a1fb1b1..8595af6 100644 --- a/captcha_solver.py +++ b/captcha_solver.py @@ -732,4 +732,4 @@ class CaptchaUnsolvable(RuntimeError): # what the address has been doing — a narrow list is how a challenge gets # reported as an empty page months later. A solver for a challenge this site # has never been observed to serve is dead code; a DETECTOR for one is cheap -# insurance. See the family note in CLAUDE.md. +# insurance. diff --git a/diff_runs.py b/diff_runs.py index 5e8f4bc..8c36954 100644 --- a/diff_runs.py +++ b/diff_runs.py @@ -65,7 +65,7 @@ # What is worth watching on a business directory, and nothing else. # # A price monitor's fields are absent because BBB has no prices — porting -# them would be dead code that looks load-bearing (CLAUDE.md §4). What +# them would be dead code that looks load-bearing. What # changes here is a business's STANDING and its CONTACT DETAILS, and those # are the two things anyone diffs a directory for. # @@ -96,7 +96,7 @@ # against a profile run would report each of them as a change on every row — # and none of it would be about the business. When the two rows disagree on # `data_source`, those fields are reported separately as `source_changed` -# rather than as changes, which is this family's rule (§8: a difference that +# rather than as changes, which is this family's rule (a difference that # comes with a provenance difference says something about our own two # snapshots, not about the site). PROFILE_ONLY_FIELDS = ( diff --git a/env_config.py b/env_config.py index be5ce59..90544f6 100644 --- a/env_config.py +++ b/env_config.py @@ -66,8 +66,8 @@ # A literal-only check reported both of those as CONFIGURED, so `cp # .env.example .env` followed by a run connected to cb.2captcha.com with the # string `{login}-zone-…` as its username and got a 401 — a confusing failure -# a long way from its cause, which is precisely what §3's rule exists to -# prevent. Any `{…}` left in a value means the line was never filled in. +# a long way from its cause, which is precisely what keeping +# credentials in `.env` was meant to prevent. Any `{…}` left in a value means the line was never filled in. _BRACED_PLACEHOLDER_RE = re.compile(r"\{[A-Za-z_][A-Za-z0-9_]*\}") _loaded_from = None diff --git a/fingerprint_client.py b/fingerprint_client.py index e84ce1e..e27f33e 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -331,7 +331,8 @@ 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 is the shape of many defects copied + # code has carried in this family. # # `.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 463be8f..aca79a3 100644 --- a/output_writer.py +++ b/output_writer.py @@ -23,14 +23,14 @@ a fact worth recording. BBB is a business directory, not a shop: it has no price, no currency, no discount, no stock and no product brand, and there is nothing on the page to measure beside those names. Six columns null on every -row of every run of every mode is exactly what CLAUDE.md §9 says must not -exist, so they are not here. `quora-scraper` dropped them for the same +row of every run of every mode is exactly what this family's schema rule says +must not exist, so they are not here. `quora-scraper` dropped them for the same reason and is the precedent. What IS kept, byte-identical and in order, is the family prefix — `source`, `scraped_at`, `url`, `sku`, `title` — so one column name works across the family and a consumer reading several of these repos reads the same first -five columns in the same order (§9). +five columns in the same order. `rating` is absent, and that one is a measurement rather than a definition ------------------------------------------------------------------------ @@ -52,14 +52,17 @@ 0.0 means "BBB has not graded this business", NOT "graded worst". Written through as 0.0 it would drag every average a consumer computes. `parse_row` therefore maps that pair to `rating_grade=None, rating_score=None`, and the -absence is the honest answer (§8: never present a guess as a fact). +absence is the honest answer (never present a guess as a fact). Everything below the dataclass is row-class-agnostic: pass `row_cls` so an empty CSV still gets the right header for the mode that produced it. """ +import contextlib import csv import json +import os +import tempfile from dataclasses import dataclass, asdict, field, fields from datetime import datetime, timezone from typing import Optional, List, Set, Sequence, Any, Type @@ -200,7 +203,7 @@ class Business: # business's officers as individuals ("Mr. Wellington Deleon, # President"). BBB publishes them, and republishing a named person's # details is a separate act from the site showing them on its own page - # (CLAUDE.md §10). A directory of businesses does not need a column of + #. A directory of businesses does not need a column of # people's names to be useful, so there is not one. Adding it later is a # product decision, not an oversight. website: Optional[str] = None @@ -253,7 +256,7 @@ class Business: position: Optional[int] = None # Which mode produced the row. The repo no longer implies it — three # modes share this schema — and `diff_runs.py` refuses to compare two - # runs whose modes it cannot line up (§9). + # runs whose modes it cannot line up. mode: Optional[str] = None # WHICH ORDERING BBB was asked for. This belongs on the row rather than # only in the sidecar, because it changes which businesses are in the @@ -266,7 +269,7 @@ class Business: # "api" BBB's own /api/search JSON — every listing row, whether # an engine read it out of the page or fetched it directly. # "profile" a rendered profile page. - # Provenance in a column, per §8, so `diff_runs.py` can tell a real + # Provenance in a column, so `diff_runs.py` can tell a real # change from a change of source. data_source: Optional[str] = None @@ -334,15 +337,95 @@ def dedupe_by_sku(rows: Sequence[Any], seen: Set[str]) -> List[Any]: def _csv_value(v: Any) -> Any: if isinstance(v, (list, tuple)): return LIST_CSV_SEPARATOR.join(str(x) for x in v) + # A mapping's Python repr is neither readable in a spreadsheet nor + # parseable by anything but Python; compact JSON is both. + if isinstance(v, dict): + return json.dumps(v, ensure_ascii=False, separators=(",", ":")) return v +@contextlib.contextmanager +def _atomic(path: str, newline: Optional[str] = None): + """Write to a temporary file beside `path`, then rename over it. + + `open(path, "w")` truncates before a single byte is written, so a crash, + a kill or a full disk halfway through `json.dump` leaves a SHORTER file + where a complete one was: the previous good run is destroyed by the + attempt to replace it rather than by its outcome. Measured here before + the fix: an interrupted write left `out.json` as 13 bytes of invalid + JSON, and an interrupted CSV write left the file without even its header. + + The temporary file goes in the SAME directory on purpose: `os.replace` + is only atomic within one filesystem, and a temp file under /tmp can be + on another one, where the rename degrades to an interruptible copy. + `fsync` before the rename makes the content durable, not merely visible. + """ + 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 the file 0600 and a rename keeps that + # mode, so without this every output would be readable by its owner + # alone. An existing target keeps its own mode (someone may have set + # it on purpose); a new one gets what `open()` would have given it, + # which honours the umask. Never a flat 0644: 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: + try: + os.unlink(handle.name) + except OSError: + pass + raise + + +# A spreadsheet treats a cell beginning with one of these as a FORMULA, not +# as text, and every string in a row here (business names, descriptions, +# phone lists, addresses) was written by a third party rather than by this +# code. `=HYPERLINK(...)`, `+cmd|...` and `@SUM(...)` are the classic shapes; +# a leading tab or newline is stripped by some readers, exposing whatever +# follows it. +CSV_FORMULA_LEADS = ("=", "+", "-", "@", "\t", "\r", "\n") + +# Prefixed to a cell that would otherwise be read as a formula. Excel, Google +# Sheets and LibreOffice treat the apostrophe as "the rest is text"; a +# consumer parsing the CSV with `csv` sees it, which is why the count goes in +# the sidecar rather than staying silent. +CSV_FORMULA_ESCAPE = "'" + + +def _csv_escape(v: Any): + """Neutralise a formula-shaped cell. Returns (value, was_escaped). + + Only STRINGS are touched: escaping a number would turn a negative + longitude into the text `'-97.7` and break every calculation over the + column. CSV only — the JSON output keeps the site's bytes as served, and + `csv_cells_escaped` in the sidecar declares that the two differ. + """ + if isinstance(v, str) and v.startswith(CSV_FORMULA_LEADS): + return CSV_FORMULA_ESCAPE + v, True + return v, False + + def write_json(rows: Sequence[Any], path: str) -> None: - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump([asdict(r) for r in rows], f, ensure_ascii=False, indent=2) -def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Business) -> None: +def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Business) -> int: + """Write the CSV; 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 @@ -351,11 +434,23 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Business) -> None: # The header comes from `row_cls`, not from the first row, so an empty # run still writes the columns of the mode that produced it. fieldnames = [f.name for f in fields(row_cls)] - with open(path, "w", encoding="utf-8", newline="") as f: + escaped = 0 + # `newline=""` is the csv module's documented requirement and has to + # survive the move to a temp file, or every row gains a stray CR on + # Windows. + with _atomic(path, newline="") as f: writer = csv.DictWriter(f, fieldnames=fieldnames) writer.writeheader() for r in rows: - writer.writerow({k: _csv_value(v) for k, v in asdict(r).items()}) + row = {} + for k, v in asdict(r).items(): + # Escape AFTER `_csv_value`: a list joined into one cell is + # text too, and its first element can be formula-shaped. + value, was_escaped = _csv_escape(_csv_value(v)) + row[k] = value + escaped += was_escaped + writer.writerow(row) + return escaped # Exit code used when a run completes but produced nothing. Distinct from 1 @@ -446,7 +541,10 @@ def write_run_meta(out_prefix: str, meta: dict) -> str: both complete, and between runs of different `mode`. """ path = f"{out_prefix}.meta.json" - with open(path, "w", encoding="utf-8") as f: + # Atomic for the same reason as the rows, and more urgently: this is the + # file a consumer BRANCHES on, so a truncated sidecar beside good rows + # reads as a broken run over data that is fine. + 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 @@ -510,14 +608,15 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, } if extra: # Merged rather than nested under a key, so a consumer reads - # `shop_rating` at the top level beside `products`. Run fields win a + # `total_results` at the top level beside `products`. Run fields win a # name collision: a caller cannot accidentally overwrite `status`. meta.update({k: v for k, v in extra.items() if k not in meta}) return meta def save(rows: Sequence[Any], out_prefix: str, fmt: str, - allow_empty: bool = False, row_cls: Type = Business) -> int: + allow_empty: bool = False, row_cls: Type = Business, + stats: Optional[dict] = None) -> int: """Write JSON/CSV and return a process exit code. Returns 0 when rows were written, EXIT_NO_PRODUCTS when there were none. @@ -544,8 +643,17 @@ def save(rows: Sequence[Any], out_prefix: str, fmt: str, write_json(rows, f"{out_prefix}.json") print(f"[+] Saved {len(rows)} businesses -> {out_prefix}.json") if fmt in ("csv", "both"): - write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) + escaped = write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) print(f"[+] Saved {len(rows)} businesses -> {out_prefix}.csv") + # Reported through `stats` because this function's return IS the + # exit code. The caller 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 rows else EXIT_NO_PRODUCTS @@ -608,8 +716,14 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, # names has a hole for every name nobody added to it. complete = stop_reason in COMPLETE_STOP_REASONS and not pages_failed row_cls = ROW_CLASS_BY_MODE.get(mode, Business) - 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 + if stats: + # Housekeeping goes UNDER the caller's `extra`: a name collision must + # never drop site data in favour of a count. + extra = {**stats, **(extra or {})} if wrote_output: status = "complete" if (rows and complete) else ( diff --git a/page_flow.py b/page_flow.py index f2e354c..0c815a7 100644 --- a/page_flow.py +++ b/page_flow.py @@ -2,7 +2,7 @@ page_flow.py ------------ The retry / solve / blocked decision, as DATA rather than as three copies of -an if-chain (CLAUDE.md §1). +an if-chain. BBB answers a request five ways, and four of them want a different response: @@ -20,7 +20,7 @@ Selenium's `execute_script` takes a function BODY with an explicit `return` while Playwright and pyppeteer take `() => expr`, so a shared snippet would quietly acquire one driver's dialect. The callbacks below are named for the -OPERATION instead, and each engine spells it in its own dialect (§1). +OPERATION instead, and each engine spells it in its own dialect. """ import logging @@ -39,7 +39,7 @@ # How many business records mean "this response is a listing". # -# `> 1` on purpose, per CLAUDE.md §5: waiting for ONE match resolves on +# `> 1` on purpose: waiting for ONE match resolves on # something unrelated long before the listing is really there. BBB serves a # fixed `pageSize` of 15 and every captured page held exactly 15, so four is # a floor that a real page clears instantly and a half-arrived one does not. @@ -47,8 +47,8 @@ # A listing page's own readiness anchor for the browser engines. BBB's search # results are a list of profile links, and a profile path is a six-segment -# contract with search engines rather than a build-generated class — §4's -# "anchor on a URL pattern, never a CSS class". +# contract with search engines rather than a build-generated class: anchor +# on a URL pattern, never on a CSS class. READY_SELECTOR_LISTING = 'a[href*="/profile/"]' # A profile page states the business's name in an `h1`. @@ -74,7 +74,7 @@ def content_timeout_ms(mode: str) -> int: # # Polled through `count(selector)` — a callback each engine implements with # its own `querySelectorAll` call — and NEVER by handing the browser a string -# to evaluate. CLAUDE.md §18: a site whose Content-Security-Policy omits +# to evaluate. Measured on a sibling site: a site whose Content-Security-Policy omits # `unsafe-eval` kills `wait_for_function` with an `EvalError` and takes the # run down with exit 1. BBB has not been measured for that, and the cheap # habit costs nothing on a site that would have allowed it. @@ -112,7 +112,7 @@ def classify(html: Optional[str], status: Optional[int] = None, url=...)` in two of three engines against a callee that took `status` second, and both crashed on their first fetch — invisible to import, `--help`, `compileall` and 400+ green offline assertions, because none of - those calls a function the way a live run does (§17). `smoke_test.py` + those calls a function the way a live run does. `smoke_test.py` binds every engine's call against this signature for that reason. """ return detect_page_state(html or "", status, url) @@ -137,6 +137,12 @@ def classify(html: Optional[str], status: Optional[int] = None, # A fresh browser from a different exit clears it too, which is why # `retry` is also True. "challenge": {"retry": True, "solve": True, "blocked": True, "parse": False}, + # The address does not exist: BBB's own 404 (by status, or by its own + # title where the driver gives no status). TERMINAL — retrying spends a + # fetch on something that will never be there — and not blocked, because + # exit 3 would send the reader after a proxy problem when the URL is + # simply wrong. Not parsed: the page holds no rows. + "not_found": {"retry": False, "solve": False, "blocked": False, "parse": False}, # Served by BBB — its own assets are on the page — but not the listing # payload. A wait, not a spend. "unknown": {"retry": True, "solve": False, "blocked": False, "parse": False}, @@ -164,7 +170,7 @@ def should_parse(state: str) -> bool: # True here, and CONSULTED rather than merely documented — the engines read # it, so setting it False really does stop the retry loop. (A sibling repo # carried this constant with a paragraph of justification and no reader, -# which is the same defect as dead code that looks load-bearing: §17.) +# which is the same defect as dead code that looks load-bearing.) # # True because on BBB a re-fetch genuinely can change the answer. The # Scraping Browser was measured serving a page normally that a datacenter @@ -236,7 +242,7 @@ def pagination_is_addressable(url: str) -> bool: ids with page 1, and the same holds for a category query. BBB paginates by an ordinary `?page=N` on an otherwise identical query. - This is the question CLAUDE.md §18 says to ask PER URL rather than per + This question is asked PER URL rather than per site — a sibling repo has one page kind that paginates and one that does not, where a `page_url()` used unconditionally reported a complete run holding page 1. On BBB both kinds answer the same way, so this returns @@ -265,7 +271,7 @@ def concurrency_limit(cdp_endpoint: Optional[str]) -> Optional[int]: The Scraping Browser API allows ONE live connection per profile, so N workers sharing a `pid` collide with `profile_locked`. Several `pid`s, - one run each, is the way to parallelise that path (§7). + one run each, is the way to parallelise that path. """ return 1 if cdp_endpoint else None @@ -313,3 +319,27 @@ def attempts(value) -> int: f"With 0 the load loop never runs, nothing is fetched, and the " f"engine reports the browser's blank page as a result.") return number + + +def at_least(minimum, kind=int): + """argparse `type=` factory: a number of `kind` no smaller than `minimum`. + + For the flags that count or wait: --pages, --concurrency, --delay, + --retry-delay, --proxy-block-retries. A negative value there is never + meaningful — `range()` quietly runs zero times and `time.sleep` raises + halfway through a run — so it is refused up front, and argparse names + the flag in the error ("argument --pages: must be at least 1, not 0"). + """ + def parse(value): + try: + number = kind(value) + except (TypeError, ValueError): + raise _argparse.ArgumentTypeError( + f"must be {'an integer' if kind is int else 'a number'}, " + f"not {value!r}") + if number < minimum: + raise _argparse.ArgumentTypeError( + f"must be at least {minimum}, not {value}") + return number + parse.__name__ = f"at_least_{minimum}" + return parse diff --git a/playwright_scraper.py b/playwright_scraper.py index 8ecbcf6..ccb19ad 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -30,7 +30,7 @@ NOT behind Cloudflare: measured 2026-09-16, HTTP 200 and 57 KB of JSON from a datacenter address that gets 403 on every HTML page on the site. Which means `--mode search` and `--mode category` need no key and no proxy. Say - so plainly rather than selling a product nobody needs here (§13). + so plainly rather than selling a product nobody needs here. * **A profile page is a different story, and it is the one that needs the paid path.** There is no profile endpoint — /api/businessprofile, /api/profile, /api/orgs, /api/reviews and /api/complaints all return BBB's @@ -167,11 +167,15 @@ class PageOutcome: # COMPLETE run, exit 0, over a page it never read. On page 1 the same # state cost the whole first page of a run that still said complete. unreadable: bool = False + # True when BBB answered with its own 404: the address does not exist. + # Terminal and not a block; its own stop_reason so a reader checks the + # URL rather than the proxy or the parser. + not_found: bool = False @property def ok(self) -> bool: return (not self.load_failed and self.blocked_by is None - and not self.unreadable) + and not self.unreadable and not self.not_found) @property def failure_reason(self) -> str: @@ -180,6 +184,8 @@ def failure_reason(self) -> str: return "page_load_timeout" if self.unreadable: return "unreadable_response" + if self.not_found: + return "not_found" return f"blocked_{self.blocked_by}" @@ -277,7 +283,7 @@ def _resolve_category(session, args): rather than stored here. Resolution REFUSES when nothing plausibly matches instead of taking BBB's first-ranked option: scraping the wrong category while reporting success is exactly the silent failure this - family exists to avoid (§5, §8). + family exists to avoid. """ slug = category_from_url(args.url) probe = api_url(country=args.country or "USA", @@ -631,15 +637,14 @@ def handle_captcha_if_present(page, args, budget=None) -> bool: against each other rather than short-circuited, because they can disagree about the variant and the parameters for one are rejected for the other. - NOTE what this cannot help with. BBB's hard refusal is not a page at - all — the HTTP/2 stream is reset and nothing arrives — so a 2Captcha key - does nothing about the state a blocked run is most likely to meet, and - `detect_page_state` reports that as "blocked" rather than "challenge" - precisely so no solve is attempted or billed. No challenge has ever been - observed on this site. This path exists because a bot manager can be - switched on between deploys, and because the family's rule is that - detection stays broad: - different geos and scenarios surface different challenges. + NOTE what this cannot help with. BBB refuses in two shapes, both HTTP + 403 pages of about 12-15 KB (measured 2026-09-16): a hard "You have been + blocked" refusal with no widget on it, and a Managed Challenge ("Just a + moment...") that is a real test. A 2Captcha key does nothing about the + first, so `detect_page_state` reports it as "blocked" rather than + "challenge" precisely so no solve is attempted or billed. Detection + stays broad on purpose: different exits and scenarios surface different + challenges. """ html = _content_when_settled(page) if html is None: @@ -768,10 +773,15 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) # Retry a navigation timeout rather than ending the run on it. One # network flap on page 12 of 50 should not break the loop. - load_failed, exit_failed = False, None + load_failed, exit_failed, status = False, None, None for attempt in range(1, args.retries + 1): try: - session.page.goto(url, wait_until="domcontentloaded", timeout=60000) + response = session.page.goto(url, wait_until="domcontentloaded", + timeout=60000) + # The HTTP status reaches the classifier: a 404 is a terminal + # answer about the address and a marker-less 403 is a block, + # and neither can be told from the body alone on every page. + status = response.status if response is not None else None load_failed = False break except (PWTimeout, PWError) as e: @@ -805,11 +815,13 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if handle_captcha_if_present(session.page, args, budget): # A solve navigated the page. Give the destination a moment - # before judging what came back. + # before judging what came back — and forget the interstitial's + # status, which describes a document that is no longer there. session.page.wait_for_timeout(1000) + status = None html = _snapshot(session.page, url) or "" - state = _classify(session.page, html) + state = _classify(session.page, html, status) # BBB server-renders its data, so a listing is parseable in the # FIRST response and there is nothing to wait for on a healthy page. @@ -837,7 +849,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome logger.info("Still nothing after %.0fs (%d match(es) for %s).", wait_timeout / 1000, found, _ready_selector(args)) html = _snapshot(session.page, url) or html - state = _classify(session.page, html) + state = _classify(session.page, html, status) # The paid path is reached only for state "challenge" — BBB's # Managed Challenge, which IS a test and can be solved. It is NOT @@ -850,7 +862,10 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if handle_captcha_if_present(session.page, args, budget): session.page.wait_for_timeout(1000) html = _snapshot(session.page, url) or html - state = _classify(session.page, html) + # The status was the challenge's; the page after a solve is + # judged by what it holds. + status = None + state = _classify(session.page, html, status) # The VERIFIED outcome, and the only one worth reporting: a # "ready" task result is not evidence the token works. This # line is what says whether the money bought anything. @@ -906,6 +921,15 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome outcome.state = state + if state == "not_found": + logger.error("%s does not exist — BBB answered with its own 404 " + "page. Nothing was scraped and nothing was retried: " + "check the URL (a withdrawn profile, or a mistyped " + "path), not the proxy.", url) + outcome.not_found = True + outcome.final_url = session.page.url + return outcome + if state == "blocked": # What a caller needs here is WHICH refusal this is, because the two # want different answers and only one of them is solvable. @@ -946,7 +970,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # BBB embeds the whole result set in the first response, so there is # nothing to wait for and nothing to scroll into view. Porting the # sibling repos' scroll loop here would be dead code that looks - # load-bearing (CLAUDE.md §4). + # load-bearing. # Dumping on success, not only on failure: a run can return the right # NUMBER of rows with a field silently unpopulated, and then the only way @@ -1490,7 +1514,7 @@ def parse_args(): "a search URL already carries find_country and a flag " "that disagreed with it would silently scrape a " "different directory than the one in the address. " - "This is the family's --country ban (CLAUDE.md §10) " + "This is the family's --country ban " "applied where it actually bites: BBB has one host " "for both countries, so the flag cannot contradict a " "HOSTNAME — only a query string, which is what the " @@ -1534,7 +1558,7 @@ def parse_args(): "override it — useful when a run's rows should carry " "the label you searched for rather than each " "business's own primary category.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.at_least(1), default=1, help="Number of listing pages to fetch. Applies to --mode " "search and --mode category; ignored in --mode " "profile. BBB caps every query at 15 pages of 15, and " @@ -1543,8 +1567,8 @@ def parse_args(): "the site states on page 1 and never asks for the " "page that errors. A request for more is honoured up " "to that cap and the sidecar records both numbers.") - p.add_argument("--delay", type=float, default=2.0, help="Delay between pages, seconds") - p.add_argument("--concurrency", type=int, default=1, metavar="N", + p.add_argument("--delay", type=page_flow.at_least(0, float), default=2.0, help="Delay between pages, seconds") + p.add_argument("--concurrency", type=page_flow.at_least(1), default=1, metavar="N", help="Fetch pages through N parallel workers (default 1 — " "unchanged sequential behaviour). Each worker runs its " "own browser and holds its own proxy exit, so N>1 " @@ -1556,7 +1580,7 @@ def parse_args(): "that comes back EMPTY is not retried — see " "page_flow.STATE_POLICY — because a query that " "matched nothing is a correct answer, not a fault.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.at_least(0, float), default=2.0, help="Seconds before the first page-load retry, doubling " "thereafter (default 2.0)") p.add_argument("--format", choices=["json", "csv", "both"], default="both") @@ -1580,7 +1604,7 @@ def parse_args(): p.add_argument("--proxy-shuffle", action="store_true", help="Shuffle the pool at startup, so concurrent runs do not " "all begin on the first exit in the file.") - p.add_argument("--proxy-block-retries", type=int, default=2, + p.add_argument("--proxy-block-retries", type=page_flow.at_least(0), default=2, help="When a page comes back refused (HTTP 403) or behind " "a captcha, retry it from this many OTHER exits before " "giving up (default 2). Needs a pool of more than one; " @@ -1591,7 +1615,7 @@ def parse_args(): p.add_argument("--allow-empty", action="store_true", help="Write output files even when 0 rows were found. Off by " "default so a failed run can't overwrite a good result " - "with an empty one; exit code is 4 either way.") + "with an empty one. It does not change the exit code: 4 for a genuinely empty result, 3 if blocked, 5 if nothing was obtained.") p.add_argument("--fingerprint", action="store_true", help="Fetch a browser fingerprint from 2captcha's Fingerprint " "API and apply it to the launched browser. Needs " @@ -1624,13 +1648,10 @@ def parse_args(): help="when-blocked (default): only pay to solve a " "reCAPTCHA if the content is not already readable. " "always: solve whenever one is detected. Neither " - "setting touches BBB's hard refusal, which is not a " - "page at all — the HTTP/2 stream is reset and nothing " - "arrives — so no solve helps there and none is " - "attempted or billed. In fact NO challenge has ever " - "been observed on this site; the path is wired up " - "because a bot manager can be switched on between " - "deploys.") + "setting touches BBB's hard refusal, a 403 page with " + "no widget on it, so no solve is attempted or billed " + "there. BBB's Managed Challenge is a Cloudflare " + "Turnstile, which this repo does not implement.") p.add_argument("--min-score", type=float, default=0.7, help="reCAPTCHA v3 minimum score to request (0.3, 0.7 or 0.9 " "— the API only accepts these three). Ignored for v2 " @@ -1651,16 +1672,28 @@ def parse_args(): args = p.parse_args() # Fill --twocaptcha-key / --cdp-endpoint / --proxy / --url from the # environment or .env when the flag was not given. An explicit flag wins. + # Captured BEFORE the environment fills anything in: after + # `env_config.apply` a URL from BBB_URL is indistinguishable from one the + # user typed, and the conflict check below then refused --text/--location + # for anyone whose .env carried a URL. + typed_url = args.url env_config.apply(args) # --url and the query flags are two ways to say the same thing, and only # one of them may win. Refused rather than merged: a --country that # disagreed with the find_country already in a URL would silently search # a different directory than the address names, which is exactly the - # hazard CLAUDE.md §10 bans a --country flag for. BBB has one host for + # hazard a --country flag is usually refused for. BBB has one host for # both countries, so the flag cannot contradict a HOSTNAME — only a query # string, and this is where that is caught. - if args.url and (args.text or args.location or args.country or args.state): + query_flags = bool(args.text or args.location or args.country or args.state) + if args.url and not typed_url and query_flags: + # The URL came from BBB_URL, not from this command line: it is a + # DEFAULT, and the query flags the user typed take its place. + logger.info("Ignoring BBB_URL from the environment: the query flags " + "on the command line describe the search instead.") + args.url = None + if args.url and query_flags: conflicting = [name for name, value in (("--text", args.text), ("--location", args.location), ("--country", args.country), ("--state", args.state)) @@ -1688,7 +1721,7 @@ def parse_args(): # Refused rather than attempted. This parser reads BBB's own state # object and BBB's profile-path shape; pointing it at another site # would not fail loudly, it would return zero rows and look like an - # empty result (§8). The REASON is given, because "is not a BBB site" + # empty result. The REASON is given, because "is not a BBB site" # about a host that plainly is one sends the reader hunting a typo. p.error(f"{args.url!r} {why}.") diff --git a/product_parser.py b/product_parser.py index d83f363..2d838c8 100644 --- a/product_parser.py +++ b/product_parser.py @@ -2,7 +2,7 @@ product_parser.py ----------------- Extracts business records from bbb.org. This module IS the site knowledge; -the engines around it carry a dozen constants and nothing else (CLAUDE.md §1). +the engines around it carry a dozen constants and nothing else. Where the data comes from, and why it is not the DOM ---------------------------------------------------- @@ -16,7 +16,7 @@ accreditation, categories, coordinates and BBB's own onward links. That is strictly more than the rendered tile shows, and it is the site's own data rather than an inference from markup, so it is the primary path — the same -call CLAUDE.md §4 records for zimmo-scraper, where the site's own embedded +call a sibling made for zimmo-scraper, where the site's own embedded JSON outranks even JSON-LD once confirmed live. There is no JSON-LD fallback here because there is nothing to fall back to: @@ -93,8 +93,8 @@ # values began `/us/` or `/ca/`, and none of them named another host. # # `bbb.org` without `www.` redirects to `www.bbb.org`, so both are accepted -# and both normalise to the canonical one — CLAUDE.md §5's "check whether -# hosts answer on www.", from the other direction. +# and both normalise to the canonical one: whether hosts answer on +# `www.` was checked, not assumed. HOSTS = ("www.bbb.org", "bbb.org") CANONICAL_HOST = "www.bbb.org" @@ -254,7 +254,7 @@ def country_from_path(path: Optional[str]) -> Optional[str]: # --------------------------------------------------------------------------- def is_supported_url(url: str) -> Tuple[bool, str]: - """(supported, reason). The reason is the point (CLAUDE.md §5). + """(supported, reason). The reason is the point. A refusal that says "is not a BBB site" about a host that plainly is one sends the reader looking for a typo, so each refusal names what is @@ -350,7 +350,7 @@ def api_url(*, country: str = "USA", text: str = "", location: str = "", # an EMPTY `find_text` answers HTTP 200 with `totalResults: 0`, # while the same filter with the category's label as the text # answers 234,844. A silent zero that looks like an empty - # category is the worst answer available here (§8), so this + # category is the worst answer available here, so this # cannot be built at all. raise ValueError( "a category filter needs a non-empty `text` as well: BBB " @@ -432,7 +432,7 @@ def pick_category_option(options: Sequence[Dict[str, str]], than scraping whatever BBB ranked first. That is the difference between a run that says "there is no such category, here is what BBB does have" and a run that quietly returns the wrong category's businesses while - reporting success (§5, §8). + reporting success. """ if not options: return None, False @@ -514,7 +514,7 @@ def parse_row(record: Dict[str, Any], *, page: Optional[int] = None, the id is what dedupe and diffing both key on, and a row without one is not comparable to anything. That has not been seen in 105 rows; it is handled because a silent partial row is this codebase's worst bug class - (§8). + . """ if not isinstance(record, dict): return None @@ -636,7 +636,7 @@ def parse_listing(payload: Any, *, page: Optional[int] = None, data = listing_payload(payload) if data is None: # Loudly, and with the reason: "0 businesses" from a page that held - # fifteen is indistinguishable from an empty category (§8). + # fifteen is indistinguishable from an empty category. log.warning("page %s: no listing payload found — neither the " "/api/search JSON nor a __PRELOADED_STATE__.searchResult " "on the page", page) @@ -653,7 +653,7 @@ def parse_listing(payload: Any, *, page: Optional[int] = None, rows.append(row) if len(rows) != len(results): - # Loudly, per §8: a row silently dropped is indistinguishable from a + # Loudly: a row silently dropped is indistinguishable from a # business the site stopped listing. log.warning("page %s: %d of %d results had no id and were skipped", page_number, len(results) - len(rows), len(results)) @@ -717,7 +717,8 @@ def pages_to_fetch(pages_requested: int, pages_available: Optional[int]) -> int: # # The negative measurement matters more than the positive one here, and this # list has been WRONG once already — see the `cf-turnstile` note below, which -# is CLAUDE.md §18's rule biting for the third time in this family. +# is "count a marker on a page you know is good" biting for the third +# time in this family. # # Counted 2026-09-16 across every capture, where "served" means a page BBB # really answered with (five of them fetched through the 2Captcha Scraping @@ -750,8 +751,8 @@ def pages_to_fetch(pages_requested: int, pages_available: Optional[int]) -> int: # good pages and misses half the bad ones is worse than no marker. # # Note what is NOT done about it: the extension's script tags are not -# stripped before matching. §19 says to add that guard "only if your -# marker set can actually match one", and with `cf-turnstile` gone, none +# stripped before matching. That guard is worth adding only if the +# marker set can actually match one, and with `cf-turnstile` gone, none # of the entries below matches anything that extension injects. Adding # the strip would be dead code that looks load-bearing. # @@ -770,7 +771,7 @@ def pages_to_fetch(pages_requested: int, pages_available: Optional[int]) -> int: # BBB HAS ITS OWN CAPTCHA, and it is not the one above. # -# §18's "no challenge rendered is not no captcha configured": every page BBB +# "No challenge rendered" is not "no captcha configured": every page BBB # serves carries a reCAPTCHA **Enterprise** configuration, for its own forms # rather than for readers — # @@ -786,18 +787,26 @@ def pages_to_fetch(pages_requested: int, pages_available: Optional[int]) -> int: # What the hard refusal says. BBB serves it under its OWN branding — the # `` is "You have been blocked | Better Business Bureau®" — so a title -# check calls it a real page, which is CLAUDE.md §18's inverted-detection -# case on a third site. +# check calls it a real page, which is the inverted-detection case on a +# third site. BLOCK_PAGE_MARKERS = ( "You have been blocked", "Sorry, you have been blocked", ) +# BBB's own 404 template names itself in the `<title>`: "Page not found | +# Better Business Bureau(R)" (captured 2026-09-16 for a withdrawn profile +# path). Read from the BODY as well as from the status because Selenium has +# no HTTP status to give the classifier, and a reader that only works for two +# engines of three makes them disagree about the same URL. +NOT_FOUND_TITLE_RE = re.compile( + r"<title>\s*Page not found\s*\|\s*Better Business Bureau", re.I) + # The positive signal, and the one that decides the ambiguous cases: a page # BBB actually served is BUILT OUT OF BBB'S OWN ASSETS. Both interstitials # reference neither host; the served 404 references them three times. # -# This is the `assets.mmsrg.com` trick from §8, and it is what catches the +# This is the `assets.mmsrg.com` trick from a sibling repo, and it is what catches the # failure modes a marker list cannot: the hard block page (which carries no # challenge marker at all), and Chromium's own network-error page (which # carries the site's hostname in its `<title>` and would otherwise read as @@ -836,10 +845,11 @@ def looks_like_api_payload(text: str) -> bool: def detect_page_state(html: str, status: Optional[int] = None, url: str = "") -> str: - """One of "content", "empty", "blocked", "challenge", "unknown". + """One of "content", "empty", "not_found", "blocked", "challenge", + "unknown". Ordered by how much each signal PROVES, not by how cheap it is to - compute — CLAUDE.md §17's classification-order trap. An unambiguous + compute — the classification-order trap. An unambiguous positive ("this response IS the API payload", "this page says it is a challenge") is decided before the asset-reference heuristic, so a real answer that happens to be minimal is never reported as blocked. @@ -847,7 +857,7 @@ def detect_page_state(html: str, status: Optional[int] = None, `status` is positional-friendly and second, matching the rest of the family: every engine calls `classify(html, status, url)` and a signature that disagreed crashed two of three engines on their first fetch in a - sibling repo (§17). + sibling repo. """ text = html or "" @@ -878,6 +888,13 @@ def detect_page_state(html: str, status: Optional[int] = None, if any(marker in text for marker in BLOCK_PAGE_MARKERS): return "blocked" + # 3b. Unambiguous: BBB's own 404, by its status or by its own title. A + # terminal answer about the ADDRESS — a mistyped or withdrawn + # profile URL — so it must not be waited on, retried or called + # blocked. Decided after the refusal markers, which are stronger. + if status == 404 or NOT_FOUND_TITLE_RE.search(text[:4096]): + return "not_found" + # 4. The status, which on this site is a real signal rather than a # formality: BBB answers a refused request 403 and a served page 200 # or 404. A 403 that reached here carries neither vendor marker, which @@ -910,7 +927,7 @@ def parse_products(payload: Any, url: str = "", page: int = 1, `__PRELOADED_STATE__.searchResult`. It RAISES on input that is neither, rather than returning `[]` — a function that quietly returns nothing on input it does not understand is this codebase's most common historical - bug class (§8), and "0 businesses" from a page that had 15 is + bug class, and "0 businesses" from a page that had 15 is indistinguishable from an empty category. """ if listing_payload(payload) is None: @@ -1089,7 +1106,7 @@ def parse_profile(source: Any, url: str = "", *, Returns None rather than an empty row when the page carries no `businessProfile`: "the profile had no data" and "this was not a profile page" are different things, and only the caller knows which it asked - for (§8). + for. `sku` is rebuilt as `{bbbId}_{businessId}_{addressId}` so a profile row JOINS a listing row on the family's own key. It has to be rebuilt @@ -1136,7 +1153,7 @@ def parse_profile(source: Any, url: str = "", *, # null rather than reconstructed from the grade: the listing # measurement shows several distinct scores map to one letter # (A+ spans 97.0 to 100.0), so inverting the mapping would invent a - # precision the page does not have (§8). + # precision the page does not have. rating_score=None, is_accredited=_bool_or_none( _nested(profile, "accreditationInformation", "isAccredited")), diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index 1e55f03..0b5dfc0 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -222,11 +222,15 @@ class PageOutcome: # after every wait and retry. Not a finished page -- see the same field # in playwright_scraper.PageOutcome for what it used to cost. unreadable: bool = False + # True when BBB answered with its own 404: the address does not exist. + # Terminal and not a block; its own stop_reason so a reader checks the + # URL rather than the proxy or the parser. + not_found: bool = False @property def ok(self) -> bool: return (not self.load_failed and self.blocked_by is None - and not self.unreadable) + and not self.unreadable and not self.not_found) @property def failure_reason(self) -> str: @@ -235,6 +239,8 @@ def failure_reason(self) -> str: return "page_load_timeout" if self.unreadable: return "unreadable_response" + if self.not_found: + return "not_found" return f"blocked_{self.blocked_by}" @@ -635,11 +641,14 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) - load_failed = False + load_failed, exit_failed, status = False, None, None for attempt in range(1, args.retries + 1): try: - bridge.run(page.goto(url, {"waitUntil": "domcontentloaded", - "timeout": 60000})) + response = bridge.run(page.goto( + url, {"waitUntil": "domcontentloaded", "timeout": 60000})) + # The HTTP status reaches the classifier, as in the + # Playwright engine. + status = response.status if response is not None else None load_failed = False break except Exception as e: # noqa: BLE001 — pyppeteer raises many types @@ -652,6 +661,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if any(marker in text for marker in _PROXY_ERROR_MARKERS): logger.warning("Exit %s is unusable (%s).", mask(pool.current) if pool else "(none)", text[:120]) + exit_failed = text[:120] break if attempt < args.retries: pause = args.retry_delay * (2 ** (attempt - 1)) @@ -660,8 +670,12 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome args.retries, text[:120], pause) time.sleep(pause) - if load_failed and block_attempt < block_retries: - pool.advance("unusable exit or repeated load failure") + # Rotate only for an unusable EXIT and only when there is another + # one to go to, exactly as the Playwright engine does. With no pool + # this used to call `pool.advance` on None and crash the run with a + # traceback (exit 1), losing every row gathered so far. + if exit_failed and has_pool and block_attempt < block_retries: + pool.advance(f"unusable exit: {exit_failed}") session.relaunch() bridge, page = session.bridge, session.page d = _driver(session) @@ -672,9 +686,10 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if handle_captcha_if_present(session, args, budget): time.sleep(1) + status = None # the interstitial's status, no longer the page's html = _snapshot(session, url) or "" - state = page_flow.classify(html, None, page.url) + state = page_flow.classify(html, status, page.url) # BBB server-renders its data, so a listing is parseable in the # FIRST response. The wait below is only for the state that says BBB @@ -695,22 +710,19 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome logger.info("Still nothing after %.0fs (%d match(es) for %s).", wait_ms / 1000.0, found, sel) html = _snapshot(session, url) or html - state = page_flow.classify(html, None, page.url) + state = page_flow.classify(html, status, page.url) # The paid path is reached only for state "challenge" — BBB's # Managed Challenge, which IS a test. It is NOT reached for # "blocked": that page carries no widget, so a solve there would be a - # charge for nothing. Mirrors the other two engines. - # - # The paid path is reached only for state "challenge", which no - # capture of this site has ever produced. Wired up because a bot - # manager can be switched on between deploys, and bounded by - # SOLVES_PER_PAGE so a speculative path cannot become a bill. + # charge for nothing. Mirrors the other two engines, and bounded by + # SOLVES_PER_PAGE so a retry loop cannot become a bill. if page_flow.should_solve(state) and budget.left: if handle_captcha_if_present(session, args, budget): time.sleep(1) html = _content(session) or html - state = page_flow.classify(html, url=page.url) + status = None + state = page_flow.classify(html, status, page.url) if state == "content": logger.info("The solve was accepted — page %d is content " "now.", page_num) @@ -730,13 +742,25 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # so a different exit is the only thing that plausibly changes the # outcome. if block_attempt < block_retries: - logger.warning("Page %d came back as %s from %s — retrying from " - "another exit (%d/%d).", page_num, state, - mask(pool.current), block_attempt + 1, block_retries) - pool.advance(f"{state} on page {page_num}") - session.relaunch() - bridge, page = session.bridge, session.page - d = _driver(session) + if has_pool: + logger.warning("Page %d came back as %s from %s — retrying " + "from another exit (%d/%d).", page_num, state, + mask(pool.current), block_attempt + 1, + block_retries) + pool.advance(f"{state} on page {page_num}") + session.relaunch() + bridge, page = session.bridge, session.page + d = _driver(session) + else: + # No pool, so nowhere else to go — re-fetch through the same + # access path, as the Playwright engine does. Not relaunched: + # over `--cdp-endpoint` a profile allows one live connection. + pause = args.retry_delay * (block_attempt + 1) + logger.warning("Page %d came back as %s — re-fetching through " + "the same access path in %.1fs (%d/%d).", + page_num, state, pause, block_attempt + 1, + block_retries) + time.sleep(pause) if load_failed: logger.error("Gave up loading %s after %d attempt(s).", url, args.retries) @@ -745,6 +769,15 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome outcome.state = state + if state == "not_found": + logger.error("%s does not exist — BBB answered with its own 404 " + "page. Nothing was scraped and nothing was retried: " + "check the URL (a withdrawn profile, or a mistyped " + "path), not the proxy.", url) + outcome.not_found = True + outcome.final_url = page.url + return outcome + if state == "blocked": # BBB refuses in TWO shapes and only one is solvable — see # playwright_scraper's twin of this block. This is the HARD refusal. @@ -1074,18 +1107,18 @@ def parse_args(): p.add_argument("--category", default=None, help="Label to tag output rows with. Defaults to BBB's own " "category for each business.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.at_least(1), default=1, help="Listing pages to fetch. BBB caps every query at 15 " "pages of 15 and answers page 16 with HTTP 500.") - p.add_argument("--delay", type=float, default=2.0, help="Delay between pages, seconds") - p.add_argument("--concurrency", type=int, default=1, metavar="N", + p.add_argument("--delay", type=page_flow.at_least(0, float), default=2.0, help="Delay between pages, seconds") + p.add_argument("--concurrency", type=page_flow.at_least(1), default=1, metavar="N", help="Accepted for flag parity and IGNORED here: parallel " "page fetching lives in playwright_scraper.py.") p.add_argument("--retries", type=page_flow.attempts, default=3, help="Attempts per page load before giving up (default 3). " "A page that comes back EMPTY is not retried: a query " "that matched nothing is a correct answer, not a fault.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.at_least(0, float), default=2.0, help="Seconds before the first retry, doubling thereafter") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="bbb_businesses", help="Output file prefix") @@ -1098,7 +1131,7 @@ def parse_args(): "Wins over --proxy.") p.add_argument("--proxy-rotate", choices=list(ROTATE_MODES), default="per-run") p.add_argument("--proxy-shuffle", action="store_true") - p.add_argument("--proxy-block-retries", type=int, default=2) + p.add_argument("--proxy-block-retries", type=page_flow.at_least(0), default=2) p.add_argument("--twocaptcha-key", default=None, help="2captcha.com API key") p.add_argument("--allow-empty", action="store_true", help="Write output files even when 0 rows were found.") @@ -1107,11 +1140,11 @@ def parse_args(): default="when-blocked", help="when-blocked (default): only pay to solve a " "reCAPTCHA if the content is not already readable. " - "always: solve whenever one is detected. Note that " - "NO challenge has ever been observed on this site — a " - "refused request gets no page at all — so neither " - "setting has anything to act on today, and neither " - "helps with a refusal.") + "always: solve whenever one is detected. Neither " + "setting touches BBB's hard refusal, a 403 page with " + "no widget on it, so no solve is attempted or billed " + "there. BBB's Managed Challenge is a Cloudflare " + "Turnstile, which this repo does not implement.") p.add_argument("--min-score", type=float, default=0.7) p.add_argument("--cdp-endpoint", default=None, help="Connect to a running browser over CDP, e.g. " @@ -1154,10 +1187,22 @@ def parse_args(): p.add_argument("--headless", action="store_true", default=True) p.add_argument("--headful", dest="headless", action="store_false") args = p.parse_args() + # Captured BEFORE the environment fills anything in: after + # `env_config.apply` a URL from BBB_URL is indistinguishable from one the + # user typed, and the conflict check below then refused --text/--location + # for anyone whose .env carried a URL. + typed_url = args.url env_config.apply(args) # Spelled identically to the other two engines. - if args.url and (args.text or args.location or args.country or args.state): + query_flags = bool(args.text or args.location or args.country or args.state) + if args.url and not typed_url and query_flags: + # The URL came from BBB_URL, not from this command line: it is a + # DEFAULT, and the query flags the user typed take its place. + logger.info("Ignoring BBB_URL from the environment: the query flags " + "on the command line describe the search instead.") + args.url = None + if args.url and query_flags: conflicting = [name for name, value in (("--text", args.text), ("--location", args.location), ("--country", args.country), ("--state", args.state)) diff --git a/scraper_api_client.py b/scraper_api_client.py index ddc08af..73fe803 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -73,6 +73,7 @@ parse_products, parse_profile) from output_writer import save import env_config +import page_flow logging.basicConfig(level=logging.INFO, format="%(asctime)s [%(levelname)s] %(message)s") logger = logging.getLogger("scraper_api_client") @@ -347,7 +348,7 @@ def parse_args(): p.add_argument("--category", default=None, help="Label to tag output rows with. Defaults to the category segment of the URL, so the column is never empty just because the flag was omitted.") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="bbb_businesses_scraperapi", help="Output file prefix") - p.add_argument("--timeout", type=int, default=60, + p.add_argument("--timeout", type=page_flow.at_least(1), default=60, help=f"API-side task timeout in seconds (1-{MAX_API_TIMEOUT}, default 60)") p.add_argument("--cdp-url", default=None, help="Route the fetch through an existing browser session over CDP " @@ -364,11 +365,11 @@ def parse_args(): p.add_argument("--allow-empty", action="store_true", help="Write output files even when 0 products were parsed. Off by " "default so a failed fetch can't overwrite a good result.") - p.add_argument("--retries", type=int, default=1, + p.add_argument("--retries", type=page_flow.at_least(0), default=1, help="Extra attempts if a bot-challenge page comes back. One retry is " "usually worth it. Each attempt is a separate billable task, so " "this defaults to 1.") - p.add_argument("--retry-delay", type=int, default=10, + p.add_argument("--retry-delay", type=page_flow.at_least(0), default=10, help="Seconds between retries (default 10)") p.add_argument("--dump-html", default=None, help="Also write the raw returned HTML to this path (always, even on success)") diff --git a/selenium_scraper.py b/selenium_scraper.py index 33046b1..6fc479b 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -139,11 +139,15 @@ class PageOutcome: # after every wait and retry. Not a finished page -- see the same field # in playwright_scraper.PageOutcome for what it used to cost. unreadable: bool = False + # True when BBB answered with its own 404: the address does not exist. + # Terminal and not a block; its own stop_reason so a reader checks the + # URL rather than the proxy or the parser. + not_found: bool = False @property def ok(self) -> bool: return (not self.load_failed and self.blocked_by is None - and not self.unreadable) + and not self.unreadable and not self.not_found) @property def failure_reason(self) -> str: @@ -152,6 +156,8 @@ def failure_reason(self) -> str: return "page_load_timeout" if self.unreadable: return "unreadable_response" + if self.not_found: + return "not_found" return f"blocked_{self.blocked_by}" @@ -597,6 +603,10 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome time.sleep(1) html = _snapshot(session, url) or "" + # `driver.get()` returns nothing and WebDriver exposes no HTTP + # status, so None is passed on purpose: BBB's 404 is still recognised + # by its own title (product_parser.NOT_FOUND_TITLE_RE), which is what + # keeps this engine agreeing with its twins about a bad URL. state = page_flow.classify(html, None, d["current_url"]()) # BBB server-renders its data, so a listing is parseable in the @@ -630,7 +640,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if handle_captcha_if_present(session, args, budget): time.sleep(1) html = d["content"]() or html - state = page_flow.classify(html, url=d["current_url"]()) + state = page_flow.classify(html, None, d["current_url"]()) if state == "content": logger.info("The solve was accepted — page %d is content " "now.", page_num) @@ -650,12 +660,25 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # so a different exit is the only thing that plausibly changes the # outcome. if block_attempt < block_retries: - logger.warning("Page %d came back as %s from %s — retrying from " - "another exit (%d/%d).", page_num, state, - mask(pool.current), block_attempt + 1, block_retries) - pool.advance(f"{state} on page {page_num}") - session.relaunch() - d = _driver(session) + if has_pool: + logger.warning("Page %d came back as %s from %s — retrying " + "from another exit (%d/%d).", page_num, state, + mask(pool.current), block_attempt + 1, + block_retries) + pool.advance(f"{state} on page {page_num}") + session.relaunch() + d = _driver(session) + else: + # No pool, so nowhere else to go — re-fetch through the same + # access path, as the Playwright engine does. Without this + # guard `pool.current` on None crashed the run (exit 1) and + # lost every row gathered on earlier pages. + pause = args.retry_delay * (block_attempt + 1) + logger.warning("Page %d came back as %s — re-fetching through " + "the same access path in %.1fs (%d/%d).", + page_num, state, pause, block_attempt + 1, + block_retries) + time.sleep(pause) if load_failed: logger.error("Gave up loading %s after %d attempt(s).", url, args.retries) @@ -664,6 +687,15 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome outcome.state = state + if state == "not_found": + logger.error("%s does not exist — BBB answered with its own 404 " + "page. Nothing was scraped and nothing was retried: " + "check the URL (a withdrawn profile, or a mistyped " + "path), not the proxy.", url) + outcome.not_found = True + outcome.final_url = d["current_url"]() + return outcome + if state == "blocked": # BBB refuses in TWO shapes and only one is solvable — see # playwright_scraper's twin of this block. This is the HARD refusal: @@ -1011,20 +1043,20 @@ def parse_args(): p.add_argument("--category", default=None, help="Label to tag output rows with. Defaults to BBB's own " "category for each business.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.at_least(1), default=1, help="Listing pages to fetch. BBB caps every query at 15 " "pages of 15 and answers page 16 with HTTP 500, so a " "run plans against the totalPages the site states on " "page 1.") - p.add_argument("--delay", type=float, default=2.0, help="Delay between pages, seconds") - p.add_argument("--concurrency", type=int, default=1, metavar="N", + p.add_argument("--delay", type=page_flow.at_least(0, float), default=2.0, help="Delay between pages, seconds") + p.add_argument("--concurrency", type=page_flow.at_least(1), default=1, metavar="N", help="Accepted for flag parity and IGNORED here: parallel " "page fetching lives in playwright_scraper.py.") p.add_argument("--retries", type=page_flow.attempts, default=3, help="Attempts per page load before giving up (default 3). " "A page that comes back EMPTY is not retried: a query " "that matched nothing is a correct answer, not a fault.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=page_flow.at_least(0, float), default=2.0, help="Seconds before the first retry, doubling thereafter") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="bbb_businesses", help="Output file prefix") @@ -1042,7 +1074,7 @@ def parse_args(): "Wins over --proxy.") p.add_argument("--proxy-rotate", choices=list(ROTATE_MODES), default="per-run") p.add_argument("--proxy-shuffle", action="store_true") - p.add_argument("--proxy-block-retries", type=int, default=2) + p.add_argument("--proxy-block-retries", type=page_flow.at_least(0), default=2) p.add_argument("--twocaptcha-key", default=None, help="2captcha.com API key") p.add_argument("--allow-empty", action="store_true", help="Write output files even when 0 rows were found.") @@ -1071,11 +1103,11 @@ def parse_args(): default="when-blocked", help="when-blocked (default): only pay to solve a " "reCAPTCHA if the content is not already readable. " - "always: solve whenever one is detected. Note that " - "NO challenge has ever been observed on this site — a " - "refused request gets no page at all — so neither " - "setting has anything to act on today, and neither " - "helps with a refusal.") + "always: solve whenever one is detected. Neither " + "setting touches BBB's hard refusal, a 403 page with " + "no widget on it, so no solve is attempted or billed " + "there. BBB's Managed Challenge is a Cloudflare " + "Turnstile, which this repo does not implement.") p.add_argument("--min-score", type=float, default=0.7) p.add_argument("--cdp-endpoint", default=None, help="Attach to a running browser at host:port. Must NOT " @@ -1088,11 +1120,23 @@ def parse_args(): p.add_argument("--headless", action="store_true", default=True) p.add_argument("--headful", dest="headless", action="store_false") args = p.parse_args() + # Captured BEFORE the environment fills anything in: after + # `env_config.apply` a URL from BBB_URL is indistinguishable from one the + # user typed, and the conflict check below then refused --text/--location + # for anyone whose .env carried a URL. + typed_url = args.url env_config.apply(args) # Spelled identically to playwright_scraper: --url and the query flags are # two ways to say the same thing, and only one may win. - if args.url and (args.text or args.location or args.country or args.state): + query_flags = bool(args.text or args.location or args.country or args.state) + if args.url and not typed_url and query_flags: + # The URL came from BBB_URL, not from this command line: it is a + # DEFAULT, and the query flags the user typed take its place. + logger.info("Ignoring BBB_URL from the environment: the query flags " + "on the command line describe the search instead.") + args.url = None + if args.url and query_flags: conflicting = [name for name, value in (("--text", args.text), ("--location", args.location), ("--country", args.country), ("--state", args.state)) diff --git a/smoke_test.py b/smoke_test.py index cd1aeda..82a3e9f 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -3,7 +3,7 @@ smoke_test.py — the offline suite for bbb-scraper. One file of plain functions with inline fixtures. No pytest, no conftest, no -fixtures directory (CLAUDE.md §10); `tests/test_smoke.py` wraps this as a +fixtures directory; `tests/test_smoke.py` wraps this as a single pytest test so `pytest` works as an entry point without a second copy of the checks. @@ -21,7 +21,7 @@ names the business's officers as individuals, and those names are replaced with REDACTED placeholders. The parser deliberately reads no such column; republishing a named person is a separate act from the site showing them on -its own page (§10). Everything BBB generates around them is untouched, and +its own page. Everything BBB generates around them is untouched, and `check_fixtures_carry_no_personal_names` guards the SHAPE so a future capture is caught too. """ @@ -90,7 +90,7 @@ def skip(group, reason): # --------------------------------------------------------------------------- # The parser, asserted on VALUES rather than on coverage # --------------------------------------------------------------------------- -# A column can be 100% populated and entirely wrong (CLAUDE.md §10), so every +# A column can be 100% populated and entirely wrong, so every # check below pins a figure from the real capture rather than counting # non-nulls. @@ -324,7 +324,7 @@ def check_profile_publishes_no_personal_names(): """The parser must not lift BBB's named officers into a column. BBB publishes them; republishing a named person is a separate act from - the site showing them on its own page (§10). This asserts the CURRENT + the site showing them on its own page. This asserts the CURRENT behaviour so that adding such a column becomes a decision rather than an accident. """ @@ -524,8 +524,11 @@ def check_page_states_on_real_captures(): P.detect_page_state(CHALLENGE_HTML, 403), "challenge") equal("the hard refusal is BLOCKED", P.detect_page_state(HARD_BLOCK_HTML, 403), "blocked") - equal("a page BBB served but that is not a listing is UNKNOWN", - P.detect_page_state(SERVED_404_HTML, 404), "unknown") + # This used to pin "unknown", which defended a gap: a mistyped or + # withdrawn profile URL then waited 30 s, re-fetched, and ended as an + # unreadable page instead of saying the address does not exist. + equal("BBB's own 404 is NOT_FOUND", + P.detect_page_state(SERVED_404_HTML, 404), "not_found") equal("the listing payload is CONTENT", P.detect_page_state(LISTING_PAYLOAD_JSON, 200), "content") equal("a profile page is CONTENT too", @@ -537,7 +540,7 @@ def check_page_states_on_real_captures(): def check_markers_do_not_match_a_page_bbb_serves(): - """CLAUDE.md §18: a marker that matches every page is WORSE than none. + """A marker that matches every page is WORSE than none. This check was once passing for the WRONG REASON, and that is why it now runs against two different kinds of served page. @@ -585,7 +588,7 @@ def check_markers_do_not_match_a_page_bbb_serves(): # Every marker must fire on at least one real refusal, or it is dead # weight. `/turnstile/v0/api.js` was in this list and matched nothing at - # all across every capture (§17). + # all across every capture. for marker in P.BOT_CHALLENGE_MARKERS: check("marker %r fires on a real challenge" % marker, marker in CHALLENGE_HTML, @@ -599,7 +602,7 @@ def check_markers_do_not_match_a_page_bbb_serves(): def check_positive_asset_detection(): """A page BBB served is built out of BBB's own assets; an interstitial is - not. §8's assets.mmsrg.com trick, on a third site.""" + not. The assets.mmsrg.com trick, on a third site.""" import product_parser as P check("the served page references BBB's asset hosts", P.references_own_assets(SERVED_404_HTML) >= 2) @@ -615,7 +618,7 @@ def check_state_policy(): import page_flow equal("every state has a policy", sorted(page_flow.STATE_POLICY), - ["blocked", "challenge", "content", "empty", "unknown"]) + ["blocked", "challenge", "content", "empty", "not_found", "unknown"]) check("content: parsed, not retried, not blocked", page_flow.should_parse("content") and not page_flow.should_retry("content") @@ -639,7 +642,7 @@ def check_state_policy(): def check_policy_constants_have_a_consumer(): - """§17: a policy constant nothing reads is the same defect as dead code. + """A policy constant nothing reads is the same defect as dead code. `RETRY_ON_BLOCKED` carried a paragraph of justification in a sibling repo and no engine consulted it, so setting it False changed nothing. @@ -673,7 +676,7 @@ def check_policy_constants_have_a_consumer(): def check_row_schema(): from output_writer import Business, ROW_CLASS_BY_MODE, UNIQUE_BY_SKU_MODES names = [f.name for f in fields(Business)] - equal("the family prefix is byte-identical and in order (§9)", + equal("the family prefix is byte-identical and in order", names[:5], ["source", "scraped_at", "url", "sku", "title"]) for gone in ("price", "currency", "original_price", "discount_pct", "in_stock", "brand"): @@ -756,7 +759,7 @@ def check_a_run_that_finds_nothing_writes_nothing(): code = save([], prefix, "json", allow_empty=True) equal("--allow-empty WRITES the empty file...", json.load(open(prefix + ".json", encoding="utf-8")), []) - # ...and still reports exit 4. Pinned deliberately (§10: pin a known + # ...and still reports exit 4. Pinned deliberately (pin a known # behaviour rather than half-guarding it): "zero businesses" is true # whether or not the file was written, and a caller that wanted the # file still wants to know the result was empty. @@ -795,7 +798,7 @@ def check_sidecar_shape(): # --------------------------------------------------------------------------- -# The engines — the five checks CLAUDE.md §17 says to steal +# The engines — five checks worth carrying to every repo # --------------------------------------------------------------------------- ENGINES = ("playwright_scraper", "selenium_scraper", "puppeteer_scraper") @@ -842,7 +845,7 @@ def check_engines_import_their_driver_at_module_level(): def check_shared_calls_bind_against_the_real_signature(): - """§17's check #1, and the one that earns its keep. + """Signature binding, and the one that earns its keep. A sibling repo shipped `classify(html, url=…)` in two of three engines against a callee taking `status` second, and BOTH crashed on their first @@ -958,7 +961,7 @@ def _argparse_flags(module_name): return flags -# The family's flag contract (CLAUDE.md §9), plus this repo's own additions. +# The family's flag contract, plus this repo's own additions. CONTRACT_FLAGS = { "--url", "--pages", "--category", "--format", "--out", "--delay", "--retries", "--retry-delay", "--concurrency", "--proxy", "--proxy-file", @@ -971,7 +974,7 @@ def _argparse_flags(module_name): def check_engine_flag_sets(): - """§17's check #2: against the contract AND against each other, both ways. + """Flag parity: against the contract AND against each other, both ways. A missing flag fails; so does closing a difference the README documents. """ @@ -988,7 +991,7 @@ def check_engine_flag_sets(): # and could not launch it on the development machine, so it needs a way # to point at another one. Its twins have no equivalent because they do # not ship a browser. Listed here so that closing the difference — or - # growing a second one — fails the build (§17). + # growing a second one — fails the build. DOCUMENTED_DIFFERENCES = {"puppeteer_scraper": {"--chromium-path"}} names = sorted(sets) for i in range(len(names) - 1): @@ -1009,7 +1012,7 @@ def check_banned_and_removed_flags(): `--country` is legitimate here and NOT banned — BBB serves both its countries from one host, so the flag cannot contradict a hostname the way - CLAUDE.md §10's rule is about. What the rule is really about is a flag + the removed-flag rule is about. What the rule is really about is a flag disagreeing with the URL, and that is enforced instead: passing --country together with --url is refused, which the next check asserts. """ @@ -1027,7 +1030,7 @@ def check_banned_and_removed_flags(): def check_undefined_names_in_every_module(): - """§10: compileall proves a file PARSES, not that its names RESOLVE. + """compileall proves a file PARSES, not that its names RESOLVE. A live run of a sibling repo's pyppeteer engine died with NameError on a line reached only while fetching, after an import had been removed — the @@ -1085,7 +1088,7 @@ def _import_graph(entrypoint): def check_dockerfile_copies_everything_the_entrypoint_imports(): - """§10: all three repos in this family shipped an image that died with + """All three repos in this family shipped an image that died with ModuleNotFoundError on every invocation, --help included, because proxy_pool.py was missing from the COPY list. CI never built the image; this check needs no Docker.""" @@ -1133,7 +1136,7 @@ def check_env_example_documents_exactly_what_the_loader_reads(): def check_a_copied_env_example_reads_as_UNSET(): - """§17: `cp .env.example .env` followed by a run must not connect. + """`cp .env.example .env` followed by a run must not connect. The placeholder check was a literal set in a sibling repo, and the two credentialled URLs are documented the way the vendor documents them — @@ -1147,15 +1150,15 @@ def check_a_copied_env_example_reads_as_UNSET(): check(".env.example exists", False) return text = open(example, encoding="utf-8").read() - values = dict(re.findall(r"^([A-Z][A-Z0-9_]+)=(.*)$", text, re.M)) - check("the example actually sets every variable", + # BBB_URL is shipped COMMENTED OUT (a copied example must not become a + # default nobody chose), so a commented assignment counts as documented. + values = dict(re.findall(r"^#?\s*([A-Z][A-Z0-9_]+)=(.*)$", text, re.M)) + check("the example shows a value for every variable", set(values) == set(env_config.ENV_KEYS), "example has %s, loader reads %s" % (sorted(values), sorted(env_config.ENV_KEYS))) - # Every CREDENTIAL must read as unset. The default TARGET must not: it is - # a real, usable URL, and blanking it would remove the one setting this - # file exists to make convenient (§17's check #3 says exactly this — the - # credentials unset, the non-credential default still usable). + # Every CREDENTIAL must read as unset. The example TARGET must not: it is + # a real, usable URL for anyone who uncomments it. CREDENTIALS = {"TWOCAPTCHA_KEY", "BBB_CDP_ENDPOINT", "BBB_PROXY"} before = dict(os.environ) try: @@ -1186,7 +1189,7 @@ def check_a_copied_env_example_reads_as_UNSET(): def check_credential_scan_is_one_implementation_invoked_from_both(): - """§17: two sources of truth, one dead and one holed. + """Two sources of truth, one dead and one holed. `.github/ci_checks.py` sat in three repos invoked by NOTHING, while tests.yml carried an inline grep doing a narrower version of the same job @@ -1216,7 +1219,7 @@ def check_credential_scan_is_one_implementation_invoked_from_both(): def check_banned_wording(): - """§12: enforced by this test rather than by review.""" + """Banned wording, enforced by this test rather than by review.""" for root, dirs, files in os.walk(HERE): dirs[:] = [d for d in dirs if d not in (".git", "__pycache__", ".pytest_cache", "node_modules")] @@ -1234,7 +1237,7 @@ def check_banned_wording(): def check_concurrency_with_the_browser_stubbed(): - """§10: a live run cannot always reach this machinery. + """A live run cannot always reach this machinery. Page 1 is fetched alone and decides how many pages there are, so a blocked page 1 means the workers never start. Driven directly instead, @@ -1380,7 +1383,7 @@ def check_worker_pools_start_on_different_exits(): def check_fingerprint_kwargs_are_ones_the_driver_accepts(): - """§10: an unknown key in new_context(**kwargs) is a TypeError at launch, + """An unknown key in new_context(**kwargs) is a TypeError at launch, on the PAID path, at runtime.""" engine = _import_engine("playwright_scraper") if engine is None: @@ -1428,7 +1431,7 @@ def check_every_engine_exposes_the_same_public_surface(): def check_engines_do_not_evaluate_a_string_in_the_browser(): - """§18: a site whose CSP omits `unsafe-eval` kills wait_for_function with + """A site whose CSP omits `unsafe-eval` kills wait_for_function with an EvalError and takes the run down with exit 1. BBB has not been measured for that, and the cheap habit costs nothing where it would have been allowed.""" @@ -1446,7 +1449,7 @@ def check_engines_do_not_evaluate_a_string_in_the_browser(): def check_credentials_never_reach_a_log(): - """§8: an EXCEPTION MESSAGE is a log, and the masker must be GLOBAL. + """An EXCEPTION MESSAGE is a log, and the masker must be GLOBAL. A Playwright connection error repeats the endpoint five times (the message plus a four-line call log), so a masker handling only the first @@ -1492,7 +1495,7 @@ def check_sample_output_matches_the_schema(): def check_readme_numbers_are_not_stale(): - """§17's check #4: diff every numeric claim against what is on disk. + """Diff every numeric claim against what is on disk. Only the figures that MUST hold are pinned: a number that legitimately varies between runs is written as a range in the README and not checked @@ -1530,7 +1533,7 @@ def _tree_state(): def check_no_test_mutates_the_working_tree(): - """§10: one suite used its own file as a fake chromedriver and chmod'd it + """One suite used its own file as a fake chromedriver and chmod'd it to 755, leaving a mode change in git status. Compares the tree against how it looked when the suite STARTED, not @@ -1548,7 +1551,7 @@ def check_no_test_mutates_the_working_tree(): def check_captcha_capability_claims_match_the_code(): - """§19: the most expensive bug this family can ship is a SENTENCE. + """The most expensive bug this family can ship is a SENTENCE. It fails in both directions and this family has shipped both: @@ -1857,7 +1860,7 @@ def page_n(n, bad=None, bad_body=moved): def check_the_solve_budget_is_spent_at_the_purchase(): - """Audit 2026-10-08, P2 (and §27.4): three solves against a limit of one. + """Audit 2026-10-08, P2: three solves against a limit of one. `handle_captcha_if_present` runs before classification AND after it, and only the second call was counted. Counting calls was also the wrong fix: @@ -1875,7 +1878,7 @@ def check_the_solve_budget_is_spent_at_the_purchase(): # Every engine: the solver call is behind the budget, the call sites pass # it, and the old per-call counter is gone. Read from source on purpose: # a check gated behind importing the engine skips for exactly the engines - # whose driver is absent (§27.4). + # whose driver is absent. for module in ("playwright_scraper", "selenium_scraper", "puppeteer_scraper"): tree = ast.parse(open(os.path.join(HERE, module + ".py"), encoding="utf-8").read()) @@ -2101,6 +2104,573 @@ def check_retries_is_validated_in_every_entry_point(): check("--retries is validated in every entry point, so 0 is a usage error", not problems, "; ".join(problems)) +# --------------------------------------------------------------------------- +# Audit 2026-10-14: writes, CSV cells, no-proxy retries, 404s, typed URLs +# --------------------------------------------------------------------------- + +def check_a_failed_write_keeps_the_previous_good_output(): + """An interrupted write must not destroy the last good file. + + `open(path, "w")` truncates first, so a failure halfway through + `json.dump` used to leave 13 bytes of invalid JSON where a good result + was, and a failed CSV write left no header at all. + """ + import output_writer as O + from output_writer import Business + good = [Business(sku="good-1", title="Good")] + with tempfile.TemporaryDirectory() as tmp: + prefix = os.path.join(tmp, "out") + O.save(good, prefix, "both") + O.write_run_meta(prefix, {"status": "complete"}) + before = {ext: open(prefix + ext, "rb").read() + for ext in (".json", ".csv", ".meta.json")} + + class Boom(Exception): + pass + + def explode(*a, **k): + raise Boom("disk full") + + saved_dump, saved_writerow = O.json.dump, O.csv.DictWriter.writerow + try: + O.json.dump = lambda obj, f, **k: (f.write('[{"partial": '), explode()) + for ext, call in ((".json", lambda: O.write_json(good, prefix + ".json")), + (".meta.json", lambda: O.write_run_meta( + prefix, {"status": "failed"}))): + try: + call() + except Boom: + pass + equal("a failed %s write leaves the previous file intact" % ext, + open(prefix + ext, "rb").read(), before[ext]) + O.json.dump = saved_dump + O.csv.DictWriter.writerow = explode + try: + O.write_csv(good, prefix + ".csv") + except Boom: + pass + equal("a failed .csv write leaves the previous file intact", + open(prefix + ".csv", "rb").read(), before[".csv"]) + finally: + O.json.dump = saved_dump + O.csv.DictWriter.writerow = saved_writerow + equal("no temporary file is left behind", + sorted(n for n in os.listdir(tmp) if n.endswith(".tmp")), []) + + +def check_output_files_get_the_mode_open_would_have_given_them(): + """A temp file is created 0600 and a rename keeps that. A new output must + get `0o666 & ~umask` (what `open()` gives it); an existing one keeps its + own mode. A flat 0644 would narrow 0664 under umask 002.""" + if os.name != "posix": + skip("output file modes", "not a POSIX system") + return + import output_writer as O + from output_writer import Business + rows = [Business(sku="m-1", title="Mode")] + with tempfile.TemporaryDirectory() as tmp: + for umask in (0o022, 0o002, 0o077): + old = os.umask(umask) + try: + path = os.path.join(tmp, "new_%03o.json" % umask) + O.write_json(rows, path) + mode = os.stat(path).st_mode & 0o777 + finally: + os.umask(old) + equal("a new file under umask %03o is %03o" % (umask, 0o666 & ~umask), + "%03o" % mode, "%03o" % (0o666 & ~umask)) + path = os.path.join(tmp, "existing.csv") + open(path, "w").close() + os.chmod(path, 0o640) + O.write_csv(rows, path) + equal("an existing file keeps its own mode", + "%03o" % (os.stat(path).st_mode & 0o777), "640") + + +def check_csv_cells_that_a_spreadsheet_would_execute_are_escaped(): + import output_writer as O + from output_writer import Business + row = Business(sku="f-1", title='=HYPERLINK("http://x","y")', + phone=["=1+1", "+1 555"], description="@SUM(A1)", + longitude=-97.7) + clean = Business(sku="f-2", title="Plain name", description="-") + with tempfile.TemporaryDirectory() as tmp: + prefix = os.path.join(tmp, "out") + code = O.finish_run([row, clean], prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", + extra={"csv_cells_escaped": "site-data"}) + equal("a run with formula-shaped cells still exits 0", code, 0) + with open(prefix + ".csv", encoding="utf-8", newline="") as f: + cells = list(csv.DictReader(f)) + equal("a formula title is prefixed with an apostrophe", + cells[0]["title"], '\'=HYPERLINK("http://x","y")') + equal("a joined list is escaped AFTER joining", cells[0]["phone"], + "'=1+1 | +1 555") + equal("an @ cell is escaped", cells[0]["description"], "'@SUM(A1)") + equal("a NUMBER stays a number, negative or not", cells[0]["longitude"], + "-97.7") + equal("a lone dash is text and is escaped too", cells[1]["description"], + "'-") + equal("a plain cell is untouched", cells[1]["title"], "Plain name") + loaded = json.load(open(prefix + ".json", encoding="utf-8")) + equal("JSON keeps the site's bytes", loaded[0]["title"], + '=HYPERLINK("http://x","y")') + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("the caller's own extra wins a name collision", + meta["csv_cells_escaped"], "site-data") + O.finish_run([row], prefix, "csv", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u") + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("the sidecar counts the escaped cells", meta.get("csv_cells_escaped"), 3) + + +def _engine_with_stub_driver(name): + """Load an engine with its driver library replaced by stand-ins. + + So the retry paths below run in every venv, including the core-only one + CI's offline job uses, rather than skipping for exactly the engines whose + driver is absent. Loaded under a private module name and with sys.modules + restored afterwards, so the import-skip checks elsewhere see the real + state of the environment. + """ + import importlib.util + import types + + class Stub(types.ModuleType): + def __getattr__(self, attr): + if attr.startswith("__"): + raise AttributeError(attr) + value = type(attr, (Exception,), {}) + setattr(self, attr, value) + return value + + stubbed = ("playwright", "playwright.sync_api", "pyppeteer", "selenium", + "selenium.webdriver", "selenium.common", + "selenium.common.exceptions", "selenium.webdriver.chrome", + "selenium.webdriver.chrome.options", + "selenium.webdriver.common", "selenium.webdriver.common.by", + "selenium.webdriver.support", "selenium.webdriver.support.ui", + "webdriver_manager", "webdriver_manager.chrome") + saved = {m: sys.modules.get(m) for m in stubbed} + try: + for m in stubbed: + sys.modules[m] = Stub(m) + spec = importlib.util.spec_from_file_location( + "_stubbed_" + name, os.path.join(HERE, name + ".py")) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + finally: + for m, v in saved.items(): + if v is None: + sys.modules.pop(m, None) + else: + sys.modules[m] = v + + +class _Response: + def __init__(self, status): + self.status = status + + +def _drive_one_page(engine_name, html, *, goto_error=None, status=None, + retries=1): + """Run one engine's `_fetch_one_page` against a fake driver, no pool. + + Returns (outcome or exception, number of navigations).""" + engine = _engine_with_stub_driver(engine_name) + navigations = [] + + def navigate(*a, **k): + navigations.append(1) + if goto_error: + raise goto_error + return _Response(status) if status is not None else None + + class Page: + url = "https://www.bbb.org/us/tx/austin/profile/plumber/x-0825-1" + + def goto(self, *a, **k): + return navigate() + + def wait_for_timeout(self, ms): + pass + + def query_selector_all(self, sel): + return [] + + def screenshot(self, **k): + pass + + class Driver: + current_url = Page.url + + def get(self, url): + navigate() + + class Bridge: + def run(self, value): + return value + + class Session: + page = Page() + driver = Driver() + bridge = Bridge() + relaunched = 0 + + def relaunch(self): + Session.relaunched += 1 + + with tempfile.TemporaryDirectory() as tmp: + class Args: + proxy_block_retries = 2 + solve_captcha = "when-blocked" + twocaptcha_key = None + captcha_api = "v2" + min_score = 0.3 + out = os.path.join(tmp, "o") + mode = "profile" + dump_html = None + pages = 1 + retry_delay = 0 + Args.retries = retries + + engine.handle_captcha_if_present = lambda *a, **k: False + engine.time = types_ns(sleep=lambda s: None, time=__import__("time").time, + monotonic=__import__("time").monotonic) + if engine_name == "playwright_scraper": + engine._snapshot = lambda page, url: html + else: + engine._snapshot = lambda session, url: html + engine._driver = lambda session: { + "count": lambda sel: 0, "sleep": lambda ms: None, + "content": lambda: html, "current_url": lambda: Page.url} + if engine_name == "puppeteer_scraper": + engine._content = lambda session: html + if goto_error is not None and engine_name == "playwright_scraper": + engine.PWError = type(goto_error) + if goto_error is not None and engine_name == "selenium_scraper": + engine.WebDriverException = type(goto_error) + import logging as _logging + _logging.disable(_logging.CRITICAL) + try: + outcome = engine._fetch_one_page(Session(), Args, None, 1, Page.url) + except Exception as e: # noqa: BLE001 — the crash IS the finding + outcome = e + finally: + _logging.disable(_logging.NOTSET) + return outcome, len(navigations) + + +def types_ns(**kw): + import types + return types.SimpleNamespace(**kw) + + +ENGINE_NAMES = ("playwright_scraper", "puppeteer_scraper", "selenium_scraper") + + +def check_the_mirror_engines_survive_a_block_with_no_proxy_pool(): + """With no --proxy, `pool` is None. A blocked page used to reach + `mask(pool.current)` / `pool.advance` in the Selenium and pyppeteer + engines: AttributeError, a traceback, exit 1, and every row from earlier + pages lost. Playwright guarded it; now all three do.""" + for name in ENGINE_NAMES: + outcome, navs = _drive_one_page(name, HARD_BLOCK_HTML) + check("%s: a block with no pool does not raise" % name, + not isinstance(outcome, Exception), repr(outcome)) + if isinstance(outcome, Exception): + continue + check("%s: ...it is reported as blocked" % name, + outcome.blocked_by is not None and not outcome.ok, + "blocked_by=%r" % outcome.blocked_by) + check("%s: ...after a plain re-fetch on the same path" % name, + navs >= 2, "%d navigation(s)" % navs) + + proxy_dead = Exception("net::ERR_PROXY_CONNECTION_FAILED at x") + outcome, navs = _drive_one_page(name, HARD_BLOCK_HTML, + goto_error=proxy_dead) + check("%s: a dead exit with no pool does not raise" % name, + not isinstance(outcome, Exception), repr(outcome)) + if not isinstance(outcome, Exception): + check("%s: ...it is a load failure" % name, outcome.load_failed) + + +def check_a_404_is_a_terminal_not_found(): + """BBB's own 404 is an answer about the ADDRESS: no wait, no retry, no + block, and its own stop_reason. Read by status where the driver gives + one, and by BBB's own title where it does not (Selenium).""" + import page_flow as PF + import product_parser as P + equal("BBB's 404 page is NOT_FOUND by its title alone", + P.detect_page_state(SERVED_404_HTML, None), "not_found") + check("not_found is terminal, not blocked, not solved, not parsed", + not PF.should_retry("not_found") and not PF.counts_as_blocked("not_found") + and not PF.should_solve("not_found") and not PF.should_parse("not_found")) + for name in ENGINE_NAMES: + outcome, navs = _drive_one_page(name, SERVED_404_HTML) + check("%s: a served 404 does not raise" % name, + not isinstance(outcome, Exception), repr(outcome)) + if isinstance(outcome, Exception): + continue + equal("%s: a served 404 is fetched once and not retried" % name, navs, 1) + equal("%s: ...and stops the run as not_found" % name, + (outcome.ok, outcome.failure_reason), (False, "not_found")) + + # The status must reach the classifier: a page with BBB's chrome but no + # title or payload is only a 404 because the response said so. + bare = SERVED_404_HTML.replace("Page not found", "Somewhere") + equal("without the title and without a status the page is unknown", + P.detect_page_state(bare, None), "unknown") + for name in ("playwright_scraper", "puppeteer_scraper"): + outcome, navs = _drive_one_page(name, bare, status=404) + check("%s: the HTTP status reaches the classifier" % name, + not isinstance(outcome, Exception) + and getattr(outcome, "failure_reason", None) == "not_found", + repr(getattr(outcome, "state", outcome))) + + +def check_the_engines_classify_with_a_status_argument(): + """Every classify call site passes a status positionally, so an engine + that drops it is a decision visible in the source (Selenium has none + to pass and says so with a literal None).""" + for name in ENGINE_NAMES: + tree = ast.parse(open(os.path.join(HERE, name + ".py"), + encoding="utf-8").read()) + calls = [n for n in ast.walk(tree) if isinstance(n, ast.Call) + and getattr(n.func, "attr", getattr(n.func, "id", None)) + in ("classify", "_classify")] + check("%s: classify call sites found" % name, len(calls) >= 3, + "found %d" % len(calls)) + bad = [] + for c in calls: + if getattr(c.func, "attr", None) == "classify": + if len(c.args) < 2: + bad.append(c.lineno) + elif getattr(c.func, "id", None) == "_classify" and c.args and \ + isinstance(c.args[0], ast.Name) and c.args[0].id == "page": + continue # the helper's own definition forwards it + elif len(c.args) < 3: + bad.append(c.lineno) + check("%s: every classify call passes a status" % name, not bad, + "lines %s" % bad) + if name != "selenium_scraper": + def status_arg(c): + index = 1 if getattr(c.func, "attr", None) == "classify" else 2 + return c.args[index] if len(c.args) > index else None + literal_none = [c.lineno for c in calls + if isinstance(status_arg(c), ast.Constant)] + check("%s: no call site throws the status away" % name, + not literal_none, "lines %s" % literal_none) + + +def _parse_engine_args(name, argv, env=None): + """Run an engine's parse_args with a stubbed driver: (args, exit, stderr). + + `env` is applied as environment variables (the same path a `.env` takes + through env_config.apply); the developer's own `.env` is not read.""" + import env_config + engine = _engine_with_stub_driver(name) + saved_argv, saved_load = sys.argv, env_config.load_env + saved_env = {k: os.environ.pop(k) for k in list(os.environ) + if k.startswith("BBB_") or k == "TWOCAPTCHA_KEY"} + err = io.StringIO() + code, args = None, None + try: + env_config.load_env = lambda *a, **k: None + os.environ.update(env or {}) + sys.argv = [name + ".py"] + argv + with contextlib.redirect_stderr(err), \ + contextlib.redirect_stdout(io.StringIO()): + try: + args = engine.parse_args() + except SystemExit as e: + code = e.code + finally: + sys.argv = saved_argv + env_config.load_env = saved_load + for k in (env or {}): + os.environ.pop(k, None) + os.environ.update(saved_env) + return args, code, err.getvalue() + + +def check_a_url_from_the_environment_does_not_block_the_query_flags(): + """BBB_URL is a DEFAULT. A user who copied `.env` from the example and + typed `--text plumber --location "Austin, TX"` used to get exit 2, + "--url already carries the whole query".""" + env = {"BBB_URL": "https://www.bbb.org/search?find_country=USA&find_text=x&find_loc=y"} + for name in ENGINE_NAMES: + args, code, err = _parse_engine_args( + name, ["--text", "plumber", "--location", "Austin, TX"], env) + check("%s: query flags replace a URL from the environment" % name, + code is None and args is not None and "find_text=plumber" in args.url, + err.strip()[-200:]) + args, code, err = _parse_engine_args(name, [], env) + check("%s: ...and with no flags the environment URL is used" % name, + code is None and args is not None and "find_text=x" in args.url, + err.strip()[-200:]) + args, code, err = _parse_engine_args( + name, ["--url", env["BBB_URL"], "--text", "plumber"]) + check("%s: a TYPED --url with --text is still refused" % name, + code == 2 and "already carries the whole query" in err, err[-200:]) + text = open(os.path.join(HERE, ".env.example"), encoding="utf-8").read() + check(".env.example ships BBB_URL commented out, not as a default", + not re.search(r"^BBB_URL=", text, re.M) + and re.search(r"^#\s*BBB_URL=", text, re.M), "") + + +def check_counting_flags_refuse_negative_values(): + """A negative --pages, --delay or --retry-delay used to be accepted: + `range()` runs zero times and `time.sleep` raises mid-run. Refused with + an argparse error that names the flag, in every engine.""" + base = ["--url", "https://www.bbb.org/search?find_country=USA&find_text=x"] + cases = (("--pages", "0"), ("--pages", "-1"), ("--delay", "-1"), + ("--retry-delay", "-0.5"), ("--concurrency", "0"), + ("--proxy-block-retries", "-1")) + for name in ENGINE_NAMES: + for flag, value in cases: + args, code, err = _parse_engine_args(name, base + [flag, value]) + check("%s: %s %s is a usage error naming the flag" % (name, flag, value), + code == 2 and ("argument %s:" % flag) in err, err[-200:]) + args, code, err = _parse_engine_args( + name, base + ["--delay", "0", "--retry-delay", "0", + "--proxy-block-retries", "0", "--pages", "2"]) + check("%s: zero waits and zero block retries stay legal" % name, + code is None, err[-200:]) + import env_config + import scraper_api_client as S + saved_argv, saved_load = sys.argv, env_config.load_env + try: + env_config.load_env = lambda *a, **k: None + for value, legal in (("0", True), ("1", True), ("-1", False)): + sys.argv = ["scraper_api_client.py", "--url", base[1], + "--retries", value] + err, code = io.StringIO(), None + try: + with contextlib.redirect_stderr(err): + S.parse_args() + except SystemExit as e: + code = e.code + check("scraper_api_client --retries %s is %s" + % (value, "legal" if legal else "refused"), + (code is None) == legal + and (legal or "argument --retries:" in err.getvalue()), + err.getvalue()[-200:]) + finally: + sys.argv, env_config.load_env = saved_argv, saved_load + + +def check_no_private_citations_in_shipped_files(): + """Public files state their reasons in their own words. A pointer into a + document the reader cannot open (an instruction file, or a numbered + section of one) explains nothing. Tokens are assembled from pieces so + this file does not match itself.""" + banned = re.compile("|".join(( + re.escape("CLAUDE" + ".md"), "claude" + "_2scraper", + "2scraper" + "_repo_rules", "play" + "book", "\u00a7"))) + hits = [] + for path in _tracked_files(): + if path.suffix.lower() in (".png", ".jpg", ".ico", ".gz", ".zip"): + continue + try: + text = path.read_text(encoding="utf-8") + except (UnicodeDecodeError, OSError): + continue + for lineno, line in enumerate(text.splitlines(), 1): + if banned.search(line): + hits.append("%s:%d" % (path.relative_to(HERE), lineno)) + check("no shipped file cites a private instruction file or its sections", + not hits, ", ".join(hits[:10])) + + +def _tracked_files(): + import pathlib + root = pathlib.Path(HERE) + try: + out = subprocess.run(["git", "ls-files"], cwd=HERE, capture_output=True, + text=True, check=True).stdout.split("\n") + files = [root / f for f in out if f] + except (OSError, subprocess.CalledProcessError): + files = [p for p in root.rglob("*") if p.is_file() + and ".git" not in p.relative_to(root).parts] + return [f for f in files if f.is_file()] + + +def check_credential_scan_is_relative_to_the_repo_and_never_scans_nothing(): + """The scan skipped a path whose ABSOLUTE parts held `venv`, so a clone + under ~/venv/ scanned 0 files and printed "ok". Exclusions are judged + relative to the repository, a zero-file scan fails, and a `.env.bak` + holding a key is read whatever its suffix.""" + ci_dir = os.path.join(HERE, ".github") + if not os.path.isdir(ci_dir): + skip("credential scan location", "no .github in this tree") + return + import shutil + key = "".join("ab" for _ in range(16)) + with tempfile.TemporaryDirectory() as tmp: + clone = os.path.join(tmp, "venv", "build", "repo") + os.makedirs(os.path.join(clone, ".github")) + shutil.copy(os.path.join(ci_dir, "ci_checks.py"), + os.path.join(clone, ".github", "ci_checks.py")) + with open(os.path.join(clone, "notes.py"), "w") as f: + f.write("x = 1\n") + run = lambda: subprocess.run( + [sys.executable, os.path.join(clone, ".github", "ci_checks.py"), + "--secret-check"], capture_output=True, text=True, cwd=clone) + r = run() + check("a clean clone under a `venv` parent scans files and passes", + r.returncode == 0 and " 0 files" not in r.stdout, + (r.stdout + r.stderr)[-300:]) + with open(os.path.join(clone, ".env.bak"), "w") as f: + f.write("TWOCAPTCHA_KEY=%s\n" % key) + r = run() + check("...and a key in an untracked .env.bak there fails the scan", + r.returncode != 0 and ".env.bak" in r.stdout + r.stderr, + (r.stdout + r.stderr)[-300:]) + os.remove(os.path.join(clone, ".env.bak")) + with open(os.path.join(clone, "fixture.json"), "w") as f: + f.write('{"note": "%s"}\n' % key) + r = run() + check("...and so does one in a .json fixture", + r.returncode != 0 and "fixture.json" in r.stdout + r.stderr, + (r.stdout + r.stderr)[-300:]) + # A scan whose exclusions matched everything must fail, not say "ok". + import importlib.util + spec = importlib.util.spec_from_file_location( + "_ci_checks_probe", os.path.join(ci_dir, "ci_checks.py")) + ci = importlib.util.module_from_spec(spec) + spec.loader.exec_module(ci) + ci.scanned_files = lambda: iter(()) + with contextlib.redirect_stdout(io.StringIO()): + failed = ci.secret_check() + check("a scan that reads 0 files fails instead of saying ok", + any("0 files" in f for f in failed), repr(failed)) + + +def check_dump_and_capture_shapes_are_ignored(): + """`--dump-html X --pages 3` writes X.page3, which `*.json` does not + cover; a raw page saved at the top of the repo, or a live-run directory, + is one `git add -A` from a commit.""" + try: + subprocess.run(["git", "rev-parse"], cwd=HERE, capture_output=True, + check=True) + except (OSError, subprocess.CalledProcessError): + skip("gitignore shapes", "not a git checkout") + return + ignored = lambda p: subprocess.run(["git", "check-ignore", "-q", p], + cwd=HERE).returncode == 0 + for path in ("x.page3", "canary_dump.json.page3", "live/x.html", + "root.html", "out/run.json", ".env.bak", ".env.local"): + check("%s is ignored" % path, ignored(path)) + for path in ("landing.html", "sample_output.json", "sample_output.csv", + ".env.example", ".github/workflows/tests.yml", + "tests/test_smoke.py", "smoke_test.py"): + check("%s is NOT ignored" % path, not ignored(path)) + + CHECKS = [v for k, v in sorted(globals().items()) if k.startswith("check_")]