From 2347138ba484e00d1a44b802e2f31cda16a00614 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 21 Jan 2026 22:09:54 +0400 Subject: [PATCH] Unified download handler exceptions (#7208) * Add DownloadTimeoutError. * Don't expect ConnectionAborted in tests. * Add DownloadCancelledError. * Add ResponseDataLoss. * Add more download handler tests. * Add DownloadConnectionRefusedError. * Add UnsupportedURLScheme. * Add CannotResolveHostError. * Add DownloadFailedError. * Remove wrapped Twisted exceptions from lists. * Update a test expectation. * Extract wrap_twisted_exceptions(). * Wrap TxTimeoutError. * Rename ResponseDataLoss and UnsupportedURLScheme. --- docs/topics/downloader-middleware.rst | 11 ++- docs/topics/settings.rst | 15 ++-- scrapy/core/downloader/handlers/http11.py | 22 +++-- scrapy/core/downloader/handlers/http2.py | 12 +-- scrapy/core/downloader/webclient.py | 4 +- scrapy/core/http2/protocol.py | 10 ++- scrapy/core/http2/stream.py | 5 +- scrapy/downloadermiddlewares/httpcache.py | 29 +++---- scrapy/exceptions.py | 28 ++++++ scrapy/settings/default_settings.py | 11 ++- scrapy/utils/_download_handlers.py | 44 ++++++++++ .../caching_hostname_resolver_ipv6.py | 2 +- .../default_name_resolver.py | 2 +- .../caching_hostname_resolver_ipv6.py | 2 +- tests/CrawlerProcess/default_name_resolver.py | 2 +- tests/mockserver/http_resources.py | 9 +- tests/test_command_runspider.py | 2 +- tests/test_command_shell.py | 2 +- tests/test_crawler.py | 8 +- .../test_downloader_handler_twisted_http10.py | 3 + .../test_downloader_handler_twisted_http2.py | 37 +++++--- tests/test_downloader_handlers_http_base.py | 85 +++++++++++++++---- tests/test_downloadermiddleware_retry.py | 48 +++++------ tests/test_downloadermiddleware_robotstxt.py | 9 +- tests/test_http2_client_protocol.py | 13 +-- tests/test_webclient.py | 6 +- 26 files changed, 281 insertions(+), 140 deletions(-) create mode 100644 scrapy/utils/_download_handlers.py diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 9be984f5d..1a4f91d93 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -1013,15 +1013,14 @@ RETRY_EXCEPTIONS Default:: [ - 'twisted.internet.defer.TimeoutError', - 'twisted.internet.error.TimeoutError', - 'twisted.internet.error.DNSLookupError', - 'twisted.internet.error.ConnectionRefusedError', + 'scrapy.exceptions.CannotResolveHostError', + 'scrapy.exceptions.DownloadConnectionRefusedError', + 'scrapy.exceptions.DownloadFailedError', + 'scrapy.exceptions.DownloadTimeoutError', + 'scrapy.exceptions.ResponseDataLossError', 'twisted.internet.error.ConnectionDone', 'twisted.internet.error.ConnectError', 'twisted.internet.error.ConnectionLost', - 'twisted.internet.error.TCPTimedOutError', - 'twisted.web.client.ResponseFailed', IOError, 'scrapy.core.downloader.handlers.http11.TunnelError', ] diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index b446f197f..5357fa72b 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -1047,12 +1047,12 @@ DOWNLOAD_FAIL_ON_DATALOSS Default: ``True`` -Whether or not to fail on broken responses, that is, declared -``Content-Length`` does not match content sent by the server or chunked -response was not properly finish. If ``True``, these responses raise a -``ResponseFailed([_DataLoss])`` error. If ``False``, these responses -are passed through and the flag ``dataloss`` is added to the response, i.e.: -``'dataloss' in response.flags`` is ``True``. +Whether or not to fail on broken responses, that is, when the declared +``Content-Length`` does not match content sent by the server or a chunked +response was not properly finished. If ``True``, these responses raise a +:exc:`~scrapy.exceptions.ResponseDataLossError` exception. If ``False``, these +responses are passed through and the flag ``dataloss`` is added to the +response, i.e.: ``'dataloss' in response.flags`` is ``True``. Optionally, this can be set per-request basis by using the :reqmeta:`download_fail_on_dataloss` Request.meta key to ``False``. @@ -1064,7 +1064,8 @@ Optionally, this can be set per-request basis by using the corruption. It is up to the user to decide if it makes sense to process broken responses considering they may contain partial or incomplete content. If :setting:`RETRY_ENABLED` is ``True`` and this setting is set to ``True``, - the ``ResponseFailed([_DataLoss])`` failure will be retried as usual. + the :exc:`~scrapy.exceptions.ResponseDataLossError` failure will be retried + as usual. .. note:: diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index cec73566a..312ecf229 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -12,9 +12,8 @@ from typing import TYPE_CHECKING, Any, TypedDict, TypeVar, cast from urllib.parse import urldefrag, urlparse from twisted.internet import ssl -from twisted.internet.defer import CancelledError, Deferred, succeed +from twisted.internet.defer import Deferred, succeed from twisted.internet.endpoints import TCP4ClientEndpoint -from twisted.internet.error import TimeoutError as TxTimeoutError from twisted.internet.protocol import Factory, Protocol, connectionDone from twisted.python.failure import Failure from twisted.web.client import ( @@ -33,9 +32,15 @@ from zope.interface import implementer from scrapy import Request, signals from scrapy.core.downloader.contextfactory import load_context_factory_from_settings from scrapy.core.downloader.handlers.base import BaseDownloadHandler -from scrapy.exceptions import StopDownload +from scrapy.exceptions import ( + DownloadCancelledError, + DownloadTimeoutError, + ResponseDataLossError, + StopDownload, +) from scrapy.http import Headers, Response from scrapy.responsetypes import responsetypes +from scrapy.utils._download_handlers import wrap_twisted_exceptions from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute from scrapy.utils.httpobj import urlparse_cached @@ -110,7 +115,8 @@ class HTTP11DownloadHandler(BaseDownloadHandler): fail_on_dataloss=self._fail_on_dataloss, crawler=self._crawler, ) - return await maybe_deferred_to_future(agent.download_request(request)) + with wrap_twisted_exceptions(): + return await maybe_deferred_to_future(agent.download_request(request)) async def close(self) -> None: from twisted.internet import reactor @@ -462,7 +468,7 @@ class ScrapyAgent: if self._txresponse: self._txresponse._transport.stopProducing() - raise TxTimeoutError(f"Getting {url} took longer than {timeout} seconds.") + raise DownloadTimeoutError(f"Getting {url} took longer than {timeout} seconds.") def _cb_latency(self, result: _T, request: Request, start_time: float) -> _T: request.meta["download_latency"] = time() - start_time @@ -534,7 +540,7 @@ class ScrapyAgent: logger.warning(warning_msg, warning_args) txresponse._transport.loseConnection() - raise CancelledError(warning_msg % warning_args) + raise DownloadCancelledError(warning_msg % warning_args) if warnsize and expected_size > warnsize: logger.warning( @@ -741,4 +747,8 @@ class _ResponseReader(Protocol): ) self._fail_on_dataloss_warned = True + exc = ResponseDataLossError() + exc.__cause__ = reason.value + reason = Failure(exc) + self._finished.errback(reason) diff --git a/scrapy/core/downloader/handlers/http2.py b/scrapy/core/downloader/handlers/http2.py index ff6e8ab98..75fa0772d 100644 --- a/scrapy/core/downloader/handlers/http2.py +++ b/scrapy/core/downloader/handlers/http2.py @@ -4,12 +4,13 @@ from time import time from typing import TYPE_CHECKING from urllib.parse import urldefrag -from twisted.internet.error import TimeoutError as TxTimeoutError from twisted.web.client import URI from scrapy.core.downloader.contextfactory import load_context_factory_from_settings from scrapy.core.downloader.handlers.base import BaseDownloadHandler from scrapy.core.http2.agent import H2Agent, H2ConnectionPool, ScrapyProxyH2Agent +from scrapy.exceptions import DownloadTimeoutError +from scrapy.utils._download_handlers import wrap_twisted_exceptions from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.python import to_bytes @@ -45,9 +46,10 @@ class H2DownloadHandler(BaseDownloadHandler): crawler=self._crawler, ) assert self._crawler.spider - return await maybe_deferred_to_future( - agent.download_request(request, self._crawler.spider) - ) + with wrap_twisted_exceptions(): + return await maybe_deferred_to_future( + agent.download_request(request, self._crawler.spider) + ) async def close(self) -> None: self._pool.close_connections() @@ -129,4 +131,4 @@ class ScrapyH2Agent: return response url = urldefrag(request.url)[0] - raise TxTimeoutError(f"Getting {url} took longer than {timeout} seconds.") + raise DownloadTimeoutError(f"Getting {url} took longer than {timeout} seconds.") diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py index e5c2255af..52d287245 100644 --- a/scrapy/core/downloader/webclient.py +++ b/scrapy/core/downloader/webclient.py @@ -11,7 +11,7 @@ from twisted.internet import defer from twisted.internet.protocol import ClientFactory from twisted.web.http import HTTPClient -from scrapy.exceptions import ScrapyDeprecationWarning +from scrapy.exceptions import DownloadTimeoutError, ScrapyDeprecationWarning from scrapy.http import Headers, Response from scrapy.responsetypes import responsetypes from scrapy.utils.httpobj import urlparse_cached @@ -80,7 +80,7 @@ class ScrapyHTTPPageGetter(HTTPClient): self.transport.stopProducing() self.factory.noPage( - defer.TimeoutError( + DownloadTimeoutError( f"Getting {self.factory.url} took longer " f"than {self.factory.timeout} seconds." ) diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index ee9211efc..9f6216157 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -21,7 +21,6 @@ from h2.events import ( WindowUpdated, ) from h2.exceptions import FrameTooLargeError, H2Error -from twisted.internet.error import TimeoutError as TxTimeoutError from twisted.internet.interfaces import ( IAddress, IHandshakeListener, @@ -33,6 +32,7 @@ from twisted.protocols.policies import TimeoutMixin from zope.interface import implementer from scrapy.core.http2.stream import Stream, StreamCloseReason +from scrapy.exceptions import DownloadTimeoutError from scrapy.http import Request, Response from scrapy.utils.deprecate import warn_on_deprecated_spider_attribute @@ -313,7 +313,7 @@ class H2ClientProtocol(Protocol, TimeoutMixin): def timeoutConnection(self) -> None: """Called when the connection times out. - We lose the connection with TimeoutError""" + We lose the connection with DownloadTimeoutError""" # Check whether there are open streams. If there are, we're going to # want to use the error code PROTOCOL_ERROR. If there aren't, use @@ -330,7 +330,11 @@ class H2ClientProtocol(Protocol, TimeoutMixin): self._write_to_transport() self._lose_connection_with_error( - [TxTimeoutError(f"Connection was IDLE for more than {self.IDLE_TIMEOUT}s")] + [ + DownloadTimeoutError( + f"Connection was IDLE for more than {self.IDLE_TIMEOUT}s" + ) + ] ) def connectionLost(self, reason: Failure = connectionDone) -> None: diff --git a/scrapy/core/http2/stream.py b/scrapy/core/http2/stream.py index afca99dcf..291419d84 100644 --- a/scrapy/core/http2/stream.py +++ b/scrapy/core/http2/stream.py @@ -7,11 +7,12 @@ from typing import TYPE_CHECKING, Any from h2.errors import ErrorCodes from h2.exceptions import H2Error, ProtocolError, StreamClosedError -from twisted.internet.defer import CancelledError, Deferred +from twisted.internet.defer import Deferred from twisted.internet.error import ConnectionClosed from twisted.python.failure import Failure from twisted.web.client import ResponseFailed +from scrapy.exceptions import DownloadCancelledError from scrapy.http.headers import Headers from scrapy.responsetypes import responsetypes from scrapy.utils.httpobj import urlparse_cached @@ -422,7 +423,7 @@ class Stream: f"size ({expected_size}) larger than download max size ({self._download_maxsize})" ) logger.error(error_msg) - self._deferred_response.errback(CancelledError(error_msg)) + self._deferred_response.errback(DownloadCancelledError(error_msg)) elif reason is StreamCloseReason.ENDED: self._fire_response_deferred() diff --git a/scrapy/downloadermiddlewares/httpcache.py b/scrapy/downloadermiddlewares/httpcache.py index 453e8a6a3..a176dcd9f 100644 --- a/scrapy/downloadermiddlewares/httpcache.py +++ b/scrapy/downloadermiddlewares/httpcache.py @@ -3,20 +3,16 @@ from __future__ import annotations from email.utils import formatdate from typing import TYPE_CHECKING -from twisted.internet import defer -from twisted.internet.error import ( - ConnectError, - ConnectionDone, - ConnectionLost, - DNSLookupError, - TCPTimedOutError, -) -from twisted.internet.error import ConnectionRefusedError as TxConnectionRefusedError -from twisted.internet.error import TimeoutError as TxTimeoutError -from twisted.web.client import ResponseFailed +from twisted.internet.error import ConnectError, ConnectionDone, ConnectionLost from scrapy import signals -from scrapy.exceptions import IgnoreRequest, NotConfigured +from scrapy.exceptions import ( + DownloadConnectionRefusedError, + DownloadFailedError, + DownloadTimeoutError, + IgnoreRequest, + NotConfigured, +) from scrapy.utils.decorators import _warn_spider_arg from scrapy.utils.misc import load_object @@ -34,16 +30,13 @@ if TYPE_CHECKING: class HttpCacheMiddleware: DOWNLOAD_EXCEPTIONS = ( - defer.TimeoutError, - TxTimeoutError, - DNSLookupError, - TxConnectionRefusedError, ConnectionDone, ConnectError, ConnectionLost, - TCPTimedOutError, - ResponseFailed, OSError, + DownloadTimeoutError, + DownloadConnectionRefusedError, + DownloadFailedError, ) crawler: Crawler diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index f37f881a7..b25add8cf 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -54,6 +54,34 @@ class StopDownload(Exception): self.fail = fail +class DownloadConnectionRefusedError(Exception): + """Indicates that a connection was refused by the server.""" + + +class CannotResolveHostError(Exception): + """Indicates that the provided hostname cannot be resolved.""" + + +class DownloadTimeoutError(Exception): + """Indicates that a request download has timed out.""" + + +class DownloadCancelledError(Exception): + """Indicates that a request download was cancelled.""" + + +class DownloadFailedError(Exception): + """Indicates that a request download has failed.""" + + +class ResponseDataLossError(Exception): + """Indicates that Scrapy couldn't get a complete response.""" + + +class UnsupportedURLSchemeError(Exception): + """Indicates that the URL scheme is not supported.""" + + # Items diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index d741bf3ac..7ef5feb9f 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -453,15 +453,14 @@ REQUEST_FINGERPRINTER_CLASS = "scrapy.utils.request.RequestFingerprinter" RETRY_ENABLED = True RETRY_EXCEPTIONS = [ - "twisted.internet.defer.TimeoutError", - "twisted.internet.error.TimeoutError", - "twisted.internet.error.DNSLookupError", - "twisted.internet.error.ConnectionRefusedError", + "scrapy.exceptions.CannotResolveHostError", + "scrapy.exceptions.DownloadConnectionRefusedError", + "scrapy.exceptions.DownloadFailedError", + "scrapy.exceptions.DownloadTimeoutError", + "scrapy.exceptions.ResponseDataLossError", "twisted.internet.error.ConnectionDone", "twisted.internet.error.ConnectError", "twisted.internet.error.ConnectionLost", - "twisted.internet.error.TCPTimedOutError", - "twisted.web.client.ResponseFailed", # OSError is raised by the HttpCompression middleware when trying to # decompress an empty response OSError, diff --git a/scrapy/utils/_download_handlers.py b/scrapy/utils/_download_handlers.py new file mode 100644 index 000000000..4c7721483 --- /dev/null +++ b/scrapy/utils/_download_handlers.py @@ -0,0 +1,44 @@ +"""Utils for built-in HTTP download handlers.""" + +from __future__ import annotations + +from contextlib import contextmanager +from typing import TYPE_CHECKING + +from twisted.internet.defer import CancelledError +from twisted.internet.error import ConnectionRefusedError as TxConnectionRefusedError +from twisted.internet.error import DNSLookupError +from twisted.internet.error import TimeoutError as TxTimeoutError +from twisted.web.client import ResponseFailed +from twisted.web.error import SchemeNotSupported + +from scrapy.exceptions import ( + CannotResolveHostError, + DownloadCancelledError, + DownloadConnectionRefusedError, + DownloadFailedError, + DownloadTimeoutError, + UnsupportedURLSchemeError, +) + +if TYPE_CHECKING: + from collections.abc import Iterator + + +@contextmanager +def wrap_twisted_exceptions() -> Iterator[None]: + """Context manager that wraps Twisted exceptions into Scrapy exceptions.""" + try: + yield + except SchemeNotSupported as e: + raise UnsupportedURLSchemeError(str(e)) from e + except CancelledError as e: + raise DownloadCancelledError(str(e)) from e + except TxConnectionRefusedError as e: + raise DownloadConnectionRefusedError(str(e)) from e + except DNSLookupError as e: + raise CannotResolveHostError(str(e)) from e + except ResponseFailed as e: + raise DownloadFailedError(str(e)) from e + except TxTimeoutError as e: + raise DownloadTimeoutError(str(e)) from e diff --git a/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py b/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py index c43f0a9c2..07e2d3684 100644 --- a/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py +++ b/tests/AsyncCrawlerProcess/caching_hostname_resolver_ipv6.py @@ -4,7 +4,7 @@ from scrapy.crawler import AsyncCrawlerProcess class CachingHostnameResolverSpider(scrapy.Spider): """ - Finishes without a twisted.internet.error.DNSLookupError exception + Finishes without a scrapy.exceptions.CannotResolveHostError exception """ name = "caching_hostname_resolver_spider" diff --git a/tests/AsyncCrawlerProcess/default_name_resolver.py b/tests/AsyncCrawlerProcess/default_name_resolver.py index af56ccd01..4c8897f8f 100644 --- a/tests/AsyncCrawlerProcess/default_name_resolver.py +++ b/tests/AsyncCrawlerProcess/default_name_resolver.py @@ -4,7 +4,7 @@ from scrapy.crawler import AsyncCrawlerProcess class IPv6Spider(scrapy.Spider): """ - Raises a twisted.internet.error.DNSLookupError: + Raises a scrapy.exceptions.CannotResolveHostError: the default name resolver does not handle IPv6 addresses. """ diff --git a/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py b/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py index d5d19e27e..5cca94bed 100644 --- a/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py +++ b/tests/CrawlerProcess/caching_hostname_resolver_ipv6.py @@ -4,7 +4,7 @@ from scrapy.crawler import CrawlerProcess class CachingHostnameResolverSpider(scrapy.Spider): """ - Finishes without a twisted.internet.error.DNSLookupError exception + Finishes without a scrapy.exceptions.CannotResolveHostError exception """ name = "caching_hostname_resolver_spider" diff --git a/tests/CrawlerProcess/default_name_resolver.py b/tests/CrawlerProcess/default_name_resolver.py index cfc4b38b7..f4c129fdf 100644 --- a/tests/CrawlerProcess/default_name_resolver.py +++ b/tests/CrawlerProcess/default_name_resolver.py @@ -4,7 +4,7 @@ from scrapy.crawler import CrawlerProcess class IPv6Spider(scrapy.Spider): """ - Raises a twisted.internet.error.DNSLookupError: + Raises a scrapy.exceptions.CannotResolveHostError: the default name resolver does not handle IPv6 addresses. """ diff --git a/tests/mockserver/http_resources.py b/tests/mockserver/http_resources.py index d5687fc44..70a9b8ac6 100644 --- a/tests/mockserver/http_resources.py +++ b/tests/mockserver/http_resources.py @@ -193,10 +193,11 @@ class Drop(Partial): request.write(b"this connection will be dropped\n") tr = request.channel.transport try: - if abort and hasattr(tr, "abortConnection"): - tr.abortConnection() - else: - tr.loseConnection() + if tr: + if abort and hasattr(tr, "abortConnection"): + tr.abortConnection() + else: + tr.loseConnection() finally: request.finish() diff --git a/tests/test_command_runspider.py b/tests/test_command_runspider.py index 2fa4ce581..b5408ccfe 100644 --- a/tests/test_command_runspider.py +++ b/tests/test_command_runspider.py @@ -119,7 +119,7 @@ class MySpider(scrapy.Spider): log = self.get_log( tmp_path, dnscache_spider, args=("-s", "DNSCACHE_ENABLED=False") ) - assert "DNSLookupError" not in log + assert "CannotResolveHostError" not in log assert "INFO: Spider opened" in log @pytest.mark.parametrize("value", [False, True]) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index d9fb96fb8..1417bc623 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -114,7 +114,7 @@ class TestShellCommand: url = "www.somedomainthatdoesntexi.st" ret, out, err = proc("shell", url, "-c", "item") assert ret == 1, out or err - assert "DNS lookup failed" in err + assert "CannotResolveHostError" in err def test_shell_fetch_async(self, mockserver: MockServer) -> None: url = mockserver.url("/html") diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 5204c9a73..350c82a0b 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -844,18 +844,18 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): log = self.run_script("default_name_resolver.py") assert "Spider closed (finished)" in log assert ( - "'downloader/exception_type_count/twisted.internet.error.DNSLookupError': 1," + "'downloader/exception_type_count/scrapy.exceptions.CannotResolveHostError': 1," in log ) assert ( - "twisted.internet.error.DNSLookupError: DNS lookup failed: no results for hostname lookup: ::1." + "scrapy.exceptions.CannotResolveHostError: DNS lookup failed: no results for hostname lookup: ::1." in log ) def test_caching_hostname_resolver_ipv6(self): log = self.run_script("caching_hostname_resolver_ipv6.py") assert "Spider closed (finished)" in log - assert "twisted.internet.error.DNSLookupError" not in log + assert "scrapy.exceptions.CannotResolveHostError" not in log def test_caching_hostname_resolver_finite_execution( self, mockserver: MockServer @@ -864,7 +864,7 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): assert "Spider closed (finished)" in log assert "ERROR: Error downloading" not in log assert "TimeoutError" not in log - assert "twisted.internet.error.DNSLookupError" not in log + assert "scrapy.exceptions.CannotResolveHostError" not in log def test_twisted_reactor_asyncio(self): log = self.run_script("twisted_reactor_asyncio.py") diff --git a/tests/test_downloader_handler_twisted_http10.py b/tests/test_downloader_handler_twisted_http10.py index 4745869e4..939e2c8ad 100644 --- a/tests/test_downloader_handler_twisted_http10.py +++ b/tests/test_downloader_handler_twisted_http10.py @@ -26,6 +26,9 @@ class HTTP10DownloadHandlerMixin: class TestHttp10(HTTP10DownloadHandlerMixin, TestHttpBase): """HTTP 1.0 test case""" + def test_unsupported_scheme(self) -> None: # type: ignore[override] + pytest.skip("Check not implemented") + @deferred_f_from_coro_f async def test_protocol(self, mockserver: MockServer) -> None: request = Request( diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index f60d5e7b5..a9197645a 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -8,10 +8,10 @@ from unittest import mock import pytest from testfixtures import LogCapture -from twisted.internet import defer, error -from twisted.web.error import SchemeNotSupported +from twisted.internet import defer from twisted.web.http import H2_ENABLED +from scrapy.exceptions import DownloadCancelledError, UnsupportedURLSchemeError from scrapy.http import Request from scrapy.utils.defer import deferred_f_from_coro_f, maybe_deferred_to_future from tests.test_downloader_handlers_http_base import ( @@ -72,7 +72,7 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): logger.error.assert_called_once_with(mock.ANY) async with self.get_dh({"DOWNLOAD_MAXSIZE": 1_500}) as download_handler: - with pytest.raises((defer.CancelledError, error.ConnectionAborted)): + with pytest.raises(DownloadCancelledError): await download_handler.download_request(request) # As the error message is logged in the dataReceived callback, we @@ -83,13 +83,6 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): reactor.callLater(0.1, d.callback, logger) await maybe_deferred_to_future(d) - @deferred_f_from_coro_f - async def test_unsupported_scheme(self) -> None: - request = Request("ftp://unsupported.scheme") - async with self.get_dh() as download_handler: - with pytest.raises(SchemeNotSupported): - await download_handler.download_request(request) - def test_download_cause_data_loss(self) -> None: # type: ignore[override] pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) @@ -99,6 +92,28 @@ class TestHttps2(H2DownloadHandlerMixin, TestHttps11Base): def test_download_allow_data_loss_via_setting(self) -> None: # type: ignore[override] pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) + def test_download_conn_failed(self) -> None: # type: ignore[override] + # Unlike HTTP11DownloadHandler which raises it from download_request() + # (without any special handling), here ConnectionRefusedError (raised in + # twisted.internet.endpoints.startConnectionAttempts()) bubbles up as + # an unhandled exception in a Deferred and the handler waits until + # DOWNLOAD_TIMEOUT. + pytest.skip("The handler doesn't properly reraise ConnectionRefusedError") + + def test_download_conn_lost(self) -> None: # type: ignore[override] + pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) + + def test_download_conn_aborted(self) -> None: # type: ignore[override] + pytest.skip(self.HTTP2_DATALOSS_SKIP_REASON) + + def test_download_dns_error(self) -> None: # type: ignore[override] + # Unlike HTTP11DownloadHandler which raises it from download_request() + # (without any special handling), here DNSLookupError (raised in + # twisted.internet.endpoints.startConnectionAttempts()) bubbles up as + # an unhandled exception in a Deferred and the handler waits until + # DOWNLOAD_TIMEOUT. + pytest.skip("The handler doesn't properly reraise DNSLookupError") + @deferred_f_from_coro_f async def test_concurrent_requests_same_domain( self, mockserver: MockServer @@ -212,7 +227,7 @@ class TestHttps2Proxy(H2DownloadHandlerMixin, TestHttpProxyBase): async def test_download_with_proxy_without_http_scheme( self, proxy_mockserver: ProxyEchoMockServer ) -> None: - with pytest.raises(SchemeNotSupported): + with pytest.raises(UnsupportedURLSchemeError): await maybe_deferred_to_future( super().test_download_with_proxy_without_http_scheme(proxy_mockserver) ) diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index e325c0463..d33de8881 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -13,10 +13,17 @@ from unittest import mock import pytest from testfixtures import LogCapture -from twisted.internet import defer, error -from twisted.web._newclient import ResponseFailed -from twisted.web.http import _DataLoss +from twisted.internet import defer +from scrapy.exceptions import ( + CannotResolveHostError, + DownloadCancelledError, + DownloadConnectionRefusedError, + DownloadFailedError, + DownloadTimeoutError, + ResponseDataLossError, + UnsupportedURLSchemeError, +) from scrapy.http import Headers, HtmlResponse, Request, Response, TextResponse from scrapy.utils.asyncio import call_later from scrapy.utils.defer import ( @@ -59,6 +66,13 @@ class TestHttpBase(ABC): finally: await dh.close() + @deferred_f_from_coro_f + async def test_unsupported_scheme(self) -> None: + request = Request("ftp://unsupported.scheme") + async with self.get_dh() as download_handler: + with pytest.raises(UnsupportedURLSchemeError): + await download_handler.download_request(request) + @deferred_f_from_coro_f async def test_download(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/text", is_secure=self.is_secure)) @@ -208,7 +222,7 @@ class TestHttpBase(ABC): request = Request(mockserver.url("/wait", is_secure=self.is_secure), meta=meta) async with self.get_dh() as download_handler: d = deferred_from_coro(download_handler.download_request(request)) - with pytest.raises((defer.TimeoutError, error.TimeoutError)): + with pytest.raises(DownloadTimeoutError): await maybe_deferred_to_future(d) @deferred_f_from_coro_f @@ -229,7 +243,7 @@ class TestHttpBase(ABC): ) async with self.get_dh() as download_handler: d = deferred_from_coro(download_handler.download_request(request)) - with pytest.raises((defer.TimeoutError, error.TimeoutError)): + with pytest.raises(DownloadTimeoutError): await maybe_deferred_to_future(d) @pytest.mark.parametrize("send_header", [True, False]) @@ -431,7 +445,7 @@ class TestHttp11Base(TestHttpBase): assert response.body == b"Works" async with self.get_dh({"DOWNLOAD_MAXSIZE": 4}) as download_handler: - with pytest.raises((defer.CancelledError, error.ConnectionAborted)): + with pytest.raises(DownloadCancelledError): await download_handler.download_request(request) @deferred_f_from_coro_f @@ -448,7 +462,7 @@ class TestHttp11Base(TestHttpBase): logger.warning.assert_called_once_with(mock.ANY, mock.ANY) async with self.get_dh({"DOWNLOAD_MAXSIZE": 1_500}) as download_handler: - with pytest.raises((defer.CancelledError, error.ConnectionAborted)): + with pytest.raises(DownloadCancelledError): await download_handler.download_request(request) # As the error message is logged in the dataReceived callback, we @@ -464,7 +478,7 @@ class TestHttp11Base(TestHttpBase): meta = {"download_maxsize": 2} request = Request(mockserver.url("/text", is_secure=self.is_secure), meta=meta) async with self.get_dh() as download_handler: - with pytest.raises((defer.CancelledError, error.ConnectionAborted)): + with pytest.raises(DownloadCancelledError): await download_handler.download_request(request) @deferred_f_from_coro_f @@ -473,7 +487,7 @@ class TestHttp11Base(TestHttpBase): ) -> None: request = Request(mockserver.url("/text", is_secure=self.is_secure)) async with self.get_dh({"DOWNLOAD_MAXSIZE": 2}) as download_handler: - with pytest.raises((defer.CancelledError, error.ConnectionAborted)): + with pytest.raises(DownloadCancelledError): await download_handler.download_request(request) @deferred_f_from_coro_f @@ -497,12 +511,10 @@ class TestHttp11Base(TestHttpBase): async def test_download_cause_data_loss( self, url: str, mockserver: MockServer ) -> None: - # TODO: this one checks for Twisted-specific exceptions request = Request(mockserver.url(f"/{url}", is_secure=self.is_secure)) async with self.get_dh() as download_handler: - with pytest.raises(ResponseFailed) as exc_info: + with pytest.raises(ResponseDataLossError): await download_handler.download_request(request) - assert any(r.check(_DataLoss) for r in exc_info.value.reasons) @pytest.mark.parametrize("url", ["broken", "broken-chunked"]) @deferred_f_from_coro_f @@ -529,6 +541,43 @@ class TestHttp11Base(TestHttpBase): response = await download_handler.download_request(request) assert response.flags == ["dataloss"] + @deferred_f_from_coro_f + async def test_download_conn_failed(self) -> None: + # copy of TestCrawl.test_retry_conn_failed() + scheme = "https" if self.is_secure else "http" + request = Request(f"{scheme}://localhost:65432/") + async with self.get_dh() as download_handler: + with pytest.raises(DownloadConnectionRefusedError): + await download_handler.download_request(request) + + @deferred_f_from_coro_f + async def test_download_conn_lost(self, mockserver: MockServer) -> None: + # copy of TestCrawl.test_retry_conn_lost() + request = Request(mockserver.url("/drop?abort=0", is_secure=self.is_secure)) + async with self.get_dh() as download_handler: + with pytest.raises(ResponseDataLossError): + await download_handler.download_request(request) + + @deferred_f_from_coro_f + async def test_download_conn_aborted(self, mockserver: MockServer) -> None: + # copy of TestCrawl.test_retry_conn_aborted() + request = Request(mockserver.url("/drop?abort=1", is_secure=self.is_secure)) + async with self.get_dh() as download_handler: + with pytest.raises(DownloadFailedError): + await download_handler.download_request(request) + + @pytest.mark.skipif( + NON_EXISTING_RESOLVABLE, reason="Non-existing hosts are resolvable" + ) + @deferred_f_from_coro_f + async def test_download_dns_error(self) -> None: + # copy of TestCrawl.test_retry_dns_error() + scheme = "https" if self.is_secure else "http" + request = Request(f"{scheme}://dns.resolution.invalid./") + async with self.get_dh() as download_handler: + with pytest.raises(CannotResolveHostError): + await download_handler.download_request(request) + @deferred_f_from_coro_f async def test_protocol(self, mockserver: MockServer) -> None: request = Request( @@ -547,6 +596,12 @@ class TestHttps11Base(TestHttp11Base): 'subject "/C=IE/O=Scrapy/CN=localhost"' ) + def test_download_conn_lost(self) -> None: # type: ignore[override] + # For some reason (maybe related to TLS shutdown flow, and maybe the + # mockserver resource can be fixed so that this works) HTTPS clients + # (not just Scrapy) hang on /drop?abort=0. + pytest.skip("Unable to test on HTTPS") + @deferred_f_from_coro_f async def test_tls_logging(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/text", is_secure=self.is_secure)) @@ -658,7 +713,7 @@ class TestHttpWithCrawlerBase(ABC): ) assert crawler.spider failure = crawler.spider.meta["failure"] # type: ignore[attr-defined] - assert isinstance(failure.value, defer.CancelledError) + assert isinstance(failure.value, DownloadCancelledError) @deferred_f_from_coro_f async def test_download(self, mockserver: MockServer) -> None: @@ -734,9 +789,9 @@ class TestHttpProxyBase(ABC): domain = "https://no-such-domain.nosuch" request = Request(domain, meta={"proxy": http_proxy, "download_timeout": 0.2}) async with self.get_dh() as download_handler: - with pytest.raises(error.TimeoutError) as exc_info: + with pytest.raises(DownloadTimeoutError) as exc_info: await download_handler.download_request(request) - assert domain in exc_info.value.osError + assert domain in str(exc_info.value) @deferred_f_from_coro_f async def test_download_with_proxy_without_http_scheme( diff --git a/tests/test_downloadermiddleware_retry.py b/tests/test_downloadermiddleware_retry.py index affcc79c1..20e8fc407 100644 --- a/tests/test_downloadermiddleware_retry.py +++ b/tests/test_downloadermiddleware_retry.py @@ -2,20 +2,15 @@ import logging import pytest from testfixtures import LogCapture -from twisted.internet import defer -from twisted.internet.error import ( - ConnectError, - ConnectionDone, - ConnectionLost, - DNSLookupError, - TCPTimedOutError, -) -from twisted.internet.error import ConnectionRefusedError as TxConnectionRefusedError -from twisted.internet.error import TimeoutError as TxTimeoutError -from twisted.web.client import ResponseFailed +from twisted.internet.error import ConnectError, ConnectionDone, ConnectionLost from scrapy.downloadermiddlewares.retry import RetryMiddleware, get_retry_request -from scrapy.exceptions import IgnoreRequest +from scrapy.exceptions import ( + CannotResolveHostError, + DownloadConnectionRefusedError, + DownloadTimeoutError, + IgnoreRequest, +) from scrapy.http import Request, Response from scrapy.settings.default_settings import RETRY_EXCEPTIONS from scrapy.spiders import Spider @@ -62,7 +57,7 @@ class TestRetry: def test_dont_retry_exc(self): req = Request("http://www.scrapytest.org/503", meta={"dont_retry": True}) - r = self.mw.process_exception(req, DNSLookupError()) + r = self.mw.process_exception(req, CannotResolveHostError()) assert r is None def test_503(self): @@ -94,12 +89,9 @@ class TestRetry: ConnectError, ConnectionDone, ConnectionLost, - TxConnectionRefusedError, - defer.TimeoutError, - DNSLookupError, - ResponseFailed, - TCPTimedOutError, - TxTimeoutError, + DownloadTimeoutError, + DownloadConnectionRefusedError, + CannotResolveHostError, ] for exc in exceptions: @@ -110,7 +102,7 @@ class TestRetry: assert stats.get_value("retry/max_reached") == len(exceptions) assert stats.get_value("retry/count") == len(exceptions) * 2 assert ( - stats.get_value("retry/reason_count/twisted.internet.defer.TimeoutError") + stats.get_value("retry/reason_count/scrapy.exceptions.DownloadTimeoutError") == 2 ) @@ -159,7 +151,7 @@ class TestMaxRetryTimes: req = Request(self.invalid_url) self._test_retry( req, - DNSLookupError("foo"), + CannotResolveHostError("foo"), max_retry_times, middleware=middleware, ) @@ -171,7 +163,7 @@ class TestMaxRetryTimes: req = Request(self.invalid_url, meta=meta) self._test_retry( req, - DNSLookupError("foo"), + CannotResolveHostError("foo"), max_retry_times, middleware=middleware, ) @@ -183,7 +175,7 @@ class TestMaxRetryTimes: req = Request(self.invalid_url) self._test_retry( req, - DNSLookupError("foo"), + CannotResolveHostError("foo"), max_retry_times, middleware=middleware, ) @@ -200,13 +192,13 @@ class TestMaxRetryTimes: self._test_retry( req1, - DNSLookupError("foo"), + CannotResolveHostError("foo"), meta_max_retry_times, middleware=middleware, ) self._test_retry( req2, - DNSLookupError("foo"), + CannotResolveHostError("foo"), middleware_max_retry_times, middleware=middleware, ) @@ -223,13 +215,13 @@ class TestMaxRetryTimes: self._test_retry( req1, - DNSLookupError("foo"), + CannotResolveHostError("foo"), meta_max_retry_times, middleware=middleware, ) self._test_retry( req2, - DNSLookupError("foo"), + CannotResolveHostError("foo"), middleware_max_retry_times, middleware=middleware, ) @@ -244,7 +236,7 @@ class TestMaxRetryTimes: req = Request(self.invalid_url, meta=meta) self._test_retry( req, - DNSLookupError("foo"), + CannotResolveHostError("foo"), 0, middleware=middleware, ) diff --git a/tests/test_downloadermiddleware_robotstxt.py b/tests/test_downloadermiddleware_robotstxt.py index bedf40279..1e99d5162 100644 --- a/tests/test_downloadermiddleware_robotstxt.py +++ b/tests/test_downloadermiddleware_robotstxt.py @@ -5,12 +5,11 @@ from typing import TYPE_CHECKING from unittest import mock import pytest -from twisted.internet import error from twisted.internet.defer import Deferred, DeferredList from twisted.python import failure from scrapy.downloadermiddlewares.robotstxt import RobotsTxtMiddleware -from scrapy.exceptions import IgnoreRequest, NotConfigured +from scrapy.exceptions import CannotResolveHostError, IgnoreRequest, NotConfigured from scrapy.http import Request, Response, TextResponse from scrapy.http.request import NO_CALLBACK from scrapy.settings import Settings @@ -165,7 +164,7 @@ Disallow: /some/randome/page.html @deferred_f_from_coro_f async def test_robotstxt_error(self, caplog: pytest.LogCaptureFixture) -> None: self.crawler.settings.set("ROBOTSTXT_OBEY", True) - err = error.DNSLookupError("Robotstxt address not found") + err = CannotResolveHostError("Robotstxt address not found") async def return_failure(request): deferred = Deferred() @@ -176,12 +175,12 @@ Disallow: /some/randome/page.html middleware = RobotsTxtMiddleware(self.crawler) await middleware.process_request(Request("http://site.local")) - assert "DNS lookup failed: Robotstxt address not found" in caplog.text + assert "Robotstxt address not found" in caplog.text @deferred_f_from_coro_f async def test_robotstxt_immediate_error(self): self.crawler.settings.set("ROBOTSTXT_OBEY", True) - err = error.DNSLookupError("Robotstxt address not found") + err = CannotResolveHostError("Robotstxt address not found") async def immediate_failure(request): raise err diff --git a/tests/test_http2_client_protocol.py b/tests/test_http2_client_protocol.py index dcc067ec9..46ef75064 100644 --- a/tests/test_http2_client_protocol.py +++ b/tests/test_http2_client_protocol.py @@ -12,14 +12,8 @@ from urllib.parse import urlencode import pytest from pytest_twisted import async_yield_fixture -from twisted.internet.defer import ( - CancelledError, - Deferred, - DeferredList, - inlineCallbacks, -) +from twisted.internet.defer import Deferred, DeferredList, inlineCallbacks from twisted.internet.endpoints import SSL4ClientEndpoint, SSL4ServerEndpoint -from twisted.internet.error import TimeoutError as TxTimeoutError from twisted.internet.ssl import Certificate, PrivateCertificate, optionsForClientTLS from twisted.web.client import URI, ResponseFailed from twisted.web.http import H2_ENABLED @@ -27,6 +21,7 @@ from twisted.web.http import Request as TxRequest from twisted.web.server import NOT_DONE_YET, Site from twisted.web.static import File +from scrapy.exceptions import DownloadCancelledError, DownloadTimeoutError from scrapy.http import JsonRequest, Request, Response from scrapy.settings import Settings from scrapy.spiders import Spider @@ -477,7 +472,7 @@ class TestHttps2ClientProtocol: url=self.get_url(server_port, "/get-data-html-large"), meta={"download_maxsize": 1000}, ) - with pytest.raises(CancelledError) as exc_info: + with pytest.raises(DownloadCancelledError) as exc_info: await make_request(client, request) error_pattern = re.compile( rf"Cancelling download of {request.url}: received response " @@ -732,7 +727,7 @@ class TestHttps2ClientProtocol: for err in exc_info.value.reasons: from scrapy.core.http2.protocol import H2ClientProtocol # noqa: PLC0415 - if isinstance(err, TxTimeoutError): + if isinstance(err, DownloadTimeoutError): assert ( f"Connection was IDLE for more than {H2ClientProtocol.IDLE_TIMEOUT}s" in str(err) diff --git a/tests/test_webclient.py b/tests/test_webclient.py index 963cb606e..50cab9a60 100644 --- a/tests/test_webclient.py +++ b/tests/test_webclient.py @@ -9,7 +9,6 @@ from urllib.parse import urlparse import OpenSSL.SSL import pytest from pytest_twisted import async_yield_fixture -from twisted.internet import defer from twisted.internet.defer import inlineCallbacks from twisted.internet.testing import StringTransport from twisted.protocols.policies import WrappingFactory @@ -18,6 +17,7 @@ from twisted.web.client import _makeGetterFactory from scrapy.core.downloader import webclient as client from scrapy.core.downloader.contextfactory import ScrapyClientContextFactory +from scrapy.exceptions import DownloadTimeoutError from scrapy.http import Headers, Request from scrapy.utils.misc import build_from_crawler from scrapy.utils.python import to_bytes, to_unicode @@ -302,9 +302,9 @@ class TestWebClient: """ When a non-zero timeout is passed to L{getPage} and that many seconds elapse before the server responds to the request. the - L{Deferred} is errbacked with a L{error.TimeoutError}. + L{Deferred} is errbacked with a L{DownloadTimeoutError}. """ - with pytest.raises(defer.TimeoutError): + with pytest.raises(DownloadTimeoutError): yield getPage(server_url + "wait", timeout=0.000001) # Clean up the server which is hanging around not doing # anything.