From f5bf9642d80ad20bdff2d79130346f510bc35da4 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 10 Aug 2026 11:31:03 +0200 Subject: [PATCH] Fail HTTP/2 requests when the connection cannot be established --- scrapy/core/http2/agent.py | 37 ++++++++++++++----- .../test_downloader_handler_twisted_http2.py | 16 -------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index 0f04c1862..8850a512d 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -48,11 +48,7 @@ class H2ConnectionPool: ) -> Deferred[H2ClientProtocol]: if key in self._pending_requests: # Received a request while connecting to remote - # Create a deferred which will fire with the H2ClientProtocol - # instance - d: Deferred[H2ClientProtocol] = Deferred() - self._pending_requests[key].append(d) - return d + return self._pending_request(key) # Check if we already have a usable connection to the remote, moving # it to the end so that the pool stays ordered from least to most @@ -71,8 +67,7 @@ class H2ConnectionPool: self, key: ConnectionKeyT, uri: URI, endpoint: HostnameEndpoint ) -> Deferred[H2ClientProtocol]: self._enforce_limit() - pending_requests: deque[Deferred[H2ClientProtocol]] = deque() - self._pending_requests[key] = pending_requests + self._pending_requests[key] = deque() conn_lost_deferred: Deferred[list[BaseException]] = Deferred() @@ -83,9 +78,25 @@ class H2ConnectionPool: tls_verbose_logging=self._tls_verbose_logging, ) conn_d = endpoint.connect(factory) - conn_d.addCallback(self.put_connection, key, conn_lost_deferred) + d = self._pending_request(key) + conn_d.addCallbacks( + self.put_connection, + self._connection_failed, + callbackArgs=(key, conn_lost_deferred), + errbackArgs=(key,), + ) + return d - d: Deferred[H2ClientProtocol] = Deferred() + def _pending_request(self, key: ConnectionKeyT) -> Deferred[H2ClientProtocol]: + """Return a deferred that fires with the connection being established + for *key*. + + Cancelling it takes it out of the queue, so that the connection, once + established or found unreachable, only fires the deferreds of the + requests that are still waiting for it. + """ + pending_requests = self._pending_requests[key] + d: Deferred[H2ClientProtocol] = Deferred(pending_requests.remove) pending_requests.append(d) return d @@ -127,6 +138,14 @@ class H2ConnectionPool: return conn + def _connection_failed(self, failure: Failure, key: ConnectionKeyT) -> None: + """Fail the requests waiting for a connection that could not be + established, and let a later request try to connect again.""" + pending_requests = self._pending_requests.pop(key) + while pending_requests: + d = pending_requests.popleft() + d.errback(failure) + def _remove_connection( self, errors: list[BaseException], key: ConnectionKeyT, conn: H2ClientProtocol ) -> None: diff --git a/tests/test_downloader_handler_twisted_http2.py b/tests/test_downloader_handler_twisted_http2.py index 3f6d1adb1..f14545b7e 100644 --- a/tests/test_downloader_handler_twisted_http2.py +++ b/tests/test_downloader_handler_twisted_http2.py @@ -82,22 +82,6 @@ class TestHttp2(H2DownloadHandlerMixin, TestHttpsBase): response = await download_handler.download_request(request) assert response.protocol == "h2" - 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_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") - @coroutine_test async def test_concurrent_requests_same_domain( self, mockserver: MockServer