From b90842379d0716a2f0011245ef5a1d7b1a876f62 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 22 Apr 2026 17:04:32 +0200 Subject: [PATCH] Extend download error coverage --- docs/topics/download-handlers.rst | 6 ++ scrapy/core/downloader/handlers/_httpx.py | 95 +++++++++++++++++++-- scrapy/exceptions.py | 12 +++ scrapy/settings/default_settings.py | 3 + scrapy/utils/_download_handlers.py | 54 +++++++++++- tests/test_downloader_handlers_http_base.py | 23 +++++ tests/test_downloadermiddleware_retry.py | 6 ++ 7 files changed, 186 insertions(+), 13 deletions(-) diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 96cf46c35..426645675 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -118,10 +118,16 @@ these exceptions. .. autoexception:: scrapy.exceptions.DownloadCancelledError +.. autoexception:: scrapy.exceptions.DownloadConnectBindError + .. autoexception:: scrapy.exceptions.DownloadConnectionRefusedError .. autoexception:: scrapy.exceptions.DownloadFailedError +.. autoexception:: scrapy.exceptions.DownloadNoRouteError + +.. autoexception:: scrapy.exceptions.DownloadTCPTimedOutError + .. autoexception:: scrapy.exceptions.DownloadTimeoutError .. autoexception:: scrapy.exceptions.ResponseDataLossError diff --git a/scrapy/core/downloader/handlers/_httpx.py b/scrapy/core/downloader/handlers/_httpx.py index 313fb1c4f..a0c171860 100644 --- a/scrapy/core/downloader/handlers/_httpx.py +++ b/scrapy/core/downloader/handlers/_httpx.py @@ -2,8 +2,10 @@ from __future__ import annotations +import errno import ipaddress import logging +import socket import ssl import time from http.cookiejar import Cookie, CookieJar @@ -14,8 +16,11 @@ from scrapy import Request, signals from scrapy.exceptions import ( CannotResolveHostError, DownloadCancelledError, + DownloadConnectBindError, DownloadConnectionRefusedError, DownloadFailedError, + DownloadNoRouteError, + DownloadTCPTimedOutError, DownloadTimeoutError, NotConfigured, ResponseDataLossError, @@ -53,6 +58,82 @@ except ImportError: logger = logging.getLogger(__name__) +def _errno_values(*names: str) -> tuple[int, ...]: + return tuple( + value for name in names if (value := getattr(errno, name, None)) is not None + ) + + +_CONNECT_ERRNO_MAP: tuple[tuple[tuple[int, ...], type[Exception]], ...] = ( + (_errno_values("ECONNREFUSED", "WSAECONNREFUSED"), DownloadConnectionRefusedError), + ( + _errno_values( + "ENETUNREACH", + "EHOSTUNREACH", + "WSAENETUNREACH", + "WSAEHOSTUNREACH", + ), + DownloadNoRouteError, + ), + (_errno_values("ETIMEDOUT", "WSAETIMEDOUT"), DownloadTCPTimedOutError), + ( + _errno_values( + "EADDRINUSE", + "EADDRNOTAVAIL", + "WSAEADDRINUSE", + "WSAEADDRNOTAVAIL", + ), + DownloadConnectBindError, + ), +) + + +def _find_nested_os_error(exc: BaseException) -> OSError | None: + fallback_os_error: OSError | None = None + to_visit: list[BaseException] = [exc] + seen: set[int] = set() + while to_visit: + current = to_visit.pop() + if id(current) in seen: + continue + seen.add(id(current)) + if isinstance(current, OSError): + if isinstance(current, socket.gaierror) or current.errno is not None: + return current + if fallback_os_error is None: + fallback_os_error = current + + next_exceptions = [current.__cause__, current.__context__] + to_visit.extend( + next_exception + for next_exception in next_exceptions + if isinstance(next_exception, BaseException) + ) + + grouped_exceptions = getattr(current, "exceptions", None) + if isinstance(grouped_exceptions, tuple): + to_visit.extend( + nested_exception + for nested_exception in grouped_exceptions + if isinstance(nested_exception, BaseException) + ) + + return fallback_os_error + + +def _to_scrapy_connect_error(exc: httpx.ConnectError) -> Exception: + os_error = _find_nested_os_error(exc) + message = str(exc) + if os_error is None: + return DownloadFailedError(message) + if isinstance(os_error, socket.gaierror): + return CannotResolveHostError(message) + for errnos, scrapy_exc_cls in _CONNECT_ERRNO_MAP: + if os_error.errno in errnos: + return scrapy_exc_cls(message) + return DownloadFailedError(message) + + class _BaseResponseArgs(TypedDict): status: int url: str @@ -133,6 +214,10 @@ class HttpxDownloadHandler(BaseHttpDownloadHandler): async with self._get_httpx_response(request, timeout) as httpx_response: request.meta["download_latency"] = time.monotonic() - start_time return await self._read_response(httpx_response, request) + except httpx.ConnectTimeout as e: + raise DownloadTCPTimedOutError( + f"Connecting to {request.url} took longer than {timeout} seconds." + ) from e except httpx.TimeoutException as e: raise DownloadTimeoutError( f"Getting {request.url} took longer than {timeout} seconds." @@ -140,15 +225,7 @@ class HttpxDownloadHandler(BaseHttpDownloadHandler): except httpx.UnsupportedProtocol as e: raise UnsupportedURLSchemeError(str(e)) from e except httpx.ConnectError as e: - error_message = str(e) - if ( - "Name or service not known" in error_message - or "getaddrinfo failed" in error_message - or "nodename nor servname" in error_message - or "Temporary failure in name resolution" in error_message - ): - raise CannotResolveHostError(error_message) from e - raise DownloadConnectionRefusedError(str(e)) from e + raise _to_scrapy_connect_error(e) from e except httpx.NetworkError as e: raise DownloadFailedError(str(e)) from e except httpx.RemoteProtocolError as e: diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index 204132973..c8dbcbc18 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -63,10 +63,22 @@ class DownloadConnectionRefusedError(Exception): """Indicates that a connection was refused by the server.""" +class DownloadConnectBindError(Exception): + """Indicates that binding to a local address failed.""" + + class CannotResolveHostError(Exception): """Indicates that the provided hostname cannot be resolved.""" +class DownloadNoRouteError(Exception): + """Indicates that no route to the remote host exists.""" + + +class DownloadTCPTimedOutError(Exception): + """Indicates that establishing a TCP connection timed out.""" + + class DownloadTimeoutError(Exception): """Indicates that a request download has timed out.""" diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index b80e48601..ccfac2671 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -458,8 +458,11 @@ REQUEST_FINGERPRINTER_CLASS = "scrapy.utils.request.RequestFingerprinter" RETRY_ENABLED = True RETRY_EXCEPTIONS = [ "scrapy.exceptions.CannotResolveHostError", + "scrapy.exceptions.DownloadConnectBindError", "scrapy.exceptions.DownloadConnectionRefusedError", "scrapy.exceptions.DownloadFailedError", + "scrapy.exceptions.DownloadNoRouteError", + "scrapy.exceptions.DownloadTCPTimedOutError", "scrapy.exceptions.DownloadTimeoutError", "scrapy.exceptions.ResponseDataLossError", "twisted.internet.error.ConnectionDone", diff --git a/scrapy/utils/_download_handlers.py b/scrapy/utils/_download_handlers.py index 9538dd81e..7a263471a 100644 --- a/scrapy/utils/_download_handlers.py +++ b/scrapy/utils/_download_handlers.py @@ -7,9 +7,14 @@ from contextlib import contextmanager from typing import TYPE_CHECKING, Any from twisted.internet.defer import CancelledError +from twisted.internet.error import ConnectBindError as TxConnectBindError +from twisted.internet.error import ConnectError as TxConnectError from twisted.internet.error import ConnectionRefusedError as TxConnectionRefusedError from twisted.internet.error import DNSLookupError +from twisted.internet.error import NoRouteError as TxNoRouteError +from twisted.internet.error import TCPTimedOutError as TxTCPTimedOutError from twisted.internet.error import TimeoutError as TxTimeoutError +from twisted.internet.error import UnknownHostError as TxUnknownHostError from twisted.python.failure import Failure from twisted.web.client import ResponseFailed from twisted.web.error import SchemeNotSupported @@ -19,8 +24,11 @@ from scrapy.core.downloader.handlers.base import BaseDownloadHandler from scrapy.exceptions import ( CannotResolveHostError, DownloadCancelledError, + DownloadConnectBindError, DownloadConnectionRefusedError, DownloadFailedError, + DownloadNoRouteError, + DownloadTCPTimedOutError, DownloadTimeoutError, StopDownload, UnsupportedURLSchemeError, @@ -54,6 +62,44 @@ class BaseHttpDownloadHandler(BaseDownloadHandler, ABC): self._fail_on_dataloss_warned: bool = False +_TWISTED_CONNECT_ERROR_MAP: tuple[ + tuple[type[TxConnectError], type[Exception]], + ..., +] = ( + (TxConnectionRefusedError, DownloadConnectionRefusedError), + (TxConnectBindError, DownloadConnectBindError), + (TxUnknownHostError, CannotResolveHostError), + (TxNoRouteError, DownloadNoRouteError), + (TxTCPTimedOutError, DownloadTCPTimedOutError), + (TxTimeoutError, DownloadTimeoutError), +) + + +def _map_twisted_connect_exception(exc: TxConnectError) -> Exception: + message = str(exc) + for twisted_exc_cls, scrapy_exc_cls in _TWISTED_CONNECT_ERROR_MAP: + if isinstance(exc, twisted_exc_cls): + return scrapy_exc_cls(message) + return DownloadFailedError(message) + + +def _map_response_failed_exception(exc: ResponseFailed) -> Exception | None: + mapped_exception: Exception | None = None + for reason in exc.reasons: + if isinstance(reason.value, TxConnectError): + current_exception: Exception = _map_twisted_connect_exception(reason.value) + elif isinstance(reason.value, DNSLookupError): + current_exception = CannotResolveHostError(str(reason.value)) + else: + return None + if mapped_exception is None: + mapped_exception = current_exception + continue + if type(mapped_exception) is not type(current_exception): + return None + return mapped_exception + + @contextmanager def wrap_twisted_exceptions() -> Iterator[None]: """Context manager that wraps Twisted exceptions into Scrapy exceptions.""" @@ -63,14 +109,14 @@ def wrap_twisted_exceptions() -> Iterator[None]: 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 TxConnectError as e: + raise _map_twisted_connect_exception(e) from e except ResponseFailed as e: + if mapped_exception := _map_response_failed_exception(e): + raise mapped_exception from e raise DownloadFailedError(str(e)) from e - except TxTimeoutError as e: - raise DownloadTimeoutError(str(e)) from e def check_stop_download( diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index 59b6d5754..2724958c9 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -6,6 +6,7 @@ import gzip import json import platform import re +import socket import sys from abc import ABC, abstractmethod from contextlib import asynccontextmanager @@ -22,6 +23,7 @@ from twisted.python.failure import Failure from scrapy.exceptions import ( CannotResolveHostError, DownloadCancelledError, + DownloadConnectBindError, DownloadConnectionRefusedError, DownloadFailedError, DownloadTimeoutError, @@ -523,6 +525,15 @@ class TestHttpBase(ABC): class TestHttp11Base(TestHttpBase): """HTTP 1.1 test case""" + @staticmethod + def _system_allows_nonlocal_bind() -> bool: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + try: + sock.bind(("198.51.100.1", 0)) + except OSError: + return False + return True + @coroutine_test async def test_download_without_maxsize_limit(self, mockserver: MockServer) -> None: request = Request(mockserver.url("/text", is_secure=self.is_secure)) @@ -748,6 +759,18 @@ class TestHttp11Base(TestHttpBase): response = await download_handler.download_request(request) assert response.body == b"127.0.0.2" + @coroutine_test + async def test_download_bind_address_nonlocal(self, mockserver: MockServer) -> None: + if self._system_allows_nonlocal_bind(): + pytest.skip("This system allows binding to non-local addresses") + + request = Request(mockserver.url("/text", is_secure=self.is_secure)) + async with self.get_dh( + {"DOWNLOAD_BIND_ADDRESS": ("198.51.100.1", 0)} + ) as download_handler: + with pytest.raises(DownloadConnectBindError): + await download_handler.download_request(request) + @pytest.mark.skipif( sys.platform == "darwin", reason="127.0.0.2 is not available on macOS by default", diff --git a/tests/test_downloadermiddleware_retry.py b/tests/test_downloadermiddleware_retry.py index 50946899a..b9fa0152f 100644 --- a/tests/test_downloadermiddleware_retry.py +++ b/tests/test_downloadermiddleware_retry.py @@ -7,7 +7,10 @@ from twisted.internet.error import ConnectError, ConnectionDone, ConnectionLost from scrapy.downloadermiddlewares.retry import RetryMiddleware, get_retry_request from scrapy.exceptions import ( CannotResolveHostError, + DownloadConnectBindError, DownloadConnectionRefusedError, + DownloadNoRouteError, + DownloadTCPTimedOutError, DownloadTimeoutError, IgnoreRequest, ) @@ -90,8 +93,11 @@ class TestRetry: ConnectionDone, ConnectionLost, DownloadTimeoutError, + DownloadTCPTimedOutError, DownloadConnectionRefusedError, + DownloadConnectBindError, CannotResolveHostError, + DownloadNoRouteError, ] for exc in exceptions: