diff --git a/CHANGELOG.md b/CHANGELOG.md index eea2cf8..78de6ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,17 @@ one, and when it does the release notes say so first. ### Fixed +- **The Scraper API path (`scraper_api_client.py`) failed whenever a wait flag + was given, and never saw the target's status.** Measured 2026-09-23 against + the live `/tasks/sync` endpoint: `waitFor` sent as a JSON-encoded string + (what this client sent) is answered HTTP 422 "params.waitFor must be an + object" and is still billed ($0.0005); sent as an object it is answered + HTTP 200. It is now an object. And the response's `status` is the API's own + verdict ("success"), while the target site's HTTP code is `http_code` — the + client logged `status`, so a target 403/503 read "success" in the log (this + engine logs the status and does not classify on it). It now reads + `http_code`, falling back to `status` only if that is an integer. After the fix, one live call (`--wait-text results` on the README's search URL) answered HTTP 200, upstream 200, 16 rows, status complete. + - **The Scraper API's `x-debug` response header is redacted before it is logged.** `SECURITY.md` names that header as one of three places credentials reach a log unmasked, and the client logged it whole: the API diff --git a/scraper_api_client.py b/scraper_api_client.py index 6cea720..24d7029 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -51,12 +51,17 @@ Content-Type: application/json {"task_type": "scrape", "url": ..., "data_format": "raw", "format": "json", "timeout": 1..120, - "waitFor": "", + "waitFor": {"text": ...}, # an OBJECT (see below) "cdpurl": "ws://user:pass@host:port" # optional } - -> 200 {"status": 200, "headers": {...}, "body": "..."} - -Note `waitFor` must be a JSON *string* (double-encoded), and the param is + -> 200 {"status": "success", "http_code": 200, "headers": {...}, + "body": "..."} + +Measured 2026-09-23 against the live endpoint: `waitFor` is an OBJECT. The +JSON-encoded string form this client sent until then was answered HTTP 422 +("params.waitFor must be an object") and still billed ($0.0005); the object +form answered 200. And `status` in the response is the API's own verdict +("success"), while the target page's HTTP code is `http_code`. The param is spelled `cdpurl` (all lowercase) while `waitFor` is camelCase — that is the API's own inconsistency, not a typo here. @@ -159,9 +164,13 @@ def _redact_debug_header(value: str) -> str: _CREDS_IN_TEXT_RE.sub(r"\1***:***@", value)) -def _build_wait_for(args) -> Optional[str]: - """`waitFor` must be a JSON STRING (double-encoded), per the API docs. - Passing a nested object is silently wrong. +def _build_wait_for(args) -> Optional[dict]: + """`waitFor` is an OBJECT. Measured 2026-09-23 against the live + /tasks/sync endpoint: the JSON-encoded string form this client used to + send was answered with HTTP 422 ("params.waitFor must be an object") + and was still billed ($0.0005); the same request with an object + answered HTTP 200. The earlier note here, that the API wanted a + double-encoded string, no longer describes the API. Default (no flag): wait for the DOM. On a challenge-protected page that resolves instantly against the challenge page itself — which is @@ -169,11 +178,11 @@ def _build_wait_for(args) -> Optional[str]: --wait-text/--wait-element exist to wait on something only the real page can contain.""" if args.wait_text: - return json.dumps({"text": args.wait_text}) + return {"text": args.wait_text} if args.wait_element: - return json.dumps({"element": args.wait_element, "checkVisible": True}) + return {"element": args.wait_element, "checkVisible": True} if args.wait_state: - return json.dumps({"state": args.wait_state}) + return {"state": args.wait_state} return None @@ -182,14 +191,14 @@ def fetch_html(args) -> str: "task_type": "scrape", "url": args.url, "data_format": "raw", # we want HTML; product_parser does the rest - "format": "json", # so we get {"status", "headers", "body"} + "format": "json", # {"status": verdict, "http_code": target status, "headers", "body"} "timeout": min(args.timeout, MAX_API_TIMEOUT), } wait_for = _build_wait_for(args) if wait_for: payload["waitFor"] = wait_for - logger.info("waitFor: %s", wait_for) + logger.info("waitFor: %s", json.dumps(wait_for)) if args.cdp_url: payload["cdpurl"] = args.cdp_url @@ -224,8 +233,16 @@ def fetch_html(args) -> str: body = resp.json() html = body.get("body") or "" - upstream_status = body.get("status") - logger.info("Upstream page status %s, %d bytes of HTML.", upstream_status, len(html)) + # The TARGET's HTTP status is `http_code`. `status` is the API's own + # verdict string ("success"), measured 2026-09-23, so this line logged + # "success" for a target 403/503 too. (This engine only LOGS the status; + # it does not classify on it.) Fall back to `status` only if it is + # itself an integer. + upstream_status = body.get("http_code") + if not isinstance(upstream_status, int): + legacy = body.get("status") + upstream_status = legacy if isinstance(legacy, int) and not isinstance(legacy, bool) else None + logger.info("Upstream page HTTP status %s, %d bytes of HTML.", upstream_status, len(html)) return html diff --git a/smoke_test.py b/smoke_test.py index 40a12cf..ae989c8 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -2126,6 +2126,61 @@ def test_ci_checks_is_actually_wired_up(): return ok +def test_scraper_api_sends_waitfor_as_an_object_and_reads_http_code(): + """Measured 2026-09-23 against the live Scraper API: a JSON-encoded + STRING waitFor is answered HTTP 422 and still billed, an object is + answered 200; and the target's status is `http_code`, while `status` is + the API's own verdict ("success"). Driven through the real fetch_html + with requests.post stubbed -- no network. This engine does not return + the status, only logs it, so the log record is what is asserted.""" + group("Scraper API payload and target status") + import logging + import types + import scraper_api_client as sac + sent = {} + records = [] + + class _Resp: + status_code = 200 + headers = {} + text = "" + + def json(self): + return {"status": "success", "http_code": 403, "headers": {}, + "body": ""} + + def _post(url, **kw): + sent.update(kw.get("json") or {}) + return _Resp() + + class _Grab(logging.Handler): + def emit(self, record): + records.append(record) + + args = types.SimpleNamespace( + url="https://www.amazon.com/s?k=wireless+headphones", key="k" * 8, + timeout=60, cdp_url=None, wait_text="results", wait_element=None, + wait_state=None) + grab = _Grab() + sac.logger.addHandler(grab) + real_post = sac.requests.post + sac.requests.post = _post + try: + sac.fetch_html(args) + finally: + sac.requests.post = real_post + sac.logger.removeHandler(grab) + upstream = [r.args[0] for r in records + if "Upstream page" in str(r.msg) and r.args] + ok = check("Scraper API: --wait-text sends waitFor as an OBJECT, not a JSON " + "string (422 + billed, 2026-09-23) -- got %r" % (sent.get("waitFor"),), + sent.get("waitFor") == {"text": "results"}) + ok &= check("Scraper API: the target status it reads is http_code (403), " + "not the API's 'success' -- got %r" % (upstream,), + upstream == [403] and isinstance(upstream[0], int)) + return ok + + def main() -> int: ok = True # Checks that could not run because an optional engine library is absent. @@ -2153,6 +2208,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_scraper_api_sends_waitfor_as_an_object_and_reads_http_code() ok &= test_proxy_failure_semantics() ok &= test_env_config() ok &= test_proxy_pool()