From 06af687662112027b4482d31e2714a3cf280a91f Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 14 Aug 2026 11:25:35 +0200 Subject: [PATCH] Fail with a clear message when Spider.start() is not an asynchronous generator (#7946) * Fail with a clear message when Spider.start() is not an asynchronous generator * Silence pylint --- scrapy/core/spidermw.py | 14 ++++++++++- tests/test_engine_loop.py | 53 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/scrapy/core/spidermw.py b/scrapy/core/spidermw.py index fbc6f2530..5d1ec246c 100644 --- a/scrapy/core/spidermw.py +++ b/scrapy/core/spidermw.py @@ -9,7 +9,7 @@ from __future__ import annotations import logging from collections.abc import AsyncIterator, Callable, Coroutine, Iterable from functools import wraps -from inspect import isasyncgenfunction +from inspect import isasyncgenfunction, iscoroutine from itertools import islice from typing import TYPE_CHECKING, Any, TypeAlias, TypeVar from warnings import warn @@ -244,8 +244,20 @@ class SpiderMiddlewareManager(MiddlewareManager): warn(msg, category=ScrapyDeprecationWarning, stacklevel=2) self._set_compat_spider(spider) start = self._spider.start() + if not hasattr(start, "__aiter__"): + if iscoroutine(start): + start.close() + start = self._reject_start(start) return await self._process_chain("process_start", start) + async def _reject_start(self, start: Any) -> AsyncIterator[Any]: + raise TypeError( + f"{global_object_name(type(self._spider))}.start() must be an" + f" asynchronous generator, i.e. an async def method with yield" + f" statements, got {type(start)}" + ) + yield # pylint: disable=unreachable # makes this method an asynchronous generator + # This method is only needed until _async compatibility methods are removed. @staticmethod def _get_process_spider_output(mw: Any) -> Callable[..., Any] | None: diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 1ecf8b8de..8e5197df7 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -4,6 +4,8 @@ from collections import deque from logging import ERROR from typing import TYPE_CHECKING, Any +import pytest + from scrapy import Request, Spider, signals from scrapy.core.scheduler import BaseScheduler from scrapy.exceptions import CloseSpider @@ -13,7 +15,7 @@ from tests.mockserver.http import MockServer from tests.utils.decorators import coroutine_test if TYPE_CHECKING: - import pytest + from collections.abc import Iterator from scrapy.http import Response @@ -50,6 +52,27 @@ class MemoryScheduler(BaseScheduler): self.paused = False +class NoneStartSpider(Spider): + name = "test" + + def start(self) -> None: # type: ignore[override] + return None + + +class CoroutineStartSpider(Spider): + name = "test" + + async def start(self) -> None: # type: ignore[override] + return None + + +class SyncStartSpider(Spider): + name = "test" + + def start(self) -> Iterator[Request]: # type: ignore[override] + yield Request("data:,a") + + class TestMain: @coroutine_test async def test_sleep(self): @@ -141,6 +164,34 @@ class TestMain: assert crawler.stats.get_value("finish_reason") == "shutdown" assert not actual_urls + @pytest.mark.parametrize( + ("spider_cls", "expected_type"), + [ + (NoneStartSpider, ""), + (CoroutineStartSpider, ""), + (SyncStartSpider, ""), + ], + ) + @coroutine_test + async def test_start_not_an_async_generator( + self, + spider_cls: type[Spider], + expected_type: str, + caplog: pytest.LogCaptureFixture, + ) -> None: + crawler = get_crawler(spider_cls) + + caplog.clear() + with caplog.at_level(ERROR): + await crawler.crawl_async() + + assert ( + f"{spider_cls.__name__}.start() must be an asynchronous generator," + f" i.e. an async def method with yield statements, got {expected_type}" + ) in caplog.text + assert crawler.stats + assert crawler.stats.get_value("finish_reason") == "start_error" + @coroutine_test async def test_start_error(self, caplog: pytest.LogCaptureFixture) -> None: class TestSpider(Spider):