From 497b8c2140ea799c62494afcb2d8425fb00638e0 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:51:18 +0000 Subject: [PATCH] Report markup drift as a parser failure, and let typed query flags win over a .env URL - A served page that still says it has rows (rows in its markup, a More link, a comment count, a profile page) and parses to none is now a failed page, stop_reason parser_found_nothing: exit 5 with nothing gathered, exit 6 after earlier pages. It used to read as an empty answer (exit 4) or as the end of the list (complete, exit 0, with part of the data). - HACKERNEWS_URL is commented out in .env.example, and a URL from the environment no longer refuses typed --feed/--item/--user/--mode in any entry point. - Key masking covers clientKey: / "key": shapes in errors and x-debug. - Comments, docstrings and the canary state their reasons in their own words; a suite check rejects references to unpublished documents. - Docs: credential lifetime claim retracted; Selenium's 403 -> exit 5 noted. - CI matrix: 3.9 and 3.14. Co-Authored-By: Claude Opus 5.5 --- .env.example | 10 +- .github/ci_checks.py | 8 +- .github/workflows/canary.yml | 8 +- .github/workflows/tests.yml | 2 +- .gitignore | 6 +- CHANGELOG.md | 28 ++++++ README.md | 12 +-- TROUBLESHOOTING.md | 15 +-- diff_runs.py | 3 +- env_config.py | 6 +- fingerprint_client.py | 14 ++- make_fixtures.py | 4 +- output_writer.py | 13 ++- page_flow.py | 119 +++++++++++++++------- playwright_scraper.py | 19 ++-- product_parser.py | 30 +++--- puppeteer_scraper.py | 22 +++-- scraper_api_client.py | 10 +- selenium_scraper.py | 15 +-- smoke_test.py | 185 ++++++++++++++++++++++++++++------- 20 files changed, 378 insertions(+), 151 deletions(-) diff --git a/.env.example b/.env.example index 0926deb..c3ac4f5 100644 --- a/.env.example +++ b/.env.example @@ -54,8 +54,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 expire (a 401 on connect means fetch fresh +# ones), so never paste a working one anywhere that is committed. HACKERNEWS_CDP_ENDPOINT=ws://{login}-zone-scraping_browser-country-de-pid-{profileId}:{password}@cb.2captcha.com:9222 # --- Proxy ---------------------------------------------------------------- @@ -80,8 +80,10 @@ HACKERNEWS_PROXY=http://{user}:{password}@eu.proxy.2captcha.com:2334 # https://news.ycombinator.com/item?id=49901736 a comment tree # https://news.ycombinator.com/user?id=dang a profile # -# The flags (--feed, --day, --item, --user, ...) do the same without a URL. -HACKERNEWS_URL=https://news.ycombinator.com/news +# The flags (--feed, --day, --item, --user, ...) do the same without a URL, +# and a flag you type wins over this default. Commented out so a copied +# example changes nothing; uncomment it to set a default target. +# HACKERNEWS_URL=https://news.ycombinator.com/news # Deliberately NOT here: an output prefix. `--out` already carries a # non-empty default, so the loader, which only fills UNSET values, would diff --git a/.github/ci_checks.py b/.github/ci_checks.py index bf38c07..391a4bb 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -88,7 +88,7 @@ # A 2captcha API key is a 32-character hex string. HEX32 = re.compile(r"\b[0-9a-f]{32}\b") -# EMPTY, and that is the stricter arrangement rather than an omission (§24). +# EMPTY, and that is the stricter arrangement rather than an omission. # # Nothing news.ycombinator.com publishes is a 32-hex string: its ids are # decimal integers (`item?id=49901736`), its usernames are words, and the two @@ -210,8 +210,8 @@ def _without_site_ids(line): # # A suffix allowlist is a scanner that cannot see the thing most likely to # leak. `--dump-html live_results` writes `live_results.page1` — no suffix -# the list knew — and a merge committed two of them, 1.5 MB each, carrying 26 -# per-seller UUIDs, 12 copies of the site's Algolia key and its Sentry keys. +# the list knew — and in a sibling repo a merge committed two of them, 1.5 MB +# each, carrying that site's front-end keys. # The check that exists to stop exactly that ran, passed, and never opened # them. # @@ -273,7 +273,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 12c0c4f..e6a889b 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -3,7 +3,7 @@ # # No secrets are needed, and that is the point of running it: the README's # central claim is "no key, no proxy, no account", and an ungated scheduled -# canary is that sentence under test every morning (CLAUDE.md §21, §24). Every +# canary is that sentence under test every morning. Every # page kind answered a datacentre address normally when this repo was written # (2026-09-30); a GitHub runner is a datacentre address too. So this is NOT # gated on a secret: a canary that CAN pass without a credential must not be @@ -83,7 +83,7 @@ jobs: playwright install --with-deps chromium # >= 3 pages on the listings, because with one page pagination is never - # exercised (§11). + # exercised. - name: Run id: run run: | @@ -100,8 +100,8 @@ jobs: case "$code" in 0) ;; 3) echo "::error::Blocked (HTTP 403). HN has never refused an address this repo used, so this is a new state: the runner's address was refused. Set the HACKERNEWS_PROXY secret (see the header)."; exit 1 ;; - 4) echo "::error::Zero rows from a page that is never empty. Either the markup has moved (see canary_dump.html*) or the site answered with its own 'nothing there' sentence."; exit 1 ;; - 5) echo "::error::The page never arrived. This job uses no credential, so this is not an expired key: a timeout or a throttle that did not lift. Check the log."; exit 1 ;; + 4) echo "::error::Zero rows from a page that is never empty: the site answered with its own 'nothing there' sentence, or a page past the end (see canary_dump.html*)."; exit 1 ;; + 5) echo "::error::No rows were obtained. Either the page never arrived (this job uses no credential, so not an expired key: a timeout or a throttle that did not lift) or it arrived and parsed to none (stop_reason parser_found_nothing: the markup moved, see canary_page*_debug.html). Check the log."; exit 1 ;; 6) echo "::error::Partial run: a page failed after others succeeded. The sidecar names it."; exit 1 ;; *) echo "::error::Unexpected exit $code."; exit 1 ;; esac diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 22f9f4b..6a9a1fd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,7 +17,7 @@ jobs: matrix: # 3.9 is the floor the README claims. Claiming it without testing it is # how a walrus operator or a `X | None` annotation ships and breaks it. - python-version: ["3.9", "3.12"] + python-version: ["3.9", "3.14"] steps: - uses: actions/checkout@v4 diff --git a/.gitignore b/.gitignore index c072ca9..48880b1 100644 --- a/.gitignore +++ b/.gitignore @@ -23,7 +23,7 @@ hackernews_rows_*.* 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 @@ -101,7 +101,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, which is the lesson: when you fix one, +# check the siblings. .env* !.env.example diff --git a/CHANGELOG.md b/CHANGELOG.md index 25b48d6..ecbee4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,36 @@ said so at the top of its release notes. ## [Unreleased] +> **A page that says it has rows and parses to none now exits 5 (or 6), not +> 4 or 0.** When the markup moves, a page can still carry its own More link or +> a comment count while the parser reads nothing from it. That used to be +> reported as an empty answer: exit 4 on page 1, and on a later page the end +> of the list, so a three-page run ended `complete`, exit 0, with a third of +> the data. Such a page is now a failed page with `stop_reason: +> parser_found_nothing`: exit 5 when nothing was gathered, exit 6 (partial) +> when earlier pages had rows. The site's own "nothing here" sentences and a +> real page past the end of a list are still exit 4 / end of list. + ### Fixed +- **`cp .env.example .env` no longer breaks every query flag.** The example + set `HACKERNEWS_URL`, and a URL from the environment was treated as typed, + so `--feed show`, `--item 1` and `--mode user --user dang` were all refused + with exit 2. The example now ships it commented out, and in every entry + point a URL from the environment is a default: a typed query flag or + `--mode` wins over it. A typed `--url` with a query flag is still refused. +- **More key shapes are masked in logged errors and the Scraper API's + `x-debug` header**: `clientKey: …` and JSON `"key": "…"` as well as + `key=…`. +- Documentation: a Scraping Browser profile's credentials expire, but not on + a fixed one-day schedule; a 401 on connect is the signal to fetch fresh + ones. The exit-code table now says Selenium, which sees no HTTP status, + reports an unrecognised 403 page as exit 5 rather than 3. +- Comments and docstrings state their reasons in their own words instead of + pointing at an unpublished document, and a check keeps it that way. Stale + comments describing other sites' columns were reworded. +- CI tests the newest Python (3.14) beside the oldest (3.9). + - **`--retries 0` sent no request and reported an unreadable page.** The flag counts attempts and the loop is `while attempt < retries`, so zero attempts fetched nothing and the run exited 5 with an empty debug file, which reads diff --git a/README.md b/README.md index 5ec1d91..8d8c7ef 100644 --- a/README.md +++ b/README.md @@ -217,10 +217,10 @@ long link. `status` is `ok`, `flagged`, `dead` or `deleted`. | 0 | ok | | 1 | crash | | 2 | bad usage (a flag combination that would scrape something nobody asked for) | -| 3 | blocked: HTTP 403 (not observed here) | -| 4 | zero rows: nothing there, a list past its end, or a served page that parsed to none (that last one is a bug in this repo and is logged as one) | -| 5 | the content was never obtained: a timeout, a dead proxy, a throttle that never lifted, a remote API error | -| 6 | partial: some pages arrived and then the run stopped; `pages_failed` in the sidecar names them | +| 3 | blocked: HTTP 403 (not observed here). Selenium sees no HTTP status, so a 403 whose body is not HN's own page reaches it as an unreadable page and ends exit 5 instead | +| 4 | zero rows: nothing there, or a list that starts past its end | +| 5 | the content was never obtained: a timeout, a dead proxy, a throttle that never lifted, a remote API error, or a served page that says it has rows (a More link, a comment count) and parsed to none (`stop_reason: parser_found_nothing`, a bug in this repo) | +| 6 | partial: some pages arrived and then the run stopped, including on a later page that parsed to none; `pages_failed` in the sidecar names them | `.meta.json` records `status` (`complete`, `partial`, `failed`), `stop_reason`, `pages_requested`, `pages_completed`, `pages_failed`, the query, @@ -265,8 +265,8 @@ secret in `argv` is readable by anything that can run `ps`). Copy Precedence: explicit flag, then exported variable, then `.env`, then default. `python3 env_config.py` prints what was picked up without printing a secret. -A Scraping Browser profile's credentials last about a day, so none is written -down anywhere in this repo; the endpoint's shape is in `.env.example`. +A Scraping Browser profile's credentials expire (a 401 on connect means it is +time to fetch fresh ones), so none is written down anywhere in this repo; the endpoint's shape is in `.env.example`. ## Tests, CI and the container diff --git a/TROUBLESHOOTING.md b/TROUBLESHOOTING.md index dafd460..b60e800 100644 --- a/TROUBLESHOOTING.md +++ b/TROUBLESHOOTING.md @@ -24,7 +24,7 @@ report with the sidecar's `stop_reason` is welcome. ## Exit 4 — zero rows -One of three things, and only the last is a bug: +One of two things: * **There is nothing there.** `No such item.`, `No such user.`, `We don't have that data yet.` and `HN didn't exist yet.` are plain-text @@ -34,15 +34,18 @@ One of three things, and only the last is a bug: and `?p=27` is empty. A run that starts inside the list and reaches the end is `complete`, exit 0; exit 4 is only for a FIRST page that is already empty, such as `--url ".../news?p=30"`. -* **A served page parsed to no rows.** That is a parser bug or a markup - change. Re-run with `--dump-html page.html` (it writes the exact bytes the - parser saw, on success too) and open a "Site changed" issue. - Nothing is written on exit 4, so an earlier good file is left alone; `--allow-empty` writes the empty file. ## Exit 5 — the content was never obtained +* **A served page parsed to no rows** (`stop_reason: parser_found_nothing`). + The page still says it has content (rows in its markup, a More link, a + comment count above zero, a profile page) and the parser read none of it: + a parser bug or a markup change, not an empty list. The page is saved as + `_pageN_debug.html`; re-run with `--dump-html page.html` if you want + the bytes on success too, and open a "Site changed" issue. On a later page + of a run that already has rows, the same thing is exit 6. * **A timeout or a dead proxy.** The log names which. A dead proxy wants a different exit; a timeout wants another try at the same one. * **A throttle that did not lift.** HN is documented to throttle by address @@ -50,7 +53,7 @@ Nothing is written on exit 4, so an earlier good file is left alone; spaced 4 s apart drew none, one measurement). Raise `--delay`, lower `--concurrency`, or spread the run over `--proxy-file`. * **HTTP 401 from a Scraping Browser endpoint.** The profile's credentials - have expired; they last about a day. Get fresh ones. + have expired. Get fresh ones. * **HTTP 500 `profile_locked` from a Scraping Browser endpoint.** The profile has one live connection and something still holds it, including a run that disconnected a second ago. Wait, or use another `pid`. diff --git a/diff_runs.py b/diff_runs.py index a804a51..0d71816 100644 --- a/diff_runs.py +++ b/diff_runs.py @@ -62,8 +62,7 @@ # ARE the story's figures.) `depth` and `parent_id` never change for a # comment, and stay tracked so a re-parenting would show. UNTRACKED_FIELDS = frozenset({ - "source", "scraped_at", "sku", "page", "position", "rank", "mode", "sort", - "data_source", "url", + "source", "scraped_at", "sku", "page", "position", "rank", "url", }) diff --git a/env_config.py b/env_config.py index 9dc0587..53f4f80 100644 --- a/env_config.py +++ b/env_config.py @@ -67,8 +67,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 the placeholder rule +# 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 @@ -204,7 +204,7 @@ def _dests_given_on_argv(dests, argv=None): first-class, as they are here: the site needs neither, so switching between them is routine rather than exceptional. - Scanning argv is the smallest fix that keeps §3's precedence intact -- + Scanning argv is the smallest fix that keeps the documented precedence intact -- explicit flag, then exported variable, then `.env`, then default -- with "explicit" now meaning what the caller typed rather than what happens to be truthy. diff --git a/fingerprint_client.py b/fingerprint_client.py index eabbc22..1d3003f 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -88,12 +88,16 @@ # Everything raised or logged from this module goes through here first. The # endpoint and the status survive, because which call failed is the useful # half and is not the secret. +# `key=value` in a URL, and `clientKey: value` / `"key": "value"` in a +# header or a JSON body: the separator and any quotes are kept, the value is +# masked. _KEY_IN_TEXT_RE = re.compile( - r"((?:client)?key|token|api[_-]?key)=([^&\s'\"]{6,})", re.IGNORECASE) + r"((?:client)?key|token|api[_-]?key)(\"?\s*[:=]\s*\"?)([^&\s'\",}]{6,})", + re.IGNORECASE) def _redact(text) -> str: - return _KEY_IN_TEXT_RE.sub(r"\1=***", str(text)) + return _KEY_IN_TEXT_RE.sub(r"\1\2***", str(text)) API_BASE = "https://api.2captcha.com" RANDOM_URL = f"{API_BASE}/fingerprint/random" @@ -247,8 +251,8 @@ def playwright_context_kwargs(fp: dict) -> dict: # option and which was previously dropped on the floor. Measured # 2026-09-11: a fingerprint stating `deviceScaleFactor: 1.25` produced a # browser reporting `window.devicePixelRatio === 1`, so the identity - # contradicted itself on an axis any fingerprinter reads for free. Same - # shape as the three defects §16 lists -- a key the API returns that + # contradicted itself on an axis any fingerprinter reads for free. A + # familiar shape -- a key the API returns that # nothing applies -- and found the same way, by comparing the live # browser against the fingerprint rather than by reading the code. scale = (fp.get("screen") or {}).get("deviceScaleFactor") @@ -350,7 +354,7 @@ def main() -> int: # loader. It used to read `os.environ` alone, which meant a key put in # `.env` exactly as README and .env.example instruct worked for every # engine and failed here with "No API key" — a documented mechanism not - # applied on one path, which is the shape of half the defects §16 lists. + # applied on one path, a familiar shape of defect. # `.env` has to be LOADED before it can be read: `env_value` looks at # os.environ, which `load_env` is what fills from the file. Calling it # here rather than relying on an engine having called it is the whole diff --git a/make_fixtures.py b/make_fixtures.py index e961014..fafc1df 100644 --- a/make_fixtures.py +++ b/make_fixtures.py @@ -7,7 +7,7 @@ `captures/` holds raw pages fetched from news.ycombinator.com on 2026-09-30 (plain `curl`, and one browser-serialised copy per page kind, see --render). It is NOT committed: a raw page dump carries the session that fetched it and -strangers' words (CLAUDE.md §10). This script turns each capture into a +strangers' words. This script turns each capture into a smaller, scrubbed fixture and PROVES the trimmed copy parses the way the original did, so the suite tests the markup of the real site and not a hand-written imitation of it. @@ -249,7 +249,7 @@ def build(captures, render): # Chromium's own network-error page, produced for real: a proxy # that refuses the connection. It is NOT the site's page, and # nothing in it names a vendor, so only "is it built out of the - # site's own markup" tells it from a served page (§18). + # site's own markup" tells it from a served page. browser = pw.chromium.launch(proxy={"server": "http://127.0.0.1:9"}) page = browser.new_page() try: diff --git a/output_writer.py b/output_writer.py index 61a8f27..5c3670c 100644 --- a/output_writer.py +++ b/output_writer.py @@ -12,7 +12,7 @@ --mode user one profile -> User Three kinds of thing, so three dataclasses (the family allows a second one -where a repo genuinely reads more than one kind, §9). What holds them +where a repo genuinely reads more than one kind). What holds them together is the id: every class carries the site's own id in `sku`, which is the HN item id for a Story and a Comment (they are one id space: a comment's `story_id` and `parent_id` are Story ids or Comment ids), and the username @@ -25,7 +25,7 @@ User `sku` -- one row What a cross-mode `diff_runs.py` would mean is nothing: two runs of DIFFERENT -modes hold different kinds of rows, and it refuses (§9). +modes hold different kinds of rows, and it refuses. Something specific to a site whose text is written by its users: **every string in a Comment and a User was typed by a stranger.** That is why the @@ -67,7 +67,7 @@ class Story: a story states. Columns that are null BY DESIGN on some rows, so a reader is not left to - guess (§9): `author`, `points` and `comments` are null on a job posting, + guess: `author`, `points` and `comments` are null on a job posting, which has none of the three -- a fact about the site, and the only way a row's `kind` is `job`. `rank` is the number the site prints beside the row, which continues across pages (page 2 starts at 31) and is null only @@ -156,7 +156,7 @@ class User: # Kept under the family's name: the workflow files, a sibling's diff tool and # a user's script may say `Product`, and a rename that breaks them all for # the sake of tidiness is a poor trade. A Story is the row a listing run is -# about, which is what `Product` means in the shop scrapers. +# about, which is the role `Product` plays across the family. Product = Story ROW_CLASS_BY_MODE = {"listing": Story, "item": Comment, "user": User} @@ -281,9 +281,8 @@ def _atomic(path: str, newline: Optional[str] = None): def _csv_value(v: Any) -> Any: if isinstance(v, (list, tuple)): return LIST_CSV_SEPARATOR.join(str(x) for x in v) - # `tags` and each entry of `variants` are mappings, and a Python repr - # of one is neither readable in a spreadsheet nor parseable by anything - # but Python. + # A mapping cell (no column here holds one today): a Python repr is + # neither readable in a spreadsheet nor parseable by anything but Python. # Compact JSON is both, and round-trips through json.loads. if isinstance(v, dict): return json.dumps(v, ensure_ascii=False, separators=(",", ":")) diff --git a/page_flow.py b/page_flow.py index eb41c3b..20fbb96 100644 --- a/page_flow.py +++ b/page_flow.py @@ -2,8 +2,7 @@ page_flow.py ------------ The retry / blocked decision, as DATA rather than as three copies of an -if-chain (CLAUDE.md §1) -- and the fetch loop itself, -written once. +if-chain -- and the fetch loop itself, written once. news.ycombinator.com answers one of this repo's requests in five ways, and they want different responses: @@ -12,19 +11,22 @@ a served page with none: `No such item.`, one page past the end of a list, a `front` day it has no data for -> parse, it is an answer: exit 4 + a served page that SAYS it has rows (a More link, a + comment count) and parses to none -> our parser + failed: exit 5/6 HTTP 429 / 503, or its "not able to serve your requests this quickly" sentence -> wait, same exit HTTP 403 -> rotate anything else (a proxy interstitial, Chromium's own network-error page) -> retry -Nothing here imports a browser, and **no JavaScript crosses this boundary** -(§1). Each engine reads the rendered document in its own driver's dialect, +Nothing here imports a browser, and **no JavaScript crosses this boundary**. +Each engine reads the rendered document in its own driver's dialect, and this is the only loop. There is no challenge state and no solve step: none was met on any page this repo reads (see product_parser.detect_bot_challenge), and a state nobody has -seen would be code that looks load-bearing and never runs (§4, §17). +seen would be code that looks load-bearing and never runs. """ import logging @@ -34,6 +36,7 @@ from output_writer import dedupe_by_key, finish_run, SOURCE_DEFAULT from product_parser import (MAX_PAGES, DEFAULT_FEED, FEEDS, # noqa: F401 Query, detect_bot_challenge, detect_page_state, + site_said_nothing, next_link_agrees, next_page_url, page_meta, page_url, pagination_is_addressable, parse_page, query_from_url, request_for) @@ -60,13 +63,13 @@ # How long one navigation may take. The largest page measured was 1.68 MB (a # 1,095-comment thread), which arrived in well under ten seconds; the bound -# exists because §8 requires every remote call to have one. +# exists because every remote call must have one. NAV_TIMEOUT_MS = 60_000 # How long to wait at the SAME exit after a throttle before trying again. # NOT OBSERVED: no request drew one while measuring. HN has said in public # that it throttles by address and answers 503, so the response is to slow -# down rather than to rotate, and it needs its own budget (§24). +# down rather than to rotate, and it needs its own budget. THROTTLE_WAIT_S = 30.0 THROTTLE_RETRIES = 2 @@ -82,7 +85,7 @@ def classify(html: Optional[str], status: Optional[int] = None, The argument ORDER is the contract: every engine calls `classify(html, status, url)`. 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). `smoke_test.py` + second, and both crashed on their first fetch. `smoke_test.py` binds every engine's call against this signature for that reason. """ return detect_page_state(html or "", status, url) @@ -96,7 +99,7 @@ def classify(html: Optional[str], status: Optional[int] = None, "empty": {"retry": False, "solve": False, "blocked": False, "parse": True}, # Rate limited. Retried at the same exit after a wait (THROTTLE_*), and # NOT counted as blocked: calling a throttle a block reports exit 3 for a - # page that was about to come back (§24). + # page that was about to come back. "throttled": {"retry": True, "solve": False, "blocked": False, "parse": False}, # HTTP 403. NOT OBSERVED by this repo. Nothing to solve on this site, so # the only move is a different exit. @@ -136,7 +139,7 @@ def concurrency_limit(cdp_endpoint: Optional[str]) -> Optional[int]: The Scraping Browser API allows ONE live connection per profile, so N workers sharing a `pid` collide with `profile_locked`. Several `pid`s, - one run each, is the way to parallelise that path (§7). + one run each, is the way to parallelise that path. """ return 1 if cdp_endpoint else None @@ -159,6 +162,8 @@ def stop_reason_for(outcome) -> str: """The run's stop_reason when `outcome` is the page that ended it.""" if getattr(outcome, "blocked_by", None): return "blocked_%s" % outcome.blocked_by + if getattr(outcome, "parse_failed", False): + return "parser_found_nothing" if getattr(outcome, "state", None) == "throttled": return "throttled" return "page_load_timeout" @@ -180,7 +185,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. """ if args.pages < 1: @@ -192,6 +197,19 @@ def build_query(args, error: Callable[[str], None]) -> Query: if getattr(args, "retries", 1) < 1: error("--retries must be at least 1: it counts attempts, and 0 " "attempts sends no request at all") + # A URL that came from the environment (HACKERNEWS_URL) is a default, not + # something the user typed: when they typed a query flag or --mode, the + # flags win and the default URL is dropped. Only a TYPED --url refuses + # the flags. The engines record `typed_url` before .env is applied; + # without it (an older caller) every URL counts as typed. + typed_url = getattr(args, "typed_url", args.url) + if args.url and not typed_url and ( + args.mode is not None + or any(getattr(args, dest, None) is not None + for dest, _ in QUERY_FLAGS)): + log.info("Ignoring the default URL from the environment (%s): the " + "query flags you typed say what to fetch.", args.url) + args.url = None if args.url: query, why = query_from_url(args.url) if query is None: @@ -246,7 +264,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): @@ -293,7 +311,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, # failure, waiting out a throttle, rotating 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): +# this boundary: # # ops.fetch(url) -> (status, html, error_or_None). Navigates to # `url` and returns the RENDERED document. @@ -304,7 +322,7 @@ def finish(args, query: Query, outcomes: List, stop_reason: str, # ops.pool -> the ProxyPool this worker owns, or None # # One copy is how "the three must agree on exit codes and on whether a run -# spends money" (§6) holds by construction. +# spends money" holds by construction. class TransportError(Exception): """A navigation that did not complete: a timeout, a dead proxy.""" @@ -316,7 +334,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 which page happened to finish first (§8). + on which page happened to finish first. """ page_num: int url: str @@ -328,7 +346,7 @@ class PageOutcome: # and they mean opposite things. state: Optional[str] = None # The site's own "More" link, absolute, or None. Where a cursor list's - # next page is only knowable from (§7). + # next page is only knowable from. next_url: Optional[str] = None # Whether that link is exactly what `?p=N` would build: True means the # list can be split across workers, False means it must be walked, None @@ -337,9 +355,13 @@ class PageOutcome: # Facts about the page that are not rows (the item at the top of a thread). meta: Dict[str, Any] = field(default_factory=dict) + # The page was served and states it has content, but parsed to none. + parse_failed: bool = False + @property def ok(self) -> bool: - return not self.load_failed and self.blocked_by is None + return (not self.load_failed and self.blocked_by is None + and not self.parse_failed) # The columns the site filled on EVERY row of every capture, per mode. Below @@ -369,7 +391,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 document the parser was given, on success too (§9).""" + """Write the exact document 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}" @@ -424,7 +446,7 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, state = classify(text, status, req.url) 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/503) — waiting " "%.0fs at the same exit (%d/%d).", page_num, pause, @@ -492,21 +514,53 @@ def fetch_one_page(ops, args, pool, query: Query, page_num: int, if page_num == 1 and query.mode == "listing": outcome.next_agrees = next_link_agrees(text, req.url) outcome.meta = page_meta(text, query) + if not rows and served_rows_we_could_not_read(state, text, query, + outcome.next_url, outcome.meta): + # The page says there is something here (rows in its markup, a More + # link onward, a comment count above zero, a profile page) and the + # parser read none of it. That is THIS repo's bug, not an empty + # answer: recorded as a failed page so a run never calls it the end + # of the list or a complete one. + outcome.parse_failed = True + debug = _save_debug(args, page_num, text) + log.error("Page %d was served with content on it and parsed to no " + "rows — the markup has probably changed. Saved to %s; " + "please open an issue with it.", page_num, debug) + return outcome if state == "empty" and not rows: log.info("Page %d: the site answered with nothing to list (%s).", page_num, _empty_reason(text)) else: log.info("Parsed %d row(s) from page %d.", len(rows), page_num) - if state == "content" and not rows: - # A page the site served, with its rows on it, that parses to none is - # THIS repo's bug rather than an empty answer (§20). - log.warning("Page %d was served with rows on it and parsed to none — " - "the markup has probably changed. Re-run with --dump-html " - "and open an issue.", page_num) _core_field_warnings(rows, query.mode, page_num) return outcome +def served_rows_we_could_not_read(state: str, text: str, query: Query, + next_url: Optional[str], + meta: Optional[dict]) -> bool: + """True when a page with NO parsed rows still says it has some. + + The site's own short "nothing" sentences (`No such item.` and the rest) + and a list past its end (its own chrome, no rows, no More link) are real + empty answers. Anything else that parsed to nothing is markup drift: + + listing rows in the markup (`content`), or a More link onward + item a comment count above zero on the story, or the site's + page with no story row at all + user the site's page without the profile box + """ + if site_said_nothing(text): + return False + if query.mode == "listing": + return state == "content" or next_url is not None + if query.mode == "item": + return state == "empty" or bool((meta or {}).get("comments")) + if query.mode == "user": + return True + return state == "content" + + def _empty_reason(text: str) -> str: from product_parser import _text_of short = _text_of(text) if len(text or "") < 600 else "" @@ -529,7 +583,7 @@ def run_pages(open_ops, close_ops, run_concurrently, args, pool, ops = open_ops() try: # Page 1 is always fetched alone: whether the site's own link to page - # 2 agrees with `?p=2` decides whether the rest can be split up (§7). + # 2 agrees with `?p=2` decides whether the rest can be split up. first = fetch_one_page(ops, args, pool, query, 1, mask) outcomes.append(first) if not first.ok: @@ -583,7 +637,7 @@ def run_pages(open_ops, close_ops, run_concurrently, args, pool, pool.advance(f"per-page rotation, page {page_num}") ops.relaunch() # The site's own link is the address, always: correct for - # a `?p=N` list and the only way for a cursor one (§7). + # a `?p=N` list and the only way for a cursor one. outcome = fetch_one_page(ops, args, pool, query, page_num, mask, url=previous.next_url) outcomes.append(outcome) @@ -599,7 +653,7 @@ def run_pages(open_ops, close_ops, run_concurrently, args, pool, seen_before = {r.sku for o in outcomes[:-1] for r in o.products} if all(r.sku in seen_before for r in outcome.products): # The DATA says the list is over, not a selector: a - # page that adds nothing new is a re-served page (§7). + # page that adds nothing new is a re-served page. log.info("Page %d added no new story — treating the " "list as ended.", page_num) stop_reason = "no_new_products" @@ -672,7 +726,7 @@ def worker_pool(pool, worker_index: int): """A private ProxyPool for one worker, starting at a different exit. Workers start on distinct exits and share no mutable state, so rotation - needs no lock (§7). + needs no lock. """ if not pool: return None @@ -687,15 +741,14 @@ def cdp_connect_hint(error_text: str) -> str: 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) + answered 401 Unauthorized, because their credentials had expired. 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. " + "Browser profile's credentials expire, so an endpoint " + "copied from an older .env has usually expired. " "Get a fresh one 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 " diff --git a/playwright_scraper.py b/playwright_scraper.py index 920ad8e..def487e 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -81,7 +81,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", @@ -108,11 +108,11 @@ def _launch_local(pw, args, pool): No user agent is set. The browser's own is the one whose TLS handshake and client hints agree with it, and a bare override leaves those reporting the - real browser (§24). This site does not care either way, and the safe + real browser. This site does not care either way, and the safe default is the one that cannot be a contradiction. A proxy rotation tears the whole browser down and calls this again, so no - state issued against one exit is replayed from another (§8). + state issued against one exit is replayed from another. """ launch_kwargs = {"headless": args.headless} proxy = to_playwright(pool.current) if pool else None @@ -146,7 +146,7 @@ class _Ops: only HOW Playwright does each step. A rotation means a genuinely FRESH browser, never a proxy swapped under - a live session (§8). + a live session. """ def __init__(self, pw, args, pool, remote: bool = False): @@ -170,7 +170,7 @@ def fetch(self, url: str): a browser built, which is also what `--dump-html` writes. `domcontent- loaded` is enough on this site, whose data is in the markup and whose scripts only wire up the vote arrows; waiting for `networkidle` would - wait on nothing (§24). + wait on nothing. """ try: resp = self.page.goto(url, wait_until="domcontentloaded", @@ -233,7 +233,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 " @@ -248,7 +248,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) @@ -263,7 +263,7 @@ def _fetch_pages_concurrently(args, pool, query, page_nums, concurrency: int): Each worker owns its own Playwright instance, browser and exit: with the sync API a browser belongs to the thread that made it, so sharing one is - not an option even in principle (§7). The page loop itself is + not an option even in principle. The page loop itself is page_flow.worker_loop, shared by all three engines. """ work = queue.Queue() @@ -436,6 +436,9 @@ def parse_args(argv=None): p.add_argument("--headless", action="store_true", default=True) p.add_argument("--headful", dest="headless", action="store_false") args = p.parse_args(argv) + # Whether --url was TYPED, read before .env fills it: a URL from the + # environment is a default, and must not refuse the query flags. + args.typed_url = args.url env_config.apply(args) args.query = page_flow.build_query(args, p.error) args.mode = args.query.mode diff --git a/product_parser.py b/product_parser.py index 4b884e2..57c046e 100644 --- a/product_parser.py +++ b/product_parser.py @@ -29,7 +29,7 @@ The site says which it is on every page: the "More" link is `