From d48a7a9b6db60f253947d3f3686e699b02660cca Mon Sep 17 00:00:00 2001 From: Sahil Sunny Date: Wed, 19 Aug 2026 20:21:37 +0530 Subject: [PATCH 1/4] fix(crawl): adapt save dispatch to Scrapy >= 2.13 engine.crawl signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scrapy 2.13 removed the 'spider' argument of ExecutionEngine.crawl() (deprecated since 2.10). Both save-dispatch sites still passed it, so on the locked Scrapy 2.16 every queued save raised TypeError, the log-and-continue handler swallowed it, and any crawl needing the discovery phase (--return-page-text, --extract-rules, --ai-query, screenshot without --json-response) reported success with zero pages saved. New _engine_crawl() helper probes the signature and dispatches correctly on both old and new Scrapy; regression tests cover both shapes plus a canary that fails loudly if a future Scrapy changes the signature again. Also: the discovery double-credit prompt told users to pass --yes; the real flag is --confirm yes. And the skill docs' custom-google price is restored to 15 credits — measured Spb-cost is 15; the API's own error message claiming 20 is what's wrong. Repro red->green: bug reproduced on main (same TypeError mechanically confirmed against installed Scrapy 2.16 signature); after the fix a live discovery crawl on the pipx build saved 2/2 pages with real content. 866 unit tests green; ruff + ty clean. --- .../reference/scrape/options.md | 2 +- .../reference/scrape/options.md | 2 +- .../reference/scrape/options.md | 2 +- .../reference/scrape/options.md | 2 +- CHANGELOG.md | 5 ++ .../reference/scrape/options.md | 2 +- src/scrapingbee_cli/commands/crawl.py | 2 +- src/scrapingbee_cli/crawl.py | 18 ++++++- tests/unit/test_crawl.py | 50 +++++++++++++++++++ 9 files changed, 77 insertions(+), 8 deletions(-) diff --git a/.agents/skills/scrapingbee-cli/reference/scrape/options.md b/.agents/skills/scrapingbee-cli/reference/scrape/options.md index 37ee290..4ec30ec 100644 --- a/.agents/skills/scrapingbee-cli/reference/scrape/options.md +++ b/.agents/skills/scrapingbee-cli/reference/scrape/options.md @@ -71,7 +71,7 @@ Blocked? See [reference/proxy/strategies.md](reference/proxy/strategies.md). |-----------|------|-------------| | `--device` | desktop \| mobile | Device type (CLI validates). | | `--timeout` | int | Timeout ms (1000–140000). Scrape job timeout on ScrapingBee. The CLI sets the HTTP client (aiohttp) timeout to this value in seconds plus 30 s (for send/receive) so the client does not give up before the API responds. | -| `--custom-google` / `--transparent-status-code` | — | Google (20 credits), target status. | +| `--custom-google` / `--transparent-status-code` | — | Google (15 credits — the API's error message for non-custom-google Google requests wrongly claims 20; 15 is correct), target status. | | `--tag` | string | Optional label included in API response headers. | | `--mode` | auto | Auto-Mode: API picks the cheapest config that succeeds; charged only for the winning config. GET only. See [Auto-Mode](#auto-mode). | | `--max-cost` | int | Cap credits a request may cost (≥ 1). Requires `--mode auto`; omit = uncapped. | diff --git a/.github/skills/scrapingbee-cli/reference/scrape/options.md b/.github/skills/scrapingbee-cli/reference/scrape/options.md index 37ee290..4ec30ec 100644 --- a/.github/skills/scrapingbee-cli/reference/scrape/options.md +++ b/.github/skills/scrapingbee-cli/reference/scrape/options.md @@ -71,7 +71,7 @@ Blocked? See [reference/proxy/strategies.md](reference/proxy/strategies.md). |-----------|------|-------------| | `--device` | desktop \| mobile | Device type (CLI validates). | | `--timeout` | int | Timeout ms (1000–140000). Scrape job timeout on ScrapingBee. The CLI sets the HTTP client (aiohttp) timeout to this value in seconds plus 30 s (for send/receive) so the client does not give up before the API responds. | -| `--custom-google` / `--transparent-status-code` | — | Google (20 credits), target status. | +| `--custom-google` / `--transparent-status-code` | — | Google (15 credits — the API's error message for non-custom-google Google requests wrongly claims 20; 15 is correct), target status. | | `--tag` | string | Optional label included in API response headers. | | `--mode` | auto | Auto-Mode: API picks the cheapest config that succeeds; charged only for the winning config. GET only. See [Auto-Mode](#auto-mode). | | `--max-cost` | int | Cap credits a request may cost (≥ 1). Requires `--mode auto`; omit = uncapped. | diff --git a/.kiro/skills/scrapingbee-cli/reference/scrape/options.md b/.kiro/skills/scrapingbee-cli/reference/scrape/options.md index 37ee290..4ec30ec 100644 --- a/.kiro/skills/scrapingbee-cli/reference/scrape/options.md +++ b/.kiro/skills/scrapingbee-cli/reference/scrape/options.md @@ -71,7 +71,7 @@ Blocked? See [reference/proxy/strategies.md](reference/proxy/strategies.md). |-----------|------|-------------| | `--device` | desktop \| mobile | Device type (CLI validates). | | `--timeout` | int | Timeout ms (1000–140000). Scrape job timeout on ScrapingBee. The CLI sets the HTTP client (aiohttp) timeout to this value in seconds plus 30 s (for send/receive) so the client does not give up before the API responds. | -| `--custom-google` / `--transparent-status-code` | — | Google (20 credits), target status. | +| `--custom-google` / `--transparent-status-code` | — | Google (15 credits — the API's error message for non-custom-google Google requests wrongly claims 20; 15 is correct), target status. | | `--tag` | string | Optional label included in API response headers. | | `--mode` | auto | Auto-Mode: API picks the cheapest config that succeeds; charged only for the winning config. GET only. See [Auto-Mode](#auto-mode). | | `--max-cost` | int | Cap credits a request may cost (≥ 1). Requires `--mode auto`; omit = uncapped. | diff --git a/.opencode/skills/scrapingbee-cli/reference/scrape/options.md b/.opencode/skills/scrapingbee-cli/reference/scrape/options.md index 37ee290..4ec30ec 100644 --- a/.opencode/skills/scrapingbee-cli/reference/scrape/options.md +++ b/.opencode/skills/scrapingbee-cli/reference/scrape/options.md @@ -71,7 +71,7 @@ Blocked? See [reference/proxy/strategies.md](reference/proxy/strategies.md). |-----------|------|-------------| | `--device` | desktop \| mobile | Device type (CLI validates). | | `--timeout` | int | Timeout ms (1000–140000). Scrape job timeout on ScrapingBee. The CLI sets the HTTP client (aiohttp) timeout to this value in seconds plus 30 s (for send/receive) so the client does not give up before the API responds. | -| `--custom-google` / `--transparent-status-code` | — | Google (20 credits), target status. | +| `--custom-google` / `--transparent-status-code` | — | Google (15 credits — the API's error message for non-custom-google Google requests wrongly claims 20; 15 is correct), target status. | | `--tag` | string | Optional label included in API response headers. | | `--mode` | auto | Auto-Mode: API picks the cheapest config that succeeds; charged only for the winning config. GET only. See [Auto-Mode](#auto-mode). | | `--max-cost` | int | Cap credits a request may cost (≥ 1). Requires `--mode auto`; omit = uncapped. | diff --git a/CHANGELOG.md b/CHANGELOG.md index 3205d58..015661c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Header-based authorization** — all API requests now authenticate via the `Authorization: Bearer` header instead of the deprecated `api_key` query parameter, so the key no longer appears in request URLs (or anything that logs them). `crawl` is the one exception: its Scrapy middleware (`scrapy-scrapingbee`) still builds `api_key` URLs and will migrate separately. +### Fixed + +- **Discovery crawls saved zero pages on Scrapy ≥ 2.13** — Scrapy 2.13 removed the `spider` argument of `ExecutionEngine.crawl()` (deprecated since 2.10), so any crawl needing the discovery phase (`--return-page-text`, `--extract-rules`, `--ai-query`, screenshot without `--json-response`) raised `TypeError` on every queued save; the error handler logged and continued, so the crawl "succeeded" with nothing saved. Save dispatch now adapts to the running Scrapy's signature (works on both old and new versions). +- **Crawl discovery prompt suggested a flag that doesn't exist** — the double-credit warning told users to pass `--yes`; the actual flag is `--confirm yes`. + ## [1.5.1] - 2026-07-20 ### Fixed diff --git a/plugins/scrapingbee-cli/skills/scrapingbee-cli/reference/scrape/options.md b/plugins/scrapingbee-cli/skills/scrapingbee-cli/reference/scrape/options.md index 37ee290..4ec30ec 100644 --- a/plugins/scrapingbee-cli/skills/scrapingbee-cli/reference/scrape/options.md +++ b/plugins/scrapingbee-cli/skills/scrapingbee-cli/reference/scrape/options.md @@ -71,7 +71,7 @@ Blocked? See [reference/proxy/strategies.md](reference/proxy/strategies.md). |-----------|------|-------------| | `--device` | desktop \| mobile | Device type (CLI validates). | | `--timeout` | int | Timeout ms (1000–140000). Scrape job timeout on ScrapingBee. The CLI sets the HTTP client (aiohttp) timeout to this value in seconds plus 30 s (for send/receive) so the client does not give up before the API responds. | -| `--custom-google` / `--transparent-status-code` | — | Google (20 credits), target status. | +| `--custom-google` / `--transparent-status-code` | — | Google (15 credits — the API's error message for non-custom-google Google requests wrongly claims 20; 15 is correct), target status. | | `--tag` | string | Optional label included in API response headers. | | `--mode` | auto | Auto-Mode: API picks the cheapest config that succeeds; charged only for the winning config. GET only. See [Auto-Mode](#auto-mode). | | `--max-cost` | int | Cap credits a request may cost (≥ 1). Requires `--mode auto`; omit = uncapped. | diff --git a/src/scrapingbee_cli/commands/crawl.py b/src/scrapingbee_cli/commands/crawl.py index 3d5c37d..b0aba29 100644 --- a/src/scrapingbee_cli/commands/crawl.py +++ b/src/scrapingbee_cli/commands/crawl.py @@ -546,7 +546,7 @@ def crawl_cmd( " an extra HTML-only discovery request — approximately doubling credits.\n\n" " Tip: Use --save-pattern '.*' to crawl with HTML (cheap, finds all links)\n" " and apply your full settings only to pages that match the pattern.\n" - " Pass --yes to skip this prompt in scripts.\n", + " Pass --confirm yes to skip this prompt in scripts.\n", err=True, ) try: diff --git a/src/scrapingbee_cli/crawl.py b/src/scrapingbee_cli/crawl.py index e70a059..60dc197 100644 --- a/src/scrapingbee_cli/crawl.py +++ b/src/scrapingbee_cli/crawl.py @@ -336,6 +336,20 @@ def _requires_discovery_phase(scrape_params: dict[str, Any]) -> bool: return False +def _engine_crawl(engine: Any, request: Any, spider: Spider) -> None: + """Dispatch a request on a running engine across Scrapy versions. + + Scrapy 2.10 deprecated (and 2.13 removed) the ``spider`` argument of + ``ExecutionEngine.crawl``; older versions require it. + """ + import inspect + + if "spider" in inspect.signature(engine.crawl).parameters: + engine.crawl(request, spider) + else: + engine.crawl(request) + + def _body_from_json_response(body: bytes) -> bytes | None: """If body is JSON with a 'body' or 'content' field (ScrapingBee json_response), return that inner content.""" @@ -654,7 +668,7 @@ def _on_spider_idle(self, spider) -> None: self._save_pending += 1 self._save_queue_next += 1 try: - engine.crawl(self._make_save_request(url), spider) + _engine_crawl(engine, self._make_save_request(url), spider) except Exception as e: # Log, don't swallow: if engine.crawl()'s signature shifts under a # Scrapy bump, every queued save would silently fail and the user @@ -1139,7 +1153,7 @@ def _on_save_error(self, failure) -> None: self._save_queue_next += 1 self._save_pending += 1 try: - engine.crawl(self._make_save_request(url), self) + _engine_crawl(engine, self._make_save_request(url), self) except Exception as e: self.logger.warning("Failed to dispatch backfill save for %s: %s", url, e) if self._save_pending > 0: diff --git a/tests/unit/test_crawl.py b/tests/unit/test_crawl.py index b707c5f..dba3ac2 100644 --- a/tests/unit/test_crawl.py +++ b/tests/unit/test_crawl.py @@ -4,6 +4,7 @@ from scrapingbee_cli.crawl import ( _body_from_json_response, + _engine_crawl, _extract_hrefs_from_body, _extract_hrefs_from_response, _normalize_url, @@ -444,6 +445,55 @@ def test_return_page_markdown_does_not_require_discovery(self): assert _requires_discovery_phase({"return_page_markdown": "true"}) is False +class TestEngineCrawlDispatch: + """_engine_crawl() must match the running Scrapy's ExecutionEngine.crawl signature. + + Scrapy 2.10 deprecated and 2.13 removed the ``spider`` argument; passing it + on a modern engine raises TypeError, which the save-dispatch error handlers + swallow — every queued save silently fails and a discovery crawl saves zero + pages (the 1.6.0 regression this guards against). + """ + + def test_modern_engine_gets_request_only(self): + from scrapy import Spider + + calls = [] + spider = Spider(name="t") + + class ModernEngine: + def crawl(self, request): + calls.append((request,)) + + _engine_crawl(ModernEngine(), "REQ", spider) + assert calls == [("REQ",)] + + def test_legacy_engine_gets_request_and_spider(self): + from scrapy import Spider + + calls = [] + spider = Spider(name="t") + + class LegacyEngine: + def crawl(self, request, spider): + calls.append((request, spider)) + + _engine_crawl(LegacyEngine(), "REQ", spider) + assert calls == [("REQ", spider)] + + def test_installed_scrapy_engine_is_dispatchable(self): + """The real installed Scrapy must match one of the two supported shapes.""" + import inspect + + from scrapy.core.engine import ExecutionEngine + + params = set(inspect.signature(ExecutionEngine.crawl).parameters) + assert "request" in params + # Either shape is fine — _engine_crawl handles both; anything else is + # a new Scrapy API break that must fail loudly here, not silently in + # a live crawl. + assert params in ({"self", "request"}, {"self", "request", "spider"}) + + class TestExtractHrefsExceptionHandling: """Tests that _extract_hrefs_from_response handles non-HTML gracefully.""" From d4da31ec37e00fe09acb7edd933488d330425356 Mon Sep 17 00:00:00 2001 From: Sahil Sunny Date: Thu, 20 Aug 2026 12:56:25 +0530 Subject: [PATCH 2/4] ci: run CI on all pull requests, not only PRs targeting main Stacked PRs (based on another PR's branch) silently got zero checks: on.pull_request was filtered to branches [main], so PRs #32 and #33 never ran CI. Drop the filter so every PR runs the suite; the push trigger stays main-only. --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 948ca10..b0cbbe0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,8 +5,9 @@ name: CI on: push: branches: [main] + # No branches filter: stacked PRs (a PR based on another PR's branch) must + # run CI too — with [main] here they silently get no checks at all. pull_request: - branches: [main] permissions: contents: read From 0d417a30c5391af75e950a0ff7637d44a3e103a7 Mon Sep 17 00:00:00 2001 From: Sahil Sunny Date: Thu, 20 Aug 2026 13:46:59 +0530 Subject: [PATCH 3/4] refactor(crawl): drop version-adaptive _engine_crawl, dispatch request-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #33: the legacy engine.crawl(request, spider) branch is unreachable in any working configuration — the spider's async start() entry point requires Scrapy >= 2.13, and 2.13 is also where the spider argument was removed. Call engine.crawl(request) directly at both dispatch sites and tighten the contract test to require the exact (self, request) signature, so any future Scrapy API change fails loudly in CI at upgrade time instead of silently adapting. Live re-verified the idle-flush path (example.com, --max-pages 10 -> 1.txt saved) on the pipx build. --- src/scrapingbee_cli/crawl.py | 23 +++++---------- tests/unit/test_crawl.py | 55 +++++++++--------------------------- 2 files changed, 21 insertions(+), 57 deletions(-) diff --git a/src/scrapingbee_cli/crawl.py b/src/scrapingbee_cli/crawl.py index 60dc197..8334ef0 100644 --- a/src/scrapingbee_cli/crawl.py +++ b/src/scrapingbee_cli/crawl.py @@ -336,20 +336,6 @@ def _requires_discovery_phase(scrape_params: dict[str, Any]) -> bool: return False -def _engine_crawl(engine: Any, request: Any, spider: Spider) -> None: - """Dispatch a request on a running engine across Scrapy versions. - - Scrapy 2.10 deprecated (and 2.13 removed) the ``spider`` argument of - ``ExecutionEngine.crawl``; older versions require it. - """ - import inspect - - if "spider" in inspect.signature(engine.crawl).parameters: - engine.crawl(request, spider) - else: - engine.crawl(request) - - def _body_from_json_response(body: bytes) -> bytes | None: """If body is JSON with a 'body' or 'content' field (ScrapingBee json_response), return that inner content.""" @@ -668,7 +654,12 @@ def _on_spider_idle(self, spider) -> None: self._save_pending += 1 self._save_queue_next += 1 try: - _engine_crawl(engine, self._make_save_request(url), spider) + # Scrapy 2.13 removed the ``spider`` argument of + # ``ExecutionEngine.crawl`` — request-only is the sole call + # shape on every Scrapy this spider runs on (its ``start()`` + # entry point requires >= 2.13). test_crawl.py's contract + # test fails loudly if a future Scrapy changes the signature. + engine.crawl(self._make_save_request(url)) except Exception as e: # Log, don't swallow: if engine.crawl()'s signature shifts under a # Scrapy bump, every queued save would silently fail and the user @@ -1153,7 +1144,7 @@ def _on_save_error(self, failure) -> None: self._save_queue_next += 1 self._save_pending += 1 try: - _engine_crawl(engine, self._make_save_request(url), self) + engine.crawl(self._make_save_request(url)) except Exception as e: self.logger.warning("Failed to dispatch backfill save for %s: %s", url, e) if self._save_pending > 0: diff --git a/tests/unit/test_crawl.py b/tests/unit/test_crawl.py index dba3ac2..f705d89 100644 --- a/tests/unit/test_crawl.py +++ b/tests/unit/test_crawl.py @@ -4,7 +4,6 @@ from scrapingbee_cli.crawl import ( _body_from_json_response, - _engine_crawl, _extract_hrefs_from_body, _extract_hrefs_from_response, _normalize_url, @@ -445,53 +444,27 @@ def test_return_page_markdown_does_not_require_discovery(self): assert _requires_discovery_phase({"return_page_markdown": "true"}) is False -class TestEngineCrawlDispatch: - """_engine_crawl() must match the running Scrapy's ExecutionEngine.crawl signature. +class TestEngineCrawlContract: + """The installed Scrapy's ExecutionEngine.crawl must be request-only. - Scrapy 2.10 deprecated and 2.13 removed the ``spider`` argument; passing it - on a modern engine raises TypeError, which the save-dispatch error handlers - swallow — every queued save silently fails and a discovery crawl saves zero - pages (the 1.6.0 regression this guards against). + Scrapy 2.13 removed the ``spider`` argument; the spider dispatches saves + with ``engine.crawl(request)``. A stale call shape raises TypeError, + which the save-dispatch error handlers swallow — every queued save + silently fails and a discovery crawl saves zero pages (the 1.6.0 + regression this guards against). If a future Scrapy changes the + signature again, this test fails loudly at upgrade time instead. """ - def test_modern_engine_gets_request_only(self): - from scrapy import Spider - - calls = [] - spider = Spider(name="t") - - class ModernEngine: - def crawl(self, request): - calls.append((request,)) - - _engine_crawl(ModernEngine(), "REQ", spider) - assert calls == [("REQ",)] - - def test_legacy_engine_gets_request_and_spider(self): - from scrapy import Spider - - calls = [] - spider = Spider(name="t") - - class LegacyEngine: - def crawl(self, request, spider): - calls.append((request, spider)) - - _engine_crawl(LegacyEngine(), "REQ", spider) - assert calls == [("REQ", spider)] - - def test_installed_scrapy_engine_is_dispatchable(self): - """The real installed Scrapy must match one of the two supported shapes.""" + def test_installed_scrapy_engine_crawl_is_request_only(self): import inspect from scrapy.core.engine import ExecutionEngine - params = set(inspect.signature(ExecutionEngine.crawl).parameters) - assert "request" in params - # Either shape is fine — _engine_crawl handles both; anything else is - # a new Scrapy API break that must fail loudly here, not silently in - # a live crawl. - assert params in ({"self", "request"}, {"self", "request", "spider"}) + params = list(inspect.signature(ExecutionEngine.crawl).parameters) + assert params == ["self", "request"], ( + f"ExecutionEngine.crawl signature changed to {params}; update the " + "engine.crawl() dispatch sites in crawl.py to match" + ) class TestExtractHrefsExceptionHandling: From c2a7a61cf3752e0351102d6bb134206d4092f694 Mon Sep 17 00:00:00 2001 From: Sahil Sunny Date: Thu, 20 Aug 2026 13:58:07 +0530 Subject: [PATCH 4/4] docs(changelog): correct crawl-fix entry scope and match simplified fix The entry overstated the blast radius (only crawls whose save queue never reached --max-pages were affected) and still described the removed version-adaptive dispatch; the fix is now a direct request-only call plus a CI contract test. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 015661c..f7e321f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- **Discovery crawls saved zero pages on Scrapy ≥ 2.13** — Scrapy 2.13 removed the `spider` argument of `ExecutionEngine.crawl()` (deprecated since 2.10), so any crawl needing the discovery phase (`--return-page-text`, `--extract-rules`, `--ai-query`, screenshot without `--json-response`) raised `TypeError` on every queued save; the error handler logged and continued, so the crawl "succeeded" with nothing saved. Save dispatch now adapts to the running Scrapy's signature (works on both old and new versions). +- **Discovery crawls saved zero pages when the site was smaller than `--max-pages`** — Scrapy 2.13 removed the `spider` argument of `ExecutionEngine.crawl()` (deprecated since 2.10), so a discovery-phase crawl (`--return-page-text`, `--extract-rules`, `--ai-query`, screenshot without `--json-response`) whose save queue never reached the `--max-pages` cap (site smaller than the cap, or no cap) raised `TypeError` on every queued save; the error handler logged and continued, so the crawl "succeeded" with nothing saved. Crawls that hit the cap dispatched saves through a different path and were unaffected — which is why this went unnoticed. Save dispatch now uses the current Scrapy call signature, and a contract test fails CI loudly if a future Scrapy changes it again. - **Crawl discovery prompt suggested a flag that doesn't exist** — the double-credit warning told users to pass `--yes`; the actual flag is `--confirm yes`. ## [1.5.1] - 2026-07-20