diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 48d6c43..b17ca50 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -224,23 +224,22 @@ jobs: # (smoke_test.FIX_VARIATION_STATE) pins the parse; this pins that the # SITE still publishes what that fixture was carved from. # - # DISPATCH-ONLY on purpose, and it must stay that way until someone runs - # it from the default branch and sees which way it goes. A /dp/ page was - # served to a plain HTTP client from a datacenter address on 2026-09-21 - # (3 of 3, HTTP 200), which is a reason to expect this to pass from a - # runner and NOT evidence that it does — GitHub's addresses are not that - # address. Putting an unverified job on a schedule is how a badge goes - # permanently red, and a check that is always red teaches everyone to - # ignore checks. + # Ran dispatch-only first, because a job nobody has watched go green does + # not belong on a schedule — an unverified live job going red every + # morning is how everyone learns to ignore a badge. Promoted on evidence: + # two manual dispatches from GitHub runners with no proxy and no secret, + # 2026-09-21, both green. # - # To promote it: merge, `gh workflow run canary.yml` (workflow_dispatch - # needs the workflow to exist on the DEFAULT branch, or it answers a 404 - # that reads like a filename typo), confirm green twice, then add it to - # the schedule above. If it proves refused from runners, gate its steps on - # AMAZON_PROXY the way the header describes and let it SKIP with a - # ::notice:: when the secret is absent. + # Those two runs are also why the assertion is a FLOOR and not the + # measured figure. The same product reported 776 variants from a + # workstation, then 744, then 745 from the runners ten minutes apart — one + # product, three numbers, two hours. A canary pinned to any of them would + # go red for the catalogue behaving normally. + # + # If it does start being refused from runners, gate its steps on + # AMAZON_PROXY the way this file's header describes and let it SKIP with a + # ::notice:: when the secret is absent, rather than leaving it red. variation-canary: - if: github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/weekly.yml b/.github/workflows/weekly.yml new file mode 100644 index 0000000..c375d34 --- /dev/null +++ b/.github/workflows/weekly.yml @@ -0,0 +1,399 @@ +name: weekly + +# The daily canary (canary.yml) covers ONE path: a listing run on amazon.com +# through Playwright. That is the right thing to run every morning — it is +# the README's headline claim and it is cheap — but it leaves most of the +# repo unexercised against the live site, and the two defects that made this +# workflow necessary were both invisible to it: +# +# * `variations` returned null on every product run for as long as the +# column existed. No detail page was ever fetched by CI. +# * pyppeteer rotated a proxy exit on an ordinary timeout while its twins +# did not. No engine but Playwright was ever run live. +# +# So this is the broader, slower half: every mode, a second marketplace, and +# the two non-primary engines. Weekly rather than daily because it is six +# runner-jobs against a site that throttles, and because the things it +# catches are slow decay rather than an outage. +# +# NOT on a schedule yet, and that is deliberate — see the block above +# variation-canary in canary.yml for the same reasoning written out. Nobody +# has watched these jobs go green, and a live job that has never been +# watched does not belong on a cron: a badge that is red every Monday +# teaches everyone to ignore it. The evidence available at the time of +# writing (2026-09-21) is narrow and worth stating exactly: +# +# listing, Playwright, amazon.com, from a runner green (daily canary) +# product, Playwright, amazon.com, from a runner green (variation canary) +# /dp/ to a plain HTTP client, datacenter address 3 of 3, HTTP 200 +# /s? to a plain HTTP client, same address HTTP 503 every time +# +# Nothing there says anything about reviews, bestsellers, a second +# marketplace, Selenium or pyppeteer from a GitHub address. +# +# To promote it: merge, `gh workflow run weekly.yml` (workflow_dispatch +# needs the workflow on the DEFAULT branch, or it answers a 404 that reads +# like a filename typo), then read what actually happened per job. Add the +# schedule below for the jobs that pass, and for any that are refused from +# runners either gate them on the AMAZON_PROXY secret with a ::notice:: skip +# — the pattern canary.yml's header describes — or drop them and say why. +# +# schedule: +# - cron: "41 5 * * 1" # Mondays, off the hour + +on: + workflow_dispatch: + +jobs: + # --------------------------------------------------------------------- + # Every mode, not just the one the daily canary runs. + modes: + strategy: + # Independent questions: one mode being refused should not hide the + # answer for the other two. + fail-fast: false + matrix: + include: + - mode: product + url: "https://www.amazon.com/dp/B07K5214NZ" + - mode: reviews + url: "https://www.amazon.com/dp/B07K5214NZ" + - mode: listing + url: "https://www.amazon.com/gp/bestsellers/electronics/" + label: bestsellers + name: "mode: ${{ matrix.label || matrix.mode }}" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Install Playwright engine + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt -r requirements-playwright.txt + playwright install --with-deps chromium + + - name: Run + id: run + # set +e: Actions runs `run:` under `bash -e`, so a non-zero exit + # aborts the step before the line that records it. continue-on-error + # stops the step failing the job; it does not touch errexit. + continue-on-error: true + env: + AMAZON_PROXY: ${{ secrets.AMAZON_PROXY }} + run: | + set +e + python3 playwright_scraper.py --mode "${{ matrix.mode }}" \ + --url "${{ matrix.url }}" \ + --out weekly_run --dump-html weekly_run.html + echo "exit_code=$?" >> "$GITHUB_OUTPUT" + + - name: Interpret the exit code + run: | + code="${{ steps.run.outputs.exit_code }}" + case "$code" in + 0) echo "OK — the page was read." ;; + 3) echo "::error::Blocked before parsing (exit 3). May be this runner's datacenter address rather than a site change — weekly_run.last_attempt.json names the stop_reason." ;; + 4) echo "::error::Served, but nothing parsed (exit 4) on a URL known to have content. This is the shape a real site-side break takes." ;; + 5) echo "::error::The page was never fetched (exit 5) — transport. More likely this runner's network than the site." ;; + 6) echo "::error::Partial run (exit 6)." ;; + *) echo "::error::Unexpected exit code $code." ;; + esac + [ "$code" = "0" ] + + - name: Check the rows are the shape this mode promises + run: | + python3 - <<'EOF' + import json + import sys + + mode = "${{ matrix.mode }}" + rows = json.load(open("weekly_run.json")) + meta = json.load(open("weekly_run.meta.json")) + fail = [] + + if not rows: + fail.append("no rows at all") + if meta.get("mode") != mode: + fail.append("sidecar says mode=%r, asked for %r" + % (meta.get("mode"), mode)) + + if mode == "reviews": + # A dozen or so is what a /dp/ page renders anonymously; the + # paginated history needs an account, so there is no --pages + # here and no point asserting a large number. What matters is + # that a review carries the fields that make it a review. + if meta.get("record_type") != "review": + fail.append("record_type is %r, not 'review'" + % meta.get("record_type")) + bodied = [r for r in rows if r.get("review_id") and r.get("body")] + if len(bodied) < 3: + fail.append("%d of %d reviews have both an id and a body" + % (len(bodied), len(rows))) + # Amazon serves a /dp/ variant whose review widget is present + # and empty, and it is sticky within a session. The engine has + # its own retry budget for exactly that, so reaching here with + # zero rows means the budget was spent, not that the page has + # no reviews. + elif mode == "product": + r = rows[0] + for column in ("sku", "title"): + if not r.get(column): + fail.append("%s is empty on the product row" % column) + else: + # The best-seller grid numbers its own items, so a gap is + # arithmetic rather than a guess: 30 rows spanning ranks 1-50 + # proves 20 cards never loaded. + if len(rows) < 20: + fail.append("only %d rows off a best-seller grid" % len(rows)) + pairs = [(r.get("page"), r.get("position")) for r in rows] + if len(set(pairs)) != len(pairs): + fail.append("page+position is not unique across the run") + + if fail: + print("::error::%s: %s" % (mode, "; ".join(fail))) + sys.exit(1) + print("weekly OK: %s, %d rows" % (mode, len(rows))) + EOF + + - uses: actions/upload-artifact@v4 + if: always() + with: + name: weekly-mode-${{ matrix.label || matrix.mode }} + path: | + weekly_run.json + weekly_run.meta.json + weekly_run.last_attempt.json + weekly_run.html + if-no-files-found: warn + + # --------------------------------------------------------------------- + # A second marketplace. The rule this exists for: running one country site + # teaches you one country site. On a sibling repo the second one found two + # bugs the first could never show — a host that answers without `www.`, + # and a title parsed as a counter because the local word for "of" collided + # with the pattern. + # + # The assertion is the CURRENCY, because it is the one column a second + # marketplace can be wrong about while every other column still looks + # right, and because it must come from the site rather than from a + # default: a guessed "USD" on amazon.de is exactly the shape of the bug. + marketplaces: + strategy: + fail-fast: false + matrix: + include: + # The query is per-marketplace on purpose. A German term sent to + # amazon.co.uk still returns something — the search is fuzzy — + # which is worse than failing, because the run looks fine while + # exercising a query no user of that site would type. + - host: amazon.de + currency: EUR + query: "bluetooth+kopfhoerer" + - host: amazon.co.uk + currency: GBP + query: "bluetooth+headphones" + name: "marketplace: ${{ matrix.host }}" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Install Playwright engine + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt -r requirements-playwright.txt + playwright install --with-deps chromium + + - name: Run + id: run + continue-on-error: true + env: + AMAZON_PROXY: ${{ secrets.AMAZON_PROXY }} + run: | + set +e + python3 playwright_scraper.py \ + --url "https://www.${{ matrix.host }}/s?k=${{ matrix.query }}" \ + --pages 2 --out weekly_run --dump-html weekly_run.html + echo "exit_code=$?" >> "$GITHUB_OUTPUT" + + - name: Interpret the exit code + run: | + code="${{ steps.run.outputs.exit_code }}" + case "$code" in + 0) echo "OK — ${{ matrix.host }} served a listing." ;; + 3) echo "::error::Blocked (exit 3) on ${{ matrix.host }}. Some marketplaces refuse a datacenter address that another accepts — that is a finding, not noise, but it is about the address." ;; + 4) echo "::error::Served, nothing parsed (exit 4) on ${{ matrix.host }} — the shape of a per-marketplace markup difference." ;; + 5) echo "::error::Never fetched (exit 5) — transport." ;; + 6) echo "::error::Partial run (exit 6)." ;; + *) echo "::error::Unexpected exit code $code." ;; + esac + [ "$code" = "0" ] + + - name: The currency must be the site's, not a default + run: | + python3 - <<'EOF' + import collections + import json + import sys + + host = "${{ matrix.host }}" + expected = "${{ matrix.currency }}" + rows = json.load(open("weekly_run.json")) + meta = json.load(open("weekly_run.meta.json")) + fail = [] + + if meta.get("source") != host: + fail.append("sidecar source is %r, not %r" + % (meta.get("source"), host)) + + seen = collections.Counter(r.get("currency") for r in rows + if r.get("price") is not None) + if not seen: + fail.append("no row carries a price, so the currency is untested") + else: + wrong = {c: n for c, n in seen.items() if c != expected} + if wrong: + fail.append("priced rows report %r; expected %s only" + % (wrong, expected)) + + if fail: + print("::error::%s: %s" % (host, "; ".join(fail))) + sys.exit(1) + print("weekly OK: %s, %d rows, %d priced, all %s" + % (host, len(rows), sum(seen.values()), expected)) + EOF + + - uses: actions/upload-artifact@v4 + if: always() + with: + name: weekly-marketplace-${{ matrix.host }} + path: | + weekly_run.json + weekly_run.meta.json + weekly_run.last_attempt.json + weekly_run.html + if-no-files-found: warn + + # --------------------------------------------------------------------- + # The two engines that are not Playwright, against the live site. + # + # "Mirror them exactly" is a design rule, not a verification. Every defect + # this family has found in a non-primary engine — a NameError on a line + # only a fetch reaches, a proxy rotation on the wrong condition — was + # invisible to import, --help, compileall and a green offline suite, and + # showed up in the first minute of a real run. + # + # Two pages, not one: with a single page the pagination path is never + # exercised, which is how a dead next-page selector once shipped as a + # complete-looking success holding a third of the data. + engines: + strategy: + fail-fast: false + matrix: + engine: [selenium, puppeteer] + name: "engine: ${{ matrix.engine }}" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install this engine only + # One engine per environment, from its own requirements file: the + # three declare mutually unsatisfiable pins, and installing them + # together lets pip resolve the conflict by reaching for whatever it + # can — which is how CI once ran green against a stub version of + # pyppeteer that the repo does not support. + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt -r requirements-${{ matrix.engine }}.txt + pip check + + - name: Show the browser this engine will drive + # No install step: pyppeteer downloads its own Chromium on first + # launch, and Selenium drives the system Chrome plus chromedriver + # that the runner image already carries. Printing the versions is + # not decoration — when one of these jobs goes red, "which browser + # was it actually driving" is the first question, and the answer is + # otherwise nowhere in the log. + run: | + google-chrome --version || echo "no system Chrome on this runner" + chromedriver --version || echo "no chromedriver on this runner" + + - name: Run two pages + id: run + continue-on-error: true + env: + AMAZON_PROXY: ${{ secrets.AMAZON_PROXY }} + run: | + set +e + python3 ${{ matrix.engine }}_scraper.py \ + --url "https://www.amazon.com/s?k=bluetooth+headphones&i=electronics" \ + --pages 2 --out weekly_run --dump-html weekly_run.html + echo "exit_code=$?" >> "$GITHUB_OUTPUT" + + - name: Interpret the exit code + run: | + code="${{ steps.run.outputs.exit_code }}" + case "$code" in + 0) echo "OK — ${{ matrix.engine }} ran live." ;; + 1) echo "::error::CRASH (exit 1) in ${{ matrix.engine }}. This is the one exit code here that is never about the address: a traceback on a line only a live fetch reaches is exactly what this job exists to find." ;; + 3) echo "::error::Blocked (exit 3) — may be this runner's address." ;; + 4) echo "::error::Served, nothing parsed (exit 4)." ;; + 5) echo "::error::Never fetched (exit 5) — transport." ;; + 6) echo "::error::Partial run (exit 6) — page 2 did not complete, so this engine's pagination is the first thing to read." ;; + *) echo "::error::Unexpected exit code $code." ;; + esac + [ "$code" = "0" ] + + - name: Two pages, and rows that agree with the primary engine's shape + run: | + python3 - <<'EOF' + import json + import sys + + engine = "${{ matrix.engine }}" + rows = json.load(open("weekly_run.json")) + meta = json.load(open("weekly_run.meta.json")) + fail = [] + + if meta.get("pages_completed", 0) < 2: + fail.append("pages_completed=%s of 2 (stop_reason=%s)" + % (meta.get("pages_completed"), meta.get("stop_reason"))) + if meta.get("status") != "complete": + fail.append("status=%r" % meta.get("status")) + if len(rows) < 20: + fail.append("only %d rows over two pages" % len(rows)) + + # page+position must be unique across a multi-page run: position + # restarts at 1 on each page, so a page number that never gets + # threaded into the parser makes half the rows claim a position + # another row already has. + pairs = [(r.get("page"), r.get("position")) for r in rows] + if len(set(pairs)) != len(pairs): + fail.append("page+position repeats — the page number is not " + "reaching the parser") + if len({r.get("page") for r in rows}) < 2: + fail.append("every row claims the same page") + + if fail: + print("::error::%s: %s" % (engine, "; ".join(fail))) + sys.exit(1) + print("weekly OK: %s, %d rows over %s pages" + % (engine, len(rows), meta.get("pages_completed"))) + EOF + + - uses: actions/upload-artifact@v4 + if: always() + with: + name: weekly-engine-${{ matrix.engine }} + path: | + weekly_run.json + weekly_run.meta.json + weekly_run.last_attempt.json + weekly_run.html + if-no-files-found: warn diff --git a/page_flow.py b/page_flow.py index 412cd30..74af646 100644 --- a/page_flow.py +++ b/page_flow.py @@ -285,6 +285,44 @@ def hydrate(count, page_height, scroll_to_bottom, sleep, scroll_into_view, HYDRATE_ATTEMPTS = {"reviews": 1} +# Chromium's own names for "the proxy is the problem, not the site". Matched +# on an exception's TEXT because every driver here surfaces them as a generic +# error: Playwright as `Error`, Selenium inside a WebDriverException, and +# pyppeteer as one of several types. +# +# Here rather than three times over, which is how it was: each engine +# declared its own copy of this tuple and inlined its own match. They were +# byte-identical when this was written, and a tuple that three files must +# keep identical is a drift waiting to happen — the same argument that put +# the retry and page-state policy in this module. +PROXY_ERROR_MARKERS = ( + "ERR_PROXY_CONNECTION_FAILED", # nothing listening / refused + "ERR_TUNNEL_CONNECTION_FAILED", # CONNECT rejected by the proxy + "ERR_PROXY_AUTH_UNSUPPORTED", # auth scheme we cannot satisfy + "ERR_PROXY_AUTH_REQUESTED", # credentials missing or wrong + "ERR_UNEXPECTED_PROXY_AUTH", + "ERR_PROXY_CERTIFICATE_INVALID", +) + + +def proxy_failure(exc) -> str: + """The Chromium proxy-error name in `exc`, or "" if it is not one. + + Telling this apart from an ordinary timeout is the whole point, because + the two want OPPOSITE responses: a timeout deserves another try at the + same exit, while an unusable exit deserves a different one — retrying it + unchanged just spends the budget on a proxy that is not going to answer. + + Takes the exception (or anything str()-able) rather than pre-extracted + text, so no caller can forget to stringify a driver's exception type. + """ + text = str(exc) + for marker in PROXY_ERROR_MARKERS: + if marker in text: + return marker + return "" + + def numeric_arg_errors(*, pages: int, retries: int, retry_delay: float, delay: float, concurrency: Optional[int] = None, min_score: Optional[float] = None) -> List[str]: diff --git a/playwright_scraper.py b/playwright_scraper.py index 913d4da..16f14e5 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -305,33 +305,6 @@ def significant(query): significant(pa.query) == significant(pb.query) -# Chromium's own names for "the proxy is the problem, not the site". Matched -# on the error text because Playwright surfaces them as a generic Error. -_PROXY_ERROR_MARKERS = ( - "ERR_PROXY_CONNECTION_FAILED", # nothing listening / refused - "ERR_TUNNEL_CONNECTION_FAILED", # CONNECT rejected by the proxy - "ERR_PROXY_AUTH_UNSUPPORTED", # auth scheme we cannot satisfy - "ERR_PROXY_AUTH_REQUESTED", # credentials missing or wrong - "ERR_UNEXPECTED_PROXY_AUTH", - "ERR_PROXY_CERTIFICATE_INVALID", -) - - -def _proxy_failure(exc) -> str: - """The Chromium proxy-error name in `exc`, or "" if it is not one. - - Distinguishing this from an ordinary timeout matters because the two want - opposite responses: a timeout deserves a retry from the same exit, while - an unusable exit deserves a different exit — retrying it unchanged just - spends the retry budget on a proxy that is not going to answer. - """ - text = str(exc) - for marker in _PROXY_ERROR_MARKERS: - if marker in text: - return marker - return "" - - def _launch_local(pw, args, pool): """Launch our own Chromium on `pool`'s current exit; return (browser, context, page). @@ -676,7 +649,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # catching only the latter lets it escape as a traceback, # which is the likeliest failure the first time anyone points # --proxy-file at a real list. - reason = _proxy_failure(e) + reason = page_flow.proxy_failure(e) if reason: exit_failed = reason load_failed = True diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index e810ff0..a1de739 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -461,7 +461,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) - load_failed = False + load_failed, exit_failed = False, None for attempt in range(1, args.retries + 1): try: bridge.run(page.goto(url, {"waitUntil": "domcontentloaded", @@ -469,25 +469,32 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome load_failed = False break except Exception as e: # noqa: BLE001 — pyppeteer raises many types - load_failed = True # pyppeteer surfaces a dead proxy as a page error whose text # carries Chromium's own name for it, exactly as Playwright # does; a timeout and an unusable exit want opposite # responses, so they are told apart by that text. - text = str(e) - if any(marker in text for marker in _PROXY_ERROR_MARKERS): - logger.warning("Exit %s is unusable (%s).", - mask(pool.current) if pool else "(none)", text[:120]) - break + reason = page_flow.proxy_failure(e) + load_failed = True + if reason: + exit_failed = reason + break # a different exit is the only thing that helps if attempt < args.retries: pause = args.retry_delay * (2 ** (attempt - 1)) logger.warning("Failed to load %s (attempt %d/%d: %s) — " "retrying in %.1fs.", url, attempt, - args.retries, text[:120], pause) + args.retries, str(e)[:120], pause) time.sleep(pause) - if load_failed and block_attempt < block_retries: - pool.advance("unusable exit or repeated load failure") + # Only a proxy failure rotates, which is what the other two engines + # do. This used to rotate on ANY load failure, so an ordinary network + # flap spent a --proxy-block-retries budget and re-fetched the page + # while its twins gave up -- three engines disagreeing about what a + # timeout means, which is the drift page_flow exists to prevent. + if exit_failed and block_attempt < block_retries: + logger.warning("Exit %s is unusable (%s) — rotating to another " + "one (%d/%d).", mask(pool.current), exit_failed, + block_attempt + 1, block_retries) + pool.advance(f"unusable exit: {exit_failed}") session.relaunch() bridge, page = session.bridge, session.page d = _driver(session) @@ -659,12 +666,6 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome return outcome -# 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", - "ERR_UNEXPECTED_PROXY_AUTH", "ERR_PROXY_CERTIFICATE_INVALID", -) def scrape(args) -> int: diff --git a/selenium_scraper.py b/selenium_scraper.py index 476a31e..d4f0db0 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -78,14 +78,6 @@ PAGE_LOAD_TIMEOUT = 60 SCRIPT_TIMEOUT = 30 -# Chromium's own names for "the proxy is the problem, not the site". A dead -# proxy and a slow page want opposite responses — a different exit versus -# another try at the same one — so they are told apart by the error text. -_PROXY_ERROR_MARKERS = ( - "ERR_PROXY_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED", - "ERR_PROXY_AUTH_UNSUPPORTED", "ERR_PROXY_AUTH_REQUESTED", - "ERR_UNEXPECTED_PROXY_AUTH", "ERR_PROXY_CERTIFICATE_INVALID", -) @dataclass @@ -481,7 +473,7 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome break except (TimeoutException, WebDriverException) as e: text = str(e) - reason = next((m for m in _PROXY_ERROR_MARKERS if m in text), "") + reason = page_flow.proxy_failure(e) load_failed = True if reason: exit_failed = reason diff --git a/smoke_test.py b/smoke_test.py index b479984..50c91dc 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -1377,6 +1377,73 @@ def test_proxy_pool(): return ok +def test_proxy_failure_semantics(): + """A dead proxy and a timeout want opposite responses, in all three + engines. + + A timeout deserves another try at the SAME exit; an unusable exit + deserves a different one, because retrying it unchanged spends the + budget on a proxy that is not going to answer. Getting that backwards is + invisible offline and expensive live. + + Two things went wrong here and both are pinned below. + + The classifier was declared THREE TIMES — each engine carried its own + copy of the marker tuple and inlined its own match. They happened to be + byte-identical, which is the state a drift starts from, not a defence + against one. + + And the responses had already drifted: pyppeteer rotated the exit on ANY + load failure, so an ordinary network flap spent a --proxy-block-retries + budget and re-fetched the page, while Playwright and Selenium gave up. + Three engines disagreeing about what a timeout means. + """ + group("proxy failure vs. timeout (shared, and identical in all three)") + ok = True + + # Real Chromium error text, as each driver surfaces it. + cases = [ + ("Page.goto: net::ERR_PROXY_CONNECTION_FAILED at https://www.amazon.com/s?k=x", + "ERR_PROXY_CONNECTION_FAILED"), + ("net::ERR_TUNNEL_CONNECTION_FAILED", "ERR_TUNNEL_CONNECTION_FAILED"), + ("net::ERR_PROXY_AUTH_REQUESTED", "ERR_PROXY_AUTH_REQUESTED"), + ("net::ERR_PROXY_CERTIFICATE_INVALID", "ERR_PROXY_CERTIFICATE_INVALID"), + # ...and the ones that are NOT a proxy problem. + ("Timeout 60000ms exceeded.", ""), + ("net::ERR_NAME_NOT_RESOLVED", ""), + ("net::ERR_CONNECTION_RESET", ""), + ("", ""), + ] + for text, expected in cases: + got = page_flow.proxy_failure(text) + ok &= check("%r -> %r" % (text[:46], expected), got == expected) + + # It takes the EXCEPTION, not a pre-extracted string, so no caller can + # forget to stringify a driver's own exception type. + ok &= check("an exception object classifies the same as its text", + page_flow.proxy_failure( + RuntimeError("net::ERR_PROXY_CONNECTION_FAILED")) + == "ERR_PROXY_CONNECTION_FAILED") + + ok &= check("every marker in the shared tuple is recognised", + all(page_flow.proxy_failure("net::" + m) == m + for m in page_flow.PROXY_ERROR_MARKERS)) + + for name in ENGINES: + src = open(os.path.join(REPO_ROOT, name + ".py"), encoding="utf-8").read() + ok &= check("%s classifies through the shared helper" % name, + "page_flow.proxy_failure(" in src) + ok &= check("%s keeps no private copy of the marker tuple" % name, + "_PROXY_ERROR_MARKERS" not in src) + # The drift that actually shipped: rotating on `load_failed` rather + # than on `exit_failed` means a timeout burns a proxy rotation. + ok &= check("%s rotates the exit only for a PROXY failure" % name, + "if exit_failed and block_attempt < block_retries:" in src) + ok &= check("%s does not rotate on a bare load failure" % name, + "if load_failed and block_attempt < block_retries:" not in src) + return ok + + def test_scraper_api_exit_contract(): """The Scraper API client must reach the SAME decision as the engines. @@ -2081,6 +2148,7 @@ def main() -> int: ok &= test_numeric_arg_validation() ok &= test_scraper_api_never_logs_a_credential() ok &= test_scraper_api_exit_contract() + ok &= test_proxy_failure_semantics() ok &= test_env_config() ok &= test_proxy_pool() ok &= test_engines(skips)