diff --git a/.dockerignore b/.dockerignore index f8079ef..f117ccf 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,6 +1,7 @@ .git/ .github/ .env +.env.* *.log *.key secrets* @@ -11,7 +12,9 @@ __pycache__/ .venv/ venv/ env/ -binance_rows.* +makemytrip_hotels.* +canary*.json +*.page[0-9]* *_debug.html *_debug.png live_results/ diff --git a/.env.example b/.env.example index 329b55c..6706294 100644 --- a/.env.example +++ b/.env.example @@ -62,8 +62,8 @@ TWOCAPTCHA_KEY=your_2captcha_api_key_here # Selenium cannot use this: chromedriver's `debuggerAddress` takes a bare # host:port with nowhere to put a password. Playwright and pyppeteer can. # -# The credentials in a profile expire in about a day, so never paste a -# working one anywhere that is committed. +# The credentials in a profile do expire (how soon varies), so never paste +# a working one anywhere that is committed. MAKEMYTRIP_CDP_ENDPOINT=ws://{login}-zone-scraping_browser-country-in-pid-{profileId}:{password}@cb.2captcha.com:9222 # --- Proxy ---------------------------------------------------------------- @@ -89,7 +89,12 @@ MAKEMYTRIP_PROXY=http://{user}:{password}@xx.proxy.2captcha.com:2334 # # Only the query in the address is read; the flags (--city, --checkin, # --checkout, --adults ...) do the same without a URL. -MAKEMYTRIP_URL=https://www.makemytrip.com/hotels/hotel-listing/?checkin=11102026&checkout=11112026&city=CTGOI&locusId=CTGOI&locusType=city&country=IN&roomStayQualifier=2e0e +# +# Shipped commented out, on purpose. A URL set here is a default for a run +# given NO query flags; a run that types --city, --checkin and the like +# ignores it. The dates in an example go stale, so put your own in when you +# uncomment it. +# MAKEMYTRIP_URL=https://www.makemytrip.com/hotels/hotel-listing/?checkin=11102026&checkout=11112026&city=CTGOI&locusId=CTGOI&locusType=city&country=IN&roomStayQualifier=2e0e # Deliberately NOT here: an output prefix. `--out` already carries a # non-empty default, so the loader, which only fills UNSET values, would diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 2d14d2d..8d99eea 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -18,8 +18,9 @@ body: was measured to work is the Scraping Browser API with a `country-in` profile (`--cdp-endpoint`). Say in the report which exit you used and what the sidecar's `stop_reason` was. - - **Exit 5 naming HTTP 401?** The Scraping Browser profile expired; - its credentials last about a day. + - **Exit 5 naming HTTP 401?** The Scraping Browser endpoint's + credentials were refused (expired, revoked or mistyped); copy it + again from your 2Captcha dashboard. - **Exit 4, zero rows?** The site answered "No Hotels Found" for that city and those dates. That is a correct answer. See [TROUBLESHOOTING.md](../blob/main/TROUBLESHOOTING.md). diff --git a/.github/canary_assert.py b/.github/canary_assert.py index 4fab115..18ebf1f 100644 --- a/.github/canary_assert.py +++ b/.github/canary_assert.py @@ -3,7 +3,7 @@ A separate script rather than a heredoc in canary.yml, and it reads only the run's own output files: a workflow that imports the repo's modules inline is -red on the first push and invisible to every local run (CLAUDE.md §26). +red on the first push and invisible to every local run. The thresholds are measured: a three-page Goa run through the Scraping Browser returned 90 rows, 90 priced, all in INR, all RECOMMENDED_HOTELS, on diff --git a/.github/ci_checks.py b/.github/ci_checks.py index 4a4fbb7..154fc09 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -285,7 +285,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 f34578e..76e1acd 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -4,21 +4,26 @@ # Two jobs, because the site has two answers and both are claims the README # makes: # -# datacentre no secrets, daily. A GitHub runner is a datacentre -# address, and from one (a netcup VPS, 2026-09-24) Akamai -# refused every client, real Chromium included, by -# dropping the connection. The job asserts the engine -# REPORTS that correctly: exit 3, a refusal, never 1, 4 -# or 5. If the runner is served instead (exit 0), the -# rows are checked as strictly as the other job's, and -# the README's datacentre sentence is due a re-measure. +# datacentre DATACENTRE REFUSAL EXPECTED. No secrets, daily. A +# GitHub runner is a datacentre address, and from one +# (a netcup VPS, 2026-09-24, and every scheduled run of +# this job since) Akamai refused every client, real +# Chromium included, by dropping the connection. The job +# asserts the engine REPORTS that correctly: exit 3, a +# refusal, never 1, 4 or 5. If the runner is served +# instead (exit 0), the rows are checked as strictly as +# the other job's, and the README's datacentre sentence +# is due a re-measure. This job does NOT test extraction; +# the next one does. # -# scraping-browser the path measured to work: a Scraping Browser profile -# with an Indian exit. Dispatch-only, not scheduled: a -# profile's credentials last about a day, so a scheduled -# job would be red every morning for a secret that had -# expired, not for the site (CLAUDE.md §19). SKIPS with a -# notice when the secret is not set (§11). +# scraping-browser the path measured to work, and the one that reads real +# rows: a Scraping Browser profile with an Indian exit, +# on the same schedule. It FAILS, with an error naming +# the secret, when MAKEMYTRIP_CDP_ENDPOINT is not set, +# and it fails on a refusal, on "never obtained" (a 401 +# is a refused endpoint: set a fresh secret) and on too +# few rows. A green canary therefore means a real +# three-page listing was read and parsed. # # gh secret set MAKEMYTRIP_CDP_ENDPOINT --repo 2scraper/makemytrip-scraper # gh workflow run canary.yml --repo 2scraper/makemytrip-scraper @@ -90,29 +95,26 @@ jobs: if-no-files-found: ignore scraping-browser: - if: github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest timeout-minutes: 20 env: MAKEMYTRIP_CDP_ENDPOINT: ${{ secrets.MAKEMYTRIP_CDP_ENDPOINT }} steps: - - name: Skip without a Scraping Browser endpoint + - name: Require a Scraping Browser endpoint if: env.MAKEMYTRIP_CDP_ENDPOINT == '' - run: echo "::notice::MAKEMYTRIP_CDP_ENDPOINT is not set, so the Scraping Browser run is skipped. It is the one path measured to get past Akamai from a datacentre; set the secret (a profile with country-in) and dispatch again." + run: | + echo "::error::The MAKEMYTRIP_CDP_ENDPOINT secret is not set, so nothing tested the site. It is the one path measured to get past Akamai from a datacentre; set it to a Scraping Browser endpoint with country-in in the login." + exit 1 - uses: actions/checkout@v4 - if: env.MAKEMYTRIP_CDP_ENDPOINT != '' - uses: actions/setup-python@v5 - if: env.MAKEMYTRIP_CDP_ENDPOINT != '' with: python-version: "3.12" - name: Install - if: env.MAKEMYTRIP_CDP_ENDPOINT != '' run: | python -m pip install --upgrade pip pip install -r requirements.txt -r requirements-playwright.txt - # >= 3 pages, because with one page pagination is never exercised (§11). + # >= 3 pages, because with one page pagination is never exercised. - name: Run - if: env.MAKEMYTRIP_CDP_ENDPOINT != '' id: run run: | set +e @@ -120,21 +122,21 @@ jobs: --out canary --format both --dump-html canary_dump.json echo "exit_code=$?" >> "$GITHUB_OUTPUT" - name: Interpret - if: env.MAKEMYTRIP_CDP_ENDPOINT != '' run: | code="${{ steps.run.outputs.exit_code }}" echo "exit code: $code" case "$code" in 0) ;; 3) echo "::error::Refused through the Scraping Browser. That is the README's central claim failing; see the log for which refusal."; exit 1 ;; - 5) echo "::error::Never obtained. A 401 in the log means the profile's credentials expired (they last about a day): set a fresh secret."; exit 1 ;; + 4) echo "::error::Zero rows, reported as an EMPTY listing. Goa is never empty."; exit 1 ;; + 5) echo "::error::Never obtained, or served and not parsed (see stop_reason in the log). A 401 in the log means the endpoint's credentials were refused: set a fresh MAKEMYTRIP_CDP_ENDPOINT secret."; exit 1 ;; + 6) echo "::error::A partial run: a page failed after rows were gathered. See stop_reason in the log."; exit 1 ;; *) echo "::error::Exit $code."; exit 1 ;; esac - name: Assert - if: env.MAKEMYTRIP_CDP_ENDPOINT != '' run: python .github/canary_assert.py canary 60 - uses: actions/upload-artifact@v4 - if: always() && env.MAKEMYTRIP_CDP_ENDPOINT != '' + if: always() with: name: canary-scraping-browser path: | diff --git a/.gitignore b/.gitignore index 53fca75..566d264 100644 --- a/.gitignore +++ b/.gitignore @@ -19,7 +19,7 @@ makemytrip_hotels_*.* live_results/ live_results.zip # ANY working directory a live run writes into, by KIND rather than by the one -# name an older repo used (CLAUDE.md §24). Measured here: `live/` was not +# name an older repo used. Measured here: `live/` was not # listed, and a `--dump-html` of a 3-page run wrote `canary_dump.json.page3`, # which `*.json` does not cover. It sat untracked and NOT ignored, one # `git add -A` from a commit, until the credential scan flagged the 32-hex @@ -93,7 +93,7 @@ secrets* # recoverable only because that repo was private at the time. On 2026-09-17 # a sibling had an untracked `.env.bak` holding a 32-hex key in a PUBLIC # repository, one `git add -A` from publication. Craigslist was fixed then -# and the fix never reached anyone else, which is §16's rule unapplied: when -# you fix one, check the siblings. +# and the fix never reached anyone else: when you fix one, check the +# siblings. .env* !.env.example diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fa7c44..45e5415 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,22 +9,80 @@ said so at the top of its release notes. ## [Unreleased] -> **Behaviour change:** `--retries 0` is now a usage error (exit 2). It used -> to be accepted and made the page-load loop unreachable, so nothing was -> fetched and the engine reported the browser's own blank page as a result: -> exit 3 or 4 with a 39-byte "capture", in tens of milliseconds. If a -> script of yours passes 0, pass **1**: the flag counts ATTEMPTS, so 1 means -> "try once and do not retry". +> **Behaviour changes.** +> +> * `--retries 0` is now a usage error (exit 2). It used to be accepted, and +> the page loop (`while attempt < args.retries`, after the landing) then +> never sent the listing request: the run fetched no page and ended with +> no rows. If a script of yours passes 0, pass **1**: the flag counts +> ATTEMPTS, so 1 means "try once and do not retry". +> * A page the site SERVED with properties in it that parses to no rows is +> now a failed page, `stop_reason: parser_found_nothing`: exit 5 when +> nothing was gathered, exit 6 (`partial`) after earlier pages. It used to +> be exit 4 ("the listing is empty") on page 1, and a complete run with +> exit 0 ("the listing ended") on a later page. +> * Negative values for `--delay`, `--retry-delay` and +> `--proxy-block-retries` are usage errors (exit 2) naming the flag. +> * Output files (`.json`, `.csv`, `.meta.json`) are written to a temporary +> file beside the target and renamed over it. A new file gets the mode +> `open()` would have given it (`0o666 & ~umask`); a file that already +> exists keeps its own mode. +> * CSV cells that begin with `=`, `+`, `-`, `@`, a tab, CR or LF get a +> leading apostrophe, so a spreadsheet reads them as text. JSON is +> unchanged, and the sidecar records `csv_cells_escaped`. +> * `.env.example` ships `MAKEMYTRIP_URL` commented out, and a URL from the +> environment no longer blocks typed query flags. ### Fixed -- **`--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 - reported a working scraper as broken. `--retries` now goes through - `page_flow.attempts` in every entry point; a check reads the source of each - one and calls the validator, so a new engine declaring the flag with a - plain int fails the suite. +- **`--retries 0` fetched nothing and called it a result.** `--retries` now + goes through `page_flow.attempts` in every entry point; a check reads the + source of each one and calls the validator, so a new engine declaring the + flag with a plain int fails the suite. +- **A served page whose rows the parser lost read as an empty or a finished + listing.** With every hotel's `id` renamed in a real Goa capture, the page + still classified as content holding 4 properties and parsed to 0 rows; + `--pages 1` exited 4, and a good page 1 followed by such a page 2 exited 0 + with `status: complete`. Both are now parser failures (see above), and the + response is saved as `_pageN_debug.html` for the issue. +- **The README's first run failed after `cp .env.example .env`.** The + example shipped a real `MAKEMYTRIP_URL`, the engines applied `.env` before + checking for a `--url`/`--city` conflict, and `--city Goa` exited 2 with + "--url already carries the query". The line is now commented out, each + engine records whether `--url` was typed before `.env` is applied, and an + environment URL gives way to typed query flags. A typed `--url` with + `--city` is still refused. +- **One page could buy three captcha solves, and a run by city name five,** + against `SOLVES_PER_PAGE = 1`: the allowance restarted on every landing, + and a refused page lands again after each relaunch. Measured with the + solver stubbed; real spend also needs `TWOCAPTCHA_KEY` and a captcha this + site has not been seen to serve. Every landing for a page now spends one + `page_flow.SolveBudget`, checked before each purchase and never reset by a + retry or a rotation, and the city lookup spends page 1's. +- **A write that failed halfway destroyed the previous good output.** A + failure injected inside `json.dump` left `out.json` and `out.meta.json` + as 10-byte fragments of invalid JSON. Writes are now atomic per file. +- **Formula-shaped text from listings reached CSV cells unescaped** (see + above). +- **A CDP 401 was always explained as credentials that "last about a + day".** That lifetime was not measured and is not a constant. The message, + README, TROUBLESHOOTING, `.env.example`, the bug template and the canary + now say the endpoint's credentials were refused (expired, revoked or + mistyped). +- `.dockerignore` named another repo's output prefix; it now lists this + repo's (`makemytrip_hotels.*`), `.env.*`, canary outputs and `.pageN` + dumps. + +### Changed + +- **The canary's Scraping Browser job runs on the schedule** instead of on + dispatch only, and FAILS with an error naming `MAKEMYTRIP_CDP_ENDPOINT` + when that secret is missing, instead of skipping green. The datacentre + job still expects Akamai's refusal (exit 3) and says it does not test + extraction; the Scraping Browser job is the one that requires real rows. +- Comments, docs and the suite state their reasons in this repo's own words + instead of pointing at section numbers in notes the public cannot read. A + suite check keeps it that way. ## [0.1.1] — 2026-09-25 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 07f87ab..9ecf56d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -65,8 +65,8 @@ break, and each is loud or guarded: behind Akamai Bot Manager, and what was measured to get through (2026-09-24) is the Scraping Browser API with a `country-in` profile. If that stops being served, the refusal is reported as exit 3 with - `blocked_akamai` or `blocked_akamai-reset`, and the dispatch-only canary - job says so. + `blocked_akamai` or `blocked_akamai-reset`, and the canary's Scraping + Browser job goes red. If you are reporting a break, say which of those four it is, and attach the `--dump-html` output: the exact JSON the parser was given, on success as well @@ -94,12 +94,13 @@ Then the rest of the presentation, in the order that matters: both halves. The daily job needs no secrets: it runs from a GitHub runner (a datacentre address) and asserts that Akamai's refusal is reported as exit 3, not as an empty or a crashed run, and if the runner is ever - served it checks the rows strictly instead. The second job is - dispatch-only and goes through a Scraping Browser profile (the - `MAKEMYTRIP_CDP_ENDPOINT` secret) to check a 3-page Goa run. A profile's - credentials last about a day, so refresh the secret before dispatching. -2. The repo description, homepage and topics set (see the family notes on - what those should say). + served it checks the rows strictly instead. The second job runs on the + same schedule through a Scraping Browser profile (the + `MAKEMYTRIP_CDP_ENDPOINT` secret) and checks a 3-page Goa run. It FAILS + when the secret is missing or refused (a 401), so a green badge always + means real rows were read. +2. The repo description, homepage and topics set: the words people search + for (the site, hotels, scraper, the engines), not a slogan. 3. Only then the row in the org profile README — and check it with an ANONYMOUS request rather than your own logged-in browser. A row pointing at a private repo is a 404 for every visitor, which costs more trust than diff --git a/README.md b/README.md index 57ec597..c6eff55 100644 --- a/README.md +++ b/README.md @@ -187,9 +187,9 @@ One key, four separately-billed products ([2captcha.com](https://2captcha.com)): * **The Scraping Browser API** (`--cdp-endpoint`, `MAKEMYTRIP_CDP_ENDPOINT`) is what makes this repo work from a datacentre: a remote browser with an - Indian exit. One live connection per `pid`. A profile's credentials last - about a day; an expired one answers HTTP 401, which the engines report as - exit 5 naming the expiry. + Indian exit. One live connection per `pid`. A profile whose credentials + are refused (expired, revoked or mistyped) answers HTTP 401, which the + engines report as exit 5 naming the status. * **The Scraper API** (`scraper_api_client.py`) works here only routed through such a session, and then reads five properties per listing. * **Captcha solving**: no captcha was met on this site. The family's @@ -254,9 +254,11 @@ no city. The [canary](.github/workflows/canary.yml) runs twice over. Daily, with no secrets, from a GitHub runner: a datacentre, so it asserts the refusal is -REPORTED as one (exit 3, never a crash or an "empty listing"). On dispatch, -through a Scraping Browser profile: a real 3-page Goa scrape, checked for -prices, currency, duplicates and zero ratings. +REPORTED as one (exit 3, never a crash or an "empty listing"); that job +tests the refusal path, not extraction. On the same schedule, through a +Scraping Browser profile: a real 3-page Goa scrape, checked for prices, +currency, duplicates and zero ratings. That job fails when its secret is +missing or refused, so the badge is green only when real rows were read. --- diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md index 195907c..4828966 100644 --- a/TROUBLESHOOTING.md +++ b/TROUBLESHOOTING.md @@ -55,9 +55,10 @@ an earlier good file is left alone; `--allow-empty` writes the empty file. same request would be rejected again, so it is not retried. If the query looks right, the API changed: open a "Site changed" issue with `--dump-html` output. -* **"HTTP 401: the endpoint's credentials were refused"** — the Scraping - Browser profile has expired. Its credentials last about a day; get a - fresh endpoint from your 2Captcha dashboard. +* **"HTTP 401: the endpoint's credentials were refused"** — the service + refused the login in the endpoint: expired, revoked or mistyped. How long + a profile's credentials live varies, so copy the endpoint again from your + 2Captcha dashboard. * **HTTP 500 / `profile_locked`** — another run still holds this `pid`. A profile allows one live connection; wait for the other run, or use a different pid. diff --git a/captcha_solver.py b/captcha_solver.py index 035b02c..c70245e 100644 --- a/captcha_solver.py +++ b/captcha_solver.py @@ -99,7 +99,7 @@ class CaptchaChallenge: page_url: str = "" # AWS WAF only. Its task needs three values that all live together in # `window.gokuProps` on the challenge page, plus the two script URLs the - # page loads. Unlike Cloudflare Turnstile (§19 of the family notes), + # page loads. Unlike Cloudflare Turnstile, # nothing has to be intercepted at runtime to get them: AWS WAF prints # all of it into the served HTML, so a static read is enough. iv: Optional[str] = None @@ -151,7 +151,7 @@ def aws_waf_action(self) -> str: def has_captcha_widget(self) -> bool: """Is there a widget on this page for a solver to work on? - Section 19's rule: "unsolvable" is a property of a PAGE, not of a + "Unsolvable" is a property of a PAGE, not of a vendor, and it means the page carries no widget. A challenge-action page carries none, so sending it to a solver buys a token for a puzzle that was never rendered -- measured at $0.00145 a time, since @@ -935,4 +935,4 @@ def solve_recaptcha(challenge: CaptchaChallenge, twocaptcha_api_key: Optional[st # what the address has been doing — a narrow list is how a challenge gets # reported as an empty page months later. A solver for a challenge this site # has never been observed to serve is dead code; a DETECTOR for one is cheap -# insurance. See the family note in CLAUDE.md. +# insurance. diff --git a/env_config.py b/env_config.py index 059f739..3514acb 100644 --- a/env_config.py +++ b/env_config.py @@ -66,8 +66,8 @@ # A literal-only check reported both of those as CONFIGURED, so `cp # .env.example .env` followed by a run connected to cb.2captcha.com with the # string `{login}-zone-…` as its username and got a 401 — a confusing failure -# a long way from its cause, which is precisely what §3's rule exists to -# prevent. Any `{…}` left in a value means the line was never filled in. +# a long way from its cause, which is precisely what treating placeholders +# as unset exists to prevent. Any `{…}` left in a value means the line was never filled in. _BRACED_PLACEHOLDER_RE = re.compile(r"\{[A-Za-z_][A-Za-z0-9_]*\}") _loaded_from = None diff --git a/fingerprint_client.py b/fingerprint_client.py index e84ce1e..a3be9ca 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -249,9 +249,8 @@ def playwright_context_kwargs(fp: dict) -> dict: # stating `deviceScaleFactor: 1.25` produced a browser reporting # `window.devicePixelRatio === 1`, so the identity contradicted itself # on an axis any fingerprinter reads for free -- and a contradiction is - # exactly what this flag exists to avoid. Same shape as the three - # defects the family notes already list for this file: a key the API - # returns that nothing applies. Found by comparing a live browser + # exactly what this flag exists to avoid. Same shape as three earlier + # defects in this file: a key the API returns that nothing applies. Found by comparing a live browser # against the fingerprint rather than by reading the code. scale = (fp.get("screen") or {}).get("deviceScaleFactor") if isinstance(scale, (int, float)) and not isinstance(scale, bool) and scale > 0: @@ -331,7 +330,7 @@ def main() -> int: # loader. It used to read `os.environ` alone, which meant a key put in # `.env` exactly as the README and .env.example instruct worked for every # engine and failed HERE with "No API key" — a documented mechanism not - # applied on one path, which is the shape of half the defects §16 lists. + # applied on one path. # # `.env` has to be LOADED before it can be read: `env_value` looks at # os.environ, and `load_env` is what fills that from the file. Calling it diff --git a/output_writer.py b/output_writer.py index 9efe643..b9b6936 100644 --- a/output_writer.py +++ b/output_writer.py @@ -17,8 +17,11 @@ family name. """ +import contextlib import csv import json +import os +import tempfile from dataclasses import dataclass, asdict, field, fields from datetime import datetime, timezone from typing import Optional, List, Set, Sequence, Any, Type @@ -70,7 +73,7 @@ class Hotel: # and villas carry `starRating: 0`, which is "unrated", not nought). star_rating: Optional[int] = None # The guest rating out of 5. None, together with its counts, when no one - # has rated the property: a 0 there would drag every average (§21). + # has rated the property: a 0 there would drag every average. rating: Optional[float] = None rating_count: Optional[int] = None review_count: Optional[int] = None @@ -166,12 +169,93 @@ def _csv_value(v: Any) -> Any: return v +@contextlib.contextmanager +def _atomic(path: str, newline: Optional[str] = None): + """Write to a temporary file beside `path`, then rename over it. + + `open(path, "w")` truncates before a single byte is written, so a crash, + a kill, a full disk or a Ctrl-C halfway through `json.dump` leaves a + SHORTER file where a complete one was. Reproduced here before this + existed: a previous good `out.json` and `out.meta.json` each came back + as a 10-byte fragment of invalid JSON after a failure inside + `json.dump`. That destroys the previous good run by the ATTEMPT to + replace it, which is exactly what `save` refusing to write an empty + result exists to prevent. + + * the temp file goes in the target's OWN directory, because `os.replace` + is atomic only within one filesystem; + * `fsync` before the rename makes the bytes durable, not merely visible; + * `newline` is passed through because the csv module needs + `newline=""`. + + Each file is replaced atomically on its own. JSON, CSV and the sidecar + are three replacements, not one transaction. + """ + directory = os.path.dirname(os.path.abspath(path)) or "." + handle = tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", newline=newline, dir=directory, + prefix=os.path.basename(path) + ".", suffix=".tmp", delete=False) + try: + with handle: + yield handle + handle.flush() + os.fsync(handle.fileno()) + # `NamedTemporaryFile` creates at 0600 and `os.replace` keeps that + # mode. An existing target keeps its OWN mode (someone may have + # tightened it on purpose); a new one gets what `open()` would have + # given it, `0o666 & ~umask`. Never a flat 0o644, which narrows + # 0664 to 0644 under umask 002. + try: + mode = os.stat(path).st_mode & 0o777 + except OSError: + umask = os.umask(0) + os.umask(umask) + mode = 0o666 & ~umask + os.chmod(handle.name, mode) + os.replace(handle.name, path) + except BaseException: + # Including KeyboardInterrupt: an interrupted run must not leave + # `out.json.ab12cd.tmp` behind beside the good file. + try: + os.unlink(handle.name) + except OSError: + pass + raise + + +# A spreadsheet treats a cell beginning with one of these as a FORMULA +# rather than as text, and every string in these rows was written by a third +# party: a property's name, its locality, a coupon code. The tab and the +# two newlines are here because some readers strip a leading one and then +# execute whatever followed it. +CSV_FORMULA_LEADS = ("=", "+", "-", "@", "\t", "\r", "\n") + +# Excel, Google Sheets and LibreOffice read a leading apostrophe as "the rest +# of this cell is text". A consumer parsing the file with the `csv` module +# DOES see it, which is why the count goes into the sidecar. +CSV_FORMULA_ESCAPE = "'" + + +def _csv_escape(v: Any): + """Neutralise a formula-shaped cell. Returns (value, was_escaped). + + Only STRINGS are touched: escaping a number would turn `-5` into text + and break every sum over the column (a negative longitude, for one). + CSV only: the JSON output keeps the site's bytes, and + `csv_cells_escaped` in the sidecar declares the divergence. + """ + if isinstance(v, str) and v.startswith(CSV_FORMULA_LEADS): + return CSV_FORMULA_ESCAPE + v, True + return v, False + + def write_json(rows: Sequence[Any], path: str) -> None: - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump([asdict(r) for r in rows], f, ensure_ascii=False, indent=2) -def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Hotel) -> None: +def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Hotel) -> int: + """Write the CSV and return how many cells were formula-escaped.""" # An empty result still gets the header row. A zero-byte file makes a # consumer fail on read (no columns to parse) instead of reading a valid # table with zero rows — and "an empty result is still a well-formed @@ -180,11 +264,20 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Hotel) -> None: # The header comes from `row_cls`, not from the first row, so an empty # run still writes the columns of the mode that produced it. fieldnames = [f.name for f in fields(row_cls)] - with open(path, "w", encoding="utf-8", newline="") as f: + escaped = 0 + with _atomic(path, newline="") as f: writer = csv.DictWriter(f, fieldnames=fieldnames) writer.writeheader() for r in rows: - writer.writerow({k: _csv_value(v) for k, v in asdict(r).items()}) + row = {} + for k, v in asdict(r).items(): + # AFTER the list serialiser: a joined list is text too, and + # its first element can be formula-shaped. + value, was_escaped = _csv_escape(_csv_value(v)) + row[k] = value + escaped += was_escaped + writer.writerow(row) + return escaped # Exit code used when a run completes but produced nothing. Distinct from 1 @@ -260,7 +353,7 @@ def write_run_meta(out_prefix: str, meta: dict) -> str: both complete, and between runs of different `mode`. """ path = f"{out_prefix}.meta.json" - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump(meta, f, ensure_ascii=False, indent=2) print(f"[+] Wrote run metadata -> {path} (status={meta.get('status')})") return path @@ -314,14 +407,15 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, } if extra: # Merged rather than nested under a key, so a consumer reads - # `shop_rating` at the top level beside `products`. Run fields win a + # `currency` at the top level beside `products`. Run fields win a # name collision: a caller cannot accidentally overwrite `status`. meta.update({k: v for k, v in extra.items() if k not in meta}) return meta def save(rows: Sequence[Any], out_prefix: str, fmt: str, - allow_empty: bool = False, row_cls: Type = Hotel) -> int: + allow_empty: bool = False, row_cls: Type = Hotel, + 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. @@ -348,8 +442,16 @@ def save(rows: Sequence[Any], out_prefix: str, fmt: str, write_json(rows, f"{out_prefix}.json") print(f"[+] Saved {len(rows)} rows -> {out_prefix}.json") if fmt in ("csv", "both"): - write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) + escaped = write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) print(f"[+] Saved {len(rows)} rows -> {out_prefix}.csv") + # Through `stats` because this function returns an EXIT CODE. + if stats is not None: + stats["csv_cells_escaped"] = escaped + if escaped: + print(f"[!] {escaped} CSV cell(s) began with a formula character " + f"and were prefixed with {CSV_FORMULA_ESCAPE!r} so a " + f"spreadsheet reads them as text. The JSON output is " + f"unchanged; see csv_cells_escaped in the sidecar.") return 0 if rows else EXIT_NO_PRODUCTS @@ -405,8 +507,13 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, # names has a hole for every name nobody added to it. complete = stop_reason in COMPLETE_STOP_REASONS and not pages_failed row_cls = ROW_CLASS_BY_MODE.get(mode, Hotel) - rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls) + stats: dict = {} + rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls, + stats=stats) wrote_output = bool(rows) or allow_empty + # Housekeeping goes UNDER the caller's `extra`: a collision must never + # let a count this module made up drop a fact about the site. + extra = {**stats, **(extra or {})} if wrote_output: status = "complete" if (rows and complete) else ( diff --git a/page_flow.py b/page_flow.py index a05cd57..554aff8 100644 --- a/page_flow.py +++ b/page_flow.py @@ -2,7 +2,7 @@ page_flow.py ------------ The retry / solve / blocked decision, as DATA rather than as three copies of -an if-chain (CLAUDE.md §1), and the one fetch loop all three engines share. +an if-chain, and the one fetch loop all three engines share. makemytrip.com answers one of this repo's requests in seven ways, and they want five different responses: @@ -19,8 +19,8 @@ 429 -> wait, same exit anything else -> retry -Nothing here imports a browser, and **no JavaScript crosses this boundary** -(§1). Each engine spells its fetch() in its own driver's dialect. +Nothing here imports a browser, and **no JavaScript crosses this boundary**. +Each engine spells its fetch() in its own driver's dialect. """ import logging @@ -31,7 +31,7 @@ from proxy_pool import ROTATE_MODES from product_parser import (DEFAULT_SORT, MAX_PAGES, ORIGIN_URL, SORTS, Cursor, Query, Room, - api_error, currency_of, default_dates, + api_error, count_rows, currency_of, default_dates, detect_page_state, is_location_code, listing_ended, listing_url, location_of, next_cursor, only_substitutes, parse_date, parse_page, @@ -48,12 +48,12 @@ # How long one fetch() may take before the engine gives up on it. The # largest response measured was 374 KB (30 Dubai properties), which arrived # in about a second. The bound exists because a browser fetch() has no -# timeout of its own, and CLAUDE.md §8 requires every remote call to have one. +# timeout of its own, and every remote call here has to be bounded. FETCH_TIMEOUT_MS = 30_000 # How long to wait at the SAME exit after a 429 before trying again. Not # observed on this site; the family's answer to a throttle is to slow down -# rather than to rotate (§24). +# rather than to rotate. THROTTLE_WAIT_S = 10.0 THROTTLE_RETRIES = 2 @@ -68,7 +68,7 @@ def classify(html: Optional[str], status: Optional[int] = None, The argument ORDER is the contract: every engine calls `classify(html, status, url, waf_action)`. A sibling repo shipped `classify(html, url=...)` in two of three engines against a callee that - took `status` second, and both crashed on their first fetch (§17). + took `status` second, and both crashed on their first fetch. `smoke_test.py` binds every engine's call against this signature for that reason. """ @@ -91,7 +91,7 @@ def classify(html: Optional[str], status: Optional[int] = None, "challenge": {"retry": True, "solve": True, "blocked": True, "parse": False}, # Rate limited: wait at the same exit (THROTTLE_*). NOT counted as # blocked, since calling a throttle a block reports exit 3 for a page - # that was about to come back (§24). Not observed on this site. + # that was about to come back. Not observed on this site. "throttled": {"retry": True, "solve": False, "blocked": False, "parse": False}, # Akamai's "Access Denied", or HTTP 403. "blocked": {"retry": True, "solve": False, "blocked": True, "parse": False}, @@ -124,7 +124,7 @@ def should_parse(state: str) -> bool: # Whether a blocked page is worth re-fetching at all. CONSULTED by the loop, -# so setting it False really does stop the retry loop (§17). +# so setting it False really does stop the retry loop. RETRY_ON_BLOCKED = True # How many times to re-fetch a refused page when there is no proxy pool to @@ -164,7 +164,7 @@ def is_reset(error_text: Optional[str]) -> bool: def concurrency_refusal(concurrency: int) -> Optional[str]: """Why --concurrency above 1 is refused, or None when it is 1. - Refused rather than quietly clamped (§18): page N's request carries a + Refused rather than quietly clamped: page N's request carries a cursor that only page N-1's response names, so there is nothing a second worker could fetch. """ @@ -202,6 +202,8 @@ def refusal_advice(state: str) -> str: def stop_reason_for(outcome) -> str: """The run's stop_reason when `outcome` is the page that ended it.""" + if getattr(outcome, "parse_failed", False): + return "parser_found_nothing" if getattr(outcome, "rejected", None): return "api_rejected" if getattr(outcome, "blocked_by", None): @@ -228,7 +230,7 @@ def build_query(args, error: Callable[[str], None]) -> Query: --url and the query flags are two ways to say the same thing. A flag the user typed that disagrees with the URL would silently scrape something neither of them named, so the combination is refused rather than merged - (the family's --country rule, §10). `error` is argparse's `p.error`, so + (the family's --country rule). `error` is argparse's `p.error`, so a refusal is exit 2 with the usage line, as in every engine. --sort is not a query flag in that sense: the site's listing address carries no ordering, so it applies to either. @@ -236,6 +238,18 @@ def build_query(args, error: Callable[[str], None]) -> Query: refusal = concurrency_refusal(getattr(args, "concurrency", 1) or 1) if refusal: error(refusal) + # Whether --url was TYPED, recorded by each engine before env_config + # filled anything in. After `apply` a URL from MAKEMYTRIP_URL looks the + # same as a typed one, and treating it as typed refused `--city Goa` for + # everyone with that line in .env: a default must not veto a flag. + typed_url = getattr(args, "typed_url", args.url) + if args.url and not typed_url: + typed_flags = [flag for dest, flag in QUERY_FLAGS + if getattr(args, dest, None) is not None] + if typed_flags: + log.info("Ignoring MAKEMYTRIP_URL from the environment: %s " + "given on the command line.", ", ".join(typed_flags)) + args.url = None if args.url: query, why = query_from_url(args.url) if query is None: @@ -290,7 +304,7 @@ def build_query(args, error: Callable[[str], None]) -> Query: def add_arguments(p, engine: str = "playwright") -> None: """The CLI the three engines share, so their flag sets agree by - construction (§17 check #2). An engine adds only what is genuinely its + construction. An engine adds only what is genuinely its own (pyppeteer's --chromium-path, Selenium's --chromedriver) after calling this, and the suite pins those differences.""" p.add_argument("--mode", choices=["hotels"], default="hotels", @@ -338,7 +352,7 @@ def add_arguments(p, engine: str = "playwright") -> None: help="Pages to fetch, 30 properties each. The site states " "no total; a run ends early, complete, when it says " "nothing follows.") - p.add_argument("--delay", type=float, default=1.0, + p.add_argument("--delay", type=non_negative("--delay"), default=1.0, help="Delay between pages, seconds (default %(default)s)") p.add_argument("--concurrency", type=int, default=1, metavar="N", help="Kept for the family's contract, and refused above 1: " @@ -349,7 +363,8 @@ def add_arguments(p, engine: str = "playwright") -> None: "The pause doubles each time. A request the endpoint " "REFUSED is not retried: its parameters would be " "refused again.") - p.add_argument("--retry-delay", type=float, default=2.0, + p.add_argument("--retry-delay", type=non_negative("--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="makemytrip_hotels", help="Output file prefix") @@ -368,7 +383,8 @@ def add_arguments(p, engine: str = "playwright") -> None: "per-page: a new exit, and a fresh browser, per page.") p.add_argument("--proxy-shuffle", action="store_true", help="Shuffle the pool at startup.") - p.add_argument("--proxy-block-retries", type=int, default=2, + p.add_argument("--proxy-block-retries", + type=non_negative("--proxy-block-retries", int), default=2, help="When a page is refused (Akamai's 403 or a dropped " "connection), retry it from this many OTHER exits " "(default 2). Needs a pool of more than one.") @@ -433,7 +449,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, """Merge the pages in PAGE order, write the output, return the exit code. One implementation for the three engines, so the merge order, the - dedupe and the sidecar cannot differ between them (§6). + dedupe and the sidecar cannot differ between them. """ rows, seen = [], set() for oc in sorted(outcomes, key=lambda o: o.page_num): @@ -476,7 +492,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, # failure, waiting out a throttle, re-connecting on a refusal, and parsing # what came back. The three engines differ only in HOW they ask their # driver, so each passes in an object with these operations and no -# JavaScript crosses this boundary (§1): +# JavaScript crosses this boundary: # # ops.goto(url) -> (status, waf_header). Raises TransportError. # ops.document_text() -> the landing document's markup @@ -499,7 +515,7 @@ class PageOutcome: Collected per page and merged afterwards, in page order, rather than folded into shared state as the loop goes, so the output cannot depend - on the order things finished in (§8). + on the order things finished in. """ page_num: int url: str @@ -512,11 +528,16 @@ class PageOutcome: cursor: Optional[Cursor] = None ended: bool = False currency: Optional[str] = None + # The site SERVED this page and its payload holds properties, and the + # parser turned none of them into rows. That is this repo's bug (or a + # payload shape that moved), not an empty listing and not the end of + # one, so the page counts as failed. + parse_failed: bool = False @property def ok(self) -> bool: return (not self.load_failed and self.blocked_by is None - and self.rejected is None) + and self.rejected is None and not self.parse_failed) # The columns the site filled on EVERY record of every capture (114 @@ -532,14 +553,49 @@ def ok(self) -> bool: } -def land(ops, args) -> Tuple[str, Optional[str]]: +class SolveBudget: + """The paid-solve allowance for ONE page, shared by every landing on it. + + Created once per page and handed to every `land()` that page makes + (and, for page 1, to the city lookup that precedes it). It is checked + BEFORE each purchase and is never reset by a transport retry, a + block-retry or an exit rotation: a fresh browser or a fresh exit is a + reason to fetch again, not a fresh allowance. + + Before this existed the allowance was a loop inside `land()`, so it + restarted on every landing. With the solver stubbed to succeed while + the page stayed challenged, one page bought 3 solves with no proxy pool, + and a run by city NAME bought 5, against SOLVES_PER_PAGE = 1. + """ + + def __init__(self, limit: int = SOLVES_PER_PAGE): + self.limit = limit + self.spent = 0 + + def spend(self, solve: Callable[[], bool]) -> bool: + """Call `solve` if the allowance has room; return what it returned.""" + if self.spent >= self.limit: + log.warning("The captcha came back, and this page's solve " + "allowance (%d) is already spent — not paying for " + "another.", self.limit) + return False + self.spent += 1 + return bool(solve()) + + +def land(ops, args, budget: Optional[SolveBudget] = None + ) -> Tuple[str, Optional[str]]: """Put the page on a www.makemytrip.com document that fetch() can use. Returns (state, error): state is a policy state for the landing, and error is a transport failure's text or None. A reset connection is a STATE ("reset"), not an error, because it is Akamai's answer rather than - a fault on the way to it. + a fault on the way to it. Every solve goes through `budget`, the + allowance of the page this landing serves; without one, nothing is + bought. """ + if budget is None: + budget = SolveBudget(limit=0) try: status, waf = ops.goto(ORIGIN_URL) except TransportError as e: @@ -548,13 +604,10 @@ def land(ops, args) -> Tuple[str, Optional[str]]: return "reset", None return "load_failed", str(e) state = classify(ops.document_text(), status, ORIGIN_URL, waf) - if state == "challenge" and should_solve(state): - for _ in range(SOLVES_PER_PAGE): - if not ops.solve_captcha(): - break - state = classify(ops.document_text(), None, ORIGIN_URL, None) - if state != "challenge": - break + while state == "challenge" and should_solve(state): + if not budget.spend(ops.solve_captcha): + break + state = classify(ops.document_text(), None, ORIGIN_URL, None) ops.landed = state == "content" return state, None @@ -573,7 +626,7 @@ def _core_field_warnings(rows: List, mode: str, page_num: int) -> None: def _dump(args, page_num: int, text: str) -> None: - """Write the exact response the parser was given, on success too (§9).""" + """Write the exact response the parser was given, on success too.""" if not args.dump_html: return path = args.dump_html if args.pages == 1 else f"{args.dump_html}.page{page_num}" @@ -592,7 +645,8 @@ def _save_debug(args, page_num: int, text: str) -> str: def fetch_one_page(ops, args, pool, query: Query, page_num: int, cursor: Optional[Cursor] = None, - mask: Callable[[str], str] = lambda s: s) -> PageOutcome: + mask: Callable[[str], str] = lambda s: s, + budget: Optional[SolveBudget] = None) -> PageOutcome: """Fetch and parse one page. Retries, re-connections and dumps live here. Never raises for an EXPECTED failure. A timeout, a refusal and a dead @@ -606,6 +660,9 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, args.proxy_block_retries if has_pool else BLOCK_RETRIES_WITHOUT_POOL) throttles = 0 state, text, last_error, exit_failed = "unknown", "", None, None + # One allowance for the whole page, across every landing below. + if budget is None: + budget = SolveBudget() for block_attempt in range(block_retries + 1): log.info("Fetching page %d/%d: %s", page_num, args.pages, req.label) @@ -615,7 +672,7 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, attempt += 1 text, last_error = "", None if not ops.landed: - state, last_error = land(ops, args) + state, last_error = land(ops, args, budget) if last_error: exit_failed = ops.proxy_failure(last_error) or None if exit_failed: @@ -634,7 +691,7 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, ops.landed = False if state == "throttled" and throttles < THROTTLE_RETRIES: throttles += 1 - attempt -= 1 # a throttle wait spends its own budget (§24) + attempt -= 1 # a throttle wait spends its own budget pause = THROTTLE_WAIT_S * throttles log.warning("Rate-limited on page %d (HTTP 429) — waiting " "%.0fs at the same exit (%d/%d).", page_num, pause, @@ -707,6 +764,20 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, log.info("The site answered page %d with %s.", page_num, api_error(text)) rows = parse_page(text, query, page_num) outcome.products = rows + stated = count_rows(text) if state == "content" else 0 + if stated and not rows: + # The response holds properties and the parser made nothing of them. + # Reported as "the listing is empty" (exit 4) or "the listing + # ended" (a complete run, exit 0), this sends the reader to check + # the city instead of the parser. It is a failed page: exit 5 when + # nothing was gathered, exit 6 (partial) after earlier pages. + outcome.parse_failed = True + debug = _save_debug(args, page_num, text) + log.error("Page %d was served with %d propert%s and NONE parsed into " + "a row — the payload shape has moved, or this is a bug. " + "Saved to %s; please open an issue with it.", page_num, + stated, "y" if stated == 1 else "ies", debug) + return outcome outcome.cursor = next_cursor(text, cursor) outcome.ended = listing_ended(text) or (bool(rows) and outcome.cursor is None) outcome.currency = currency_of(text) @@ -741,7 +812,9 @@ def _check_location(text: str, query: Query) -> None: loc["code"], loc["country"], query.country_code) -def resolve_city(ops, args, query: Query) -> Tuple[Optional[str], Optional[str]]: +def resolve_city(ops, args, query: Query, + budget: Optional[SolveBudget] = None + ) -> Tuple[Optional[str], Optional[str]]: """Fill in city_code/country_code from a city NAME, via the site's own autosuggest. Returns (refusal, blocked_state): a reason the run is refused (exit 2), or the refusal STATE that stopped the lookup, which @@ -757,7 +830,7 @@ def resolve_city(ops, args, query: Query) -> Tuple[Optional[str], Optional[str]] last_block = None for _ in range(max(1, args.retries) + BLOCK_RETRIES_WITHOUT_POOL): if not ops.landed: - state, err = land(ops, args) + state, err = land(ops, args, budget) if not ops.landed: if counts_as_blocked(state): last_block = state @@ -801,7 +874,10 @@ def run_pages(open_ops, close_ops, args, pool, query: Query, blocked, stop_reason = False, "completed" ops = open_ops() try: - refusal, refused_state = resolve_city(ops, args, query) + # The lookup is part of getting page 1, so it spends page 1's + # allowance: a landing it solved is the landing page 1 then uses. + first_budget = SolveBudget() + refusal, refused_state = resolve_city(ops, args, query, first_budget) if refusal: log.error("%s", refusal) return 2 @@ -822,7 +898,8 @@ def run_pages(open_ops, close_ops, args, pool, query: Query, if pool and pool.rotates_per_page(): pool.advance(f"per-page rotation, page {page_num}") ops.relaunch() - outcome = fetch_one_page(ops, args, pool, query, page_num, cursor, mask) + outcome = fetch_one_page(ops, args, pool, query, page_num, cursor, mask, + budget=first_budget if page_num == 1 else None) outcomes.append(outcome) if not outcome.ok: stop_reason = stop_reason_for(outcome) @@ -842,18 +919,18 @@ def run_pages(open_ops, close_ops, args, pool, query: Query, def cdp_connect_hint(error_text: str) -> str: """What a failed --cdp-endpoint connection means, from its status. - Two answers that want opposite fixes. Measured 2026-09-24 against four - Scraping Browser endpoints left in sibling repos' .env files: all four - answered 401 Unauthorized, because a profile's credentials last about a - day. The message used to explain a 500 (a pid another run still holds) + Two answers that want opposite fixes. A 401 is the service refusing the + credentials in the endpoint: expired, revoked or mistyped. How long a + profile's credentials live is not a constant (one endpoint was measured + still working after ten days), so the message does not guess which. + The message used to explain a 500 (a pid another run still holds) whatever the status was, which sent the reader to wait for a run that did not exist. """ if "401" in (error_text or ""): - return ("HTTP 401: the endpoint's credentials were refused. A Scraping " - "Browser profile's credentials last about a day, so an " - "endpoint copied from an older .env has usually expired. " - "Get a fresh one from your 2Captcha dashboard.") + return ("HTTP 401: the endpoint's credentials were refused, for " + "example an expired, revoked or mistyped profile login. " + "Copy the endpoint again from your 2Captcha dashboard.") return ("A Scraping Browser profile allows ONE live connection at a time, " "so an HTTP 500 here usually means another run still holds this " "`pid`. Wait for it to finish, or use a different pid.") @@ -892,26 +969,37 @@ def cdp_should_retry(error_text: str) -> bool: import argparse as _argparse # noqa: E402 (kept beside its only user) -def attempts(value) -> int: - """argparse `type=` for --retries. Rejects anything below 1. +def non_negative(flag: str, kind=float): + """argparse `type=` for a count or a delay that may be 0 but not less. - WHY THIS IS A USAGE ERROR AND NOT A CLAMP. The page-load loop is - `for attempt in range(1, retries + 1)`, so `--retries 0` makes its body - unreachable: nothing is fetched, and the engine then judges whatever the - browser holds — `about:blank`, 39 bytes of - `` — and reports a block or an - empty result in a few dozen milliseconds. That is a confident, fast, - plausible answer to a question nobody asked. + A negative --delay or --retry-delay is not a shorter wait, and a + negative --proxy-block-retries makes the page loop run no times at all, + so the page is never fetched. Refused with the flag's name instead. + """ + def parse(value): + try: + number = kind(value) + except (TypeError, ValueError): + raise _argparse.ArgumentTypeError( + f"{flag} must be a number, not {value!r}") + if number < 0: + raise _argparse.ArgumentTypeError( + f"{flag} must be 0 or more, not {value}") + return number + parse.__name__ = "non_negative" + return parse - Measured on 2026-10-13 against sleepnumber-scraper, from an address the - site genuinely does refuse, so the conclusion even looked right: - --retries 0 exit 3, 39 bytes, 36 ms <- never left the process - --retries 1 exit 3, 923 bytes, 116 ms <- the real CloudFront 403 +def attempts(value) -> int: + """argparse `type=` for --retries. Rejects anything below 1. - A network round trip does not finish in 36 ms. Three third-party audits of - the 2scraper scrapers have now run `--retries 0`, read that output, and - reported a working scraper as broken or a site as unreachable. + WHY THIS IS A USAGE ERROR AND NOT A CLAMP. The page loop here is + `while attempt < args.retries`, after the landing, so with `--retries 0` + the listing request is never sent: the run lands on the site's origin, + fetches no page, and ends as "never obtained" (exit 5) with no rows. + That is a confident, fast, plausible answer to a question nobody asked, + and third-party audits of scrapers built this way have read such output + as a broken scraper or an unreachable site. The flag counts ATTEMPTS, not re-tries, so a caller who writes 0 almost certainly means "do not retry", which is 1. Clamping would hide the diff --git a/playwright_scraper.py b/playwright_scraper.py index 1c90885..70871a7 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -68,7 +68,7 @@ # Chromium's own names for a proxy that could not be used. Distinguished -# from a timeout because the two want opposite responses (CLAUDE.md §8). +# from a timeout because the two want opposite responses. _PROXY_ERROR_MARKERS = ( "ERR_PROXY_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED", @@ -82,11 +82,11 @@ # The fetch() every page goes through. It is handed to `page.evaluate` as a # FUNCTION, which Playwright sends through `Runtime.callFunctionOn` rather -# than evaluating a string, so it works under any Content-Security-Policy -# (§18). `credentials: "include"` sends the cookies Akamai's sensor set on +# than evaluating a string, so it works under any Content-Security-Policy. +# `credentials: "include"` sends the cookies Akamai's sensor set on # the landing, as the site's own front end does. The AbortController is the -# timeout: a browser fetch() has none of its own, and §8 requires every -# remote call to be bounded. +# timeout: a browser fetch() has none of its own, and every remote call +# here has to be bounded. FETCH_JS = """ async ([url, method, body, headers, timeoutMs]) => { const ctl = new AbortController(); @@ -109,7 +109,7 @@ def _chrome_ua(chromium_version: str) -> str: - """A desktop-Chrome UA naming the browser's OWN real version (§8).""" + """A desktop-Chrome UA naming the browser's OWN real version.""" return (f"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " f"(KHTML, like Gecko) Chrome/{chromium_version} Safari/537.36") @@ -128,7 +128,7 @@ def _launch_local(pw, args, pool): A proxy rotation tears the whole browser down and calls this again: cookies a bot manager issued against one exit, replayed from another, - are a stronger signal than either address alone (§8). + are a stronger signal than either address alone. """ launch_kwargs = {"headless": args.headless} proxy = to_playwright(pool.current) if pool else None @@ -263,7 +263,7 @@ def _connect_remote(pw, args): break if browser is None: # The endpoint carries a password, and Playwright repeats it five - # times in its error text (§8). Rewritten with it masked, keeping + # times in its error text. Rewritten with it masked, keeping # host and port, which are the useful half. raise PWError( f"could not connect to --cdp-endpoint " @@ -292,7 +292,7 @@ def _connect_remote(pw, args): # Every `scheme://user:pass@` in a string, however many times it occurs. # Matching GLOBALLY is the point: a Playwright connection error repeats the -# endpoint five times (§8). +# endpoint five times. _CREDENTIALS_IN_URL_RE = re.compile(r"([a-z][a-z0-9+.\-]*://)[^\s/@]+:[^\s/@]+@", re.IGNORECASE) @@ -311,11 +311,11 @@ def handle_captcha_if_present(page, args) -> bool: site, so this path has never run against it; it is the family's broad detection kept as insurance. The static-HTML and live-page detectors are reconciled rather than short-circuited, because they can disagree about - the variant and the parameters for one are rejected for the other (§8). + the variant and the parameters for one are rejected for the other. This repo implements reCAPTCHA v2 and v3 here. Should the site ever render another kind, 2Captcha has task types for it and this client - would need the matching one added: a TODO, not a limit (§19). + would need the matching one added: a TODO, not a limit. """ try: html = page.content() @@ -340,7 +340,7 @@ def handle_captcha_if_present(page, args) -> bool: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, min_score=args.min_score) - except Exception as e: # noqa: BLE001 — a solver error is a warning (§8) + except Exception as e: # noqa: BLE001 — a solver error is a warning logger.error("Solving the captcha failed (%s) — continuing.", _mask_credentials(str(e))) return False @@ -370,6 +370,9 @@ def parse_args(argv=None): description="makemytrip.com hotel listing scraper (Playwright edition)") page_flow.add_arguments(p, engine="playwright") args = p.parse_args(argv) + # Before env_config fills anything in: a URL from .env is a default, + # and must not be mistaken for one the user typed. + args.typed_url = args.url env_config.apply(args) if args.category is not None: p.error("--category: this site has no category to pick; pass --city.") diff --git a/product_parser.py b/product_parser.py index 2e7b6dd..c257613 100644 --- a/product_parser.py +++ b/product_parser.py @@ -1,7 +1,7 @@ """ product_parser.py ----------------- -Everything this repo knows about makemytrip.com lives here (CLAUDE.md §1). +Everything this repo knows about makemytrip.com lives here. One mode, the front end's own listing API ----------------------------------------- @@ -39,7 +39,7 @@ No captcha was met anywhere on this site ---------------------------------------- Every served page was counted for captcha markers after removing what the -Scraping Browser's auto-solve extension injects (§24): 0 reCAPTCHA, 0 +Scraping Browser's auto-solve extension injects: 0 reCAPTCHA, 0 hCaptcha, 0 Turnstile, 0 sitekeys, 0 GeeTest. Akamai refuses by dropping the connection or by its 400-byte "Access Denied" page, and neither carries a widget. So the solver path is the family's broad DETECTION, kept as @@ -332,7 +332,7 @@ class Cursor: The listing is not addressed by page number. Each response names the last hotel it returned and a window marker, and the next request sends both back with a running count. So page N cannot be fetched before page - N-1 (§7, §18), and --concurrency is refused. + N-1, and --concurrency is refused. """ last_hotel_id: str = "" window: str = "" @@ -513,10 +513,10 @@ def query_from_url(url: str) -> Tuple[Optional[Query], Optional[str]]: # bytes, titled "Access Denied", with a reference id and a link to # errors.edgesuite.net. Raw bytes entity-escape the punctuation # (`errors.edgesuite.net`) while a browser's DOM spells it plainly, -# so markers are matched after html.unescape over a bounded prefix (§20). +# so markers are matched after html.unescape over a bounded prefix. AKAMAI_DENY_MARKERS = ("errors.edgesuite.net", "You don't have permission to access") # The reference id's own shape, whitespace-tolerant: Akamai has been seen to -# write it with two spaces on a sibling site (§24). +# write it with two spaces on a sibling site. _AKAMAI_REF_RE = re.compile(r"Reference\s+#\s*\d+\.[0-9a-f]+\.\d+\.[0-9a-f]+") # A page the site actually SERVED is built from its own assets. Counted @@ -525,7 +525,7 @@ def query_from_url(url: str) -> Tuple[Optional[Query], Optional[str]]: # Chromium's own error page reference it 0 times. So a served page is # positively identified rather than inferred from the absence of a marker, # which also catches Chromium's network-error page wearing the site's -# hostname (§18). +# hostname. OWN_ASSET_MARKER = "mmtcdn.com" OWN_ASSET_MIN = 3 @@ -535,7 +535,7 @@ def query_from_url(url: str) -> Tuple[Optional[Query], Optional[str]]: # Browser: 0 of each. The bare words `captcha`, `recaptcha` and # `cf-turnstile` are NOT markers: the Scraping Browser's auto-solve # extension injects 16 hunter scripts that say them 21, 2 and 1 times on -# every page it loads (§24), and none of them loads one of these paths. +# every page it loads, and none of them loads one of these paths. CAPTCHA_WIDGET_MARKERS = ("recaptcha/api.js", "recaptcha/enterprise.js", "recaptcha/api2/anchor", "hcaptcha.com/1/api.js", "challenges.cloudflare.com/turnstile") @@ -611,7 +611,7 @@ def detect_page_state(text: Optional[str], status: Optional[int] = None, throttled 429 unknown anything else - Signals are ordered by what they PROVE (§17): the API's own envelope + Signals are ordered by what they PROVE: the API's own envelope first, since no interstitial carries it; then Akamai's page; then the status; the asset count last. `waf_action` is accepted for the family's call signature and unused: this site's refusals carry no WAF header. @@ -629,7 +629,7 @@ def detect_page_state(text: Optional[str], status: Optional[int] = None, if own_assets < OWN_ASSET_MIN and any(m in (text or "") for m in CAPTCHA_WIDGET_MARKERS): # A widget on a page the site did not otherwise serve. On a SERVED # page a widget guards nothing this repo reads (a login form, say), - # and solving it would be paying for nothing (§8). + # and solving it would be paying for nothing. return "challenge" if status == 403: return "blocked" @@ -843,7 +843,7 @@ def parse_page(payload: Any, query: Query, page: int = 1) -> List[Hotel]: `position` counts the rows EMITTED on this page, not the slots in the payload, so a record the parser drops cannot shift every later - position (§24). + position. """ p = _as_dict(payload) currency = currency_of(p) diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index e972f41..4a26cb4 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -46,8 +46,7 @@ # At module level, deliberately, and not inside the launch path. The offline # suite guards `import puppeteer_scraper` behind try/except ImportError and # REPORTS the skip, and CI's engine-smoke job fails on any reported skip. -# That only works if importing this module actually requires the driver -# (CLAUDE.md §10). +# That only works if importing this module actually requires the driver. from pyppeteer import launch, connect from captcha_solver import (detect_recaptcha_v3, detect_recaptcha_in_page, @@ -64,11 +63,11 @@ # Every await in this file goes through the bridge below with a timeout, so a # hung remote call ends the operation instead of the run. pyppeteer provides -# no connect timeout of its own (§8). +# no connect timeout of its own. DEFAULT_OP_TIMEOUT = 120 CONNECT_TIMEOUT = 30 -# Chromium's own names for a proxy that could not be used (§8). +# Chromium's own names for a proxy that could not be used. _PROXY_ERROR_MARKERS = ( "ERR_PROXY_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED", @@ -82,7 +81,7 @@ # pyppeteer's dialect of the fetch() in playwright_scraper.FETCH_JS: the # arguments arrive POSITIONALLY rather than as one array. Same body, same -# return shape, same AbortController timeout (§8), same `credentials: +# return shape, same AbortController timeout, same `credentials: # "include"` and the same headers the site's own front end sends. FETCH_JS = """ async (url, method, body, headers, timeoutMs) => { @@ -172,7 +171,7 @@ def _cancel_and_stop(): self._thread.join(timeout=5) -# Every `scheme://user:pass@` in a string, however many times it occurs (§8). +# Every `scheme://user:pass@` in a string, however many times it occurs. _CREDENTIALS_IN_URL_RE = re.compile(r"([a-z][a-z0-9+.\-]*://)[^\s/@]+:[^\s/@]+@", re.IGNORECASE) @@ -183,7 +182,7 @@ def _mask_credentials(text: str) -> str: def _chrome_ua(version: str) -> str: - """A desktop-Chrome UA naming the browser's OWN real version (§8). + """A desktop-Chrome UA naming the browser's OWN real version. pyppeteer's `browser.version()` returns "HeadlessChrome/115.0.0.0".""" number = version.split("/")[-1] if "/" in version else version return (f"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " @@ -202,7 +201,7 @@ class _Ops: """One pyppeteer browser + page, exposed as page_flow's named operations. Same contract as playwright_scraper._Ops, including the rule that a - rotation means a genuinely FRESH browser (§8). + rotation means a genuinely FRESH browser. """ def __init__(self, bridge: _AsyncBridge, args, pool): @@ -270,7 +269,7 @@ def open(self): exit_url = self.pool.current # Credentials are answered over CDP (_authenticate_proxy), never # put on the command line: --proxy-server= is part of the browser's argv, - # readable by anything that can run `ps` (§8). + # readable by anything that can run `ps`. scrubbed, credentials = split_credentials(exit_url) launch_args.append(f"--proxy-server={scrubbed}") logger.info("Using proxy exit %s", mask(exit_url)) @@ -305,7 +304,7 @@ def _authenticate_proxy(self, username: str, password: str): Every request is paused and continued unchanged; only an auth challenge FROM THE PROXY is answered with its credentials, which - never leave this process's memory (§8). A site's own challenge gets + never leave this process's memory. A site's own challenge gets the browser's default answer. """ async def _enable(): @@ -338,7 +337,7 @@ def _enable_autosolve(self): engine enables it: if a captcha ever appears in front of the landing, the extension can clear it before the local solver gets a turn. It was missing from this engine in a sibling repo until a live run over - --cdp-endpoint showed the difference (CLAUDE.md §26).""" + --cdp-endpoint showed the difference.""" async def _enable(): # One coroutine for both calls: pyppeteer's CDPSession.send # returns a Future rather than a coroutine, and the bridge's @@ -466,7 +465,7 @@ def handle_captcha_if_present(ops, args) -> bool: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, min_score=args.min_score) - except Exception as e: # noqa: BLE001 — a solver error is a warning (§8) + except Exception as e: # noqa: BLE001 — a solver error is a warning logger.error("Solving the captcha failed (%s) — continuing.", _mask_credentials(str(e))) return False @@ -508,6 +507,9 @@ def parse_args(argv=None): "to open its DevTools socket. Point it at a Chrome or " "Chromium of your own — Playwright's, if installed.") args = p.parse_args(argv) + # Before env_config fills anything in: a URL from .env is a default, + # and must not be mistaken for one the user typed. + args.typed_url = args.url env_config.apply(args) if args.category is not None: p.error("--category: this site has no category to pick; pass --city.") diff --git a/selenium_scraper.py b/selenium_scraper.py index 8197d86..970a3d5 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -27,7 +27,7 @@ accepts no credentials. They are stripped and a warning says so. * **Selenium reports no HTTP status, and does not raise on a network error.** Chrome renders its own error page instead, carrying the site's - hostname as its title (§18). `goto` therefore reads the document for + hostname as its title. `goto` therefore reads the document for Chrome's error names and raises them, so a connection Akamai dropped is reported as the refusal it is (exit 3), exactly as in the twins. @@ -69,7 +69,7 @@ # script timeout cuts the call off with an exception. SCRIPT_TIMEOUT = page_flow.FETCH_TIMEOUT_MS // 1000 + 15 -# Chromium's own names for "the proxy is the problem, not the site" (§8). +# Chromium's own names for "the proxy is the problem, not the site". _PROXY_ERROR_MARKERS = ( "ERR_PROXY_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED", "ERR_PROXY_AUTH_UNSUPPORTED", "ERR_PROXY_AUTH_REQUESTED", @@ -81,7 +81,7 @@ # function BODY run by execute_async_script, with the arguments in # `arguments` and the result handed to the callback Selenium appends last. # Same request, same headers, same return shape, same AbortController -# timeout (§8). +# timeout. FETCH_JS = """ var done = arguments[arguments.length - 1]; var url = arguments[0], method = arguments[1], body = arguments[2]; @@ -105,7 +105,7 @@ """ -# Every `scheme://user:pass@` in a string, however many times it occurs (§8). +# Every `scheme://user:pass@` in a string, however many times it occurs. _CREDENTIALS_IN_URL_RE = re.compile(r"([a-z][a-z0-9+.\-]*://)[^\s/@]+:[^\s/@]+@", re.IGNORECASE) @@ -116,7 +116,7 @@ def _mask_credentials(text: str) -> str: def _chrome_ua(version: str) -> str: - """A desktop-Chrome UA naming the installed Chrome's own version (§8).""" + """A desktop-Chrome UA naming the installed Chrome's own version.""" return (f"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " f"(KHTML, like Gecko) Chrome/{version} Safari/537.36") @@ -151,7 +151,7 @@ class _Ops: """One Chrome driver, exposed as page_flow's named operations. Same contract as playwright_scraper._Ops, including the rule that a - rotation means a genuinely FRESH browser (§8). + rotation means a genuinely FRESH browser. """ def __init__(self, args, pool): @@ -168,7 +168,7 @@ def open(self): logger.info("Attaching to an existing browser at %s.", options.debugger_address) # No UA, no proxy, no fingerprint on this path: the remote browser - # brings its own (§8). + # brings its own. self.driver = webdriver.Chrome(options=options) self._apply_timeouts() return self @@ -208,7 +208,7 @@ def open(self): def _apply_timeouts(self): # Explicit, because a driver that stops answering otherwise hangs - # the run (§8). + # the run. self.driver.set_page_load_timeout(PAGE_LOAD_TIMEOUT) self.driver.set_script_timeout(SCRIPT_TIMEOUT) @@ -325,7 +325,7 @@ def handle_captcha_if_present(ops, args) -> bool: url = ops.driver.current_url html_challenge = detect_recaptcha_v3(html, url) # execute_script takes a function BODY with an explicit return, so the - # discovery arrow function is wrapped and invoked (§1). + # discovery arrow function is wrapped and invoked. runtime_challenge = detect_recaptcha_in_page( lambda js: ops.driver.execute_script("return (%s)();" % js), page_url=url) challenge = reconcile_detections(html_challenge, runtime_challenge) @@ -344,7 +344,7 @@ def handle_captcha_if_present(ops, args) -> bool: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, min_score=args.min_score) - except Exception as e: # noqa: BLE001 — a solver error is a warning (§8) + except Exception as e: # noqa: BLE001 — a solver error is a warning logger.error("Solving the captcha failed (%s) — continuing.", _mask_credentials(str(e))) return False @@ -373,6 +373,9 @@ def parse_args(argv=None): description="makemytrip.com hotel listing scraper (Selenium edition)") page_flow.add_arguments(p, engine="selenium") args = p.parse_args(argv) + # Before env_config fills anything in: a URL from .env is a default, + # and must not be mistaken for one the user typed. + args.typed_url = args.url env_config.apply(args) if args.category is not None: p.error("--category: this site has no category to pick; pass --city.") diff --git a/smoke_test.py b/smoke_test.py index 2cf3eec..2342126 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -128,7 +128,7 @@ def check_fixture_corpus_is_real_and_scrubbed(): # --------------------------------------------------------------------------- -# The parser, asserted on VALUES rather than on coverage (§10) +# The parser, asserted on VALUES rather than on coverage # --------------------------------------------------------------------------- def check_goa_listing_parses_to_the_captured_values(): @@ -166,7 +166,7 @@ def check_goa_listing_parses_to_the_captured_values(): def check_unrated_properties_carry_no_zero_rating(): - """§21: zero is not a rating. Taken from a real record with its rating + """Zero is not a rating. Taken from a real record with its rating counts zeroed, which is how the site writes an unrated property.""" import product_parser as P payload = copy.deepcopy(FIXTURES["search_goa_p1"]) @@ -182,7 +182,7 @@ def check_unrated_properties_carry_no_zero_rating(): for row in _rows(name, code="CTDUB" if name == "search_dubai" else "CTGOI")] check("no row anywhere carries a 0 rating or 0 stars", not any(row.rating == 0 or row.star_rating == 0 for row in rows)) - check("no row has an original price at or below its price (§4's canary)", + check("no row has an original price at or below its price", not any(row.original_price is not None and row.original_price <= row.price for row in rows)) @@ -257,7 +257,7 @@ def check_position_counts_emitted_rows(): hotels = payload["response"]["personalizedSections"][0]["hotels"] hotels.insert(1, {"name": "no id"}) # a record the parser drops rows = P.parse_page(json.dumps(payload), _q(), 1) - equal("a dropped record does not shift later positions (§24)", + equal("a dropped record does not shift later positions", [r.position for r in rows], [1, 2, 3, 4]) @@ -432,11 +432,11 @@ def check_page_states_on_real_captures(): equal("%s (HTTP %s) is %s" % (name, status, want), P.detect_page_state(fx(name), status), want) raw = fx("akamai_denied_www") - check("the raw refusal is entity-escaped (not vacuous, §20)", + check("the raw refusal is entity-escaped (not vacuous)", "errors.edgesuite.net" in raw) equal("...and the BROWSER's spelling of it is caught too", P.detect_bot_challenge(html_lib.unescape(raw)), "akamai") - check("Chrome's error page wears the site's hostname as its title (§18)", + check("Chrome's error page wears the site's hostname as its title", "www.makemytrip.com" in fx("chrome_error_page")) equal("an unknown 403 is blocked", P.detect_page_state("", 403), "blocked") equal("a 429 is throttled", P.detect_page_state("", 429), "throttled") @@ -456,7 +456,7 @@ def check_the_decoy_marker_is_exact(): def check_markers_do_not_match_a_page_the_scraping_browser_served(): - """§24: the Scraping Browser's extension injects captcha hunters into + """The Scraping Browser's extension injects captcha hunters into EVERY page. The marker set must score zero against that injection WITHOUT any strip, or the strip is load-bearing.""" import product_parser as P @@ -478,7 +478,7 @@ def check_a_widget_only_counts_on_a_page_the_site_did_not_serve(): loader = '' equal("a widget loader on a bare page is a challenge", P.detect_page_state("%s" % loader, 200), "challenge") - equal("the same loader on a SERVED page guards nothing (§8)", + equal("the same loader on a SERVED page guards nothing", P.detect_page_state(fx("landing_served") + loader, 200), "content") @@ -496,7 +496,7 @@ def check_state_policy(): check("blocked and reset: retried from a fresh browser, blocked, never solved", all(F.should_retry(s) and F.counts_as_blocked(s) and not F.should_solve(s) for s in ("blocked", "reset"))) - check("throttled: retried, NOT blocked (§24)", + check("throttled: retried, NOT blocked", F.should_retry("throttled") and not F.counts_as_blocked("throttled")) check("challenge: solved", F.should_solve("challenge")) equal("at most one solve per page", F.SOLVES_PER_PAGE, 1) @@ -509,14 +509,15 @@ def check_state_policy(): "net::ERR_CONNECTION_RESET", "net::ERR_SSL_PROTOCOL_ERROR"): check("%s is Akamai hanging up" % text.split("::")[1].split()[0], F.is_reset(text)) check("a timeout is not a reset", not F.is_reset("Timeout 60000ms exceeded.")) - check("a CDP 401 is explained as expired credentials", - "expired" in F.cdp_connect_hint("WebSocket error: 401 Unauthorized")) + hint = F.cdp_connect_hint("WebSocket error: 401 Unauthorized") + check("a CDP 401 is explained as refused credentials, without guessing " + "a lifetime", "refused" in hint and "about a day" not in hint, hint) check("...and a 500 as a held pid", "pid" in F.cdp_connect_hint("HTTP 500")) check("--concurrency 1 is not refused", F.concurrency_refusal(1) is None) def check_policy_constants_have_a_consumer(): - """§17: a policy constant nothing reads is the same defect as dead code.""" + """A policy constant nothing reads is the same defect as dead code.""" src = open(os.path.join(HERE, "page_flow.py"), encoding="utf-8").read() engines = "".join(open(os.path.join(HERE, m + ".py"), encoding="utf-8").read() for m in ENGINES) @@ -615,7 +616,7 @@ def check_the_shared_loop_end_to_end(): equal("...complete, stopped because the plan was done", (meta["status"], meta["stop_reason"]), ("complete", "completed")) equal("...rows in page order", [r["page"] for r in rows], [1, 1, 1, 1, 2, 2, 2]) - check("...page+position unique across the run (§18)", + check("...page+position unique across the run", len({(r["page"], r["position"]) for r in rows}) == len(rows)) equal("the browser landed ONCE for the run", ops.gotos, 1) sent = ops.bodies[1]["searchCriteria"] @@ -663,7 +664,7 @@ def check_the_shared_loop_end_to_end(): equal("...at the SAME exit (no relaunch)", ops.relaunches, 0) rc, meta, rows, ops = _run({1: [(429, ""), (200, fx("search_goa_p1"))]}, pages=1, retries=1) - equal("a throttle wait spends its OWN budget, not --retries (§24): " + equal("a throttle wait spends its OWN budget, not --retries: " "with --retries 1 the page still arrives", rc, 0) dup = {1: [(200, fx("search_goa_p1"))], 2: [(200, fx("search_goa_p1"))]} @@ -742,7 +743,7 @@ def check_a_remote_relaunch_is_a_new_connection(): def check_row_schema(): from output_writer import Hotel, Product, ROW_CLASS_BY_MODE, UNIQUE_BY_SKU_MODES names = [f.name for f in fields(Hotel)] - equal("the family prefix is byte-identical and in order (§9)", names[:8], + equal("the family prefix is byte-identical and in order", names[:8], ["source", "scraped_at", "url", "sku", "title", "price", "original_price", "currency"]) check("the run-describing tail is present", @@ -783,7 +784,7 @@ def check_exit_codes(): equal("3 blocked / 4 empty / 5 never obtained / 6 partial", (O.EXIT_BLOCKED, O.EXIT_NO_PRODUCTS, O.EXIT_FETCH_FAILED, O.EXIT_PARTIAL), (3, 4, 5, 6)) - check("end_of_listing is a COMPLETE stop reason (§24)", + check("end_of_listing is a COMPLETE stop reason", "end_of_listing" in O.COMPLETE_STOP_REASONS) check("api_rejected is NOT complete", "api_rejected" not in O.COMPLETE_STOP_REASONS) @@ -842,7 +843,7 @@ def check_sidecar_shape(): # --------------------------------------------------------------------------- -# The engines — the checks CLAUDE.md §17 says to steal +# The engines: signature binding, flag parity, undefined names # --------------------------------------------------------------------------- def check_engines_import_their_driver_at_module_level(): @@ -859,9 +860,9 @@ def check_engines_import_their_driver_at_module_level(): def check_shared_calls_bind_against_the_real_signature(): - """§17's check #1. Every call from an engine (and the Scraper API client, + """Every call from an engine (and the Scraper API client, diff_runs and page_flow itself) into a shared module is bound against the - callee's real signature. A name that does not exist FAILS (§22). A name + callee's real signature. A name that does not exist FAILS. A name bound in the calling file shadows a same-named module.""" import captcha_solver import output_writer @@ -950,8 +951,8 @@ def _engine_flags(module_name): return flags, bool(m) -# The family's flag contract (CLAUDE.md §9, including the five it omitted -# for months), plus this repo's own query flags. +# The flag contract every scraper in this family shares, plus this repo's +# own query flags. CONTRACT_FLAGS = { "--url", "--pages", "--category", "--format", "--out", "--delay", "--retries", "--retry-delay", "--concurrency", "--proxy", "--proxy-file", @@ -965,7 +966,7 @@ def _engine_flags(module_name): def check_engine_flag_sets(): - """§17's check #2: against the contract AND against each other, both + """Engine flags: against the contract AND against each other, both ways. The exception list IS the documentation.""" sets = {} for module in ENGINES: @@ -997,7 +998,7 @@ def check_banned_and_removed_flags(): def check_undefined_names_in_every_module(): - """§10: compileall proves a file PARSES, not that its names RESOLVE. + """compileall proves a file PARSES, not that its names RESOLVE. Kept COARSE (pooled bindings) so it under-reports rather than invents.""" import builtins for filename in sorted(f for f in os.listdir(HERE) if f.endswith(".py")): @@ -1077,7 +1078,8 @@ def check_dockerfile_copies_everything_the_entrypoint_imports(): def check_env_example_documents_exactly_what_the_loader_reads(): import env_config text = open(os.path.join(HERE, ".env.example"), encoding="utf-8").read() - documented = set(re.findall(r"^([A-Z][A-Z0-9_]+)=", text, re.M)) + # A variable shipped commented out (`# NAME=value`) is still documented. + documented = set(re.findall(r"^(?:# ?)?([A-Z][A-Z0-9_]+)=", text, re.M)) read = set(env_config.ENV_KEYS) equal("the example and the loader name the same variables", sorted(documented), sorted(read)) @@ -1091,6 +1093,9 @@ def check_a_copied_env_example_reads_as_UNSET(): import env_config text = open(os.path.join(HERE, ".env.example"), encoding="utf-8").read() values = dict(re.findall(r"^([A-Z][A-Z0-9_]+)=(.*)$", text, re.M)) + examples = dict(re.findall(r"^# ?([A-Z][A-Z0-9_]+)=(.*)$", text, re.M)) + check("MAKEMYTRIP_URL ships commented out, so a copied .env sets no URL", + "MAKEMYTRIP_URL" not in values and "MAKEMYTRIP_URL" in examples) credentials = {"TWOCAPTCHA_KEY", "MAKEMYTRIP_CDP_ENDPOINT", "MAKEMYTRIP_PROXY"} before = dict(os.environ) try: @@ -1108,7 +1113,7 @@ def check_a_copied_env_example_reads_as_UNSET(): os.environ.clear() os.environ.update(before) import product_parser as P - q, why = P.query_from_url(values["MAKEMYTRIP_URL"]) + q, why = P.query_from_url(examples.get("MAKEMYTRIP_URL", "")) check("the example's MAKEMYTRIP_URL is an address this repo reads", q is not None, why or "") @@ -1116,7 +1121,7 @@ def check_credential_scan_is_one_implementation_invoked_from_both(): script = os.path.join(HERE, ".github", "ci_checks.py") if not os.path.isdir(os.path.join(HERE, ".github")): # Inside the Docker image, which copies no .github at all. Triggered - # by the WHOLE directory being absent, never by one file in it (§22). + # by the WHOLE directory being absent, never by one file in it. skip("ci_checks", "no .github directory (the image)") return check("the credential scan exists as a script", os.path.exists(script)) @@ -1167,8 +1172,8 @@ def check_the_hex_exemption_is_one_context_only(): # Assembled from pieces, so this file can be scanned like every other rather -# than exempted (§22: the file most likely to acquire a stray phrase is the -# one a wholesale exemption never reads). +# than exempted (the file most likely to acquire a stray phrase is the one +# a wholesale exemption never reads). BANNED_WORDING = ( "cloud" + " browser", "anti" + "detect browser", "2scraper " + "Anti" + "detect Browser", "gate." + "2prx.com", "ANTI" + "DETECT_LOCAL_API", @@ -1176,7 +1181,7 @@ def check_the_hex_exemption_is_one_context_only(): def check_banned_wording(): - """§12, enforced by this test rather than by review.""" + """Banned product wording, enforced by this test rather than by review.""" scanned = 0 for root, dirs, files in os.walk(HERE): dirs[:] = [d for d in dirs if d not in (".git", "__pycache__", ".pytest_cache", @@ -1211,7 +1216,7 @@ def check_fingerprint_kwargs_are_ones_the_driver_accepts(): def check_engines_do_not_evaluate_a_string_in_the_browser(): - """§18: wait_for_function evaluates a string, which a CSP without + """wait_for_function evaluates a string, which a CSP without unsafe-eval kills. page.evaluate with a real function is fine.""" for module in ENGINES: tree = ast.parse(open(os.path.join(HERE, module + ".py"), encoding="utf-8").read()) @@ -1346,7 +1351,7 @@ def post(url, **kw): def check_captcha_capability_claims_match_the_code(): - """§19: the most expensive bug this family can ship is a SENTENCE.""" + """The most expensive bug this family can ship is a SENTENCE.""" readme = open(os.path.join(HERE, "README.md"), encoding="utf-8").read() # Whitespace-normalised: Markdown wraps a sentence wherever it likes, # and a phrase split across two lines is still the phrase. @@ -1360,7 +1365,7 @@ def check_captcha_capability_claims_match_the_code(): def check_readme_numbers_are_not_stale(): - """§17's check #4: a count claimed in the README is the code's.""" + """A count claimed in the README is the code's.""" readme = open(os.path.join(HERE, "README.md"), encoding="utf-8").read() from output_writer import Hotel import product_parser as P @@ -1445,6 +1450,298 @@ def check_retries_is_validated_in_every_entry_point(): check("--retries is validated in every entry point, so 0 is a usage error", not problems, "; ".join(problems)) +# --------------------------------------------------------------------------- +# Output files: atomic replacement, file modes, formula-safe CSV +# --------------------------------------------------------------------------- + +def check_a_failed_write_keeps_the_previous_good_files(): + """A failure halfway through a write must leave the previous file whole. + `open(path, "w")` truncates first, and a run that died inside json.dump + used to replace yesterday's good output with a 10-byte fragment.""" + import output_writer as O + rows = _rows("search_goa_p1") + with tempfile.TemporaryDirectory() as tmp: + prefix = os.path.join(tmp, "out") + O.write_json(rows, prefix + ".json") + O.write_csv(rows, prefix + ".csv") + O.write_run_meta(prefix, {"status": "complete"}) + before = {ext: open(prefix + ext, "rb").read() + for ext in (".json", ".csv", ".meta.json")} + real_dump, real_writer = O.json.dump, O.csv.DictWriter + + def broken_dump(obj, f, **kw): + f.write('[{"sku": ') + raise OSError("disk full (planted)") + + class BrokenWriter(real_writer): + def writerow(self, row): + raise OSError("disk full (planted)") + + O.json.dump, O.csv.DictWriter = broken_dump, BrokenWriter + raised = [] + try: + for name, call in (("json", lambda: O.write_json(rows, prefix + ".json")), + ("csv", lambda: O.write_csv(rows, prefix + ".csv")), + ("meta", lambda: O.write_run_meta(prefix, {"status": "x"}))): + try: + call() + except OSError: + raised.append(name) + finally: + O.json.dump, O.csv.DictWriter = real_dump, real_writer + equal("each planted failure surfaced (not vacuous)", raised, ["json", "csv", "meta"]) + for ext, data in before.items(): + equal("a failed write left the previous %s byte-identical" % ext, + open(prefix + ext, "rb").read(), data) + equal("...and no temporary file behind", + sorted(os.listdir(tmp)), ["out.csv", "out.json", "out.meta.json"]) + + +def check_output_files_get_the_mode_open_would_give(): + """A temp file is created 0600 and os.replace keeps that, so a naive + atomic writer makes every output owner-only. A new file gets + 0o666 & ~umask; an existing one keeps its own mode.""" + import output_writer as O + rows = _rows("search_goa_p1") + old = os.umask(0o022) + try: + for umask in (0o022, 0o002, 0o077): + os.umask(umask) + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "out.json") + O.write_json(rows, path) + equal("a new file under umask %03o is %03o" % (umask, 0o666 & ~umask), + oct(os.stat(path).st_mode & 0o777), oct(0o666 & ~umask)) + os.chmod(path, 0o640) + O.write_json(rows, path) + equal("...and a rewritten one keeps its own mode (umask %03o)" % umask, + oct(os.stat(path).st_mode & 0o777), oct(0o640)) + finally: + os.umask(old) + + +def check_csv_cells_that_a_spreadsheet_would_execute_are_escaped(): + import output_writer as O + from output_writer import Hotel + row = Hotel(sku="1", title="=HYPERLINK(\"http://x\")", locality="+cmd", + categories=["@SUM(1)", "ok"], coupon_code="\tTAB", + latitude=-15.5, longitude=73.8, price=4000.0) + with tempfile.TemporaryDirectory() as tmp: + prefix = os.path.join(tmp, "out") + rc = O.finish_run([row], prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", + mode="hotels", extra={"csv_cells_escaped": "site fact wins"}) + equal("a run with formula-shaped cells still exits 0", rc, 0) + cells = dict(zip(*list(csv.reader(open(prefix + ".csv", encoding="utf-8"))))) + equal("a leading = is escaped", cells["title"], "'=HYPERLINK(\"http://x\")") + equal("a leading + is escaped", cells["locality"], "'+cmd") + equal("a joined list is escaped AFTER joining", cells["categories"], "'@SUM(1) | ok") + equal("a leading tab is escaped", cells["coupon_code"], "'\tTAB") + equal("a negative NUMBER is left a number", cells["latitude"], "-15.5") + loaded = json.load(open(prefix + ".json", encoding="utf-8"))[0] + equal("JSON keeps the site's bytes", loaded["title"], "=HYPERLINK(\"http://x\")") + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("the caller's extra wins a collision with the housekeeping count", + meta["csv_cells_escaped"], "site fact wins") + O.finish_run([row], prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", mode="hotels") + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + equal("the sidecar counts the escaped cells", meta.get("csv_cells_escaped"), 4) + + +# --------------------------------------------------------------------------- +# Paid solves: one allowance per page, whatever lands again +# --------------------------------------------------------------------------- + +class _SolvingOps(_FakeOps): + """A landing that stays a captcha, and a solver that always 'succeeds'.""" + + def solve_captcha(self): + self.solves += 1 + if self.solves > 20: + # A loop with no allowance at all would never end; fail loudly. + raise RuntimeError("the solver was asked %d times for one run" + % self.solves) + return True + + +CAPTCHA_LANDING = ('') + + +def _solve_count(query=None, **extra): + import page_flow + with tempfile.TemporaryDirectory() as tmp: + args = types.SimpleNamespace( + pages=1, retries=3, retry_delay=0, delay=0, proxy_block_retries=2, + out=os.path.join(tmp, "out"), format="json", allow_empty=False, + dump_html=None, url=None, cdp_endpoint=None, concurrency=1) + for k, v in extra.items(): + setattr(args, k, v) + ops = _SolvingOps({}, landing=CAPTCHA_LANDING) + rc = page_flow.run_pages(lambda: ops, lambda o: None, args, None, + query or _q()) + return rc, ops.solves + + +def check_one_page_buys_at_most_one_solve(): + import page_flow + from product_parser import Query + equal("the planted landing really classifies as a captcha (not vacuous)", + page_flow.classify(CAPTCHA_LANDING, 200, page_flow.ORIGIN_URL), "challenge") + rc, solves = _solve_count() + equal("a page whose captcha survives the token: exit 3", rc, 3) + equal("...and ONE solve bought across every relaunch, not one per landing", + solves, page_flow.SOLVES_PER_PAGE) + q = Query(city="Goa", checkin=date(2026, 10, 15), checkout=date(2026, 10, 16)) + rc, solves = _solve_count(query=q) + equal("a run by city NAME: the lookup spends page 1's allowance", solves, + page_flow.SOLVES_PER_PAGE) + + +# --------------------------------------------------------------------------- +# A served page whose properties the parser loses is a failure +# --------------------------------------------------------------------------- + +def _renamed_ids(name): + """A real page with every hotel's `id` renamed: the site's shape moving.""" + data = copy.deepcopy(FIXTURES[name]) + for section in data["response"]["personalizedSections"]: + for hotel in section.get("hotels") or []: + hotel["hotelIdRenamed"] = hotel.pop("id") + return json.dumps(data) + + +def check_a_served_page_that_parses_to_nothing_is_a_parser_failure(): + import product_parser as P + broken = _renamed_ids("search_goa_p1") + check("the broken page still states its properties (not vacuous)", + P.count_rows(broken) > 0 and P.detect_page_state(broken, 200) == "content") + equal("...and parses to nothing", len(P.parse_page(broken, _q(), 1)), 0) + rc, meta, rows, ops = _run({1: [(200, broken)]}, pages=1) + equal("page 1 served and lost: exit 5, not 4 (an empty listing)", rc, 5) + rc, meta, rows, ops = _run({1: [(200, broken)]}, pages=3) + equal("...also at --pages 3", rc, 5) + rc, meta, rows, ops = _run({1: [(200, fx("search_goa_p1"))], + 2: [(200, _renamed_ids("search_goa_p2"))]}, pages=3) + equal("page 2 lost after a good page 1: exit 6, not a complete run", rc, 6) + equal("...partial, named, with the page listed", + (meta["status"], meta["stop_reason"], meta["pages_failed"]), + ("partial", "parser_found_nothing", [2])) + rc, meta, rows, ops = _run({1: [(200, fx("search_no_hotels"))]}) + equal("'No Hotels Found' is still exit 4", rc, 4) + + +# --------------------------------------------------------------------------- +# Flags: .env defaults, and values that cannot be meant +# --------------------------------------------------------------------------- + +def _parse_cli(argv, env=None): + """add_arguments -> parse_args -> typed_url -> env_config.apply -> + build_query, exactly as every engine's parse_args does it.""" + import env_config + import page_flow + p = argparse.ArgumentParser() + page_flow.add_arguments(p) + saved = {k: os.environ.get(k) for k in env_config.ENV_KEYS} + try: + for k in env_config.ENV_KEYS: + os.environ.pop(k, None) + os.environ.update(env or {}) + try: + args = p.parse_args(argv) + args.typed_url = args.url + env_config.apply(args, quiet=True) + return page_flow.build_query(args, p.error), None + except SystemExit as e: + return None, e.code + finally: + for k, v in saved.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + + +def check_an_env_url_does_not_veto_typed_query_flags(): + import io + import contextlib + text = open(os.path.join(HERE, ".env.example"), encoding="utf-8").read() + found = re.search(r"^# ?MAKEMYTRIP_URL=(.*)$", text, re.M) + check(".env.example carries a MAKEMYTRIP_URL example", found is not None) + example = found.group(1).strip() if found else ( + "https://www.makemytrip.com/hotels/hotel-listing/?checkin=11102026" + "&checkout=11112026&city=CTGOI&locusId=CTGOI&locusType=city" + "&country=IN&roomStayQualifier=2e0e") + env = {"MAKEMYTRIP_URL": example} + with contextlib.redirect_stderr(io.StringIO()): + q, code = _parse_cli(["--city", "Goa"], env) + check("MAKEMYTRIP_URL in .env, then the README's `--city Goa`: a query", + q is not None and q.city == "Goa", "exit %r" % code) + q, code = _parse_cli([], env) + check("...and with no flags the .env URL is the default", + q is not None and q.city_code == "CTGOI", "exit %r" % code) + q, code = _parse_cli(["--url", example, "--city", "Goa"]) + equal("a TYPED --url with --city is still refused", code, 2) + + +def check_negative_counts_and_delays_are_usage_errors(): + import io + import contextlib + for argv in (["--delay", "-1"], ["--retry-delay", "-0.5"], + ["--proxy-block-retries", "-1"], ["--retries", "0"], + ["--pages", "0"]): + err = io.StringIO() + with contextlib.redirect_stderr(err): + q, code = _parse_cli(["--city", "CTGOI"] + argv) + check("%s %s is exit 2, naming the flag" % tuple(argv), + code == 2 and argv[0] in err.getvalue(), err.getvalue()[-160:]) + q, code = _parse_cli(["--city", "CTGOI", "--delay", "0", "--proxy-block-retries", "0"]) + check("0 is still a legal delay and block-retry count", q is not None, "exit %r" % code) + + +# --------------------------------------------------------------------------- +# Shipped files cite nothing a reader cannot open +# --------------------------------------------------------------------------- + +# Assembled from pieces so this file does not match itself. +PRIVATE_CITATIONS = re.compile( + "|".join([re.escape("CLAUDE" + ".md"), re.escape("claude" + "_2scraper"), + re.escape("2scraper" + "_repo_rules"), "§" + r"\s*\d", + r"\b[Pp]lay" + r"book\b", "family" + " note"])) + + +def check_no_private_citations_in_shipped_files(): + """Comments, docs and the CHANGELOG state their rules in this repo's own + words. A pointer into notes the public cannot open reads as a reference + nobody can follow.""" + import subprocess + try: + listed = subprocess.run(["git", "ls-files"], cwd=HERE, capture_output=True, + text=True, check=True).stdout.split() + except (OSError, subprocess.CalledProcessError): + listed = [os.path.relpath(os.path.join(d, f), HERE) + for d, _, fs in os.walk(HERE) if "/." not in d + "/" + for f in fs] + hits, scanned = [], 0 + for rel in listed: + path = os.path.join(HERE, rel) + if not os.path.isfile(path) or rel.endswith((".png", ".jpg", ".gz")): + continue + try: + text = open(path, encoding="utf-8").read() + except UnicodeDecodeError: + continue + scanned += 1 + for n, line in enumerate(text.splitlines(), 1): + if PRIVATE_CITATIONS.search(line.replace("CLAUDE_CODE", "")): + hits.append("%s:%d" % (rel, n)) + check("the citation scan read the repo (not vacuous)", scanned >= 20, str(scanned)) + check("no shipped file cites private notes", not hits, ", ".join(hits[:10])) + + CHECKS = [v for k, v in sorted(globals().items()) if k.startswith("check_")]