diff --git a/.github/ci_checks.py b/.github/ci_checks.py index c4c0d5e..7223f03 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -83,7 +83,7 @@ # dubizzle publishes ends in `---{32 hex}`, and the committed fixtures, # sample output and documentation are full of them by design. A pattern that # cannot tell the two apart fails on its own repository on day one — which is -# how a check stops being read (§17) — so the ad-id shape is subtracted from +# how a check stops being read — so the ad-id shape is subtracted from # the text BEFORE the scan rather than added to a context allowlist. A bare # 32-hex string anywhere else still fails, which is the point. HEX32 = re.compile(r"\b[0-9a-f]{32}\b") @@ -242,6 +242,12 @@ def _without_site_ids(line): re.compile(r"_debug\.(?:html|png)$"), re.compile(r"^live_results\."), re.compile(r"^captures?/"), + # Working directories by KIND rather than by one name, and a bare HTML + # file at the root: a dump arrives through whatever directory someone + # called it. Narrow enough to leave .github/, tests/ and the generated + # fixtures alone, which the suite pins. + re.compile(r"(?:^|/)(?:live|out|output|runs?|results?|tmp|scratch|dumps?)/"), + re.compile(r"^[^/]+\.html?$"), ) @@ -269,7 +275,7 @@ def tracked_files(): # into pip's vendored code and flags a 32-hex string in `_elffile.py` as # key-shaped. Correct about the string, wrong about the file — and it is the # FIRST thing a new user sees from `python3 smoke_test.py`. A guard people -# have to argue with is one they learn to suppress (CLAUDE.md §22). +# have to argue with is one they learn to suppress. # # Deliberately NOT a narrowing of what gets scanned: an untracked file is # still read, because a key pasted into a scratch file beside the scripts is diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 962adba..9df9280 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -28,9 +28,9 @@ name: canary # secret with `region-ae` would work as well; the CDP path is the one # measured here. # -# The live steps are SKIPPED, not failed, when the secret is absent. The job -# goes green with a notice saying why. A check that is always red teaches -# everyone to ignore checks, which is worse than having no check at all. Set +# Without that secret the job FAILS, naming the missing secret. A canary that +# skipped, was blocked, reached nothing or parsed nothing has not tested the +# site, and a green badge for it would read as "the parser still works". Set # the secret and the same workflow starts doing real work with no edit: # # gh secret set DUBIZZLE_CDP_ENDPOINT --repo 2scraper/dubizzle-scraper @@ -49,19 +49,14 @@ name: canary # loader already treats as "not configured", so there is no commented-out # line for someone to forget to uncomment. # -# ON DEMAND, NOT ON A SCHEDULE — forced by the credential rather than chosen. -# A Scraping Browser endpoint on these accounts does not survive long: of the -# credentials inherited from the sibling repos in this family, most were -# already returning 401 when this repo was written. A daily cron against a -# credential with that lifetime produces one of two useless outcomes: a red -# badge every day, or a green badge every day that has tested nothing. The -# second is the more dangerous, because green reads as "the parser still -# works". -# -# Put the schedule back the day a long-lived credential exists: -# schedule: -# - cron: "23 6 * * *" +# WEEKLY, and on demand. The schedule used to be removed on the argument +# that a Scraping Browser credential does not live long; that lifetime was +# never measured on this repo, and a sibling's endpoint stayed valid for more +# than ten days. If the credential has stopped working, the run says so in +# red (exit 5, with the HTTP status in the log), which is the point. on: + schedule: + - cron: "23 6 * * 1" workflow_dispatch: jobs: @@ -74,10 +69,10 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Explain a skipped run + - name: Fail when the live run cannot happen if: env.HAVE_CDP != 'true' run: | - echo "::notice::Skipping the live run: no DUBIZZLE_CDP_ENDPOINT secret is set." + echo "::error::No DUBIZZLE_CDP_ENDPOINT secret is set, so this canary cannot reach dubizzle and tested nothing." echo "Imperva refuses this site to any exit outside the UAE - a European" echo "residential address got HTTP 403 and a Finnish datacentre address" echo "got a 'Pardon Our Interruption' page under HTTP 200. A GitHub" @@ -90,6 +85,7 @@ jobs: echo echo "The offline suite (tests.yml) covers everything that does not" echo "need the network, and it runs on every push." + exit 1 - uses: actions/setup-python@v5 if: env.HAVE_CDP == 'true' @@ -135,64 +131,57 @@ jobs: continue-on-error: true env: DUBIZZLE_CDP_ENDPOINT: ${{ secrets.DUBIZZLE_CDP_ENDPOINT }} + # + # The log is kept (tee) because a parser failure is named only there: + # a page served with ads in its payload that parses to zero rows ends + # the run with stop_reason `parser_found_nothing` and exit 5 (or 6 + # with rows from earlier pages), and the verdict below must fail on + # that by name rather than read it as an access condition. run: | set +e python3 playwright_scraper.py \ --url "https://uae.dubizzle.com/motors/used-cars/" \ - --pages 3 --retries 4 --out canary_run --dump-html canary_run.html - echo "exit_code=$?" >> "$GITHUB_OUTPUT" + --pages 3 --retries 4 --out canary_run --dump-html canary_run.html \ + 2>&1 | tee canary_run.log + echo "exit_code=${PIPESTATUS[0]}" >> "$GITHUB_OUTPUT" - name: Interpret the exit code id: verdict if: env.HAVE_CDP == 'true' - # ACCESS CONDITIONS ARE NOT DEFECTS, and on this site that - # distinction decides whether the badge means anything. - # - # Failing the job on ANY non-zero exit makes the canary red for two - # things that are nobody's bug: - # - # * the SECRET EXPIRED. A Scraping Browser credential is not - # forever. When it stops authenticating the run exits 5. - # * ACCESS IS INTERMITTENT PER PROFILE, and a profile allows one - # live connection. A canary that collides with a person's run - # exits 5 with `profile_locked`, which says nothing about the - # site or the code. - # - # So: 3 (blocked), 5 (remote API error) and 6 (partial) are reported - # as WARNINGS and do not fail the job. What stays red is a run that - # got in and found the wrong thing, or one that broke: - # - # 1 crashed - # 2 the workflow's own arguments are wrong - # 4 served a page and parsed ZERO rows from a motors listing that - # holds 25 ads per page — the payload action or the tile anchor - # moved, which is exactly what this canary exists to catch - # - # A warning must not read as a pass, either. Each one writes to the - # step summary as well, so the run page says in words that the canary - # did not actually test anything this time. + # ONLY exit 0 is a pass. A blocked run (3), a run that never obtained + # its pages (5) and a partial run (6) all mean the site was not fully + # tested, so each is red with the reason written to the step summary; + # so are a crash (1), bad arguments (2) and zero rows (4). Access + # problems are not code defects, but a canary that goes green when it + # tested nothing is worse than one that says "fix the access". run: | set -e code="${{ steps.run.outputs.exit_code }}" tested=true + # A parser failure is a regression whatever the exit code says: + # exit 5/6 would otherwise be downgraded to a warning below. + if grep -q "parser_found_nothing" canary_run.log; then + echo "::error::A page was served with ads in its payload and parsed to 0 rows (stop_reason parser_found_nothing, exit $code). The payload shape or the parser moved — this is the regression this canary exists to catch. The HTML dump in the artefacts shows what was served." + exit 1 + fi case "$code" in 0) echo "OK — ads parsed from a live run." ;; 3) - tested=false - echo "::warning::Blocked before parsing (exit 3) — an ACCESS condition, not a code defect, so this run is not failed. On this site a block means the exit is not in the UAE: check that country-ae is in the endpoint's login. There is no challenge on the page to solve and no captcha key would help." + echo "::error::Blocked before parsing (exit 3), so nothing was tested. On this site a block means the exit is not in the UAE: check that country-ae is in the endpoint's login. There is no challenge on the page to solve and no captcha key would help." echo "### Canary did NOT test anything (exit 3, blocked)" >> "$GITHUB_STEP_SUMMARY" - echo "The site did not serve the request, so the parser was not exercised. This is access, not a regression — the first thing to check is the exit's COUNTRY, not the browser." >> "$GITHUB_STEP_SUMMARY" ;; + echo "The site did not serve the request, so the parser was not exercised. This is access, not a regression — the first thing to check is the exit's COUNTRY, not the browser." >> "$GITHUB_STEP_SUMMARY" + exit 1 ;; 5) - tested=false - echo "::warning::Remote API error (exit 5) — the Scraping Browser endpoint refused the connection, so nothing was tested and this run is not failed. If the log says 'profile_locked', another run still holds this pid: give CI its own. Otherwise the DUBIZZLE_CDP_ENDPOINT secret has most likely expired and needs refreshing." - echo "### Canary did NOT test anything (exit 5, endpoint refused)" >> "$GITHUB_STEP_SUMMARY" - echo "The most likely cause is an expired \`DUBIZZLE_CDP_ENDPOINT\` secret. Refresh it, or remove it to go back to the explicit SKIP path." >> "$GITHUB_STEP_SUMMARY" ;; + echo "::error::Content never obtained (exit 5), so nothing was tested. The log names the cause: 'profile_locked' means another run still holds this pid (give CI its own); an HTTP 401 means the DUBIZZLE_CDP_ENDPOINT credential is no longer accepted and needs refreshing; a navigation timeout is neither." + echo "### Canary did NOT test anything (exit 5, content never obtained)" >> "$GITHUB_STEP_SUMMARY" + echo "Read the run log for the cause (profile_locked, an HTTP 401 on the endpoint, or a navigation timeout) before refreshing the \`DUBIZZLE_CDP_ENDPOINT\` secret." >> "$GITHUB_STEP_SUMMARY" + exit 1 ;; 6) - tested=false - echo "::warning::Partial run (exit 6) — some pages were gathered and the run then stopped early. canary_run.meta.json names which pages failed. Not failed, because on this site that is usually an access condition; if it repeats on every run, suspect pagination instead." + echo "::error::Partial run (exit 6) — some pages were gathered and the run then stopped early. canary_run.meta.json names which pages failed." echo "### Canary tested only PART of the run (exit 6)" >> "$GITHUB_STEP_SUMMARY" - echo "See \`pages_failed\` in the sidecar artefact." >> "$GITHUB_STEP_SUMMARY" ;; + echo "See \`pages_failed\` in the sidecar artefact." >> "$GITHUB_STEP_SUMMARY" + exit 1 ;; 4) echo "::error::Ran fine but parsed 0 rows (exit 4) on a motors listing that holds 25 ads per page. Either the Redux action the payload lives in (listings/fetchListingDataForQuery/fulfilled) or the ad-URL pattern the DOM fallback anchors on has moved — this is the regression this canary exists to catch. The HTML dump in the artefacts tells you which." exit 1 ;; @@ -209,7 +198,7 @@ jobs: # Only when the run actually got in. On a blocked or refused run # there is no output file to check, and a missing file would fail for # the wrong reason. - if: env.HAVE_CDP == 'true' && steps.verdict.outputs.tested == 'true' + if: env.HAVE_CDP == 'true' # Exit 0 only means "some rows were written". It says nothing about # whether the page actually rendered: a snapshot taken too early # still writes rows, just with fields empty. Every floor below is set @@ -252,7 +241,7 @@ jobs: f"read has most likely broken") # A row with no price must carry no currency, and vice versa. A - # missing currency is null, never a default (§4). + # missing currency is null, never a default. mismatched = [r.get("sku") for r in rows if (r.get("price") is None) != (r.get("currency") is None)] if mismatched: @@ -311,7 +300,7 @@ jobs: fail.append(f"{got}/{len(rows)} motors rows have `{field}`, " f"which only a property ad publishes") - # §4's guard, one line, and it catches the regression forever. + # One line, a guard against the was-price regression, and it catches the regression forever. bad = [r.get("sku") for r in rows if r.get("original_price") is not None and r.get("price") is not None @@ -345,10 +334,9 @@ jobs: kinds = {r.get("listing_kind") for r in rows} if kinds - {"organic", "car_of_the_week"}: fail.append(f"unknown listing_kind(s) {sorted(kinds)}") - if "car_of_the_week" not in kinds: - fail.append("no row is labelled car_of_the_week - every motors " - "page carries one, so the promoted slot is being " - "passed off as a result") + # Whether a promoted slot is present is the site's merchandising + # and is not required; when one IS present it must be labelled + # (the unknown-kind check above). # `position` restarts at 1 on each page, so the pair is the key. pairs = [(r.get("page"), r.get("position")) for r in rows] @@ -406,6 +394,7 @@ jobs: canary_run.csv canary_run.meta.json canary_run.html* + canary_run.log canary_run_page*_debug.* if-no-files-found: ignore retention-days: 14 diff --git a/.gitignore b/.gitignore index 4011e13..3bdaa3f 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,24 @@ live_results.zip # the multi-megabyte originals they were cut from are not. captures/ +# Working directories by KIND, not by the one name that leaked once: a raw +# dump reaches `git add -A` through whatever directory someone happened to +# call it, and `live_results/` above covered only one spelling. A bare .html +# at the repo root is a dump too; the repo ships none. +live/ +out/ +output/ +run/ +runs/ +result/ +results/ +tmp/ +scratch/ +dump/ +dumps/ +/*.html +/*.htm + # --- the landing page and the proxy list --------------------------------- # Both live outside the repo on purpose: the landing page is published # elsewhere, and a downloaded 2captcha proxy list is a credential file. @@ -86,7 +104,7 @@ secrets* # recoverable only because that repo was private at the time. On 2026-09-17 # a sibling had an untracked `.env.bak` holding a 32-hex key in a PUBLIC # repository, one `git add -A` from publication. Craigslist was fixed then -# and the fix never reached anyone else, which is §16's rule unapplied: when +# and the fix never reached anyone else, which is the lesson unapplied: when # you fix one, check the siblings. .env* !.env.example diff --git a/CHANGELOG.md b/CHANGELOG.md index 42c0345..fd5ee29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,55 @@ user would notice, the release notes say so first. > script of yours passes 0, pass **1**: the flag counts ATTEMPTS, so 1 means > "try once and do not retry". +> **Behaviour change:** a served listing whose payload names ads but parses +> to zero rows now ends with `stop_reason: parser_found_nothing` and exit 5 +> (exit 6 if earlier pages gave rows). It used to be exit 4 ("nothing +> listed") on page 1 and a complete run with exit 0 on page 2 or later. + +> **Behaviour change:** `--pages 0`, `--concurrency 0` and negative +> `--delay`, `--retry-delay` or `--proxy-block-retries` (and, in +> `scraper_api_client.py`, negative `--retries`/`--retry-delay` or a +> `--timeout` below 1) are now usage errors (exit 2) naming the flag. + +> **Behaviour change:** the CSV output prefixes an apostrophe to any text +> cell that begins with `=`, `+`, `-`, `@`, a tab or a line break, so a +> spreadsheet reads it as text. JSON is unchanged; the sidecar records +> `csv_cells_escaped`. + ### Fixed +- **A paid solve was budgeted at one of its two call sites.** Each engine + checks for a challenge right after navigation and again after + classification; only the second was counted, so with the solver stubbed + one challenged page bought 3 tokens against `SOLVES_PER_PAGE = 1`. Both + call sites now go through one `page_flow.SolveBudget`, checked immediately + before the purchase and not reset by a retry or a rotation. A source check + and a stubbed run of every engine pin it. +- **pyppeteer crashed when a page failed to load and no proxy was set** + (`AttributeError` on `pool.advance`, exit 1). It now rotates only when there + is a pool, as the Playwright engine does, and reports the failed load. +- **The HTTP status never reached the classifier**, so dubizzle's own 404 was + read as an unpainted page, waited on and parsed. Playwright and pyppeteer + now pass it; Selenium, which has no status, recognises the 404 from its + body (`product_parser.is_not_found_page`). +- **Formula-shaped CSV cells** (seller-written titles such as + `=HYPERLINK(...)`) are neutralised in CSV only. +- **The canary now passes only when it actually tested the site.** Without + the `DUBIZZLE_CDP_ENDPOINT` secret it fails and names the secret (it used + to skip green); exit 3, 5 and 6 are red with the reason in the step + summary (they used to be warnings); a logged `parser_found_nothing` is red + as a parser failure; it runs weekly as well as on demand; and it no longer + requires a promoted "car of the week" row, which is merchandising rather + than parsing. +- `.gitignore` and the tracked-capture check now cover working directories + by kind (`live/`, `out/`, `runs/`, `tmp/`, `dumps/`, ...) and bare HTML + files at the root. +- Comments, help text and the Dockerfile example no longer describe another + site's search grid, tracking parameters or scroll counts; the README's + exit-code line says what 5 means now; references to private notes were + replaced by the rule in plain words, and a suite check keeps them out. The + wording check now scans the suite file too. + - **`--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/CONTRIBUTING.md b/CONTRIBUTING.md index 7ae8ec8..f61b83d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,13 +86,13 @@ Then the rest of the presentation, in the order that matters: 1. `python3 smoke_test.py` green, and the canary dispatched at least once — including its SKIP branch, which is what runs when the - `DUBIZZLE_CDP_ENDPOINT` secret is absent. The canary has **no schedule**: a - Scraping Browser credential on this account does not survive a day, so it - runs on demand with a fresh secret rather than painting a badge that has - tested nothing. Put a schedule back the day a long-lived credential - exists. -2. The repo description, homepage and topics set (see the family notes on - what those should say). + `DUBIZZLE_CDP_ENDPOINT` secret is absent. The canary has **no schedule** + and runs on demand. How long a Scraping Browser credential stays valid has + not been measured on this repo, so re-measure it before deciding whether + to put a schedule back. +2. The repo description, homepage and topics set: a description that names + the engines and the 2Captcha Scraping Browser API, `https://2captcha.com` + as the homepage, and topics people actually search for. 3. Only then the row in the org profile README — and check it with an ANONYMOUS request rather than your own logged-in browser. A row pointing at a private repo is a 404 for every visitor, which costs more trust than diff --git a/Dockerfile b/Dockerfile index ae07d7a..c0bc8ce 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,7 +5,7 @@ # docker build -t dubizzle-scraper . # docker run --rm -v "$PWD/out:/out" dubizzle-scraper \ # --url "https://uae.dubizzle.com/motors/used-cars/" \ -# --pages 3 --out /out/mugs +# --pages 3 --out /out/used-cars # # Pass --proxy/--twocaptcha-key the same way as running locally, or mount a # .env at /app/.env — nothing here bakes in a credential. diff --git a/README.md b/README.md index e3047fc..14250e3 100644 --- a/README.md +++ b/README.md @@ -179,8 +179,10 @@ rather than `changed`. ### Exit codes -`0` ok · `1` crash · `2` bad usage · `3` blocked · `4` zero ads · `5` remote -API error · `6` partial. Every run writes `.meta.json` beside its output +`0` ok · `1` crash · `2` bad usage · `3` blocked · `4` zero ads · `5` the +content was never obtained (a navigation timeout, a dead proxy, a refused +remote endpoint, or a served page with ads in its payload that parsed to zero +rows — `stop_reason: parser_found_nothing`) · `6` partial. Every run writes `.meta.json` beside its output with `status`, `stop_reason` and **which** pages failed by number. A failed run writes no sidecar and leaves the previous good output in place — a run that finds nothing does not overwrite last night's data unless you pass @@ -504,12 +506,12 @@ and a check that no `.env`, test suite or fixture was baked in. **What the canary badge here does and does not mean.** The workflow is a real three-page run against a real listing URL, with a floor on the rows, -page+position uniqueness and a dozen other assertions. But this repository -deliberately holds **no live credential**, so the live steps are skipped and -the job goes green with a `::notice::` saying why — a check that is always red -teaches everyone to ignore checks, and a check that is always green teaches -the same lesson more quietly. So read the green canary badge as *"the workflow -is wired up"*, not as *"the site was verified today"*. +page+position uniqueness and a dozen other assertions, run weekly and on +demand. It needs a UAE exit, so it reads a `DUBIZZLE_CDP_ENDPOINT` secret; +**without that secret the job fails** and names it, and a blocked, partial or +never-obtained run is red too. A green badge therefore means the site was +read and parsed on that run, and a red one says in its summary whether the +cause was access or the parser. To make it do real work in your own fork, set one secret and change nothing else: diff --git a/env_config.py b/env_config.py index 2b0f1d9..85d62bf 100644 --- a/env_config.py +++ b/env_config.py @@ -67,7 +67,7 @@ # A literal-only check reported both of those as CONFIGURED, so `cp # .env.example .env` followed by a run connected to cb.2captcha.com with the # string `{login}-zone-…` as its username and got a 401 — a confusing failure -# a long way from its cause, which is precisely what §3's rule exists to +# a long way from its cause, which is precisely what the placeholder check exists to # prevent. Any `{…}` left in a value means the line was never filled in. _BRACED_PLACEHOLDER_RE = re.compile(r"\{[A-Za-z_][A-Za-z0-9_]*\}") @@ -204,7 +204,7 @@ def _dests_given_on_argv(dests, argv=None): headful path and its Scraping Browser path are both first-class, and switching between them is routine rather than exceptional. - Scanning argv is the smallest fix that keeps §3's precedence intact -- + Scanning argv is the smallest fix that keeps the documented precedence intact -- explicit flag, then exported variable, then `.env`, then default -- with "explicit" now meaning what the caller typed rather than what happens to be truthy. diff --git a/fingerprint_client.py b/fingerprint_client.py index ea2e008..c16b62c 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -248,7 +248,7 @@ def playwright_context_kwargs(fp: dict) -> dict: # 2026-09-11: a fingerprint stating `deviceScaleFactor: 1.25` produced a # browser reporting `window.devicePixelRatio === 1`, so the identity # contradicted itself on an axis any fingerprinter reads for free. Same - # shape as the three defects §16 lists -- a key the API returns that + # shape as other fingerprint-client defects fixed here -- a key the API returns that # nothing applies -- and found the same way, by comparing the live # browser against the fingerprint rather than by reading the code. scale = (fp.get("screen") or {}).get("deviceScaleFactor") @@ -329,7 +329,7 @@ def main() -> int: # loader. It used to read `os.environ` alone, which meant a key put in # `.env` exactly as README and .env.example instruct worked for every # engine and failed here with "No API key" — a documented mechanism not - # applied on one path, which is the shape of half the defects §16 lists. + # applied on one path, a defect shape this client has had before. # `.env` has to be LOADED before it can be read: `env_value` looks at # os.environ, which `load_env` is what fills from the file. Calling it # here rather than relying on an engine having called it is the whole diff --git a/make_fixtures.py b/make_fixtures.py index 6f824df..43ef327 100644 --- a/make_fixtures.py +++ b/make_fixtures.py @@ -85,7 +85,7 @@ # that `position` increments or that a dedupe has anything to do. KEEP_ADS = 2 -# Patterns, not literals, so the NEXT capture's values are caught too (§10). +# Patterns, not literals, so the NEXT capture's values are caught too. SCRUB = [ # A real person's name, on every property tile. (re.compile(r'("agent_profile":\s*\{(?:[^{}]|\{[^{}]*\})*?"name":\s*\{"en":\s*)"[^"]*"(,\s*"ar":\s*)"[^"]*"'), @@ -309,7 +309,7 @@ def main(): "expect_state": P.detect_page_state(trimmed, status, url), "expect_rows": len(after), "expect_skus": sorted(after), - # Pinned VALUES, not coverage (§10): a column can be 100% + # Pinned VALUES, not coverage: a column can be 100% # populated and entirely wrong. "expect": {sku: {f: getattr(after[sku], f) for f in after[sku].__dataclass_fields__ @@ -363,7 +363,7 @@ def cut_sample(run="live_p1.json"): """Cut `sample_output.{json,csv}` from a real run's output. Shipped here rather than as a second script because CI checks the sample - against the schema on every push (§10) and a reader who changes a column + against the schema on every push and a reader who changes a column needs one obvious way to regenerate it: python3 playwright_scraper.py --url ... --out live_p1 --format json diff --git a/output_writer.py b/output_writer.py index 8d442d7..b19066f 100644 --- a/output_writer.py +++ b/output_writer.py @@ -67,7 +67,7 @@ class Product: # The listing's own absolute URL, on its emirate subdomain — the address # the site itself publishes, not one rebuilt from the browsed host. Two # sibling repos lost a whole column by rebuilding a URL that the page - # then never matched (§5). + # then never matched. url: str = "" # The listing's URL PATH, with a leading `/ar` stripped and no trailing # slash. @@ -100,7 +100,7 @@ class Product: # # Deliberately NOT derived by splitting a title or reading the URL's # make segment. Both would be a guess wearing the costume of a fact - # (§8), and the URL segment is the CATEGORY's make, which for a listing + #, and the URL segment is the CATEGORY's make, which for a listing # filed under the wrong node is not the car's. brand: Optional[str] = None # What the seller is asking. An integer in the site's payload — no @@ -117,7 +117,7 @@ class Product: # motors and property JSON-LD state `priceCurrency: "AED"` outright, and # the tile prints the ISO code `AED` in its own node beside the amount. # Still null rather than defaulted when there is no price — a row with - # no price has no currency either (§4). + # no price has no currency either. currency: Optional[str] = None # The was-price, from `pre_discount_price`. Sparse ON PURPOSE and worth # keeping: only a promoted "Car of the Week" or a classified listing @@ -128,7 +128,7 @@ class Product: # Computed from the two prices, never read off the printed badge, and # None rather than 0 or a negative when `original_price` is not above # `price` — two figures that are not what they were taken for should - # produce no number at all (§4). + # produce no number at all. discount_pct: Optional[float] = None # Null on every listing row of every run measured so far, and kept only # because it is in the family prefix: this site rates SELLERS on a @@ -307,6 +307,20 @@ def dedupe_by_sku(rows: Sequence[Any], seen: Set[str]) -> List[Any]: # handled) is neither readable nor parseable by anything but Python. LIST_CSV_SEPARATOR = " | " +# A CSV cell beginning with one of these is evaluated as a formula by Excel, +# Google Sheets and LibreOffice. Every string in a row here was written by +# whoever posted the ad, so a title such as `=HYPERLINK(...)` would otherwise +# reach a spreadsheet as live code. `=`, `+`, `-` and `@` are the classic +# shapes; the tab and the newlines are here because some readers strip a +# leading one, which exposes whatever follows it. +CSV_FORMULA_LEADS = ("=", "+", "-", "@", "\t", "\r", "\n") + +# Prefixed to a cell that would otherwise be read as a formula. Spreadsheets +# treat the apostrophe as "the rest of this cell is text" and do not show it; +# 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_value(v: Any) -> Any: if isinstance(v, (list, tuple)): @@ -319,12 +333,27 @@ def _csv_value(v: Any) -> Any: return v +def _csv_escape(v: Any) -> Any: + """Neutralise a formula-shaped cell. Returns (value, was_escaped). + + Only STRINGS are touched. Escaping a number would turn `-5` into the text + `'-5` and break every sum a consumer writes over the column. + + CSV only. The JSON output keeps the site's bytes exactly as served, so + the two files deliberately differ; `csv_cells_escaped` in the sidecar is + what declares that divergence instead of leaving it to be discovered. + """ + 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: 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 = Product) -> None: +def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> int: # 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 @@ -332,12 +361,24 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> 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. + # + # Returns how many cells were formula-escaped (see _csv_escape). fieldnames = [f.name for f in fields(row_cls)] + escaped = 0 with open(path, "w", encoding="utf-8", 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 + # while the list itself is not a string. + 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 @@ -478,7 +519,8 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, def save(rows: Sequence[Any], out_prefix: str, fmt: str, - allow_empty: bool = False, row_cls: Type = Product) -> int: + allow_empty: bool = False, row_cls: Type = Product, + stats: Optional[dict] = None) -> int: """Write JSON/CSV and return a process exit code. Returns 0 when rows were written, EXIT_NO_PRODUCTS when there were none. @@ -505,8 +547,19 @@ def save(rows: Sequence[Any], out_prefix: str, fmt: str, write_json(rows, f"{out_prefix}.json") print(f"[+] Saved {len(rows)} products -> {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)} products -> {out_prefix}.csv") + # Reported through `stats` rather than as a return value because + # this function's return IS the exit code. The caller threads it + # into the sidecar, where a consumer comparing the CSV against the + # JSON can see that they diverge and by how much. + 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 @@ -561,12 +614,20 @@ 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, Product) - rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls) + write_stats: dict = {} + rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls, + stats=write_stats) wrote_output = bool(rows) or allow_empty if wrote_output: status = "complete" if (rows and complete) else ( "partial" if rows else "failed") + # The CSV/JSON divergence is a property of THIS run's output, so it + # belongs beside the other run facts. Merged UNDER the caller's + # `extra`: an engine's own extras win a name collision, because a + # collision here would be housekeeping silently dropping site data. + if write_stats: + extra = {**write_stats, **(extra or {})} write_run_meta(out_prefix, run_meta( status=status, stop_reason=stop_reason, pages_requested=pages_requested, pages_completed=pages_completed, diff --git a/page_flow.py b/page_flow.py index 54fabda..082ead8 100644 --- a/page_flow.py +++ b/page_flow.py @@ -2,7 +2,7 @@ dubizzle answers a request five ways, and four of them want a different response, which is why this module exists rather than the same triage being -written three times inside three engines and drifting apart (§1): +written three times inside three engines and drifting apart: content the site served its listing payload empty served exactly as asked and holding no listing — a page past @@ -29,7 +29,7 @@ content() -> Optional[str] current HTML, None if unavailable sleep(ms) -> None wait -No JavaScript crosses that boundary in either direction (§1): Selenium's +No JavaScript crosses that boundary in either direction: Selenium's `execute_script` takes a function BODY with an explicit `return` where Playwright and pyppeteer take `() => expr`, so the shared module names the OPERATION and the engine spells it in its own driver's dialect. @@ -64,8 +64,8 @@ # --------------------------------------------------------------------------- # Readiness # --------------------------------------------------------------------------- -# The anchor is a price node WITH SOMETHING IN IT, not a card count, for the -# reason §8 gives: a selector matching the wrong element is worse than one +# The anchor is a price node WITH SOMETHING IN IT, not a card count, because +# a selector matching the wrong element is worse than one # matching nothing. A tile's shell and its price node can both exist while # the amount is still blank, and a wait satisfied by the shell is a wait that # did not wait. @@ -80,7 +80,7 @@ # agree. READY_SELECTOR_LISTING = '[data-testid="listing-price"]' -# Above 1, per §5: waiting for a single match resolves on an unrelated node +# Above 1: waiting for a single match resolves on an unrelated node # long before a grid paints. Below the smallest real page, which is 25 ads on # motors, classified, jobs and community and 35 on both property indexes — # so 8 leaves room for a page that is genuinely short (a jobs category with @@ -112,7 +112,7 @@ def min_matches(mode: str = "listing", expected: Optional[int] = None) -> int: this site states it: a last page of a jobs category holds 23 ads and waiting for 8 of them is right, while waiting for 8 on a page that holds 3 would time out on a page that is fully painted. Never above the floor - and never below 2 (§5). + and never below 2. """ floor = _MIN.get(mode, MIN_CARD_MATCHES) if expected is None: @@ -148,7 +148,7 @@ def wait_for_count(count: Callable[[str], int], sleep: Callable[[int], None], `EvalError: Evaluating a string as JavaScript violates the following Content Security Policy directive` on a site whose CSP has no `unsafe-eval`, and took the run down with exit 1 on the site's most - obvious URL (§18). Counting elements goes through the protocol and works + obvious URL. Counting elements goes through the protocol and works under any CSP, and spells the same in all three drivers. """ waited = 0 @@ -177,7 +177,7 @@ def classify(html: Optional[str], status: Optional[int] = None, Getting that wrong is not a style question: a sibling repo shipped two of three engines calling this as `classify(html, url=...)`, both crashed on their first fetch, and nothing short of a live run or a signature-binding - check saw it (§17). + check saw it. """ if html is None: return "blocked" @@ -185,7 +185,7 @@ def classify(html: Optional[str], status: Optional[int] = None, # The retry/solve/blocked decision as DATA rather than as three copies of an -# if-chain in three engines (§1). +# if-chain in three engines. # # parse is there anything on this page worth writing down? # retry would fetching it again, from a different exit, plausibly help? @@ -203,7 +203,7 @@ def classify(html: Optional[str], status: Optional[int] = None, # results. A hub URL is the same story with 45 rail links instead of one. "empty": {"parse": False, "retry": False, "solve": False, "blocked": False}, # Served, and still painting. Wants the readiness wait, not another - # fetch: refetching a shell just buys another shell (§18). + # fetch: refetching a shell just buys another shell. "shell": {"parse": True, "retry": False, "solve": False, "blocked": False}, "challenge": {"parse": False, "retry": True, "solve": True, "blocked": False}, "blocked": {"parse": False, "retry": True, "solve": False, "blocked": True}, @@ -252,7 +252,7 @@ def is_unpainted(state: str, html: Optional[str]) -> bool: # the country the site serves. A retry from a different exit is worth # spending, and the engines read these constants rather than computing their # own budget — a policy constant nothing consults is the same defect as dead -# code (§17). +# code. RETRY_ON_BLOCKED = True BLOCK_RETRIES_WITHOUT_POOL = 1 BLOCK_RETRIES_WITH_POOL = 3 @@ -262,6 +262,51 @@ def is_unpainted(state: str, html: Optional[str]) -> bool: SOLVES_PER_PAGE = 1 +class SolveBudget: + """The one allowance every paid solve on a page goes through. + + Created once per page by each engine's `_fetch_one_page` and handed to + BOTH places that can buy a token: the check that runs right after + navigation (so a challenge is cleared before anything is judged) and the + one that runs after classification, for the state that says the page + really is gated. The allowance used to be counted at the second place + only, so the first one bought a token on every block-retry: with the + solver stubbed, one page made 3 purchases against a limit of 1. + + `allows()` is checked immediately BEFORE a purchase and `spend()` records + one; detecting a challenge and deciding not to solve it costs nothing. + It is deliberately NOT reset by a retry or an exit rotation: a fresh exit + is a reason to fetch again, not a fresh allowance to spend. + """ + + def __init__(self, limit: int = SOLVES_PER_PAGE): + self.limit = limit + self.spent = 0 + + def allows(self) -> bool: + return self.spent < self.limit + + def spend(self) -> None: + self.spent += 1 + + +def failure_reason(outcome) -> str: + """The stop reason for a page that did not yield data. + + One function for every path in every engine, so page 1, the sequential + loop and the concurrent workers cannot name the same failure three ways. + `parser_found_nothing` is a page the site SERVED with ads in its payload + that the parser turned into zero rows — a reading failure on our side, + not an empty listing, so it must not end as exit 4 ("nothing listed") + or, on page 2 and later, as a complete run. + """ + if getattr(outcome, "load_failed", False): + return "page_load_timeout" + if getattr(outcome, "parse_failed", False): + return "parser_found_nothing" + return f"blocked_{outcome.blocked_by}" + + def block_advice(html: Optional[str], headless: bool, has_pool: bool) -> str: """What a reader should actually DO about this block. @@ -289,7 +334,7 @@ def block_advice(html: Optional[str], headless: bool, # --------------------------------------------------------------------------- # Pagination # --------------------------------------------------------------------------- -# Three layers, weakest last (§7), and on this site every one of them is +# Three layers, weakest last, and on this site every one of them is # available and they agree: # # 1. `` in — a standards-based signal, present on @@ -319,7 +364,7 @@ def pagination_is_addressable(page1_url: str, next_href: Optional[str] = None) -> bool: """Whether page N can be fetched without walking pages 2..N-1. - True for every listing grid here, and verified the way §7 asks rather + True for every listing grid here, and verified rather than assumed: page 1's own `` is `https://uae.dubizzle.com/motors/used-cars/?page=2`, which is exactly what the convention builds, and the fetched page 2 reported @@ -344,7 +389,7 @@ def _resolve(base_url: str, href: str) -> str: so a comparison that does not resolve first fails for half the links it is given. That reads as "the site's link disagrees with the convention", which sends the caller down the link-chaining path and quietly costs it - `--concurrency` (§7) — while looking like a deliberate safety decision. + `--concurrency` — while looking like a deliberate safety decision. """ return urljoin(base_url, href) if href else href @@ -358,7 +403,7 @@ def _as_hrefs(next_href: Union[str, Iterable[str], None]) -> List[str]: is what took a sibling repo's FIRST live run down — `TypeError: Cannot mix str and non-str arguments`, from `urljoin` being handed a list — and nothing short of running it saw that, because the offline checks called - it the way its author was thinking (§17). + it the way its author was thinking. """ if next_href is None: return [] @@ -455,7 +500,7 @@ def concurrency_limit(url: str) -> Optional[int]: def concurrency_refusal(url: str) -> Optional[str]: """Why concurrency above 1 is refused for this URL, or None. - Refused WITH the reason (§18): an individual ad and a hub page have no + Refused WITH the reason: an individual ad and a hub page have no page 2 at all, so there is nothing for a second worker to fetch, and silently running one worker would look like the flag did something. """ @@ -514,3 +559,41 @@ def attempts(value) -> int: f"With 0 the load loop never runs, nothing is fetched, and the " f"engine reports the browser's blank page as a result.") return number + + +def _bounded_number(flag: str, cast, minimum, what: str): + """An argparse `type=` that rejects a value below `minimum`, naming `flag`. + + A negative page count, delay or worker count is never what the caller + meant, and argparse accepts all of them silently. `--delay -1` reaches + `time.sleep(-1)`, which raises ValueError between page 1 and page 2, so + the run dies with a traceback after doing real work; `--pages 0` and + `--concurrency 0` are silently read as 1. + """ + def check(value): + try: + number = cast(value) + except (TypeError, ValueError): + raise _argparse.ArgumentTypeError( + f"{flag} must be {what}, not {value!r}") + if number < minimum: + raise _argparse.ArgumentTypeError( + f"{flag} must be at least {minimum}, not {value}") + return number + check.__name__ = "at_least_%s" % str(minimum).replace(".", "_") + return check + + +def positive_int(flag: str): + """`type=` for a count that must be 1 or more (--pages, --concurrency).""" + return _bounded_number(flag, int, 1, "an integer") + + +def non_negative_int(flag: str): + """`type=` for a count where 0 is meaningful (--proxy-block-retries).""" + return _bounded_number(flag, int, 0, "an integer") + + +def non_negative_float(flag: str): + """`type=` for a number of seconds (--delay, --retry-delay).""" + return _bounded_number(flag, float, 0, "a number") diff --git a/playwright_scraper.py b/playwright_scraper.py index 730fa7a..e843a77 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -13,7 +13,7 @@ language is a path segment on a single platform, so a country flag could only disagree with the URL it was given; a detail mode is not shipped because the ad pages were not measured, and a mode whose markup nobody has read is a mode -that reports success without checking (§8). +that reports success without checking. WHAT IS DIFFERENT ABOUT THIS SITE --------------------------------- @@ -174,9 +174,14 @@ class PageOutcome: # reported it as complete would read as a shrinking catalogue. scroll: Optional[dict] = None + # Served with ads in the payload, parsed to zero rows: our failure, not + # an empty listing. See page_flow.failure_reason. + parse_failed: bool = False + @property def ok(self) -> bool: - return not self.load_failed and self.blocked_by is None + return (not self.load_failed and self.blocked_by is None + and not self.parse_failed) ITEM_LINK_SELECTOR = page_flow.READY_SELECTOR_LISTING @@ -383,10 +388,9 @@ def _same_url(a: str, b: str) -> bool: fetching — the exact divergence page_flow.py exists to prevent, reproduced inside one engine. - On this site the comparison has to strip a long tracking tail: a listing - anchor arrives with `?extParam=…keyword=kopi&search_id=…&src=search` and - a detail page's own canonical arrives with a UTM triple, so two views of - one page never match unless both sides are cleaned. + The comparison drops campaign parameters (`utm_*` and the rest of + product_parser.TRACKING_PARAMS), a `www.` prefix and a trailing slash, + so two spellings of one dubizzle page compare equal. """ return page_flow.comparable(a) == page_flow.comparable(b) @@ -602,8 +606,8 @@ def _content_when_settled(page, attempts: int = 4, pause_ms: int = 700): Playwright raises `Page.content: Unable to retrieve content because the page is navigating and changing the content` if the document swaps under - it. dubizzle does not geo-redirect: the locale is a path segment, and euro - from two exit countries — but its shell navigates on its own during + it. dubizzle does not geo-redirect (the locale is a path segment), but + its shell navigates on its own during hydration, and the remote Scraping Browser target was observed closing mid-scroll on three of six captures, so a snapshot taken right after goto() can land exactly on a swap. @@ -628,7 +632,7 @@ def _content_when_settled(page, attempts: int = 4, pause_ms: int = 700): return None -def handle_captcha_if_present(page, args) -> bool: +def handle_captcha_if_present(page, args, budget=None) -> bool: """Detect and solve a challenge. True if something was solved. Runs after EVERY navigation, for ANY page — not scoped to one URL. The @@ -685,6 +689,16 @@ def handle_captcha_if_present(page, args) -> bool: "solved — continuing with whatever the page already " "holds.") return False + # Checked immediately before the purchase, against the page's single + # allowance (page_flow.SolveBudget), so the pre- and post-classification + # calls together cannot buy more than SOLVES_PER_PAGE tokens. + if budget is None or not budget.allows(): + logger.info("Not solving: this page's solve allowance (%d) is spent. " + "A challenge that survives a solved token is not one this " + "run can pass, and a second solve is a second charge.", + page_flow.SOLVES_PER_PAGE) + return False + budget.spend() try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -713,7 +727,7 @@ def _parse_for_mode(html: str, url: str, args, page_num: int = 1) -> List: restarts at 1 on every page: without the page number beside it, a row from page 2 claims the same position as one from page 1 and the two are indistinguishable in the output. A sibling repo's first live run wrote - 120 rows all labelled page 1 for exactly that reason (§18). + 120 rows all labelled page 1 for exactly that reason. """ return parse_products(html, url, page=page_num, category=args.category) @@ -752,11 +766,20 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome block_retries = 0 if not page_flow.RETRY_ON_BLOCKED else ( args.proxy_block_retries if has_pool else page_flow.BLOCK_RETRIES_WITHOUT_POOL) - # Counted across the whole block-retry loop, not per attempt: a page that + # ONE allowance for every paid solve on this page, handed to both call + # sites below and counted across the whole block-retry loop: a page that # keeps coming back as a challenge would otherwise buy one solve per - # rotation, which is how a run quietly turns into a bill. - solves_bought = 0 + # rotation, which is how a run quietly turns into a bill. See + # page_flow.SolveBudget. + budget = page_flow.SolveBudget(page_flow.SOLVES_PER_PAGE) html, state, load_failed = None, "ok", False + # The HTTP status of the navigation, handed to the classifier. Without + # it a 404 category — which dubizzle serves with its asset references but + # no grid — classified as a `shell`, cost the full readiness wait and was + # then parsed. Reset to None whenever the document changes under us (a + # solve reloads the page), so a later page is judged by what it holds + # rather than by the status of the one it replaced. + status = None for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) @@ -765,7 +788,9 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome load_failed, exit_failed = False, 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) + status = response.status if response is not None else None load_failed = False break except (PWTimeout, PWError) as e: @@ -797,25 +822,23 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if load_failed: break - if handle_captcha_if_present(session.page, args): + if handle_captcha_if_present(session.page, args, budget): # A solve navigated the page. Give the destination a moment # before judging what came back. session.page.wait_for_timeout(1000) + status = None html = _content_when_settled(session.page) or "" - state = _classify(session.page, html) - - # "Not painted yet" is not a fault, and telling it apart from one is - # what the first live search run of this engine got wrong. A CATEGORY - # listing server-renders its grid container, so it classifies as - # content at domcontentloaded; a SEARCH grid arrives with the - # client-side GraphQL response, so at that moment the page is a - # 608 KB shell with no grid in it. Classified naively that is - # "unknown", "unknown" retries, and the run fetched the page twice, - # scrolled not at all and reported 0 rows with exit 4. - # - # So wait for the anchor and re-classify BEFORE the retry decision. - # See page_flow.is_unpainted. + state = _classify(session.page, html, status) + + # "Not painted yet" is not a fault. A page dubizzle served (it is + # built from the site's own assets) but whose listings payload is not + # in the document yet classifies as `shell`. Fetching it again would + # only buy another shell, so wait for the anchor and re-classify + # BEFORE the retry decision. None of the captured pages in + # fixtures_generated.json is a shell — a category grid arrives + # server-rendered — so this wait is a safety net, not a measured + # path. See page_flow.is_unpainted. if page_flow.is_unpainted(state, html): wait_timeout = page_flow.content_timeout_ms(args.mode) logger.info("Page %d is a shell the site served but has not " @@ -830,7 +853,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome logger.info("The grid still had not painted after %.0fs " "(%d match(es)).", wait_timeout / 1000, found) html = _content_when_settled(session.page) or html - state = _classify(session.page, html) + state = _classify(session.page, html, status) # No interstitial-settling step here, and its absence is measured # rather than an omission. This site has no interstitial to settle: a @@ -844,12 +867,12 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # cannot name what stopped it is much harder to fix — and bounded by # SOLVES_PER_PAGE so a speculative path cannot become a bill. if (page_flow.should_solve(state) - and solves_bought < page_flow.SOLVES_PER_PAGE): - solves_bought += 1 - if handle_captcha_if_present(session.page, args): + and budget.allows()): + if handle_captcha_if_present(session.page, args, budget): session.page.wait_for_timeout(1000) + status = None html = _content_when_settled(session.page) or html - state = _classify(session.page, html) + 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. @@ -873,8 +896,9 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # Blocked or challenged. A different exit is the one thing that # plausibly changes the outcome: the ADDRESS is what was scored, not # the URL, so retrying it unchanged would only confirm it. Measured - # 2026-09-09 — the same URL that answers 403 from a datacentre exit - # answers 200 from a residential one. + # 2026-09-14 — the same listing refused to a European residential + # exit and to a Finnish datacentre exit was served in full to a UAE + # residential one (see page_flow's notes above RETRY_ON_BLOCKED). if block_attempt < block_retries: if has_pool: logger.warning("Page %d came back as %s from %s — retrying " @@ -951,10 +975,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # # NO scroll follows, and that is measured: three scrolls to the # document's own bottom added zero cards and left the page height - # unchanged on all three page kinds. The whole page is served at once, - # so without the scroll below a listing run returns whatever the - # first batch happened to have painted. Measured on one search URL: - # 11 tiles at round 0, 95 after one scroll. + # unchanged on all three page kinds. The whole page is served at once. selector, threshold = _ready_selector(args), _min_matches(args, html) content_timeout = page_flow.content_timeout_ms(args.mode) # A POLL, not wait_for_function. This site's CSP does allow @@ -967,13 +988,10 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome session.page.wait_for_timeout, selector, threshold, content_timeout) session.page.wait_for_timeout(500) if found <= threshold: - # Not an error on its own, and what it MEANS depends on the - # mode — which is why the message does too. A listing page with - # no grid is a correct answer (a taxonomy hub, or one page past - # the end); a detail page whose buy box never painted is a - # different thing entirely, and on this site it is usually just - # slow rather than absent, because the row is parsed out of the - # page's JSON-LD and not out of the buy box. + # Not an error on its own. A listing page with no grid is a + # correct answer (a hub page, or one page past the end), and + # classification has already told those apart from a page whose + # payload names ads. logger.info("No listing tiles appeared within %.0fs. If this " "URL is a hub page or one page " "past the end of a listing, that is the expected " @@ -1004,7 +1022,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # so this runs only for a state the policy has already given up on and # only REFINES the reason. # - # The marker set itself is chosen the same way (§18): `_Incapsula_Resource` + # The marker set itself is chosen the same way: `_Incapsula_Resource` # on its own is NOT in it, because it appears once on pages dubizzle # plainly served — a marker that matches a good page is worse than no # marker. @@ -1050,14 +1068,26 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if page_flow.should_parse(state) else []) logger.info("Parsed %d row(s) from page %d.", len(products), page_num) - # This site DOES publish a result total, in its own payload, so the - # arithmetic completeness check is available here. What it does not have - # is a printed header worth reading — the old note about - # untuk "kopi"" — and the total is empty on every capture; the range it - # does print is not a count of loaded rows either (it claimed 61-180 - # while the DOM held 95). So the header is recorded verbatim and - # `total_available` stays None rather than carrying a number nobody can - # act on. + # A page classified `content` has ads in its own payload — that is what + # the classifier keys on — so zero rows from it is OUR reading failing, + # not an empty listing. Marked as a failed page so the run cannot end as + # exit 4 ("nothing listed") on page 1 or as a complete run on page 2+; + # finish_run then reports exit 5 with no rows or exit 6 with some. + if state == "content" and not products: + outcome.parse_failed = True + logger.error("Page %d was served with ads in its payload (the " + "payload's own total: %s) and parsed to 0 rows. That is " + "a parser failure, not an empty listing — the site's " + "markup has probably changed. Re-run with --dump-html " + "and compare against the fixtures.", page_num, + total_results(html)) + + # This site DOES publish a result total, in its own payload + # (`pagination.totalHits`), so `total_available` is read from there and + # the arithmetic completeness check below is available. The "header" + # recorded beside it is the Algolia index the page queried + # (product_parser.search_header), the closest thing the page states to + # "what was asked for". if args.mode == "listing" and page_num == 1: outcome.total_available = total_results(html) outcome.header = search_header(html) @@ -1270,8 +1300,7 @@ def scrape(args) -> int: outcomes.append(first) if not first.ok: - stop_reason = ("page_load_timeout" if first.load_failed - else f"blocked_{first.blocked_by}") + stop_reason = page_flow.failure_reason(first) blocked = first.blocked_by is not None seen_keys.update(p.sku for p in first.products if p.sku is not None) planned = _plan_page_urls(session.page, args, first.final_url) @@ -1299,8 +1328,7 @@ def scrape(args) -> int: failed = [o for o in rest if not o.ok] if failed: worst = min(failed, key=lambda o: o.page_num) - stop_reason = ("page_load_timeout" if worst.load_failed - else f"blocked_{worst.blocked_by}") + stop_reason = page_flow.failure_reason(worst) blocked = any(o.blocked_by for o in rest) elif exhausted: stop_reason = "no_new_products" @@ -1323,8 +1351,7 @@ def scrape(args) -> int: outcome = _fetch_one_page(session, args, pool, page_num, url) outcomes.append(outcome) if not outcome.ok: - stop_reason = ("page_load_timeout" if outcome.load_failed - else f"blocked_{outcome.blocked_by}") + stop_reason = page_flow.failure_reason(outcome) blocked = outcome.blocked_by is not None break @@ -1502,13 +1529,13 @@ def parse_args(): "fact about the ad rather than about the request — " "the browsed URL is in the run's sidecar. Pass this " "to override it with a label of your own.") - p.add_argument("--pages", type=int, default=1, + p.add_argument("--pages", type=page_flow.positive_int("--pages"), default=1, help="Number of listing pages to crawl. Applies to " "--mode listing, which is the only mode. Every " "listing page has its own ?page=N address, and the " "site's own totalPages caps how many there are.") - 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.non_negative_float("--delay"), default=2.0, help="Delay between pages, seconds") + p.add_argument("--concurrency", type=page_flow.positive_int("--concurrency"), 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 " @@ -1520,7 +1547,7 @@ def parse_args(): "that comes back EMPTY is not retried — see " "page_flow.STATE_POLICY — because an empty hub " "category 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.non_negative_float("--retry-delay"), 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") @@ -1547,7 +1574,8 @@ 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, + p.add_argument("--proxy-block-retries", + type=page_flow.non_negative_int("--proxy-block-retries"), default=page_flow.BLOCK_RETRIES_WITH_POOL, help="When a page comes back refused, retry it from this " "many OTHER exits before giving up (default %d, from " diff --git a/product_parser.py b/product_parser.py index bcc3517..423444b 100644 --- a/product_parser.py +++ b/product_parser.py @@ -16,7 +16,7 @@ anywhere, a different DOM, `` reading "دوبيزل (أوليكس)" — dubizzle (OLX)), and `dubizzle.com.lb` redirects to `olx.com.lb` outright. Measured 2026-09-14. `unsupported_reason` names that reason rather than saying "not a -dubizzle site", which is false and sends the reader looking for a typo (§5). +dubizzle site", which is false and sends the reader looking for a typo. Where the data is, and why it is not where you would expect ----------------------------------------------------------- @@ -31,7 +31,7 @@ jobs 25 0 0 community 25 0 0 -So JSON-LD is NOT the primary path here, against this family's default (§4). +So JSON-LD is NOT the primary path here, against this family's default. It is absent on three of the six verticals, and a JSON-LD-primary parser would have produced a scraper that works on cars and flats and silently returns nothing on everything else. The payload — @@ -66,7 +66,7 @@ </div> — so reading the price node alone gets "389,000" with no currency, and -reading the wrapper's text gets "AED 389,000". §4's split-price trap, in the +reading the wrapper's text gets "AED 389,000". The split-price trap, in the shape where it is the SYMBOL rather than the cents that lives elsewhere. The trap on this site @@ -117,7 +117,7 @@ HOSTS = BROWSE_HOSTS + EMIRATE_HOSTS -# Same brand, different platform. Refused WITH the reason (§5). +# Same brand, different platform. Refused WITH the reason. OLX_PLATFORM_HOSTS = ("dubizzle.com.bh", "www.dubizzle.com.bh", "dubizzle.com.om", "www.dubizzle.com.om", "dubizzle.com.eg", "www.dubizzle.com.eg", @@ -131,7 +131,7 @@ # AED, and a fact rather than a default: the motors and property JSON-LD # state `offers.priceCurrency: "AED"` outright, and every tile on every # vertical prints the ISO code in its own node. Still never written onto a -# row that has no price (§4). +# row that has no price. CURRENCY = "AED" # The site's own asset hosts. A page dubizzle SERVED is built out of them — @@ -333,7 +333,7 @@ def paginates_by_url(url: str) -> bool: `https://uae.dubizzle.com/motors/used-cars/?page=2` — exactly what the convention below builds — and page 2's payload reports `pagination.page: 1` for it. So every page's address is knowable up front and workers can - be handed independent pages (§7). + be handed independent pages. """ return listing_kind(url) in PAGINATED_KINDS @@ -486,7 +486,7 @@ def hits_per_page(html: Optional[str]) -> int: def total_pages(html: Optional[str], url: str = "") -> Optional[int]: """Pages this listing has, from the site's OWN count. - The strongest form §7 asks for, and stronger than the arithmetic a + The strongest pagination signal available, and stronger than the arithmetic a sibling repo needs: page 1's payload states `totalPages` directly, so no selector is followed and no page count is inferred. The site applies its own result cap before publishing this number, which is why it is 400 on a @@ -641,7 +641,7 @@ def _epoch_iso(value: Any) -> Optional[str]: # --------------------------------------------------------------------------- # A rendered page uses a no-break variant so the number cannot wrap, so the -# plain space is not enough (§4). +# plain space is not enough. _GROUP_SPACES = " \u00a0\u202f\u2009" _AMOUNT = (r"\d{1,3}(?:[.,%s]\d{3})+(?:[.,]\d{1,2})?" % _GROUP_SPACES + r"|\d+(?:[.,]\d{1,2})?") @@ -650,7 +650,7 @@ def _epoch_iso(value: Any) -> Optional[str]: _PRICE_RE = re.compile(r"(?:AED|DHS|Dhs|د\.إ)\s*(" + _AMOUNT + r")" r"|(" + _AMOUNT + r")\s*(?:AED|DHS|Dhs|د\.إ)") # Stripped BEFORE matching, not rejected after: a rejected match has already -# consumed the currency symbol beside it (§4). +# consumed the currency symbol beside it. _PCT_RE = re.compile(r"-?\s*\d{1,3}(?:[.,]\d+)?\s*%|-?\s*%\s*\d{1,3}(?:[.,]\d+)?") @@ -731,7 +731,7 @@ def price_in(text: str) -> Optional[float]: # as opposed to one refusing outright. # # `_Incapsula_Resource` on its OWN is deliberately NOT in this list, and that -# is the §18 rule applied before it could cost anything: the marker appears +# is the "count it on a good page first" rule applied before it could cost anything: the marker appears # once on pages dubizzle plainly served (1 occurrence on the motors page 1 # capture and 1 on the property capture, against 3 and 4 on the two refusals). # A marker that matches a good page is worse than no marker, so the entries @@ -756,7 +756,7 @@ def price_in(text: str) -> Optional[float]: ) # WHICH CAPTCHA THIS SITE ACTUALLY USES, found by reading its own bundles -# rather than by guessing from a vendor list (§18). +# rather than by guessing from a vendor list. # # Google reCAPTCHA. The key below is dubizzle's, and it is in the site's own # JavaScript twice — as `RECAPTCHA_KEY` in the argument object handed to @@ -771,7 +771,7 @@ def price_in(text: str) -> Optional[float]: # `recaptcha/api.js` loader or a single `grecaptcha.*` call. The widget is # rendered by a separate auth application that the listing pages never load, # so the loader's `render=` parameter, which is what settles v2 against v3 -# (§8), is not observable from anything this scraper fetches. +#, is not observable from anything this scraper fetches. # # Kept as a MARKER because it is safe to be one: the key appears in 0 of 17 # captures, so its presence in a page's HTML means the site has rendered its @@ -782,7 +782,7 @@ def price_in(text: str) -> Optional[float]: # The site also ships an EMPTY mount point on every page it serves — # `<captcha-widgets></captcha-widgets>`, 1 occurrence on all 17 captures, # good pages included. So the bare tag is a fact about the site and not a -# marker (§18), and it is deliberately absent from the list above. What a +# marker, and it is deliberately absent from the list above. What a # rendered challenge would look like is the same element with something # INSIDE it, which is a structural question and therefore a function rather # than a substring. @@ -794,7 +794,7 @@ def captcha_mount_is_populated(html: Optional[str]) -> bool: """Whether the site's own captcha mount point has rendered anything. Asked rather than assumed: "did we meet a challenge" and "is one - configured" are different questions (§18), and on this site the answer to + configured" are different questions, and on this site the answer to the second is yes while the answer to the first has been no on every capture taken. """ @@ -807,7 +807,7 @@ def captcha_mount_is_populated(html: Optional[str]) -> bool: # The Scraping Browser's auto-solve extension injects its own hunters into # every page it loads, so a challenge marker can be OURS rather than the -# site's (§8). Stripped before the scan because two of the markers above +# site's. Stripped before the scan because two of the markers above # (`recaptcha/api.js`, `challenges.cloudflare.com/turnstile`) are exactly # what that extension injects — unlike two sibling repos, where the same # guard would have been dead code, this marker set can genuinely match one. @@ -833,7 +833,7 @@ def detect_block_marker(html: Optional[str]) -> Optional[str]: def detect_bot_challenge(html: Optional[str], url: str = "") -> Optional[str]: """Which challenge is on this page, or None. - Broad on purpose (§8): different geos and scenarios surface different + Broad on purpose: different geos and scenarios surface different challenges, and a detection that only knows the one we happened to meet is a detection that fails the first time the site changes vendor. """ @@ -865,7 +865,7 @@ def detect_bot_challenge(html: Optional[str], url: str = "") -> Optional[str]: def is_no_results(html: Optional[str]) -> bool: """Whether the site served this page and told us it holds nothing. - Ordered by what each signal PROVES, not by what is cheap to check (§17): + Ordered by what each signal PROVES, not by what is cheap to check: the payload stating `totalHits: 0` is the site's own answer and settles it; the sentence is a fallback for a page with no payload. """ @@ -879,12 +879,38 @@ def is_no_results(html: Optional[str]) -> bool: return any(marker in html for marker in NO_RESULTS_MARKERS) +_TITLE_RE = re.compile(r"<title[^>]*>(.*?)", re.S | re.I) + + +def is_not_found_page(html: Optional[str]) -> bool: + """dubizzle's own 404, recognised from the body alone. + + Needed because one engine never sees a status: Selenium's `get()` + returns nothing. Measured on the captured 404 against the twelve other + captures (listings in four verticals, Arabic, past-the-end, the home + hub, two refusals): only the 404 has BOTH an empty `` and a + `__NEXT_DATA__` whose `reduxWrapperActionsGIPP` is an empty list. The + home hub also has an empty action list, but a real title; every listing + has exactly one action. Two conditions, so neither alone decides it. + """ + props = next_data(html) + if not isinstance(props, dict): + return False + page_props = (props.get("props") or {}).get("pageProps") or {} + actions = page_props.get("reduxWrapperActionsGIPP") if isinstance( + page_props, dict) else None + if not (isinstance(actions, list) and not actions): + return False + title = _TITLE_RE.search(html or "") + return title is not None and not title.group(1).strip() + + def detect_page_state(html: Optional[str], status: Optional[int] = None, url: str = "") -> str: """Which of the five states this response is. Signals are ordered by how much each one PROVES, which on this site is - not the order they are cheapest in (§17): + not the order they are cheapest in: 1. No body at all -> blocked. Nothing to read. 2. An explicit refusal marker -> blocked, even at HTTP 200, because @@ -894,7 +920,7 @@ def detect_page_state(html: Optional[str], status: Optional[int] = None, and Chromium's own network-error page, whose <title> is the site's hostname and which carries no vendor - marker at all (§18). + marker at all. 4. A challenge widget -> challenge. 5. The payload names hits -> content. Positive and unambiguous. 6. The payload says zero -> empty. Checked AFTER content so a @@ -923,6 +949,9 @@ def detect_page_state(html: Optional[str], status: Optional[int] = None, # Served by dubizzle, no grid, and the site said so with a status. # A 404 category is an empty answer, not a refusal. return "empty" + if is_not_found_page(html): + # The same 404, for an engine that has no status to pass. + return "empty" if url and listing_kind(url) != "listing": # A HUB, not a grid: `uae.dubizzle.com/` and `/jobs/` are landing # pages of category tiles and promo rails, and they never paint a @@ -955,7 +984,7 @@ def jsonld_blocks(html: Optional[str]) -> List[Any]: def _ld_image(value: Any) -> Optional[str]: - """A schema.org `image` in any of its four legal shapes (§4).""" + """A schema.org `image` in any of its four legal shapes.""" if isinstance(value, str): return value or None if isinstance(value, dict): @@ -973,7 +1002,7 @@ def _ld_offer(value: Any) -> dict: `.get("offers", {})` is not enough: a JSON null is a PRESENT key, so the default never applies and the caller gets None where it expected a dict - (§4's first row). + . """ if isinstance(value, dict): return value @@ -1018,7 +1047,7 @@ def jsonld_items_by_url(html: Optional[str]) -> Dict[str, dict]: def jsonld_currency(html: Optional[str]) -> Optional[str]: """`priceCurrency` from the page's AggregateOffer — the site naming its - own currency, which outranks anything read off the DOM (§4).""" + own currency, which outranks anything read off the DOM.""" for block in jsonld_blocks(html): if isinstance(block, dict): code = _ld_offer(block.get("offers")).get("priceCurrency") @@ -1120,7 +1149,7 @@ def _discount(price: Optional[float], `pre_discount_price: 0` is how this site writes "no discount", and a naive read turns it into a 100%-off car. Anything not strictly above the price produces None for BOTH columns — two figures that are not what they - were taken for should produce no number at all (§4). + were taken for should produce no number at all. """ if price is None or original is None: return None, None @@ -1240,7 +1269,7 @@ def parse_products(html: str, url: str, page: int = 1, category: Optional[str] = None) -> List[Product]: """Rows for one listing page, in the payload's own order. - Payload order is page order, which is what §8 requires: a dedupe that + Payload order is page order, which is what a deterministic merge needs: a dedupe that mutates a running set inside a fetch loop makes the output depend on which page finished first. @@ -1289,7 +1318,7 @@ def parse_products(html: str, url: str, page: int = 1, def _tile_of(anchor: Any) -> Any: """The smallest ancestor that still covers exactly ONE ad. - Counts distinct ad URLs, not links (§4). Capped at 8 levels so a + Counts distinct ad URLs, not links. Capped at 8 levels so a malformed document cannot walk to <body>: on this site the tile is one level up and the grid — 26 ads — is two, so stopping late would give every row its neighbours' price. diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index 613206d..4707d98 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -232,9 +232,14 @@ class PageOutcome: # run that reported it as complete would read as a shrinking catalogue. scroll: Optional[dict] = None + # Served with ads in the payload, parsed to zero rows: our failure, not + # an empty listing. See page_flow.failure_reason. + parse_failed: bool = False + @property def ok(self) -> bool: - return not self.load_failed and self.blocked_by is None + return (not self.load_failed and self.blocked_by is None + and not self.parse_failed) # Every `scheme://user:pass@` in a string, however many times it occurs. @@ -304,7 +309,7 @@ def open(self): # it unwind printed a 40-line asyncio traceback where the # Playwright engine prints one sentence and exits 5 — # "reporting a timeout is not the same as exiting on one" - # (§8), and the three engines must agree on exit codes. + #, and the three engines must agree on exit codes. # # The message is masked on the way out: pyppeteer and # websockets both put the endpoint they tried into the @@ -427,7 +432,7 @@ def _parse_for_mode(html: str, url: str, args, page_num: int = 1) -> List: restarts at 1 on every page: without the page number beside it, a row from page 2 claims the same position as one from page 1 and the two are indistinguishable in the output. A sibling repo's first live run wrote - 120 rows all labelled page 1 for exactly that reason (§18). + 120 rows all labelled page 1 for exactly that reason. """ return parse_products(html, url, page=page_num, category=args.category) @@ -436,13 +441,13 @@ def _same_url(a: str, b: str) -> bool: """Whether two URLs address the same page. Delegates to page_flow rather than reimplementing the comparison, so all - three engines cannot drift on it. On this site the comparison has to - strip a long tracking tail: a listing anchor arrives with - `?extParam=…keyword=kopi&search_id=…&src=search` and a detail page's own - canonical arrives with a UTM triple, so two views of one page never - match unless both sides are cleaned. An engine with its own copy of - this in a sibling repo got the equivalent wrong and silently fell back - to sequential fetching. + three engines cannot drift on it. An engine with its own copy of this + in a sibling repo got the equivalent wrong and silently fell back to + sequential fetching. + + The comparison drops campaign parameters (`utm_*` and the rest of + product_parser.TRACKING_PARAMS), a `www.` prefix and a trailing slash, + so two spellings of one dubizzle page compare equal. """ return page_flow.comparable(a) == page_flow.comparable(b) @@ -468,7 +473,7 @@ def _next_page_candidates(session, page_num: int) -> List[str]: return page_flow.next_page_candidates(page.url, hrefs or []) -def handle_captcha_if_present(session, args) -> bool: +def handle_captcha_if_present(session, args, budget=None) -> bool: """Detect and solve a challenge. True if something was solved. Same two families, same order, same "detected is not blocking" rule as @@ -500,6 +505,16 @@ def handle_captcha_if_present(session, args) -> bool: if not args.twocaptcha_key: logger.warning("No 2captcha API key, so this challenge cannot be solved.") return False + # Checked immediately before the purchase, against the page's single + # allowance (page_flow.SolveBudget), so the pre- and post-classification + # calls together cannot buy more than SOLVES_PER_PAGE tokens. + if budget is None or not budget.allows(): + logger.info("Not solving: this page's solve allowance (%d) is spent. " + "A challenge that survives a solved token is not one this " + "run can pass, and a second solve is a second charge.", + page_flow.SOLVES_PER_PAGE) + return False + budget.spend() try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -538,19 +553,29 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome block_retries = 0 if not page_flow.RETRY_ON_BLOCKED else ( args.proxy_block_retries if has_pool else page_flow.BLOCK_RETRIES_WITHOUT_POOL) - # Counted across the whole block-retry loop, not per attempt: a page that + # ONE allowance for every paid solve on this page, handed to both call + # sites below and counted across the whole block-retry loop: a page that # keeps coming back as a challenge would otherwise buy one solve per - # rotation, which is how a run quietly turns into a bill. - solves_bought = 0 + # rotation, which is how a run quietly turns into a bill. See + # page_flow.SolveBudget. + budget = page_flow.SolveBudget(page_flow.SOLVES_PER_PAGE) html, state, load_failed = None, "ok", False + # The HTTP status of the navigation, handed to the classifier. Without + # it a 404 category — which dubizzle serves with its asset references but + # no grid — classified as a `shell`, cost the full readiness wait and was + # then parsed. Reset to None whenever the document changes under us (a + # solve reloads the page), so a later page is judged by what it holds + # rather than by the status of the one it replaced. + status = None for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) load_failed = False 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})) + status = response.status if response is not None else None load_failed = False break except Exception as e: # noqa: BLE001 — pyppeteer raises many types @@ -571,7 +596,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: + # Only with a pool to rotate through, as in the Playwright engine. + # Without one, `block_retries` is still BLOCK_RETRIES_WITHOUT_POOL + # (1 on this site), so this branch used to be reached with + # `pool=None` and crash the run with an AttributeError (exit 1) + # where a page that never loaded should report exit 5. + if load_failed and has_pool and block_attempt < block_retries: pool.advance("unusable exit or repeated load failure") session.relaunch() bridge, page = session.bridge, session.page @@ -581,18 +611,21 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome break - if handle_captcha_if_present(session, args): + if handle_captcha_if_present(session, args, budget): time.sleep(1) + status = None html = _content(session) or "" - state = page_flow.classify(html, url=page.url) - - # "Not painted yet" is not a fault. A SEARCH grid arrives with the - # client-side GraphQL response, so at domcontentloaded the page is a - # shell with no grid in it — classified naively that is "unknown", - # and "unknown" retries. Wait for the anchor and re-classify BEFORE - # the retry decision. Mirrors playwright_scraper exactly; see - # page_flow.is_unpainted. + state = page_flow.classify(html, status, url=page.url) + + # "Not painted yet" is not a fault. A page dubizzle served (it is + # built from the site's own assets) but whose listings payload is not + # in the document yet classifies as `shell`. Fetching it again would + # only buy another shell, so wait for the anchor and re-classify + # BEFORE the retry decision. None of the captured pages in + # fixtures_generated.json is a shell — a category grid arrives + # server-rendered — so this wait is a safety net, not a measured + # path. See page_flow.is_unpainted. if page_flow.is_unpainted(state, html): wait_timeout = page_flow.content_timeout_ms(args.mode) logger.info("Page %d is a shell the site served but has not " @@ -607,7 +640,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome logger.info("The grid still had not painted after %.0fs " "(%d match(es)).", wait_timeout / 1000, found) html = _content(session) or html - state = page_flow.classify(html, url=page.url) + state = page_flow.classify(html, status, url=page.url) # No interstitial-settling step, and its absence is measured rather # than an omission: this site has no interstitial. A refused request @@ -620,12 +653,12 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # manager can be switched on between deploys, and bounded by # SOLVES_PER_PAGE so a speculative path cannot become a bill. if (page_flow.should_solve(state) - and solves_bought < page_flow.SOLVES_PER_PAGE): - solves_bought += 1 - if handle_captcha_if_present(session, args): + and budget.allows()): + if handle_captcha_if_present(session, args, budget): time.sleep(1) + status = None html = _content(session) or html - state = page_flow.classify(html, url=page.url) + state = page_flow.classify(html, status, url=page.url) if state == "content": logger.info("The solve was accepted — page %d is content " "now.", page_num) @@ -706,12 +739,9 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if state == "content": - # Wait for the prices to fill in. There is NO scroll: three scrolls - # to the document's own bottom added zero cards on every page kind. - # What the wait is for is hydration — the server sends 24 card shells - # with 24 EMPTY price nodes, so a card count is satisfied instantly - # while every price is still blank: - # 95 after one scroll on the measured search URL. + # Wait for the listing anchors. There is NO scroll: three scrolls + # to the document's own bottom added zero cards on every page kind, + # because the whole page is served at once. selector = page_flow.ready_selector(args.mode) threshold = page_flow.min_matches(args.mode, page_flow.expected_cards(html)) @@ -723,13 +753,10 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome page_flow.content_timeout_ms(args.mode)) time.sleep(0.5) if found <= threshold: - # Not an error on its own, and what it MEANS depends on the - # mode — which is why the message does too. A listing page with - # no grid is a correct answer (a taxonomy hub, or one page past - # the end); a detail page whose buy box never painted is a - # different thing entirely, and on this site it is usually just - # slow rather than absent, because the row is parsed out of the - # page's JSON-LD and not out of the buy box. + # Not an error on its own. A listing page with no grid is a + # correct answer (a hub page, or one page past the end), and + # classification has already told those apart from a page whose + # payload names ads. logger.info("No listing tiles appeared in time. If this URL " "is a hub page or one page past " "the end of a listing, that is the expected " @@ -791,6 +818,20 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if page_flow.should_parse(state) else []) logger.info("Parsed %d row(s) from page %d.", len(products), page_num) + # A page classified `content` has ads in its own payload — that is what + # the classifier keys on — so zero rows from it is OUR reading failing, + # not an empty listing. Marked as a failed page so the run cannot end as + # exit 4 ("nothing listed") on page 1 or as a complete run on page 2+; + # finish_run then reports exit 5 with no rows or exit 6 with some. + if state == "content" and not products: + outcome.parse_failed = True + logger.error("Page %d was served with ads in its payload (the " + "payload's own total: %s) and parsed to 0 rows. That is " + "a parser failure, not an empty listing — the site's " + "markup has probably changed. Re-run with --dump-html " + "and compare against the fixtures.", page_num, + total_results(html)) + if args.mode == "listing" and page_num == 1: # The payload states the listing's own total, so the arithmetic # completeness check is available here and the number is recorded @@ -886,8 +927,7 @@ def scrape(args) -> int: outcomes.append(first) if not first.ok: - stop_reason = ("page_load_timeout" if first.load_failed - else f"blocked_{first.blocked_by}") + stop_reason = page_flow.failure_reason(first) blocked = first.blocked_by is not None elif args.mode == "listing": seen_keys.update(p.sku for p in first.products if p.sku is not None) @@ -929,8 +969,7 @@ def scrape(args) -> int: outcome = _fetch_one_page(session, args, pool, page_num, url) outcomes.append(outcome) if not outcome.ok: - stop_reason = ("page_load_timeout" if outcome.load_failed - else f"blocked_{outcome.blocked_by}") + stop_reason = page_flow.failure_reason(outcome) blocked = outcome.blocked_by is not None break @@ -1077,16 +1116,16 @@ def parse_args(): "not been measured, and a mode that ships untested " "is worse than one that is absent.") p.add_argument("--category", default=None, help="Label to tag output rows with.") - p.add_argument("--pages", type=int, default=1, help="Listing pages to crawl") - 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("--pages", type=page_flow.positive_int("--pages"), default=1, help="Listing pages to crawl") + p.add_argument("--delay", type=page_flow.non_negative_float("--delay"), default=2.0, help="Delay between pages, seconds") + p.add_argument("--concurrency", type=page_flow.positive_int("--concurrency"), 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: an empty " "hub category 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.non_negative_float("--retry-delay"), default=2.0, help="Seconds before the first retry, doubling thereafter") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="dubizzle_products", help="Output file prefix") @@ -1099,7 +1138,8 @@ 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, + p.add_argument("--proxy-block-retries", + type=page_flow.non_negative_int("--proxy-block-retries"), default=page_flow.BLOCK_RETRIES_WITH_POOL, help="When a page comes back refused, retry it from this " "many OTHER exits before giving up (default %d, from " diff --git a/scraper_api_client.py b/scraper_api_client.py index 62478aa..0d21285 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -57,6 +57,7 @@ detect_page_state, BOT_CHALLENGE_MARKERS) 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") @@ -314,7 +315,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="dubizzle_products_scraperapi", help="Output file prefix") - p.add_argument("--timeout", type=int, default=60, + p.add_argument("--timeout", type=page_flow.positive_int("--timeout"), 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 " @@ -331,11 +332,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.non_negative_int("--retries"), 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.non_negative_int("--retry-delay"), 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 a9b3f30..e69460e 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -136,9 +136,14 @@ class PageOutcome: # whose grid was still growing when the budget ran out is partial. scroll: Optional[dict] = None + # Served with ads in the payload, parsed to zero rows: our failure, not + # an empty listing. See page_flow.failure_reason. + parse_failed: bool = False + @property def ok(self) -> bool: - return not self.load_failed and self.blocked_by is None + return (not self.load_failed and self.blocked_by is None + and not self.parse_failed) # Every `scheme://user:pass@` in a string, however many times it occurs. @@ -366,7 +371,7 @@ def _parse_for_mode(html: str, url: str, args, page_num: int = 1) -> List: restarts at 1 on every page: without the page number beside it, a row from page 2 claims the same position as one from page 1 and the two are indistinguishable in the output. A sibling repo's first live run wrote - 120 rows all labelled page 1 for exactly that reason (§18). + 120 rows all labelled page 1 for exactly that reason. """ return parse_products(html, url, page=page_num, category=args.category) @@ -375,11 +380,13 @@ def _same_url(a: str, b: str) -> bool: """Whether two URLs address the same page. Delegates to page_flow rather than reimplementing the comparison, so all - three engines cannot drift on it. In particular this site writes some of its - next-links percent-DECODED (".../kühlen-gefrieren-32.html") while a - pasted URL is encoded (".../k%C3%BChlen-gefrieren-32.html"); an engine - with its own copy of this got that wrong and silently fell back to - sequential fetching on every accented category. + three engines cannot drift on it. An engine with its own copy of this + in a sibling repo got the equivalent wrong and silently fell back to + sequential fetching. + + The comparison drops campaign parameters (`utm_*` and the rest of + product_parser.TRACKING_PARAMS), a `www.` prefix and a trailing slash, + so two spellings of one dubizzle page compare equal. """ return page_flow.comparable(a) == page_flow.comparable(b) @@ -412,7 +419,7 @@ def _next_page_candidates(session, page_num: int) -> List[str]: hrefs or []) -def handle_captcha_if_present(session, args) -> bool: +def handle_captcha_if_present(session, args, budget=None) -> bool: """Detect and solve a challenge. True if something was solved. Same detectors, same reconciliation and the same "detected is not @@ -450,6 +457,16 @@ def handle_captcha_if_present(session, args) -> bool: if not args.twocaptcha_key: logger.warning("No 2captcha API key, so this challenge cannot be solved.") return False + # Checked immediately before the purchase, against the page's single + # allowance (page_flow.SolveBudget), so the pre- and post-classification + # calls together cannot buy more than SOLVES_PER_PAGE tokens. + if budget is None or not budget.allows(): + logger.info("Not solving: this page's solve allowance (%d) is spent. " + "A challenge that survives a solved token is not one this " + "run can pass, and a second solve is a second charge.", + page_flow.SOLVES_PER_PAGE) + return False + budget.spend() try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -491,10 +508,18 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome block_retries = 0 if not page_flow.RETRY_ON_BLOCKED else ( args.proxy_block_retries if has_pool else page_flow.BLOCK_RETRIES_WITHOUT_POOL) - # Counted across the whole block-retry loop, not per attempt: a page that + # ONE allowance for every paid solve on this page, handed to both call + # sites below and counted across the whole block-retry loop: a page that # keeps coming back as a challenge would otherwise buy one solve per - # rotation, which is how a run quietly turns into a bill. - solves_bought = 0 + # rotation, which is how a run quietly turns into a bill. See + # page_flow.SolveBudget. + budget = page_flow.SolveBudget(page_flow.SOLVES_PER_PAGE) + # Selenium's `driver.get()` returns None and the driver exposes no HTTP + # status at all, so the classifier gets None here and relies on the + # body: dubizzle's own 404 is recognised by product_parser's + # `is_not_found_page`, which is what makes this engine agree with the + # two that do see a status. + status = None for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) @@ -530,18 +555,20 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome break - if handle_captcha_if_present(session, args): + if handle_captcha_if_present(session, args, budget): time.sleep(1) html = d["content"]() or "" - state = page_flow.classify(html, url=d["current_url"]()) - - # "Not painted yet" is not a fault. A SEARCH grid arrives with the - # client-side GraphQL response, so at domcontentloaded the page is a - # shell with no grid in it — classified naively that is "unknown", - # and "unknown" retries. Wait for the anchor and re-classify BEFORE - # the retry decision. Mirrors playwright_scraper exactly; see - # page_flow.is_unpainted. + state = page_flow.classify(html, status, url=d["current_url"]()) + + # "Not painted yet" is not a fault. A page dubizzle served (it is + # built from the site's own assets) but whose listings payload is not + # in the document yet classifies as `shell`. Fetching it again would + # only buy another shell, so wait for the anchor and re-classify + # BEFORE the retry decision. None of the captured pages in + # fixtures_generated.json is a shell — a category grid arrives + # server-rendered — so this wait is a safety net, not a measured + # path. See page_flow.is_unpainted. if page_flow.is_unpainted(state, html): wait_s = page_flow.content_timeout_ms(args.mode) / 1000.0 sel = page_flow.ready_selector(args.mode) @@ -556,7 +583,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome logger.info("The grid still had not painted after %.0fs " "(%d match(es)).", wait_s, found) html = d["content"]() or html - state = page_flow.classify(html, url=d["current_url"]()) + state = page_flow.classify(html, status, url=d["current_url"]()) # No interstitial-settling step, and its absence is measured rather # than an omission: this site has no interstitial. A refused request @@ -569,12 +596,11 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # manager can be switched on between deploys, and bounded by # SOLVES_PER_PAGE so a speculative path cannot become a bill. if (page_flow.should_solve(state) - and solves_bought < page_flow.SOLVES_PER_PAGE): - solves_bought += 1 - if handle_captcha_if_present(session, args): + and budget.allows()): + 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, status, url=d["current_url"]()) if state == "content": logger.info("The solve was accepted — page %d is content " "now.", page_num) @@ -656,11 +682,9 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if state == "content": - # Wait for paint, then SCROLL — and on this site the scroll is not an - # optimisation. This site serves the whole page at once - # (links=0 at round 0 on every capture) and hydrates 60 tiles at a - # time from client-side GraphQL: 11 tiles against 95 after one scroll - # on the measured search URL. + # Wait for the listing anchors. There is NO scroll: three scrolls + # to the document's own bottom added zero cards on every page kind, + # because the whole page is served at once. selector = page_flow.ready_selector(args.mode) threshold = page_flow.min_matches(args.mode, page_flow.expected_cards(html)) @@ -674,13 +698,10 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome int(timeout_s * 1000)) time.sleep(0.5) if found <= threshold: - # Not an error on its own, and what it MEANS depends on the - # mode — which is why the message does too. A listing page with - # no grid is a correct answer (a taxonomy hub, or one page past - # the end); a detail page whose buy box never painted is a - # different thing entirely, and on this site it is usually just - # slow rather than absent, because the row is parsed out of the - # page's JSON-LD and not out of the buy box. + # Not an error on its own. A listing page with no grid is a + # correct answer (a hub page, or one page past the end), and + # classification has already told those apart from a page whose + # payload names ads. logger.info("No listing tiles appeared within %.0fs. If this " "URL is a hub page or one page " "past the end of a listing, that is the expected " @@ -743,6 +764,20 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome if page_flow.should_parse(state) else []) logger.info("Parsed %d row(s) from page %d.", len(products), page_num) + # A page classified `content` has ads in its own payload — that is what + # the classifier keys on — so zero rows from it is OUR reading failing, + # not an empty listing. Marked as a failed page so the run cannot end as + # exit 4 ("nothing listed") on page 1 or as a complete run on page 2+; + # finish_run then reports exit 5 with no rows or exit 6 with some. + if state == "content" and not products: + outcome.parse_failed = True + logger.error("Page %d was served with ads in its payload (the " + "payload's own total: %s) and parsed to 0 rows. That is " + "a parser failure, not an empty listing — the site's " + "markup has probably changed. Re-run with --dump-html " + "and compare against the fixtures.", page_num, + total_results(html)) + if args.mode == "listing" and page_num == 1: # The payload states the listing's own total, so the arithmetic # completeness check is available here and the number is recorded @@ -826,8 +861,7 @@ def scrape(args) -> int: outcomes.append(first) if not first.ok: - stop_reason = ("page_load_timeout" if first.load_failed - else f"blocked_{first.blocked_by}") + stop_reason = page_flow.failure_reason(first) blocked = first.blocked_by is not None elif args.mode == "listing": seen_keys.update(p.sku for p in first.products if p.sku is not None) @@ -866,8 +900,7 @@ def scrape(args) -> int: outcome = _fetch_one_page(session, args, pool, page_num, url) outcomes.append(outcome) if not outcome.ok: - stop_reason = ("page_load_timeout" if outcome.load_failed - else f"blocked_{outcome.blocked_by}") + stop_reason = page_flow.failure_reason(outcome) blocked = outcome.blocked_by is not None break @@ -1013,16 +1046,16 @@ def parse_args(): "not been measured, and a mode that ships untested " "is worse than one that is absent.") p.add_argument("--category", default=None, help="Label to tag output rows with.") - p.add_argument("--pages", type=int, default=1, help="Listing pages to crawl") - 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("--pages", type=page_flow.positive_int("--pages"), default=1, help="Listing pages to crawl") + p.add_argument("--delay", type=page_flow.non_negative_float("--delay"), default=2.0, help="Delay between pages, seconds") + p.add_argument("--concurrency", type=page_flow.positive_int("--concurrency"), 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: an empty " "hub category 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.non_negative_float("--retry-delay"), default=2.0, help="Seconds before the first retry, doubling thereafter") p.add_argument("--format", choices=["json", "csv", "both"], default="both") p.add_argument("--out", default="dubizzle_products", help="Output file prefix") @@ -1036,7 +1069,8 @@ 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, + p.add_argument("--proxy-block-retries", + type=page_flow.non_negative_int("--proxy-block-retries"), default=page_flow.BLOCK_RETRIES_WITH_POOL, help="When a page comes back refused, retry it from this " "many OTHER exits before giving up (default %d, from " diff --git a/smoke_test.py b/smoke_test.py index 366d2db..5e89590 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -73,6 +73,7 @@ listings_payload, vertical_of, SELECTORS, BLOCK_MARKERS, BOT_CHALLENGE_MARKERS) import page_flow +import output_writer from proxy_pool import (ProxyPool, mask, to_playwright, split_credentials, parse_proxy_line) @@ -105,13 +106,29 @@ def _raises(fn): return False +# Assembled from pieces, so this file can be scanned like every other one: +# the literal phrases are never written down here, and the suite no longer +# has to exempt itself — the file most likely to acquire a stray phrase was +# the one file nobody scanned. BANNED_PHRASES = ( - "cloud browser", - "antidetect browser", - "anti-detect browser", - "2scraper Antidetect Browser", - "gate.2prx.com", - "2prx.com", + "cloud" + " browser", + "anti" + "detect browser", + "anti-" + "detect browser", + "2scraper Anti" + "detect Browser", + "gate.2" + "prx.com", + "2prx" + ".com", +) + +# References to private instruction files and their section numbers. The +# public cannot open them, so a citation reads as a pointer to nothing; the +# rule is stated in this repo's own words instead. Assembled from pieces for +# the same reason as above. +PRIVATE_CITATIONS = ( + "\u00a7", # the section sign + "CLAUDE" + ".md", + "claude_" + "2scraper", + "2scraper_" + "repo_rules", + "play" + "book", ) # Flags that were removed and must stay removed. Scoped to the ENGINES: @@ -123,7 +140,7 @@ def _raises(fn): "selenium_scraper.py") # A fingerprint as the 2Captcha API really returns one, with this site's -# region. Kept as a fixture because three of the six defects §16 lists were +# region. Kept as a fixture because three fingerprint-client defects fixed earlier were # one wrong key each in exactly this structure -- `userAgent.value` where the # API says `userAgent.userAgent`, a locale built as `en-{country}`, a # timezone never applied at all. @@ -239,7 +256,7 @@ def test_price_parsing(): ok &= check("%-28r -> %-12s (%s)" % (text, want, why), price_in(text) == want) - # §4's percentage trap, in both word orders. Rejecting the match + # The percentage trap, in both word orders. Rejecting the match # afterwards is not enough — a rejected match has already consumed the # currency code beside it, so the percentage is stripped BEFORE matching. ok &= check("a -16% badge beside a price does not become the price", @@ -247,7 +264,7 @@ def test_price_parsing(): ok &= check("a -%10,34 badge (percent first) does not become the price", price_in("-%10,34 AED 25.999") == 25999.0) - # §4's ISO allowlist: a bare three-letter token is not a currency. + # The ISO allowlist: a bare three-letter token is not a currency. ok &= check("a size chart is not a price", price_in("XXL 100") is None) ok &= check("a bare number is not a price", price_in("389000") is None) ok &= check("a year is not a price", price_in("2024") is None) @@ -263,7 +280,7 @@ def test_price_parsing(): # --------------------------------------------------------------------------- -# Rows: VALUES on real fixtures, not coverage (§10) +# Rows: VALUES on real fixtures, not coverage # --------------------------------------------------------------------------- def test_listing_values(): group("listing rows: pinned VALUES on every vertical, not coverage") @@ -272,7 +289,7 @@ def test_listing_values(): # make_fixtures.py pinned every column of every kept ad against the # untrimmed capture. Re-asserting all of it here is what stops a parser # change from being "still 100% populated" and entirely wrong — the - # defect §10 names, where a review_count of 445279961 passed a coverage + # failure seen in a sibling repo, where a review_count of 445279961 passed a coverage # check on every row of every run. for name in ("motors_p1", "motors_p2", "motors_ar_p1", "property_rent", "property_sale", "classified", "jobs", "community"): @@ -332,7 +349,7 @@ def test_listing_values(): ok &= check("it is emitted AFTER the organic results, so position 1 is a " "real result", all(r.position > 1 for r in cotw)) - # page + position, together, must identify a row (§18). + # page + position, together, must identify a row. two = fx_rows("motors_p1") + fx_rows("motors_p2") pairs = [(r.page, r.position) for r in two] ok &= check("page+position is unique across a two-page run", @@ -345,7 +362,7 @@ def test_listing_values(): def test_null_price_is_the_site_and_not_the_parser(): group("a null price: which of them are the site's answer") ok = True - # §9: a column that is null on every row of every run should not exist — + # A column that is null on every row of every run should not exist — # so the fact that `price` is null on every jobs row has to be a # MEASUREMENT written down, not an unexplained gap. It is also why the # price-coverage floor is per-vertical. @@ -363,7 +380,7 @@ def test_null_price_is_the_site_and_not_the_parser(): ok &= check("and none of them claims a no-price source", not any(r.price_source.endswith(":no-price") for r in priced)) - # §4's guard, one line, and it catches the regression forever. + # One line, and it catches the regression forever. ok &= check("no row has an original_price at or below its price", not [r for r in priced if r.original_price is not None and r.price is not None @@ -385,7 +402,7 @@ def test_null_price_is_the_site_and_not_the_parser(): def test_jsonld_enrichment(): group("JSON-LD: an enrichment, joined on the PATH and not on the URL") ok = True - # The measurement that decided the payload is primary (§4). + # The measurement that decided the payload is primary. have_itemlist = {name: len(product_parser.jsonld_items_by_url(fx(name)[0])) for name in ("motors_p1", "property_rent", "classified", "jobs", "community")} @@ -414,7 +431,7 @@ def test_jsonld_enrichment(): ok &= check("a vertical with no ItemList has no brand, rather than one " "guessed from the title", all(r.brand is None for r in cls)) - # `offers` in each of its legal shapes (§4's table). + # `offers` in each of its legal shapes. ok &= check("an explicit `\"offers\": null` does not raise", product_parser._ld_offer(None) == {}) ok &= check("`offers` as a list takes the first dict", @@ -455,7 +472,7 @@ def test_dom_fallback_agrees_with_the_payload(): all(r.currency == "AED" for r in dom.values() if r.price is not None)) - # §4's tile-scoping failure, in the shape it takes here. + # The tile-scoping failure, in the shape it takes here. html, url, _ = fx("motors_p1") soup = BeautifulSoup(html, "html.parser") anchors = [a for a in soup.select("a[href]") @@ -486,7 +503,7 @@ def test_urls(): all(is_supported_host("https://%s/motors/used-cars/" % h) for h in product_parser.EMIRATE_HOSTS)) - # §5: refuse WITH the reason. "is not a dubizzle site" would be false for + # Refuse WITH the reason. "is not a dubizzle site" would be false for # a dubizzle-branded OLX site and would send the reader hunting for a typo. for host in ("www.dubizzle.com.bh", "www.dubizzle.com.om", "dubizzle.com.eg"): why = unsupported_reason("https://%s/" % host) @@ -653,7 +670,7 @@ def test_page_state(): for n in ("motors_p1", "property_rent", "classified", "jobs", "community", "hub_home", "not_found"))) - # §18's inverted-detection case: Chromium's own network-error page carries + # The inverted-detection case: Chromium's own network-error page carries # the site's hostname in its <title> and no vendor marker of any kind. chromium_error = ( '<html><head><title>www.dubizzle.com' @@ -665,7 +682,7 @@ def test_page_state(): ok &= check("and no marker list would have caught it — only the assets do", detect_block_marker(chromium_error) is None) - # §18's other rule: a marker that matches a good page is worse than none. + # The other rule: a marker that matches a good page is worse than none. served = fx("motors_p1")[0] ok &= check("_Incapsula_Resource appears on pages the site SERVED, so it " "is not on its own a challenge marker", @@ -675,7 +692,7 @@ def test_page_state(): '') is not None) - # The extension trap (§8), and here the guard is NOT dead code: this + # The extension trap, and here the guard is NOT dead code: this # marker set contains exactly what the auto-solve extension injects. ok &= check("a Scraping Browser extension's own hunters are not mistaken " "for the site's challenge", @@ -743,7 +760,7 @@ def test_page_flow(): # a sibling repo, where the client mattered and the address did not. So # the retry budget is spent rather than saved, and the engines read these # constants rather than computing their own (a policy constant nothing - # consults is the same defect as dead code, §17). + # consults is the same defect as dead code). ok &= check("RETRY_ON_BLOCKED is True here: rotating the exit DOES help", page_flow.RETRY_ON_BLOCKED is True) ok &= check("and the block-retry budgets are non-zero", @@ -787,7 +804,7 @@ def test_page_flow(): # A poll, not an evaluated string: `wait_for_function` hands the browser a # STRING, and a site whose CSP lacks `unsafe-eval` refuses it outright — # which took a sibling repo's run down with exit 1 on its most obvious - # URL (§18). Counting elements goes through the protocol instead. + # URL. Counting elements goes through the protocol instead. calls = {"n": 0} def count(_sel): @@ -854,13 +871,13 @@ def test_output_contract(): "posted_at", "bumped_at", "payment_frequency", "bedrooms", "bathrooms", "size_sqft", "year", "kilometers", "attributes"]) # One KIND of thing here, so one dataclass — there is no second row class - # to keep in step, and no mode that reads something else (§9). + # to keep in step, and no mode that reads something else. ok &= check("one mode maps to one row class", ROW_CLASS_BY_MODE == {"listing": Product}) ok &= check("that mode is one row per sku", set(UNIQUE_BY_SKU_MODES) == {"listing"}) - # §9: a column that is null on every row of every run should not exist. + # A column that is null on every row of every run should not exist. # `rating` and `review_count` ARE null on every row here, and they are # kept only because they are in the family prefix — which is a decision, # so it is written down and pinned rather than left to be rediscovered. @@ -1180,15 +1197,19 @@ def default_exception_handler(self, context): def test_canary_separates_access_from_defect(): - group("the canary fails on defects and only WARNS on access conditions") + group("the canary passes ONLY when it actually tested the site") ok = True wf_path = os.path.join(REPO_ROOT, ".github", "workflows", "canary.yml") wf = open(wf_path, encoding="utf-8").read() - # The data checks must not run on a blocked or refused run: there is no - # output file, and a missing file would fail for the wrong reason. - ok &= check("the data checks are gated on the run having got in", - "steps.verdict.outputs.tested == 'true'" in wf) + # A canary that could not reach the site must not end green: without the + # secret the job fails and names it. + missing = wf[wf.index("- name: Fail when the live run cannot happen"):] + missing = missing[:missing.index("- uses:")] + ok &= check("a missing DUBIZZLE_CDP_ENDPOINT secret fails the job, by name", + "if: env.HAVE_CDP != 'true'" in missing + and "::error::No DUBIZZLE_CDP_ENDPOINT" in missing + and "exit 1" in missing) # Extract the real interpret-the-exit-code script and run it under bash # for every code, rather than asserting on the YAML text. What matters is @@ -1202,30 +1223,25 @@ def test_canary_separates_access_from_defect(): script = "\n".join(line[10:] if line.startswith(" " * 10) else line for line in wf[start:end].splitlines()) - # WHY EACH CODE LANDS WHERE IT DOES: - # 0 got in and parsed -> pass, and the assertions then run - # 3 blocked before parsing -> ACCESS. Measured intermittent per - # profile on this site, so a daily red badge would be noise. - # 5 endpoint refused -> ACCESS, and the commonest cause is an - # EXPIRED SECRET. A credential is not forever; failing on it paints - # the badge red every day until someone notices. - # 6 partial -> ACCESS, usually a mid-run block. - # 1 crashed -> DEFECT. - # 2 bad arguments -> DEFECT (in the workflow itself). - # 4 served a page, ZERO rows -> DEFECT, and precisely the regression - # this canary exists to catch: the tile anchor moved. - expected = {0: "pass", 3: "warn", 5: "warn", 6: "warn", - 1: "fail", 2: "fail", 4: "fail", 99: "fail"} + # Only exit 0 means the site was tested. 3 (blocked), 5 (never + # obtained) and 6 (partial) are access problems rather than code + # defects, but each means the site was NOT fully tested, so each is red + # and says why in the step summary. 1, 2 and 4 are defects. A parser + # failure named in the log is red whatever the exit code. + expected = {0: "pass", 3: "access", 5: "access", 6: "access", + 1: "fail", 2: "fail", 4: "fail", 99: "fail", -5: "fail"} for code, want in sorted(expected.items()): body = script.replace('code="${{ steps.run.outputs.exit_code }}"', - 'code="%d"' % code) + 'code="%d"' % abs(code)) with tempfile.TemporaryDirectory() as td: + with open(os.path.join(td, "canary_run.log"), "w") as f: + f.write("stop_reason parser_found_nothing\n" if code < 0 else "") out_file = os.path.join(td, "gh_output") summary = os.path.join(td, "gh_summary") open(out_file, "w").close() open(summary, "w").close() done = subprocess.run( - ["bash", "-c", body], capture_output=True, text=True, + ["bash", "-c", body], capture_output=True, text=True, cwd=td, env=dict(os.environ, GITHUB_OUTPUT=out_file, GITHUB_STEP_SUMMARY=summary)) failed = done.returncode != 0 @@ -1236,31 +1252,24 @@ def test_canary_separates_access_from_defect(): if want == "pass": got = not failed and not warned and not errored and tested - elif want == "warn": - # A warning must NOT read as a pass: it also has to say in the - # step summary that nothing was actually tested, and it must not - # claim `tested`. - got = not failed and warned and wrote_summary and not tested + elif want == "access": + # Red, and the summary says in words that nothing was tested. + got = failed and errored and wrote_summary and not tested else: got = failed and errored - ok &= check("exit %-2d is treated as %s" % (code, want), got) - - ok &= check("the reason access is not a defect is written down", - "ACCESS CONDITIONS ARE NOT DEFECTS" in wf) - - # NO SCHEDULE, and the reason has to travel with the decision. A daily - # cron against a credential that does not survive a day gives either a - # permanently red badge or a permanently green one that tested nothing — - # and the green is worse, because it reads as "the parser still works". - # Restoring the cron is a legitimate change the day a long-lived - # credential exists; this check makes it a decision rather than a habit. - ok &= check("the canary has no cron schedule", - not re.search(r"^\s*-\s*cron:", wf, re.M)) - ok &= check("it is dispatchable by hand", "workflow_dispatch:" in wf) - ok &= check("and the reason the schedule is off is written down", - "does not survive long" in wf) - ok &= check("the expired-secret case is named", - "expired" in wf.lower() and "refresh" in wf.lower()) + if code < 0: + # Named as a parser failure, not passed off as an access problem. + got = got and "parser_found_nothing" in done.stdout \ + and "Content never obtained" not in done.stdout + label = ("a logged parser_found_nothing" if code < 0 + else "exit %-2d" % code) + ok &= check("%s is treated as %s" % (label, want), got and not warned) + + ok &= check("the canary runs on a schedule, not only by hand", + re.search(r"^\s*-\s*cron:", wf, re.M) is not None + and "workflow_dispatch:" in wf) + ok &= check("a credential the endpoint refuses is named in the exit-5 " + "message", "401" in wf and "refresh" in wf.lower()) return ok @@ -1501,13 +1510,15 @@ def test_wording(): listed = subprocess.run(["git", "ls-files"], cwd=REPO_ROOT, capture_output=True, text=True) if listed.returncode == 0 and listed.stdout.strip(): - shipped = [f for f in listed.stdout.split("\n") - if f.endswith((".py", ".md", ".txt", ".toml", ".yml", ".yaml")) - and os.path.basename(f) != os.path.basename(__file__)] + tracked = [f for f in listed.stdout.split("\n") if f] else: # not a git checkout (a release tarball): fall back to the root - shipped = [f for f in os.listdir(REPO_ROOT) - if f.endswith((".py", ".md", ".txt", ".toml", ".yml", ".yaml")) - and f != os.path.basename(__file__)] + tracked = [f for f in os.listdir(REPO_ROOT) + if os.path.isfile(os.path.join(REPO_ROOT, f))] + shipped = [f for f in tracked + if f.endswith((".py", ".md", ".txt", ".toml", ".yml", ".yaml"))] + ok &= check("the wording scan covers this suite itself", + os.path.basename(__file__) in + {os.path.basename(f) for f in shipped}) ok &= check("the wording scan reaches beyond the repo root", any(os.sep in f or "/" in f for f in shipped)) for phrase in BANNED_PHRASES: @@ -1521,6 +1532,22 @@ def test_wording(): offenders.append(f) ok &= check("no shipped file says %r" % phrase, not offenders) + # Private citations, over EVERY tracked text file — CHANGELOG, the + # Dockerfile, .gitignore and .env.example included — not just the + # suffixes above. + for token in PRIVATE_CITATIONS: + offenders = [] + for f in tracked: + try: + text = open(os.path.join(REPO_ROOT, f), encoding="utf-8").read() + except (OSError, UnicodeDecodeError): + continue + if token.lower() in text.lower(): + offenders.append(f) + ok &= check("no tracked file cites private instruction files (%s)" + % token.encode("ascii", "backslashreplace").decode(), + not offenders) + for flag in REMOVED_ENGINE_FLAGS: offenders = [] for f in ENGINE_FILES: @@ -1556,10 +1583,10 @@ def test_fingerprint_client_reads_env(): # THE DEFECT THIS PINS, found the first time --fingerprint was run live # here and confirmed present in five sibling repos: `--key` defaulted to # `os.environ.get("TWOCAPTCHA_KEY")` alone. So a key put in `.env` — - # which is exactly what §3, the README and .env.example instruct — worked + # which is exactly what 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. + # mechanism not applied on one path, which is a defect shape this + # client has had before. src = inspect.getsource(fpc.main) ok &= check("it loads .env itself, rather than hoping an engine did", "env_config.load_env()" in src) @@ -1623,7 +1650,7 @@ def test_fingerprint_application(): # possible, but it is not what this fingerprint describes, and a locale # that contradicts the rest of the identity is the mismatch again. This # was one of the six defects a sibling repo inherited from copied core - # and never ran (§16). + # and never ran. ok &= check("the locale is the fingerprint's own, not en-", kw.get("locale") == "nl-NL") ok &= check("the timezone is carried, so the browser cannot contradict it", @@ -2020,7 +2047,7 @@ def test_captcha(): {"recaptcha/api2/anchor", "recaptcha/api2/bframe", "recaptcha/api.js", "data-sitekey", "hcaptcha.com/captcha"} <= markers) - # The other half of the question §18 asks: "did we meet a challenge" and + # The other half of the question: "did we meet a challenge" and # "is one configured, and would we recognise it" are different, and here # the answers are no and yes. This site ships its own captcha mount point # EMPTY on every page it serves — ``, @@ -2043,7 +2070,7 @@ def test_captcha(): for n in ("motors_p1", "property_rent", "classified", "jobs", "community"))) # WHICH captcha this site uses, found by reading its own bundles rather - # than by guessing from a vendor list (§18). Google reCAPTCHA, on the + # than by guessing from a vendor list. Google reCAPTCHA, on the # login / phone-verification flow, never on a listing. # Against the raw tuple, not the lowercased set above: a reCAPTCHA key # is case-sensitive and the page emits it verbatim, so the marker has to @@ -2076,7 +2103,7 @@ def test_captcha(): # recaptcha on EVERY page fetched over --cdp-endpoint: 3, 2 and 2 # occurrences on a good motors page, and 0 of each after the extension #