diff --git a/docs/examples/code_examples/file_download_stream.py b/docs/examples/code_examples/file_download_stream.py index 82f1b2018a..1d4801b1e8 100644 --- a/docs/examples/code_examples/file_download_stream.py +++ b/docs/examples/code_examples/file_download_stream.py @@ -14,9 +14,9 @@ async def main() -> None: # been read yet. crawler = FileDownloadCrawler( stream=True, - # Bounds establishing the connection and receiving the response headers. + # Bounds the whole download: the connection, the response headers and the body. navigation_timeout=timedelta(minutes=5), - # The body is downloaded inside the handler, so this bounds the transfer itself. + # The body is downloaded inside the handler, so this bounds the transfer as well. request_handler_timeout=timedelta(minutes=5), # Limit the crawl to max requests. Remove or increase it for crawling all links. max_requests_per_crawl=10, diff --git a/docs/guides/code_examples/http_clients/parsel_httpx_example.py b/docs/guides/code_examples/http_clients/parsel_httpx_example.py index 06d6ccdd1d..9c48a671ae 100644 --- a/docs/guides/code_examples/http_clients/parsel_httpx_example.py +++ b/docs/guides/code_examples/http_clients/parsel_httpx_example.py @@ -8,7 +8,6 @@ async def main() -> None: http_client = HttpxHttpClient( # Optional additional keyword arguments for `httpx2.AsyncClient`. timeout=10, - follow_redirects=True, ) crawler = ParselCrawler( diff --git a/docs/guides/extending_crawlee.mdx b/docs/guides/extending_crawlee.mdx index 6e4a3d3977..554e230b8b 100644 --- a/docs/guides/extending_crawlee.mdx +++ b/docs/guides/extending_crawlee.mdx @@ -132,11 +132,11 @@ An HTTP client performs network calls for crawlers. Swapping it changes the tran The contract is `HttpClient`: -- `crawl` performs a request inside the crawler's pipeline and returns the result the crawler consumes. -- `send_request` covers standalone calls made from a handler. -- `stream` yields a response you read incrementally. +- `_fetch` sends a single request and returns the response with its body still unread. It doesn't follow redirects or keep cookies. - `cleanup` releases whatever the client holds open. +The base class builds `send_request` on top of `_fetch`. The crawler's pipeline and the handlers both call it. It follows redirects and sends and stores the session cookies, so every client handles them the same way. + Crawlee ships `ImpitHttpClient`, `HttpxHttpClient`, and `CurlImpersonateHttpClient`. See the [HTTP clients guide](./http-clients) for the libraries these clients wrap and their installation requirements. diff --git a/docs/guides/http_clients.mdx b/docs/guides/http_clients.mdx index f79fe1941d..09e20aaeb4 100644 --- a/docs/guides/http_clients.mdx +++ b/docs/guides/http_clients.mdx @@ -101,15 +101,14 @@ python -m pip install 'crawlee[all]' Crawlee provides an abstract base class, `HttpClient`, which defines the interface that all HTTP clients must implement. This allows you to create custom HTTP clients tailored to your specific requirements. -HTTP clients are responsible for several key operations: +To create a custom HTTP client, inherit from `HttpClient` and implement two methods: -- sending HTTP requests and receiving responses, -- managing cookies and sessions, -- handling headers and authentication, -- managing proxy configurations, -- connection pooling with timeout management. +- `_fetch` sends a single request, through the proxy from `proxy_info` if there is one, and returns the response without reading its body. It doesn't follow redirects or keep cookies of its own. Its `timeout` bounds both receiving the response and reading the body, and without one the default timeout of the underlying library applies. +- `cleanup` closes the connections and other resources the client holds. -To create a custom HTTP client, you need to inherit from the `HttpClient` base class and implement all required abstract methods. Your implementation must be async-compatible and include proper cleanup and resource management to work seamlessly with Crawlee's concurrent processing model. +The response is a subclass of the abstract `HttpResponse` class. Its body is read on `read` or `read_stream`, and `aclose` releases the connection when the body isn't needed. The base class provides the async context manager, which closes the response on exit. + +The base class provides `send_request` on top of `_fetch`. It sends the cookies of the session with every request, stores the cookies the server sets, and follows up to 10 redirects unless the client is created with `follow_redirects=False`. A custom client therefore handles redirects and cookies the same way as the built-in ones. ## Conclusion diff --git a/docs/guides/secure_scraping.mdx b/docs/guides/secure_scraping.mdx index 3cfa3097d6..2f04ba18e2 100644 --- a/docs/guides/secure_scraping.mdx +++ b/docs/guides/secure_scraping.mdx @@ -63,7 +63,7 @@ If only your crawler's storage sees the response, the same request is a *blind* A hostile site can try to use up your CPU, memory, or queue rather than read anything: - A *crawler trap* is a maze of randomly generated URLs linking to more generated pages. It's built to keep a crawler running forever and grow its queue without bound. Bound how far link-following goes with `max_crawl_depth`, and cap a whole run with `max_requests_per_crawl`. Both are unlimited by default, so set them yourself. -- A single *oversized or slow response* can tie up a worker or blow up memory. Both the fetch and the handler are capped at 60 seconds by default. `navigation_timeout` covers the fetch and is passed straight to the HTTP client. `request_handler_timeout` covers the handler. Raise them if a legitimate target needs longer, but keep the cap as tight as your workload allows. +- A single *oversized or slow response* can tie up a worker or blow up memory. Both the fetch and the handler are capped at 60 seconds by default. `navigation_timeout` covers the fetch, including reading the body, and is passed straight to the HTTP client. `request_handler_timeout` covers the handler. Raise them if a legitimate target needs longer, but keep the cap as tight as your workload allows. - A *decompression bomb* is an archive that expands to gigabytes when unpacked. If your crawler downloads and extracts archives, cap the output stream during extraction. The declared size is part of the attack and easy to forge. ## Untrusted content diff --git a/docs/upgrading/upgrading_to_v2.md b/docs/upgrading/upgrading_to_v2.md index 032cff0100..4a86c5571b 100644 --- a/docs/upgrading/upgrading_to_v2.md +++ b/docs/upgrading/upgrading_to_v2.md @@ -231,6 +231,79 @@ A `None` from `fetch_next_request` no longer means the loader is finished. It on The crawler now reads a stall from the status, so it also sees a `ThrottlingRequestManager` wrapped in a `RequestManagerTandem`. Such a crawl now ends with `PersistentRateLimitError` once a domain stalls, as it already did with a `ThrottlingRequestManager` passed directly. Before, the crawler didn't detect the stall. +## HTTP clients + +`HttpClient` follows the design of Crawlee JS v4. A client sends a single request, and the base class handles redirects and cookies the same way for every client. + +### Responses are read lazily + +`HttpClient.crawl` and `HttpClient.stream` are removed. `send_request` returns the response as soon as its headers arrive and reads the body only on `read` or `read_stream`. A response whose body you don't read keeps its connection until you call `aclose`. Use the response as an async context manager to close it: + +```python +# v1 +async with http_client.stream(url) as response: + async for chunk in response.read_stream(): + ... + +# v2 +async with await http_client.send_request(url) as response: + async for chunk in response.read_stream(): + ... +``` + +Responses returned by `context.send_request` are closed after the request is processed, so a request handler doesn't need to close them. + +### `timeout` covers the redirects and the body + +The `timeout` of `send_request` is one deadline for the whole redirect chain and for reading the body, the same as Crawlee JS. A body read past it raises `TimeoutError`. Without a `timeout`, the default of the underlying library applies. It's 30 seconds for `ImpitHttpClient`, including reading the body, and for `CurlImpersonateHttpClient`, and 5 seconds per operation for `HttpxHttpClient`. In v1, `ImpitHttpClient` and `CurlImpersonateHttpClient` disabled the timeout altogether in that case. To change the default, pass `timeout` when you create the client. + +The crawlers pass the remaining `navigation_timeout` to the HTTP client, so it bounds reading the body too. With `FileDownloadCrawler(stream=True)`, the deadline also covers the body streamed in the request handler, with every HTTP client. In v1, only `ImpitHttpClient` bounded the streamed body, while `HttpxHttpClient` and `CurlImpersonateHttpClient` bounded only the connection. For long downloads, raise `navigation_timeout` together with `request_handler_timeout`. + +### `send_request` follows at most 10 redirects + +`send_request` follows up to 10 redirects and raises `RuntimeError` when a server redirects more often. The limit isn't configurable. In v1, each client raised the `TooManyRedirects` error of its library instead, after 20 redirects with `ImpitHttpClient` and `HttpxHttpClient` and after 30 with `CurlImpersonateHttpClient`. Every client now takes `follow_redirects`. With `follow_redirects=False`, `send_request` returns the first response as it is. The redirect options of the underlying libraries are ignored with a warning. For the list, see [Some client arguments are ignored](#some-client-arguments-are-ignored). + +### Client headers follow the redirect rules + +The `headers` argument of a client still applies to every request. Its `Authorization`, `Proxy-Authorization` and `Cookie` headers are dropped when a redirect leaves the origin, the same as the headers passed with a request. With `HttpxHttpClient`, the headers of the client now win over the generated browser headers. In v1, the generated browser headers replaced them. + +### Some client arguments are ignored + +The following keyword arguments of the underlying libraries are now ignored with a warning: + +- `cookies` of every client and `cookie_jar` of `ImpitHttpClient`, since cookies are kept in sessions. +- `auth` and `params` of `HttpxHttpClient` and `CurlImpersonateHttpClient`, which would reach every request of a redirect chain. Pass the credentials in the `Authorization` header and the query in the URL instead. +- `max_redirects` of every client and `allow_redirects` of `CurlImpersonateHttpClient`, since `send_request` follows the redirects. + +### Custom clients implement `_fetch` + +A custom client implements `_fetch` and `cleanup` instead of `crawl`, `send_request` and `stream`. `_fetch` sends one request and returns the response without following redirects or keeping cookies. The client inherits `send_request` from `HttpClient`. + +For example: + +```python +# v1 +class MyHttpClient(HttpClient): + async def crawl(self, request, **kwargs): ... + + async def send_request(self, url, **kwargs): ... + + def stream(self, url, **kwargs): ... + + async def cleanup(self): ... + + +# v2 +class MyHttpClient(HttpClient): + async def _fetch(self, url, *, method, headers, payload, proxy_info, timeout): ... + + async def cleanup(self): ... +``` + +`HttpClient.__init__` is no longer abstract, so a custom client doesn't have to define it. A custom `__init__` must call `super().__init__()`, because the base `__init__` stores the `follow_redirects` and `headers` options that `send_request` reads. + +`HttpResponse` is now an abstract base class instead of a protocol, so a custom response subclasses it. It has three new abstract members, `url`, `headers_set_cookie` and `aclose`, and works as an async context manager that closes the response. + ## Logs changed If you configure loggers by name or match log messages, update them for the following changes. diff --git a/src/crawlee/_types.py b/src/crawlee/_types.py index b973e6b2e2..e7b02eb293 100644 --- a/src/crawlee/_types.py +++ b/src/crawlee/_types.py @@ -566,7 +566,9 @@ class SendRequestFunction(Protocol): """A function for sending HTTP requests. It simplifies the process of sending HTTP requests. It is implemented by the crawling context and is used - within request handlers to send additional HTTP requests to target URLs. + within request handlers to send additional HTTP requests to target URLs. The body of the response is not read yet. + The response is closed once the request is processed, and until then it stays in memory with the body read by + `read`, so a handler that sends many requests keeps all of their responses. """ def __call__( @@ -585,7 +587,8 @@ def __call__( method: The HTTP method to use. headers: The headers to include in the request. payload: The payload to include in the request. - timeout: Maximum time allowed to process the request. + timeout: Maximum time allowed to receive the response, including all redirects, and to read its body. + Without it, the default timeout of the HTTP client applies. Returns: The HTTP response received from the server. diff --git a/src/crawlee/_utils/http.py b/src/crawlee/_utils/http.py index 3c240c7d6a..d2f791df89 100644 --- a/src/crawlee/_utils/http.py +++ b/src/crawlee/_utils/http.py @@ -2,13 +2,25 @@ from __future__ import annotations +import asyncio import re +from contextlib import suppress from datetime import UTC, datetime, timedelta from email.utils import parsedate_to_datetime from logging import getLogger +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from crawlee.http_clients import HttpResponse logger = getLogger(__name__) +_MAX_DRAINED_BODY_SIZE = 64 * 1024 +"""The largest unread body that `drain_small_body` reads.""" + +_MAX_DRAIN_SECONDS = 1 +"""How long `drain_small_body` waits for the body.""" + _CHARSET_PATTERN = re.compile(r'(?:^|;)\s*charset\s*=\s*(?:"([^"]*)"|([^;\s]*))', re.IGNORECASE) @@ -66,3 +78,16 @@ def parse_retry_after_header(value: str | None) -> timedelta | None: pass return None + + +async def drain_small_body(response: HttpResponse) -> None: + """Read the body of a response known to be small, so that closing the response keeps its HTTP/1.1 connection. + + `httpx2` and `impit` drop the connection of a response closed before its body arrives. + """ + content_length = response.headers.get('content-length', '') + if content_length.isdigit() and int(content_length) <= _MAX_DRAINED_BODY_SIZE: + # If reading fails or takes too long, closing the response drops the connection anyway. + with suppress(Exception): + async with asyncio.timeout(_MAX_DRAIN_SECONDS): + await response.read() diff --git a/src/crawlee/_utils/robots.py b/src/crawlee/_utils/robots.py index 68a2ff59f2..c066755500 100644 --- a/src/crawlee/_utils/robots.py +++ b/src/crawlee/_utils/robots.py @@ -57,15 +57,18 @@ async def load(cls, url: str, http_client: HttpClient, proxy_info: ProxyInfo | N try: response = await http_client.send_request(url, proxy_info=proxy_info) - status_code = response.status_code - if is_status_code_client_error(status_code): - # A 4xx response means the file is unavailable, so crawling is unrestricted (RFC 9309, section 2.3.1.3). - body = b'User-agent: *\nAllow: /' - elif is_status_code_server_error(status_code): - # A 5xx response means the file is unreachable, so assume complete disallow (RFC 9309, section 2.3.1.4). - body = b'User-agent: *\nDisallow: /' - else: - body = await response.read() + async with response: + status_code = response.status_code + if is_status_code_client_error(status_code): + # A 4xx response means the file is unavailable, so crawling is unrestricted (RFC 9309, section + # 2.3.1.3). + body = b'User-agent: *\nAllow: /' + elif is_status_code_server_error(status_code): + # A 5xx response means the file is unreachable, so assume complete disallow (RFC 9309, section + # 2.3.1.4). + body = b'User-agent: *\nDisallow: /' + else: + body = await response.read() robots = Protego.parse(body.decode('utf-8')) except Exception as e: diff --git a/src/crawlee/_utils/sitemap.py b/src/crawlee/_utils/sitemap.py index 8a565619a1..35eba444bb 100644 --- a/src/crawlee/_utils/sitemap.py +++ b/src/crawlee/_utils/sitemap.py @@ -400,9 +400,10 @@ async def _fetch_and_process_sitemap( while retries_left > 0: retries_left -= 1 try: - async with http_client.stream( + response = await http_client.send_request( sitemap_url, method='GET', headers=SITEMAP_HEADERS, proxy_info=proxy_info, timeout=timeout - ) as response: + ) + async with response: status_code = response.status_code if not _is_successful_sitemap_status(status_code): # Retryable statuses raise and route to the retry handler below; the rest skip this source. @@ -740,6 +741,8 @@ def _check_and_add(url: str) -> bool: response = await http_client.send_request( candidate, method=method_for_checking, proxy_info=proxy_info, timeout=request_timeout ) + # Only the status code is checked, the body is never read. + await response.aclose() if is_status_code_successful(response.status_code) and _check_and_add(candidate): yield candidate except ProxyError: diff --git a/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py b/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py index aa7f03ba3c..5f189b5b97 100644 --- a/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py +++ b/src/crawlee/crawlers/_abstract_http/_abstract_http_crawler.py @@ -123,6 +123,7 @@ def _create_static_content_crawler_pipeline(self) -> ContextPipeline[ParsedHttpC .compose(self._make_http_request) .compose(self._execute_post_navigation_hooks) .compose(self._handle_status_code_response) + .compose(self._read_http_response) .compose(self._parse_http_response) .compose(self._handle_blocked_request_by_content) ) @@ -280,16 +281,30 @@ async def _make_http_request(self, context: BasicCrawlingContext) -> AsyncGenera The original crawling context enhanced by HTTP response. """ async with self._shared_navigation_timeouts[id(context.request)] as remaining_timeout: - result = await self._http_client.crawl( - request=context.request, + response = await self._http_client.send_request( + context.request.url, + method=context.request.method, + headers=context.request.headers, + payload=context.request.payload, session=context.session, proxy_info=context.proxy_info, - statistics=self._statistics, timeout=remaining_timeout, ) - context.request.state = RequestState.AFTER_NAV - yield HttpCrawlingContext.from_basic_crawling_context(context=context, http_response=result.http_response) + try: + self._statistics.register_status_code(response.status_code) + context.request.loaded_url = response.url + context.request.state = RequestState.AFTER_NAV + yield HttpCrawlingContext.from_basic_crawling_context(context=context, http_response=response) + finally: + await response.aclose() + + async def _read_http_response(self, context: HttpCrawlingContext) -> AsyncGenerator[HttpCrawlingContext, None]: + """Read the body of the HTTP response within the navigation timeout.""" + async with self._shared_navigation_timeouts[id(context.request)]: + await context.http_response.read() + + yield context async def _handle_status_code_response( self, context: HttpCrawlingContext diff --git a/src/crawlee/crawlers/_basic/_basic_crawler.py b/src/crawlee/crawlers/_basic/_basic_crawler.py index aeb8b9e158..60d1bfa621 100644 --- a/src/crawlee/crawlers/_basic/_basic_crawler.py +++ b/src/crawlee/crawlers/_basic/_basic_crawler.py @@ -86,6 +86,7 @@ from crawlee._types import ( ConcurrencySettings, + DeferredCleanupCallback, EnqueueLinksFunction, ExtractLinksFunction, GetDataKwargs, @@ -1402,6 +1403,7 @@ def _prepare_send_request_function( self, session: Session | None, proxy_info: ProxyInfo | None, + register_deferred_cleanup: Callable[[DeferredCleanupCallback], None], ) -> SendRequestFunction: async def send_request( url: str, @@ -1411,7 +1413,7 @@ async def send_request( headers: HttpHeaders | dict[str, str] | None = None, timeout: timedelta | None = None, ) -> HttpResponse: - return await self._http_client.send_request( + response = await self._http_client.send_request( url=url, method=method, payload=payload, @@ -1420,6 +1422,9 @@ async def send_request( proxy_info=proxy_info, timeout=timeout, ) + # A response whose body the handler never read would hold its connection open. + register_deferred_cleanup(response.aclose) + return response return send_request @@ -1569,7 +1574,7 @@ async def __run_task_function(self, task_id: str) -> None: # noqa: ARG002 request=request, session=session, proxy_info=proxy_info, - send_request=self._prepare_send_request_function(session, proxy_info), + send_request=self._prepare_send_request_function(session, proxy_info, deferred_cleanup.append), add_requests=result.add_requests, push_data=result.push_data, get_key_value_store=result.get_key_value_store, diff --git a/src/crawlee/crawlers/_file_download/_file_download_crawler.py b/src/crawlee/crawlers/_file_download/_file_download_crawler.py index 4df3cd365f..28474914f2 100644 --- a/src/crawlee/crawlers/_file_download/_file_download_crawler.py +++ b/src/crawlee/crawlers/_file_download/_file_download_crawler.py @@ -1,12 +1,9 @@ from __future__ import annotations -from contextlib import AsyncExitStack from typing import TYPE_CHECKING -from crawlee._request import RequestState from crawlee._utils.docs import docs_group from crawlee.crawlers._abstract_http import AbstractHttpCrawler -from crawlee.crawlers._abstract_http._http_crawling_context import HttpCrawlingContext from crawlee.crawlers._basic import ContextPipeline from crawlee.crawlers._http._http_parser import NoParser @@ -16,8 +13,8 @@ from collections.abc import AsyncGenerator from typing import Unpack - from crawlee._types import BasicCrawlingContext from crawlee.crawlers._abstract_http import HttpCrawlerOptions + from crawlee.crawlers._abstract_http._http_crawling_context import HttpCrawlingContext @docs_group('Crawlers') @@ -32,7 +29,8 @@ class FileDownloadCrawler(AbstractHttpCrawler[FileDownloadCrawlingContext, bytes `await context.http_response.read()`. For large files, construct the crawler with `stream=True`. The request handler then receives a response whose body has not been read yet and can consume it in chunks via `context.http_response.read_stream()`. The connection stays open while the handler runs and is closed - afterwards. A handler that returns without consuming the body just skips the download. + afterwards. A handler that returns without consuming the body just skips the download. In both modes, reading + the body counts towards `navigation_timeout`, so raise it for long downloads. ### Usage @@ -108,47 +106,20 @@ def __init__( def _create_file_download_pipeline(self) -> ContextPipeline[FileDownloadCrawlingContext]: """Create the file download context pipeline with expected pipeline steps.""" - make_http_request = self._make_http_stream_request if self._stream_response_body else self._make_http_request - - return ( + pipeline = ( ContextPipeline() .compose(self._manage_shared_navigation_timeout) .compose(self._execute_pre_navigation_hooks) - .compose(make_http_request) + .compose(self._make_http_request) .compose(self._execute_post_navigation_hooks) .compose(self._handle_status_code_response) - .compose(self._to_file_download_crawling_context) ) - async def _make_http_stream_request( - self, context: BasicCrawlingContext - ) -> AsyncGenerator[HttpCrawlingContext, None]: - """Make a streamed HTTP request and create context enhanced by the streamed HTTP response. - - Args: - context: The current crawling context. + # In stream mode the request handler consumes the body. + if not self._stream_response_body: + pipeline = pipeline.compose(self._read_http_response) - Yields: - The original crawling context enhanced by the streamed HTTP response. - """ - async with AsyncExitStack() as exit_stack: - async with self._shared_navigation_timeouts[id(context.request)] as remaining_timeout: - response = await exit_stack.enter_async_context( - self._http_client.stream( - url=context.request.url, - method=context.request.method, - headers=context.request.headers, - payload=context.request.payload, - session=context.session, - proxy_info=context.proxy_info, - timeout=remaining_timeout, - ) - ) - - self._statistics.register_status_code(response.status_code) - context.request.state = RequestState.AFTER_NAV - - yield HttpCrawlingContext.from_basic_crawling_context(context=context, http_response=response) + return pipeline.compose(self._to_file_download_crawling_context) async def _to_file_download_crawling_context( self, context: HttpCrawlingContext diff --git a/src/crawlee/crawlers/_playwright/_playwright_http_client.py b/src/crawlee/crawlers/_playwright/_playwright_http_client.py index 70b9d25f3b..be8bef2596 100644 --- a/src/crawlee/crawlers/_playwright/_playwright_http_client.py +++ b/src/crawlee/crawlers/_playwright/_playwright_http_client.py @@ -1,15 +1,15 @@ from __future__ import annotations import contextvars -from contextlib import AbstractAsyncContextManager, asynccontextmanager +from contextlib import asynccontextmanager from typing import TYPE_CHECKING from typing_extensions import override -from crawlee._types import HttpHeaders -from crawlee._utils.urls import validate_http_url from crawlee.crawlers._playwright._types import PlaywrightHttpResponse -from crawlee.http_clients import HttpClient, HttpCrawlingResult, HttpResponse +from crawlee.http_clients import HttpClient +from crawlee.sessions import Session +from crawlee.sessions._cookies import PlaywrightCookieParam if TYPE_CHECKING: from collections.abc import AsyncGenerator @@ -17,11 +17,9 @@ from playwright.async_api import Page - from crawlee import Request - from crawlee._types import HttpMethod, HttpPayload + from crawlee._types import HttpHeaders, HttpMethod, HttpPayload + from crawlee.http_clients import HttpResponse from crawlee.proxy_configuration import ProxyInfo - from crawlee.sessions import Session - from crawlee.statistics import Statistics _browser_page_context_var: contextvars.ContextVar[Page | None] = contextvars.ContextVar('browser_context', default=None) @@ -48,22 +46,6 @@ class PlaywrightHttpClient(HttpClient): Note: This class is pre-designated for use in `PlaywrightCrawler` only """ - def __init__(self) -> None: - """Initialize a new instance.""" - self._active = False - - @override - async def crawl( - self, - request: Request, - *, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - statistics: Statistics | None = None, - timeout: timedelta | None = None, - ) -> HttpCrawlingResult: - raise NotImplementedError('The `crawl` method should not be used for `PlaywrightHttpClient`') - @override async def send_request( self, @@ -76,57 +58,61 @@ async def send_request( proxy_info: ProxyInfo | None = None, timeout: timedelta | None = None, ) -> HttpResponse: - # `proxy_info` are not used because `APIRequestContext` inherits the proxy from `BrowserContext` # TODO: Use `session` to restore all the fingerprint headers according to the `BrowserContext`, after resolved # https://github.com/apify/crawlee-python/issues/1055 - validate_http_url(url) - - if isinstance(headers, dict) or headers is None: - headers = HttpHeaders(headers or {}) + page = _browser_page_context_var.get() + + # The browser context holds the cookies of the session, added before the navigation, with the changes the page + # made since. The browser stores the response cookies there too, and the crawler saves them to the session after + # the request handler. + if page is not None: + context_cookies = await page.context.cookies() + session = Session() + session.cookies.set_cookies_from_playwright_format( + [PlaywrightCookieParam(**cookie) for cookie in context_cookies] + ) + + return await super().send_request( + url, + method=method, + headers=headers, + payload=payload, + session=session, + proxy_info=proxy_info, + timeout=timeout, + ) + @override + async def _fetch( + self, + url: str, + *, + method: HttpMethod, + headers: HttpHeaders, + payload: HttpPayload | None, + proxy_info: ProxyInfo | None, + timeout: timedelta | None, + ) -> HttpResponse: + # `proxy_info` are not used because `APIRequestContext` inherits the proxy from `BrowserContext` browser_context = _browser_page_context_var.get() if browser_context is None: raise RuntimeError('Unable to create an `APIRequestContext` outside the browser context') - self._raise_if_non_positive_timeout(timeout) - # Proxies appropriate to the browser context are used response = await browser_context.request.fetch( url_or_request=url, method=method.lower(), - headers=dict(headers) if headers else None, + headers=dict(headers) or None, data=payload, timeout=timeout.total_seconds() * 1000 if timeout is not None else None, + # Redirects are followed hop by hop by `send_request`. + max_redirects=0, ) return await PlaywrightHttpResponse.from_playwright_response(response, protocol='') - @override - def stream( - self, - url: str, - *, - method: HttpMethod = 'GET', - headers: HttpHeaders | dict[str, str] | None = None, - payload: HttpPayload | None = None, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - timeout: timedelta | None = None, - ) -> AbstractAsyncContextManager[HttpResponse]: - raise NotImplementedError('The `stream` method should not be used for `PlaywrightHttpClient`') - - @staticmethod - def _raise_if_non_positive_timeout(timeout: timedelta | None) -> None: - """Raise `TimeoutError` for a non-positive timeout. - - Playwright treats an explicit `0` timeout as "disable timeout", so a non-positive timeout must be - rejected here instead of being forwarded as-is. - """ - if timeout is not None and timeout.total_seconds() <= 0: - raise TimeoutError - async def cleanup(self) -> None: # The `browser_page_context` is responsible for resource cleanup return diff --git a/src/crawlee/crawlers/_playwright/_types.py b/src/crawlee/crawlers/_playwright/_types.py index afc01a36dd..853d7300ae 100644 --- a/src/crawlee/crawlers/_playwright/_types.py +++ b/src/crawlee/crawlers/_playwright/_types.py @@ -1,12 +1,12 @@ from __future__ import annotations -from dataclasses import dataclass from typing import TYPE_CHECKING, Literal, Protocol, TypedDict from playwright.async_api import APIResponse from crawlee import HttpHeaders from crawlee._utils.docs import docs_group +from crawlee.http_clients import HttpResponse if TYPE_CHECKING: from collections.abc import AsyncGenerator @@ -34,26 +34,64 @@ async def __call__( """ -@dataclass(frozen=True) -class PlaywrightHttpResponse: - """Wrapper class for playwright `Response` and `APIResponse` objects to implement `HttpResponse` protocol.""" +class PlaywrightHttpResponse(HttpResponse): + """Wrapper class for playwright `Response` and `APIResponse` objects to implement `HttpResponse`.""" - http_version: str - status_code: int - headers: HttpHeaders - _content: bytes + def __init__( + self, + *, + url: str, + http_version: str, + status_code: int, + headers: HttpHeaders, + headers_set_cookie: list[str], + content: bytes, + ) -> None: + self._url = url + self._http_version = http_version + self._status_code = status_code + self._headers = headers + self._headers_set_cookie = headers_set_cookie + self._content = content + + @property + def url(self) -> str: + return self._url + + @property + def http_version(self) -> str: + return self._http_version + + @property + def status_code(self) -> int: + return self._status_code + + @property + def headers(self) -> HttpHeaders: + return self._headers + + @property + def headers_set_cookie(self) -> list[str]: + return self._headers_set_cookie async def read(self) -> bytes: return self._content async def read_stream(self) -> AsyncGenerator[bytes, None]: # Playwright does not support `streaming` responses. - # This is a workaround to make it compatible with `HttpResponse` protocol. + # This is a workaround to make it compatible with `HttpResponse`. yield self._content + async def aclose(self) -> None: + # The body is read when the instance is created, so there is no connection to release. + return + @classmethod async def from_playwright_response(cls, response: Response | APIResponse, protocol: str) -> Self: headers = HttpHeaders(response.headers) + # Unlike `headers`, the headers array lists every `Set-Cookie` header separately. + headers_array = response.headers_array if isinstance(response, APIResponse) else await response.headers_array() + headers_set_cookie = [header['value'] for header in headers_array if header['name'].lower() == 'set-cookie'] status_code = response.status # Used http protocol version cannot be obtained from `Response` and has to be passed as additional argument. http_version = protocol @@ -62,7 +100,14 @@ async def from_playwright_response(cls, response: Response | APIResponse, protoc if isinstance(response, APIResponse): await response.dispose() - return cls(http_version=http_version, status_code=status_code, headers=headers, _content=_content) + return cls( + url=response.url, + http_version=http_version, + status_code=status_code, + headers=headers, + headers_set_cookie=headers_set_cookie, + content=_content, + ) class GotoOptions(TypedDict): diff --git a/src/crawlee/http_clients/_base.py b/src/crawlee/http_clients/_base.py index 0bc53ec761..3fa4354996 100644 --- a/src/crawlee/http_clients/_base.py +++ b/src/crawlee/http_clients/_base.py @@ -1,62 +1,172 @@ from __future__ import annotations +import asyncio from abc import ABC, abstractmethod from dataclasses import dataclass -from typing import TYPE_CHECKING, Protocol +from datetime import timedelta +from http import HTTPStatus +from time import monotonic +from typing import TYPE_CHECKING, Self +from yarl import URL + +from crawlee._types import HttpHeaders from crawlee._utils.docs import docs_group +from crawlee._utils.urls import validate_http_url +from crawlee.sessions._cookies import SessionCookies if TYPE_CHECKING: - from collections.abc import AsyncIterator - from contextlib import AbstractAsyncContextManager - from datetime import timedelta + from collections.abc import AsyncIterator, Mapping from types import TracebackType - from crawlee import Request - from crawlee._types import HttpHeaders, HttpMethod, HttpPayload + from crawlee._types import HttpMethod, HttpPayload from crawlee.proxy_configuration import ProxyInfo from crawlee.sessions import Session - from crawlee.statistics import Statistics + +_MAX_REDIRECTS = 10 + +_REDIRECT_STATUS_CODES = frozenset( + { + HTTPStatus.MOVED_PERMANENTLY, + HTTPStatus.FOUND, + HTTPStatus.SEE_OTHER, + HTTPStatus.TEMPORARY_REDIRECT, + HTTPStatus.PERMANENT_REDIRECT, + } +) + +# Status codes that redirect a `POST` as a `GET`. `HTTPStatus.SEE_OTHER` does so for any method but `GET` and `HEAD`. +_MOVED_STATUS_CODES = frozenset({HTTPStatus.MOVED_PERMANENTLY, HTTPStatus.FOUND}) + +_HTTP_SCHEMES = frozenset({'http', 'https'}) + +# Headers scoped to a single origin, dropped as soon as a redirect leaves it. +_CROSS_ORIGIN_HEADERS = frozenset({'authorization', 'cookie', 'proxy-authorization'}) + +_REQUEST_BODY_HEADERS = frozenset( + {'content-encoding', 'content-language', 'content-location', 'content-type', 'content-length'} +) + + +def _with_ascii_host(url: URL) -> URL: + """Encode a non-ASCII host of `url` with IDNA, the form in which the HTTP clients report `HttpResponse.url`. + + The cookies are stored under the URL of the response and looked up by the URL of the request, so both must spell the + host the same way. + """ + raw_host = url.raw_host + if raw_host is None or raw_host.isascii(): + return url + + return url.with_host(raw_host) + + +def _is_cross_origin(url: URL, next_url: URL) -> bool: + """Check whether a redirect from `url` to `next_url` leaves the origin. + + Origins are compared as lowercase strings, because `yarl` keeps the case of a host parsed with `encoded=True` and + considers an explicitly written default port different from an omitted one. + """ + return str(url.origin()).lower() != str(next_url.origin()).lower() + + +def _redirect_method(status_code: int, method: HttpMethod) -> HttpMethod: + """Resolve the method of a redirected request, following the `HTTP-redirect fetch` algorithm. + + See https://fetch.spec.whatwg.org/#http-redirect-fetch. + """ + if (status_code in _MOVED_STATUS_CODES and method == 'POST') or ( + status_code == HTTPStatus.SEE_OTHER and method not in {'GET', 'HEAD'} + ): + return 'GET' + + return method + + +def _add_cookie_header(headers: HttpHeaders, cookies: SessionCookies, url: str) -> HttpHeaders: + """Add the stored cookies matching `url` as the `Cookie` header, unless the headers already have one.""" + if 'cookie' in headers or not (cookie_header := cookies.get_cookie_string(url)): + return headers + + return headers | HttpHeaders({'cookie': cookie_header}) @docs_group('Other') -class HttpResponse(Protocol): - """Define the interface that any HTTP response object must implement.""" +class HttpResponse(ABC): + """Base class for the HTTP responses returned by an `HttpClient`. + + The body is not read when the response is received. Read it with `read` or `read_stream`, or release the connection + with `aclose` when the body is not needed. Used as an async context manager, the response is closed on exit. + """ @property + @abstractmethod + def url(self) -> str: + """The URL of the response, after all redirects.""" + + @property + @abstractmethod def http_version(self) -> str: """The HTTP version used in the response.""" @property + @abstractmethod def status_code(self) -> int: """The HTTP status code received from the server.""" @property + @abstractmethod def headers(self) -> HttpHeaders: """The HTTP headers received in the response.""" + @property + @abstractmethod + def headers_set_cookie(self) -> list[str]: + """The values of all `Set-Cookie` headers of the response. + + `headers` holds one value per header name, so repeated `Set-Cookie` headers are only available here. + """ + + @abstractmethod async def read(self) -> bytes: """Read the entire content of the response body. - This method loads the complete response body into memory at once. It should be used - for responses received from regular HTTP requests (via `send_request` or `crawl` methods). + This method loads the complete response body into memory at once. Repeated calls return the same content. Raises: - RuntimeError: If called on a response received from the `stream` method. + RuntimeError: If the body was already consumed by `read_stream`. """ + @abstractmethod def read_stream(self) -> AsyncIterator[bytes]: """Iterate over the content of the response body in chunks. - This method should be used for responses received from the `stream` method to process - large response bodies without loading them entirely into memory. It allows for efficient - processing of potentially large data by yielding chunks sequentially. + This method should be used to process large response bodies without loading them entirely into memory. Raises: - RuntimeError: If the stream has already been consumed or if the response was not obtained from the `stream` - method. + RuntimeError: If the body was already read or streamed. """ + @abstractmethod + async def aclose(self) -> None: + """Close the response and release its connection. + + Reading the whole body releases the connection as well. Repeated calls have no effect. + """ + + async def __aenter__(self) -> Self: + """Return the response itself.""" + return self + + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc_value: BaseException | None, + traceback: TracebackType | None, + ) -> None: + """Close the response.""" + await self.aclose() + @dataclass(frozen=True) @docs_group('Crawling contexts') @@ -73,20 +183,30 @@ class HttpCrawlingResult: @docs_group('HTTP clients') class HttpClient(ABC): - """An abstract base class for HTTP clients used in crawlers (`BasicCrawler` subclasses).""" + """An abstract base class for HTTP clients used in crawlers (`BasicCrawler` subclasses). + + A subclass implements `_fetch`, which sends a single request. `send_request` builds on it, and follows redirects and + handles cookies the same way for every client. + """ - @abstractmethod def __init__( self, *, persist_cookies_per_session: bool = True, + follow_redirects: bool = True, + headers: Mapping[str, str] | None = None, ) -> None: """Initialize a new instance. Args: persist_cookies_per_session: Whether to persist cookies per HTTP session. + follow_redirects: Whether `send_request` follows redirects. Without following them, it returns the first + response as it is. + headers: The headers to include in every request. The headers passed to `send_request` win over them. """ self._persist_cookies_per_session = persist_cookies_per_session + self._follow_redirects = follow_redirects + self._headers = HttpHeaders(headers or {}) # Flag to indicate the context state. self._active = False @@ -96,35 +216,6 @@ def active(self) -> bool: """Indicate whether the context is active.""" return self._active - @abstractmethod - async def crawl( - self, - request: Request, - *, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - statistics: Statistics | None = None, - timeout: timedelta | None = None, - ) -> HttpCrawlingResult: - """Perform the crawling for a given request. - - This method is called from `crawler.run()`. - - Args: - request: The request to be crawled. - session: The session associated with the request. - proxy_info: The information about the proxy to be used. - statistics: The statistics object to register status codes. - timeout: Maximum time allowed to process the request. - - Raises: - ProxyError: Raised if a proxy-related error occurs. - - Returns: - The result of the crawling. - """ - - @abstractmethod async def send_request( self, url: str, @@ -136,57 +227,132 @@ async def send_request( proxy_info: ProxyInfo | None = None, timeout: timedelta | None = None, ) -> HttpResponse: - """Send an HTTP request via the client. + """Send an HTTP request via the client, following up to 10 redirects unless `follow_redirects` is disabled. - This method is called from `context.send_request()` helper. + Every request of the redirect chain sends the cookies matching its URL and stores the cookies of its response. A + `Cookie` header passed by the caller is sent instead of the stored cookies, and its cookies are not stored. The + `Authorization`, `Proxy-Authorization` and `Cookie` headers are dropped when a redirect leaves the origin. + + The body of the returned response is not read yet. Read it, or call `aclose` on the response to release its + connection. Args: url: The URL to send the request to. method: The HTTP method to use. headers: The headers to include in the request. payload: The data to be sent as the request body. - session: The session associated with the request. + session: The session whose cookies are sent. The response cookies are stored in it when + `persist_cookies_per_session` is enabled. Otherwise, and without a session, they are kept only for the + redirect chain of this request. proxy_info: The information about the proxy to be used. - timeout: Maximum time allowed to process the request. + timeout: Maximum time allowed to receive the response, including all redirects, and to read its body. + Without it, the default timeout of the HTTP client applies. Raises: ProxyError: Raised if a proxy-related error occurs. + TimeoutError: If the response does not arrive within `timeout`. Reading the body raises it as well. + RuntimeError: If the server redirects more than 10 times. Returns: The HTTP response received from the server. """ + validate_http_url(url) + + # The headers of the client go through the redirect rules below too, so its credentials never leave the origin. + headers = self._headers | HttpHeaders(headers or {}) + + if session is None: + cookies = SessionCookies() + elif self._persist_cookies_per_session: + cookies = session.cookies + else: + # The cookies of the session are still sent, but the response cookies go to a copy. + cookies = SessionCookies(session.cookies) + + # `encoded=True` keeps the path and the query byte for byte, including a safe `%2F` in query. + current_url = _with_ascii_host(URL(url, encoded=True)) + deadline = monotonic() + timeout.total_seconds() if timeout is not None else None + + for _ in range(_MAX_REDIRECTS + 1): + remaining = deadline - monotonic() if deadline is not None else None + if remaining is not None and remaining <= 0: + raise TimeoutError + + # One wall-clock deadline for the whole chain, whatever timeouts the client library applies on its own. + async with asyncio.timeout(remaining): + response = await self._fetch( + str(current_url), + method=method, + headers=_add_cookie_header(headers, cookies, str(current_url)), + payload=payload, + proxy_info=proxy_info, + timeout=timedelta(seconds=remaining) if remaining is not None else None, + ) + + try: + set_cookie_headers = response.headers_set_cookie + if set_cookie_headers: + cookies.extract_cookies_from_headers(response.url, set_cookie_headers) + + location = response.headers.get('location') + if not self._follow_redirects or response.status_code not in _REDIRECT_STATUS_CODES or not location: + return response + + next_url = _with_ascii_host(current_url.join(URL(location, encoded=True))) + if next_url.scheme not in _HTTP_SCHEMES: + return response + except BaseException: + await response.aclose() + raise + + await response.aclose() + + next_method = _redirect_method(response.status_code, method) + if next_method != method: + method = next_method + payload = None + kept_headers = {key: value for key, value in headers.items() if key not in _REQUEST_BODY_HEADERS} + headers = HttpHeaders(kept_headers) + + if _is_cross_origin(current_url, next_url): + kept_headers = {key: value for key, value in headers.items() if key not in _CROSS_ORIGIN_HEADERS} + headers = HttpHeaders(kept_headers) + + current_url = next_url + + raise RuntimeError(f'Too many redirects ({_MAX_REDIRECTS}) while requesting {url}.') @abstractmethod - def stream( + async def _fetch( self, url: str, *, - method: HttpMethod = 'GET', - headers: HttpHeaders | dict[str, str] | None = None, - payload: HttpPayload | None = None, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - timeout: timedelta | None = None, - ) -> AbstractAsyncContextManager[HttpResponse]: - """Stream an HTTP request via the client. + method: HttpMethod, + headers: HttpHeaders, + payload: HttpPayload | None, + proxy_info: ProxyInfo | None, + timeout: timedelta | None, + ) -> HttpResponse: + """Send a single HTTP request and return the response without reading its body. - This method should be used for downloading potentially large data where you need to process - the response body in chunks rather than loading it entirely into memory. + The client must neither follow redirects nor keep cookies of its own. `send_request` does both, and passes the + cookies in the `Cookie` header. Args: url: The URL to send the request to. method: The HTTP method to use. headers: The headers to include in the request. payload: The data to be sent as the request body. - session: The session associated with the request. proxy_info: The information about the proxy to be used. - timeout: The maximum time to wait for establishing the connection. + timeout: Maximum time allowed to receive the response and read its body. Without it, the default timeout + of the client library applies. `send_request` also cancels the call when the time is up. Raises: ProxyError: Raised if a proxy-related error occurs. + TimeoutError: If the response does not arrive within `timeout`. Reading the body raises it as well. Returns: - An async context manager yielding the HTTP response with streaming capabilities. + The HTTP response received from the server. """ @abstractmethod diff --git a/src/crawlee/http_clients/_curl_impersonate.py b/src/crawlee/http_clients/_curl_impersonate.py index c4308af0b4..f36155ac34 100644 --- a/src/crawlee/http_clients/_curl_impersonate.py +++ b/src/crawlee/http_clients/_curl_impersonate.py @@ -1,11 +1,10 @@ from __future__ import annotations import asyncio -from contextlib import asynccontextmanager -from http.cookiejar import Cookie -from typing import TYPE_CHECKING, Any, cast +import warnings +from functools import cached_property +from typing import TYPE_CHECKING, Any -from curl_cffi import CurlInfo from curl_cffi.const import CurlHttpVersion from curl_cffi.requests import AsyncSession from curl_cffi.requests.cookies import Cookies as CurlCookies @@ -19,24 +18,19 @@ from crawlee._types import HttpHeaders, HttpMethod, HttpPayload from crawlee._utils.blocked import ROTATE_PROXY_ERRORS from crawlee._utils.docs import docs_group -from crawlee._utils.urls import validate_http_url from crawlee.errors import ProxyError -from crawlee.http_clients import HttpClient, HttpCrawlingResult, HttpResponse +from crawlee.http_clients import HttpClient, HttpResponse if TYPE_CHECKING: from collections.abc import AsyncGenerator from datetime import timedelta - from curl_cffi import Curl from curl_cffi.requests import Request as CurlRequest from curl_cffi.requests import Response from curl_cffi.requests.session import HttpMethod as CurlHttpMethod - from crawlee import Request from crawlee._types import HttpMethod from crawlee.proxy_configuration import ProxyInfo - from crawlee.sessions import Session - from crawlee.statistics import Statistics _CURL_PROXY_ERRORS = [ @@ -68,11 +62,19 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self._cookies = _EmptyCookies() -class _CurlImpersonateResponse: - """Adapter class for `curl_cffi.requests.Response` to conform to the `HttpResponse` protocol.""" +class _CurlImpersonateResponse(HttpResponse): + """Adapter class for `curl_cffi.requests.Response` to implement `HttpResponse`.""" - def __init__(self, response: Response) -> None: + def __init__(self, response: Response, *, deadline: float | None) -> None: self._response = response + self._deadline = deadline + # `curl_cffi` does not keep the body of a streamed response once it was read. + self._content: bytes | None = None + self._consumed = False + + @property + def url(self) -> str: + return self._response.url @property def http_version(self) -> str: @@ -97,33 +99,53 @@ def http_version(self) -> str: def status_code(self) -> int: return self._response.status_code - @property + @cached_property def headers(self) -> HttpHeaders: return HttpHeaders({key: value for key, value in self._response.headers.items() if value}) - async def read(self) -> bytes: - if self._response.astream_task: - raise RuntimeError('Use `read_stream` to read the body of the Response received from the `stream` method') + @property + def headers_set_cookie(self) -> list[str]: + return [value for value in self._response.headers.get_list('set-cookie') if value] - return self._response.content + async def read(self) -> bytes: + if self._content is None: + # `curl_cffi` returns an empty body to a second read of a failed transfer, so the body counts as consumed as + # soon as reading starts. + if self._consumed: + raise RuntimeError('Stream is already consumed.') + self._consumed = True + try: + async with asyncio.timeout_at(self._deadline): + self._content = await self._response.acontent() + except Timeout as exc: + raise TimeoutError from exc + return self._content async def read_stream(self) -> AsyncGenerator[bytes, None]: - if not self._response.astream_task: - raise RuntimeError('Cannot read stream, Response not obtained from `stream` method.') - - # A finished transfer task only means the whole body arrived into the response queue. The stream is consumed - # once the queue has also been drained. - if ( - isinstance(self._response.astream_task, asyncio.Future) - and self._response.astream_task.done() - and self._response.queue is not None - and self._response.queue.empty() - ): - raise RuntimeError('Cannot read stream, it was already consumed.') - - async for chunk in self._response.aiter_content(): + if self._consumed: + raise RuntimeError('Stream is already consumed.') + self._consumed = True + chunks = self._response.aiter_content() + while True: + try: + async with asyncio.timeout_at(self._deadline): + chunk = await anext(chunks) + except StopAsyncIteration: + return + except Timeout as exc: + raise TimeoutError from exc yield chunk + async def aclose(self) -> None: + # Reading a body after its transfer is cancelled would raise the `CancelledError` of the transfer. + if self._content is None: + self._consumed = True + # `aclose` of `curl_cffi` waits for the rest of the body, while cancelling the transfer closes its connection. + transfer = self._response.astream_task + if isinstance(transfer, asyncio.Task): + transfer.cancel() + await asyncio.wait([transfer]) + @docs_group('HTTP clients') class CurlImpersonateHttpClient(HttpClient): @@ -149,137 +171,68 @@ def __init__( self, *, persist_cookies_per_session: bool = True, + follow_redirects: bool = True, **async_session_kwargs: Any, ) -> None: """Initialize a new instance. Args: persist_cookies_per_session: Whether to persist cookies per HTTP session. - async_session_kwargs: Additional keyword arguments for `curl_cffi.requests.AsyncSession`. + follow_redirects: Whether to follow HTTP redirects. + async_session_kwargs: Additional keyword arguments for `curl_cffi.requests.AsyncSession`. Some of them are + ignored with a warning. Cookies are kept in sessions, so `cookies` is ignored. The `auth` and `params` + arguments would reach every request of a redirect chain, so pass credentials in the `Authorization` + header and the query in the URL instead. Redirects are followed by `send_request`, so `allow_redirects` + and `max_redirects` are ignored. """ super().__init__( persist_cookies_per_session=persist_cookies_per_session, + follow_redirects=follow_redirects, + headers=async_session_kwargs.pop('headers', None), ) - self._async_session_kwargs = async_session_kwargs - - self._client_by_proxy_url = dict[str | None, AsyncSession]() - - @override - async def crawl( - self, - request: Request, - *, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - statistics: Statistics | None = None, - timeout: timedelta | None = None, - ) -> HttpCrawlingResult: - client = self._get_client(proxy_info.url if proxy_info else None) - self._raise_if_non_positive_timeout(timeout) - try: - response = await client.request( - url=request.url, - method=self._convert_method(request.method), - headers=request.headers, - data=request.payload, - cookies=session.cookies.jar if session else None, - timeout=timeout.total_seconds() if timeout is not None else None, - ) - except Timeout as exc: - raise TimeoutError from exc - except CurlRequestError as exc: - if self._is_proxy_error(exc): - raise ProxyError from exc - raise - - if statistics: - statistics.register_status_code(response.status_code) + for ignored_kwarg in ('cookies', 'auth', 'params', 'allow_redirects', 'max_redirects'): + if async_session_kwargs.pop(ignored_kwarg, None) is not None: + warnings.warn( + f'The `{ignored_kwarg}` argument of `CurlImpersonateHttpClient` is ignored.', + UserWarning, + stacklevel=2, + ) - if self._persist_cookies_per_session and session and response.curl: - response_cookies = self._get_cookies(response.curl) - session.cookies.store_cookies(response_cookies) - - request.loaded_url = response.url + self._async_session_kwargs = async_session_kwargs - return HttpCrawlingResult( - http_response=_CurlImpersonateResponse(response), - ) + self._client_by_proxy_url = dict[str | None, AsyncSession]() @override - async def send_request( + async def _fetch( self, url: str, *, - method: HttpMethod = 'GET', - headers: HttpHeaders | dict[str, str] | None = None, - payload: HttpPayload | None = None, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - timeout: timedelta | None = None, - ) -> HttpResponse: - validate_http_url(url) - - if isinstance(headers, dict) or headers is None: - headers = HttpHeaders(headers or {}) - - proxy_url = proxy_info.url if proxy_info else None - client = self._get_client(proxy_url) - self._raise_if_non_positive_timeout(timeout) - - try: - response = await client.request( - url=url, - method=self._convert_method(method), - headers=dict(headers) if headers else None, - data=payload, - cookies=session.cookies.jar if session else None, - timeout=timeout.total_seconds() if timeout is not None else None, - ) - except Timeout as exc: - raise TimeoutError from exc - except CurlRequestError as exc: - if self._is_proxy_error(exc): - raise ProxyError from exc - raise - - if self._persist_cookies_per_session and session and response.curl: - response_cookies = self._get_cookies(response.curl) - session.cookies.store_cookies(response_cookies) - - return _CurlImpersonateResponse(response) + method: HttpMethod, + headers: HttpHeaders, + payload: HttpPayload | None, + proxy_info: ProxyInfo | None, + timeout: timedelta | None, + ) -> _CurlImpersonateResponse: + client = self._get_client(proxy_info.url if proxy_info else None) + deadline = asyncio.get_running_loop().time() + timeout.total_seconds() if timeout is not None else None - @asynccontextmanager - @override - async def stream( - self, - url: str, - *, - method: HttpMethod = 'GET', - headers: HttpHeaders | dict[str, str] | None = None, - payload: HttpPayload | None = None, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - timeout: timedelta | None = None, - ) -> AsyncGenerator[HttpResponse]: - validate_http_url(url) - - if isinstance(headers, dict) or headers is None: - headers = HttpHeaders(headers or {}) - - proxy_url = proxy_info.url if proxy_info else None - client = self._get_client(proxy_url) - self._raise_if_non_positive_timeout(timeout) + # Without a timeout of its own, the request gets the default timeout of the session. + timeout_kwargs: dict[str, Any] = {} + if timeout is not None: + # `curl_cffi` cuts the timeout down to whole milliseconds, and `libcurl` reads 0 as no timeout. + timeout_kwargs['timeout'] = max(timeout.total_seconds(), 0.001) try: response = await client.request( url=url, method=self._convert_method(method), - headers=dict(headers) if headers else None, + headers=dict(headers) or None, data=payload, - cookies=session.cookies.jar if session else None, + # Redirects are followed hop by hop by `send_request`. + allow_redirects=False, stream=True, - timeout=timeout.total_seconds() if timeout is not None else None, + **timeout_kwargs, ) except Timeout as exc: raise TimeoutError from exc @@ -288,14 +241,7 @@ async def stream( raise ProxyError from exc raise - if self._persist_cookies_per_session and session and response.curl: - response_cookies = self._get_cookies(response.curl) - session.cookies.store_cookies(response_cookies) - - try: - yield _CurlImpersonateResponse(response) - finally: - await response.aclose() + return _CurlImpersonateResponse(response, deadline=deadline) def _get_client(self, proxy_url: str | None) -> AsyncSession: """Retrieve or create an asynchronous HTTP session for the given proxy URL. @@ -321,16 +267,6 @@ def _get_client(self, proxy_url: str | None) -> AsyncSession: return self._client_by_proxy_url[proxy_url] - @staticmethod - def _raise_if_non_positive_timeout(timeout: timedelta | None) -> None: - """Raise `TimeoutError` for a non-positive timeout. - - `curl_cffi` (like `libcurl`) treats an explicit `0` timeout as "no timeout", so a non-positive timeout - must be rejected here instead of being forwarded as-is. - """ - if timeout is not None and timeout.total_seconds() <= 0: - raise TimeoutError - def _convert_method(self, method: HttpMethod) -> CurlHttpMethod: """Convert from Crawlee HTTP method to curl-cffi HTTP method. @@ -380,20 +316,6 @@ def _is_proxy_error(error: CurlRequestError) -> bool: return False - @staticmethod - def _get_cookies(curl: Curl) -> list[Cookie]: - cookies = list[Cookie]() - - # Implementation of getinfo always returns list[bytes] for CurlInfo.COOKIELIST. - cookie_list = cast('list[bytes]', curl.getinfo(CurlInfo.COOKIELIST)) - - for curl_cookie in cookie_list: - curl_morsel = CurlMorsel.from_curl_format(curl_cookie) - cookie = curl_morsel.to_cookiejar_cookie() - cookies.append(cookie) - - return cookies - async def cleanup(self) -> None: for client in self._client_by_proxy_url.values(): await client.close() diff --git a/src/crawlee/http_clients/_httpx.py b/src/crawlee/http_clients/_httpx.py index 0efe018976..b7d305333b 100644 --- a/src/crawlee/http_clients/_httpx.py +++ b/src/crawlee/http_clients/_httpx.py @@ -1,9 +1,11 @@ from __future__ import annotations +import asyncio import warnings -from contextlib import asynccontextmanager +from functools import cached_property +from http.cookiejar import CookieJar, DefaultCookiePolicy from logging import DEBUG, WARNING, getLogger -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any import httpx2 from typing_extensions import override @@ -12,30 +14,34 @@ from crawlee._types import HttpHeaders from crawlee._utils.blocked import ROTATE_PROXY_ERRORS from crawlee._utils.docs import docs_group -from crawlee._utils.urls import validate_http_url +from crawlee._utils.http import drain_small_body from crawlee.errors import ProxyError from crawlee.fingerprint_suite import HeaderGenerator -from crawlee.http_clients import HttpClient, HttpCrawlingResult, HttpResponse +from crawlee.http_clients import HttpClient, HttpResponse if TYPE_CHECKING: - from collections.abc import AsyncGenerator, AsyncIterator + from collections.abc import AsyncIterator from datetime import timedelta from ssl import SSLContext - from crawlee import Request from crawlee._types import HttpMethod, HttpPayload from crawlee.proxy_configuration import ProxyInfo from crawlee.sessions import Session - from crawlee.statistics import Statistics logger = getLogger(__name__) -class _HttpxResponse: - """Adapter class for `httpx2.Response` to conform to the `HttpResponse` protocol.""" +class _HttpxResponse(HttpResponse): + """Adapter class for `httpx2.Response` to implement `HttpResponse`.""" - def __init__(self, response: httpx2.Response) -> None: + def __init__(self, response: httpx2.Response, *, deadline: float | None) -> None: self._response = response + self._deadline = deadline + self._streamed = False + + @property + def url(self) -> str: + return str(self._response.url) @property def http_version(self) -> str: @@ -45,65 +51,43 @@ def http_version(self) -> str: def status_code(self) -> int: return self._response.status_code - @property + @cached_property def headers(self) -> HttpHeaders: return HttpHeaders(dict(self._response.headers)) + @property + def headers_set_cookie(self) -> list[str]: + return self._response.headers.get_list('set-cookie') + async def read(self) -> bytes: - if not self._response.is_closed: - raise RuntimeError('Use `read_stream` to read the body of the Response received from the `stream` method') - return await self._response.aread() + if self._streamed: + raise RuntimeError('Stream is already consumed.') + try: + async with asyncio.timeout_at(self._deadline): + return await self._response.aread() + except httpx2.TimeoutException as exc: + raise TimeoutError from exc async def read_stream(self) -> AsyncIterator[bytes]: - if self._response.is_stream_consumed: + if self._streamed or self._response.is_stream_consumed: raise RuntimeError('Stream is already consumed.') - else: - async for chunk in self._response.aiter_bytes(): - yield chunk - - -def _same_origin(url: httpx2.URL, other: httpx2.URL) -> bool: - """Check whether two URLs share an origin.""" - return url.scheme == other.scheme and url.host == other.host and url.port == other.port - - -class _HttpxTransport(httpx2.AsyncHTTPTransport): - """HTTP transport adapter that keeps cookies in a `Session` instead of in the `HTTPX2` client. - - Response cookies are stored in the session when `persist_cookies_per_session` is enabled, and the `Cookie` - header is rebuilt from it before every hop, so one client can be shared by all sessions. A `Cookie` header - passed by the caller wins for as long as the redirect chain stays on its origin. - """ - - def __init__(self, *, persist_cookies_per_session: bool, **kwargs: Any) -> None: - """Initialize a new instance. Extra arguments are passed to `httpx2.AsyncHTTPTransport`.""" - self._persist_cookies_per_session = persist_cookies_per_session - super().__init__(**kwargs) - - @override - async def handle_async_request(self, request: httpx2.Request) -> httpx2.Response: - session = cast('Session | None', request.extensions.get('crawlee_session')) - original_url, user_cookie = request.extensions.get('crawlee_caller_cookie', (None, None)) - - # The transport owns the `Cookie` header. Anything already on the request came from the `httpx2` jar, - # which is scoped to no session and no origin, so it is always replaced or dropped. - if original_url is not None and _same_origin(original_url, request.url): - request.headers['cookie'] = user_cookie - elif session and (cookies := session.cookies.get_cookie_string(str(request.url))): - request.headers['cookie'] = cookies - else: - request.headers.pop('cookie', None) - - response = await super().handle_async_request(request) - response.request = request - - if self._persist_cookies_per_session and session: - session.cookies.store_cookies(list(response.cookies.jar)) - - if 'Set-Cookie' in response.headers: - del response.headers['Set-Cookie'] - - return response + self._streamed = True + chunks = self._response.aiter_bytes() + while True: + try: + async with asyncio.timeout_at(self._deadline): + chunk = await anext(chunks) + except StopAsyncIteration: + return + except httpx2.TimeoutException as exc: + raise TimeoutError from exc + yield chunk + + async def aclose(self) -> None: + try: + await drain_small_body(self) + finally: + await self._response.aclose() @docs_group('HTTP clients') @@ -136,6 +120,7 @@ def __init__( http2: bool = True, verify: str | bool | SSLContext = True, header_generator: HeaderGenerator | None = _DEFAULT_HEADER_GENERATOR, + follow_redirects: bool = True, **async_client_kwargs: Any, ) -> None: """Initialize a new instance. @@ -146,13 +131,19 @@ def __init__( http2: Whether to enable HTTP/2 support. verify: SSL certificates used to verify the identity of requested hosts. header_generator: Header generator instance to use for generating browser-like headers. - async_client_kwargs: Additional keyword arguments for `httpx2.AsyncClient`. The `mounts` and `transport` - arguments are ignored, they would bypass the cookie handling. The `proxy` argument covers only the - requests made without a `ProxyInfo`, a `ProxyConfiguration` takes precedence over it. The `limits` - argument applies per proxy, because every proxy gets a connection pool of its own. + follow_redirects: Whether to follow HTTP redirects. + async_client_kwargs: Additional keyword arguments for `httpx2.AsyncClient`. Some of them are ignored with a + warning. The `mounts` and `transport` arguments would bypass the proxy of a `ProxyInfo`. Cookies are + kept in sessions, so `cookies` is ignored. The `auth` and `params` arguments would reach every request + of a redirect chain, so pass credentials in the `Authorization` header and the query in the URL instead. + Redirects are followed by `send_request`, so `max_redirects` is ignored. The `proxy` argument covers + only the requests made without a `ProxyInfo`, a `ProxyConfiguration` takes precedence over it. The + `limits` argument applies per proxy, because every proxy gets a connection pool of its own. """ super().__init__( persist_cookies_per_session=persist_cookies_per_session, + follow_redirects=follow_redirects, + headers=async_client_kwargs.pop('headers', None), ) # `httpx2` logs one INFO line per request, which is too noisy for the default log level. Silence it down to @@ -163,16 +154,14 @@ def __init__( self._http1 = http1 self._http2 = http2 - # `httpx2.AsyncClient` turns a `proxy` into a mount that bypasses the cookie handling, so it is handed to - # the transport instead. It covers the requests that carry no `ProxyInfo` of their own. + # `httpx2.AsyncClient` turns a `proxy` into a mount of its own, so it is handed to the transport instead. It + # covers the requests that carry no `ProxyInfo` of their own. self._proxy = async_client_kwargs.pop('proxy', None) - # These two would put a transport of their own in front of the one that handles the cookies. - for ignored_kwarg in ('mounts', 'transport'): + for ignored_kwarg in ('mounts', 'transport', 'cookies', 'auth', 'params', 'max_redirects'): if async_client_kwargs.pop(ignored_kwarg, None) is not None: warnings.warn( - f'The `{ignored_kwarg}` argument of `HttpxHttpClient` is ignored, requests are sent through ' - 'the transport that handles the cookies.', + f'The `{ignored_kwarg}` argument of `HttpxHttpClient` is ignored.', UserWarning, stacklevel=2, ) @@ -184,46 +173,6 @@ def __init__( self._client_by_proxy_url = dict[str | None, httpx2.AsyncClient]() - @override - async def crawl( - self, - request: Request, - *, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - statistics: Statistics | None = None, - timeout: timedelta | None = None, - ) -> HttpCrawlingResult: - client = self._get_client(proxy_info.url if proxy_info else None) - - http_request = self._build_request( - client=client, - session=session, - url=request.url, - method=request.method, - headers=request.headers, - payload=request.payload, - timeout=httpx2.Timeout(timeout.total_seconds()) if timeout is not None else None, - ) - - try: - response = await client.send(http_request) - except httpx2.TimeoutException as exc: - raise TimeoutError from exc - except httpx2.TransportError as exc: - if self._is_proxy_error(exc): - raise ProxyError from exc - raise - - if statistics: - statistics.register_status_code(response.status_code) - - request.loaded_url = str(response.url) - - return HttpCrawlingResult( - http_response=_HttpxResponse(response), - ) - @override async def send_request( self, @@ -236,100 +185,53 @@ async def send_request( proxy_info: ProxyInfo | None = None, timeout: timedelta | None = None, ) -> HttpResponse: - validate_http_url(url) - - client = self._get_client(proxy_info.url if proxy_info else None) + # The headers are generated once per call, so that every request of a redirect chain carries the same browser + # profile. + if not isinstance(headers, HttpHeaders): + headers = HttpHeaders(headers or {}) - http_request = self._build_request( - client=client, - url=url, + return await super().send_request( + url, method=method, - headers=headers, + headers=self._combine_headers(headers), payload=payload, session=session, - timeout=httpx2.Timeout(timeout.total_seconds()) if timeout is not None else None, + proxy_info=proxy_info, + timeout=timeout, ) - try: - response = await client.send(http_request) - except httpx2.TimeoutException as exc: - raise TimeoutError from exc - except httpx2.TransportError as exc: - if self._is_proxy_error(exc): - raise ProxyError from exc - raise - - return _HttpxResponse(response) - - @asynccontextmanager @override - async def stream( + async def _fetch( self, url: str, *, - method: HttpMethod = 'GET', - headers: HttpHeaders | dict[str, str] | None = None, - payload: HttpPayload | None = None, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - timeout: timedelta | None = None, - ) -> AsyncGenerator[HttpResponse]: - validate_http_url(url) - + method: HttpMethod, + headers: HttpHeaders, + payload: HttpPayload | None, + proxy_info: ProxyInfo | None, + timeout: timedelta | None, + ) -> _HttpxResponse: client = self._get_client(proxy_info.url if proxy_info else None) + deadline = asyncio.get_running_loop().time() + timeout.total_seconds() if timeout is not None else None - http_request = self._build_request( - client=client, + http_request = client.build_request( url=url, method=method, - headers=headers, - payload=payload, - session=session, - timeout=httpx2.Timeout(None, connect=timeout.total_seconds()) if timeout is not None else None, + headers=dict(headers) or None, + content=payload, + timeout=httpx2.Timeout(timeout.total_seconds()) if timeout is not None else httpx2.USE_CLIENT_DEFAULT, ) try: response = await client.send(http_request, stream=True) except httpx2.TimeoutException as exc: raise TimeoutError from exc + except httpx2.TransportError as exc: + if self._is_proxy_error(exc): + raise ProxyError from exc + raise - try: - yield _HttpxResponse(response) - finally: - await response.aclose() - - def _build_request( - self, - *, - client: httpx2.AsyncClient, - url: str, - method: HttpMethod, - headers: HttpHeaders | dict[str, str] | None, - payload: HttpPayload | None, - session: Session | None = None, - timeout: httpx2.Timeout | None = None, - ) -> httpx2.Request: - """Build an `httpx2.Request` using the provided parameters.""" - if isinstance(headers, dict) or headers is None: - headers = HttpHeaders(headers or {}) - - headers = self._combine_headers(headers) - - request = client.build_request( - url=url, - method=method, - headers=dict(headers) if headers else None, - content=payload, - extensions={'crawlee_session': session}, - timeout=timeout or httpx2.USE_CLIENT_DEFAULT, - ) - - # Extensions survive a redirect, the `Cookie` header does not, so the caller's value rides along there. - # An empty value is kept too: it means "no cookies" and outranks the session on the origin it came from. - if (caller_cookie := request.headers.get('cookie')) is not None: - request.extensions['crawlee_caller_cookie'] = (request.url, caller_cookie) - - return request + return _HttpxResponse(response, deadline=deadline) def _get_client(self, proxy_url: str | None) -> httpx2.AsyncClient: """Retrieve or create an HTTP client for the given proxy URL. @@ -338,13 +240,12 @@ def _get_client(self, proxy_url: str | None) -> httpx2.AsyncClient: """ if proxy_url not in self._client_by_proxy_url: # A client built with `proxy=` mounts its own transport and never calls the one given to `transport=`, - # so the proxy has to go on the transport for the cookie handling to run. - transport = _HttpxTransport( + # so the proxy goes on the transport. + transport = httpx2.AsyncHTTPTransport( http1=self._http1, http2=self._http2, verify=self._ssl_context, proxy=proxy_url or self._proxy, - persist_cookies_per_session=self._persist_cookies_per_session, # Above the `httpx2` default of 20 kept-alive connections every request pays a TCP and TLS handshake. limits=self._async_client_kwargs.get( 'limits', @@ -356,7 +257,6 @@ def _get_client(self, proxy_url: str | None) -> httpx2.AsyncClient: kwargs: dict[str, Any] = { 'http1': self._http1, 'http2': self._http2, - 'follow_redirects': True, } # Update the default kwargs with any additional user-provided kwargs. @@ -366,6 +266,10 @@ def _get_client(self, proxy_url: str | None) -> httpx2.AsyncClient: { 'transport': transport, 'verify': self._ssl_context, + # Redirects are followed hop by hop by `send_request`, which also handles the cookies. + 'follow_redirects': False, + # With both cookie protocols off, the jar of the client never stores a cookie. + 'cookies': CookieJar(policy=DefaultCookiePolicy(netscape=False, rfc2965=False)), } ) @@ -374,11 +278,11 @@ def _get_client(self, proxy_url: str | None) -> httpx2.AsyncClient: return self._client_by_proxy_url[proxy_url] - def _combine_headers(self, explicit_headers: HttpHeaders | None) -> HttpHeaders: - """Merge generated headers with explicit headers for an HTTP request. + def _combine_headers(self, explicit_headers: HttpHeaders) -> HttpHeaders: + """Merge generated headers with the headers of the client and explicit headers for an HTTP request. - The generated headers come from a single browser profile, so that the set stays consistent. Explicit - headers win over the generated ones. + The generated headers come from a single browser profile, so that the set stays consistent. The headers of the + client win over the generated ones, and explicit headers win over both. """ if self._header_generator: generated_headers = self._header_generator.get_specific_headers( @@ -387,8 +291,7 @@ def _combine_headers(self, explicit_headers: HttpHeaders | None) -> HttpHeaders: else: generated_headers = HttpHeaders() - explicit_headers = explicit_headers or HttpHeaders() - return generated_headers | explicit_headers + return generated_headers | self._headers | explicit_headers @staticmethod def _is_proxy_error(error: httpx2.TransportError) -> bool: diff --git a/src/crawlee/http_clients/_impit.py b/src/crawlee/http_clients/_impit.py index 87cb489c14..6694229470 100644 --- a/src/crawlee/http_clients/_impit.py +++ b/src/crawlee/http_clients/_impit.py @@ -1,85 +1,45 @@ from __future__ import annotations -from contextlib import asynccontextmanager -from http import HTTPStatus +import asyncio +import warnings +from functools import cached_property from logging import getLogger -from time import monotonic from typing import TYPE_CHECKING, Any -from impit import AsyncClient, Browser, HTTPError, Response, TimeoutException, TooManyRedirects, TransportError +from impit import USE_CLIENT_DEFAULT, AsyncClient, Browser, HTTPError, Response, TimeoutException, TransportError from impit import ProxyError as ImpitProxyError from typing_extensions import override -from yarl import URL from crawlee._types import HttpHeaders from crawlee._utils.blocked import ROTATE_PROXY_ERRORS from crawlee._utils.docs import docs_group -from crawlee._utils.urls import validate_http_url +from crawlee._utils.http import drain_small_body from crawlee.errors import ProxyError -from crawlee.http_clients import HttpClient, HttpCrawlingResult, HttpResponse +from crawlee.http_clients import HttpClient, HttpResponse if TYPE_CHECKING: - from collections.abc import AsyncGenerator, AsyncIterator + from collections.abc import AsyncIterator from datetime import timedelta - from crawlee import Request from crawlee._types import HttpMethod, HttpPayload from crawlee.proxy_configuration import ProxyInfo - from crawlee.sessions import Session - from crawlee.statistics import Statistics logger = getLogger(__name__) -_REDIRECT_STATUS_CODES = frozenset( - { - HTTPStatus.MOVED_PERMANENTLY, - HTTPStatus.FOUND, - HTTPStatus.SEE_OTHER, - HTTPStatus.TEMPORARY_REDIRECT, - HTTPStatus.PERMANENT_REDIRECT, - } -) -# Status codes that redirect a `POST` as a `GET`. `HTTPStatus.SEE_OTHER` does so for any method but `GET` and `HEAD`. -_MOVED_STATUS_CODES = frozenset({HTTPStatus.MOVED_PERMANENTLY, HTTPStatus.FOUND}) +class _ImpitResponse(HttpResponse): + """Adapter class for `impit.Response` to implement `HttpResponse`.""" -_HTTP_SCHEMES = frozenset({'http', 'https'}) - -# Headers scoped to a single origin, dropped as soon as a redirect leaves it. -_CROSS_ORIGIN_HEADERS = frozenset({'authorization', 'cookie', 'proxy-authorization'}) - -_REQUEST_BODY_HEADERS = frozenset( - {'content-encoding', 'content-language', 'content-location', 'content-type', 'content-length'} -) - - -def _is_cross_origin(url: URL, next_url: URL) -> bool: - """Check whether a redirect from `url` to `next_url` leaves the origin. - - Origins are compared as strings, because `yarl` considers an explicitly written default port different from - an omitted one. - """ - return str(url.origin()) != str(next_url.origin()) - - -def _redirect_method(status_code: int, method: str) -> str: - """Resolve the method of a redirected request, following the `HTTP-redirect fetch` algorithm. - - See https://fetch.spec.whatwg.org/#http-redirect-fetch. - """ - if (status_code in _MOVED_STATUS_CODES and method == 'POST') or ( - status_code == HTTPStatus.SEE_OTHER and method not in {'GET', 'HEAD'} - ): - return 'GET' - - return method - - -class _ImpitResponse: - """Adapter class for `impit.Response` to conform to the `HttpResponse` protocol.""" - - def __init__(self, response: Response) -> None: + def __init__(self, response: Response, *, deadline: float | None) -> None: self._response = response + self._deadline = deadline + self._streamed = False + # A repeated `aread()` of `impit` still goes through the event loop, so past the deadline it would time out. + self._content: bytes | None = None + + @property + def url(self) -> str: + return str(self._response.url) @property def http_version(self) -> str: @@ -89,21 +49,45 @@ def http_version(self) -> str: def status_code(self) -> int: return int(self._response.status_code) - @property + @cached_property def headers(self) -> HttpHeaders: return HttpHeaders(dict(self._response.headers)) + @property + def headers_set_cookie(self) -> list[str]: + return self._response.headers.get_list('set-cookie') + async def read(self) -> bytes: - if not self._response.is_closed: - raise RuntimeError('Use `read_stream` to read the body of the Response received from the `stream` method') - return self._response.content + if self._streamed: + raise RuntimeError('Stream is already consumed.') + if self._content is None: + try: + async with asyncio.timeout_at(self._deadline): + self._content = await self._response.aread() + except TimeoutException as exc: + raise TimeoutError from exc + return self._content async def read_stream(self) -> AsyncIterator[bytes]: - if self._response.is_stream_consumed: + if self._streamed or self._response.is_stream_consumed: raise RuntimeError('Stream is already consumed.') - else: - async for chunk in self._response.aiter_bytes(): - yield chunk + self._streamed = True + chunks = self._response.aiter_bytes() + while True: + try: + async with asyncio.timeout_at(self._deadline): + chunk = await anext(chunks) + except StopAsyncIteration: + return + except TimeoutException as exc: + raise TimeoutError from exc + yield chunk + + async def aclose(self) -> None: + try: + await drain_small_body(self) + finally: + await self._response.aclose() @docs_group('HTTP clients') @@ -134,7 +118,6 @@ def __init__( verify: bool = True, browser: Browser | None = 'firefox', follow_redirects: bool = True, - max_redirects: int = 20, **async_client_kwargs: Any, ) -> None: """Initialize a new instance. @@ -145,82 +128,54 @@ def __init__( verify: SSL certificates used to verify the identity of requested hosts. browser: Browser to impersonate. follow_redirects: Whether to follow HTTP redirects. - max_redirects: Maximum number of redirects to follow before raising `impit.TooManyRedirects`. - async_client_kwargs: Additional keyword arguments for `impit.AsyncClient`. + async_client_kwargs: Additional keyword arguments for `impit.AsyncClient`. Some of them are ignored with a + warning. Cookies are kept in sessions, so `cookies` and `cookie_jar` are ignored. Redirects are followed + by `send_request`, so `max_redirects` is ignored. """ super().__init__( persist_cookies_per_session=persist_cookies_per_session, + follow_redirects=follow_redirects, + headers=async_client_kwargs.pop('headers', None), ) self._http3 = http3 self._verify = verify self._browser = browser - self._follow_redirects = follow_redirects - self._max_redirects = max_redirects + + for ignored_kwarg in ('cookies', 'cookie_jar', 'max_redirects'): + if async_client_kwargs.pop(ignored_kwarg, None) is not None: + warnings.warn( + f'The `{ignored_kwarg}` argument of `ImpitHttpClient` is ignored.', + UserWarning, + stacklevel=2, + ) self._async_client_kwargs = async_client_kwargs self._client_by_proxy_url = dict[str | None, AsyncClient]() @override - async def crawl( - self, - request: Request, - *, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - statistics: Statistics | None = None, - timeout: timedelta | None = None, - ) -> HttpCrawlingResult: - try: - response = await self._request_with_redirects( - method=request.method, - url=request.url, - headers=dict(request.headers) if request.headers else {}, - payload=request.payload, - session=session, - proxy_info=proxy_info, - timeout=timeout, - ) - except TimeoutException as exc: - raise TimeoutError from exc - except (TransportError, HTTPError) as exc: - if self._is_proxy_error(exc): - raise ProxyError from exc - raise - - if statistics: - statistics.register_status_code(response.status_code) - - request.loaded_url = str(response.url) - - return HttpCrawlingResult(http_response=_ImpitResponse(response)) - - @override - async def send_request( + async def _fetch( self, url: str, *, - method: HttpMethod = 'GET', - headers: HttpHeaders | dict[str, str] | None = None, - payload: HttpPayload | None = None, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - timeout: timedelta | None = None, - ) -> HttpResponse: - validate_http_url(url) - - if isinstance(headers, dict) or headers is None: - headers = HttpHeaders(headers or {}) + method: HttpMethod, + headers: HttpHeaders, + payload: HttpPayload | None, + proxy_info: ProxyInfo | None, + timeout: timedelta | None, + ) -> _ImpitResponse: + client = self._get_client(proxy_info.url if proxy_info else None) + deadline = asyncio.get_running_loop().time() + timeout.total_seconds() if timeout is not None else None try: - response = await self._request_with_redirects( + response = await client.request( method=method, url=url, - headers=dict(headers), - payload=payload, - session=session, - proxy_info=proxy_info, - timeout=timeout, + content=payload, + headers=dict(headers) or None, + # `None` would disable the timeout, the client default applies instead. + timeout=timeout.total_seconds() if timeout is not None else USE_CLIENT_DEFAULT, + stream=True, ) except TimeoutException as exc: raise TimeoutError from exc @@ -229,139 +184,7 @@ async def send_request( raise ProxyError from exc raise - return _ImpitResponse(response) - - @asynccontextmanager - @override - async def stream( - self, - url: str, - *, - method: HttpMethod = 'GET', - headers: HttpHeaders | dict[str, str] | None = None, - payload: HttpPayload | None = None, - session: Session | None = None, - proxy_info: ProxyInfo | None = None, - timeout: timedelta | None = None, - ) -> AsyncGenerator[HttpResponse]: - validate_http_url(url) - - if isinstance(headers, dict) or headers is None: - headers = HttpHeaders(headers or {}) - - try: - response = await self._request_with_redirects( - method=method, - url=url, - headers=dict(headers), - payload=payload, - session=session, - proxy_info=proxy_info, - timeout=timeout, - stream=True, - ) - except TimeoutException as exc: - raise TimeoutError from exc - - try: - yield _ImpitResponse(response) - finally: - response.close() - - async def _request_with_redirects( - self, - *, - method: str, - url: str, - headers: dict[str, str], - payload: HttpPayload | None, - session: Session | None, - proxy_info: ProxyInfo | None, - timeout: timedelta | None, - stream: bool = False, - ) -> Response: - """Perform a request, following redirects one hop at a time. - - Redirects are resolved here instead of by `impit`, so that cookies of the given session are attached to and - collected from every single hop. A client is therefore never bound to a session and can be shared by all - of them. - - Args: - method: The HTTP method to use. - url: The URL to send the request to. - headers: The headers to include in the request. - payload: The data to be sent as the request body. - session: The session whose cookies are sent and updated. - proxy_info: The information about the proxy to be used. - timeout: Maximum time allowed to process the request. - stream: Whether the body of the final response should be streamed. - - Raises: - TooManyRedirects: If the number of redirects exceeds `max_redirects`. - - Returns: - The final response of the redirect chain. - """ - client = self._get_client(proxy_info.url if proxy_info else None) - # `encoded=True` keeps the URL byte for byte and safe `%2F` in query. - current_url = URL(url, encoded=True) - content = payload - # The timeout bounds the whole chain rather than each hop, which is how `impit` treats it as well. - deadline = monotonic() + timeout.total_seconds() if timeout is not None else None - - for _ in range(self._max_redirects + 1): - remaining = deadline - monotonic() if deadline is not None else None - if remaining is not None and remaining <= 0: - raise TimeoutError - - # Rebuilt per hop, so that the `Cookie` header never reaches a URL its cookies do not match. A header - # passed by the caller wins over the session, which is how `impit` treats its own cookie jar. - request_headers = dict(headers) - if ( - session - and 'cookie' not in request_headers - and (cookie_string := session.cookies.get_cookie_string(str(current_url))) - ): - request_headers['cookie'] = cookie_string - - response = await client.request( - method=method, - url=str(current_url), - content=content, - headers=request_headers or None, - timeout=remaining, - stream=stream, - ) - - if session and self._persist_cookies_per_session: - session.cookies.extract_cookies_from_headers(str(current_url), response.headers.get_list('set-cookie')) - - if not self._follow_redirects or response.status_code not in _REDIRECT_STATUS_CODES: - return response - - location = response.headers.get('location') - if not location: - return response - - next_url = current_url.join(URL(location, encoded=True)) - if next_url.scheme not in _HTTP_SCHEMES: - return response - - next_method = _redirect_method(response.status_code, method) - if next_method != method: - method = next_method - content = None - headers = {key: value for key, value in headers.items() if key not in _REQUEST_BODY_HEADERS} - - if _is_cross_origin(current_url, next_url): - headers = {key: value for key, value in headers.items() if key not in _CROSS_ORIGIN_HEADERS} - - if stream: - response.close() - - current_url = next_url - - raise TooManyRedirects(f'Exceeded the limit of {self._max_redirects} redirects while requesting {url}.') + return _ImpitResponse(response, deadline=deadline) def _get_client(self, proxy_url: str | None) -> AsyncClient: """Retrieve or create an HTTP client for the given proxy URL. @@ -382,8 +205,10 @@ def _get_client(self, proxy_url: str | None) -> AsyncClient: # Update the default kwargs with any additional user-provided kwargs. kwargs.update(self._async_client_kwargs) - # Redirects are followed hop by hop by `_request_with_redirects`. - client = AsyncClient(**kwargs, follow_redirects=False) + # Redirects are followed hop by hop by `send_request`. + kwargs['follow_redirects'] = False + + client = AsyncClient(**kwargs) self._client_by_proxy_url[proxy_url] = client diff --git a/tests/unit/_utils/test_http.py b/tests/unit/_utils/test_http.py index af704d327a..1be55d6d61 100644 --- a/tests/unit/_utils/test_http.py +++ b/tests/unit/_utils/test_http.py @@ -1,8 +1,14 @@ from __future__ import annotations +import asyncio +from unittest.mock import Mock + import pytest -from crawlee._utils.http import parse_content_type_charset +from crawlee import HttpHeaders +from crawlee._utils import http as http_utils +from crawlee._utils.http import drain_small_body, parse_content_type_charset +from crawlee.http_clients import HttpResponse @pytest.mark.parametrize( @@ -20,3 +26,42 @@ def test_parse_content_type_charset(content_type: str | None, expected: str | None) -> None: """The charset parameter is read from a `Content-Type` header.""" assert parse_content_type_charset(content_type) == expected + + +@pytest.mark.parametrize( + ('headers', 'expected_reads'), + [ + pytest.param({'content-length': '1024'}, 1, id='small-body'), + pytest.param({'content-length': str(1024 * 1024)}, 0, id='large-body'), + pytest.param({}, 0, id='unknown-length'), + ], +) +async def test_drain_small_body_reads_only_small_body(headers: dict[str, str], expected_reads: int) -> None: + """Test that `drain_small_body` reads the body only when it is known to be small.""" + response = Mock(spec=HttpResponse, headers=HttpHeaders(headers)) + + await drain_small_body(response) + + assert response.read.await_count == expected_reads + + +async def test_drain_small_body_ignores_failed_read() -> None: + """Test that `drain_small_body` doesn't raise when reading the body fails.""" + response = Mock(spec=HttpResponse, headers=HttpHeaders({'content-length': '10'})) + response.read.side_effect = ConnectionError + + await drain_small_body(response) + + response.read.assert_awaited_once() + + +async def test_drain_small_body_gives_up_on_slow_body(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that `drain_small_body` stops waiting for a body that doesn't arrive in time.""" + monkeypatch.setattr(http_utils, '_MAX_DRAIN_SECONDS', 0.01) + response = Mock(spec=HttpResponse, headers=HttpHeaders({'content-length': '10'})) + response.read.side_effect = asyncio.Event().wait + + async with asyncio.timeout(1): + await drain_small_body(response) + + response.read.assert_awaited_once() diff --git a/tests/unit/_utils/test_robots.py b/tests/unit/_utils/test_robots.py index a099e46f80..29732a7002 100644 --- a/tests/unit/_utils/test_robots.py +++ b/tests/unit/_utils/test_robots.py @@ -1,14 +1,16 @@ from __future__ import annotations from typing import TYPE_CHECKING +from unittest.mock import AsyncMock, MagicMock + +import pytest from crawlee._utils.robots import RobotsTxtFile +from crawlee.http_clients import HttpClient, HttpResponse if TYPE_CHECKING: from yarl import URL - from crawlee.http_clients._base import HttpClient - async def test_generation_robots_txt_url(server_url: URL, http_client: HttpClient) -> None: """`RobotsTxtFile.find` constructs the correct /robots.txt URL and successfully parses the response.""" @@ -99,3 +101,24 @@ async def test_bind_robots_txt_url() -> None: robots = await RobotsTxtFile.from_content('http://check.com/robots.txt', content) assert not robots.is_allowed('http://check.com/test.html') assert robots.is_allowed('http://othercheck.com/robots.txt') + + +@pytest.mark.parametrize( + 'status_code', + [ + pytest.param(200, id='success'), + pytest.param(404, id='client-error'), + pytest.param(500, id='server-error'), + ], +) +async def test_load_closes_response(status_code: int) -> None: + """Test that `RobotsTxtFile.load` closes the response, also when the status code leaves the body unread.""" + response = MagicMock(spec=HttpResponse) + response.status_code = status_code + response.read = AsyncMock(return_value=b'User-agent: *\nAllow: /') + http_client = AsyncMock(spec=HttpClient) + http_client.send_request.return_value = response + + await RobotsTxtFile.load('http://example.com/robots.txt', http_client) + + response.__aexit__.assert_awaited_once() diff --git a/tests/unit/_utils/test_sitemap.py b/tests/unit/_utils/test_sitemap.py index 4253d609b5..6e03598f48 100644 --- a/tests/unit/_utils/test_sitemap.py +++ b/tests/unit/_utils/test_sitemap.py @@ -1,7 +1,6 @@ import base64 import gzip import logging -from contextlib import asynccontextmanager from datetime import UTC, datetime, timedelta from typing import TYPE_CHECKING, Any, cast from unittest.mock import AsyncMock, MagicMock, patch @@ -11,6 +10,7 @@ from yarl import URL from crawlee._utils.sitemap import ( + COMMON_SITEMAP_PATHS, DEFAULT_MAX_DEPTH, ParseSitemapOptions, Sitemap, @@ -31,7 +31,7 @@ ) if TYPE_CHECKING: - from collections.abc import AsyncGenerator, AsyncIterator, Callable + from collections.abc import AsyncIterator, Callable def _make_mock_client(url_map: dict[str, tuple[int, bytes]]) -> AsyncMock: @@ -52,11 +52,10 @@ async def send_request(url: str, **_kwargs: Any) -> HttpResponse: def _make_flaky_stream_client(body: bytes, *, fail_times: int) -> tuple[AsyncMock, list[int]]: - """Create a mock client whose `stream` fails with a network error for the first `fail_times` calls.""" + """Create a mock client whose `send_request` fails with a network error for the first `fail_times` calls.""" attempts: list[int] = [] - @asynccontextmanager - async def stream(_url: str, **_kwargs: Any) -> 'AsyncGenerator[HttpResponse]': + async def send_request(_url: str, **_kwargs: Any) -> HttpResponse: attempt = len(attempts) + 1 attempts.append(attempt) if attempt <= fail_times: @@ -69,19 +68,18 @@ async def read_stream() -> 'AsyncIterator[bytes]': response.status_code = 200 response.headers = {'content-type': 'application/xml; charset=utf-8'} response.read_stream = read_stream - yield cast('HttpResponse', response) + return cast('HttpResponse', response) client = AsyncMock(spec=HttpClient) - client.stream = stream + client.send_request.side_effect = send_request return client, attempts def _make_stream_client(body_for_url: 'Callable[[str], bytes]') -> tuple[AsyncMock, list[str]]: - """Create a mock client whose `stream` serves `body_for_url(url)` in a single chunk and records fetched URLs.""" + """Create a mock client whose `send_request` serves `body_for_url(url)` in one chunk and records fetched URLs.""" fetched: list[str] = [] - @asynccontextmanager - async def stream(url: str, **_kwargs: Any) -> 'AsyncGenerator[HttpResponse]': + async def send_request(url: str, **_kwargs: Any) -> HttpResponse: fetched.append(url) async def read_stream() -> 'AsyncIterator[bytes]': @@ -91,10 +89,10 @@ async def read_stream() -> 'AsyncIterator[bytes]': response.status_code = 200 response.headers = {'content-type': 'application/xml; charset=utf-8'} response.read_stream = read_stream - yield cast('HttpResponse', response) + return cast('HttpResponse', response) client = AsyncMock(spec=HttpClient) - client.stream = stream + client.send_request.side_effect = send_request return client, fetched @@ -598,8 +596,7 @@ async def test_gzip_sitemap_stops_reading_after_member_end() -> None: member = compress_gzip(get_basic_sitemap()) chunks_read = 0 - @asynccontextmanager - async def stream(_url: str, **_kwargs: Any) -> 'AsyncGenerator[HttpResponse]': + async def send_request(_url: str, **_kwargs: Any) -> HttpResponse: async def read_stream() -> 'AsyncIterator[bytes]': nonlocal chunks_read chunks_read += 1 @@ -613,10 +610,10 @@ async def read_stream() -> 'AsyncIterator[bytes]': response.status_code = 200 response.headers = {'content-type': 'application/gzip'} response.read_stream = read_stream - yield cast('HttpResponse', response) + return cast('HttpResponse', response) client = AsyncMock(spec=HttpClient) - client.stream = stream + client.send_request.side_effect = send_request items = [item async for item in parse_sitemap([{'type': 'url', 'url': f'{DEFAULT_URL}sitemap.xml.gz'}], client)] @@ -630,8 +627,7 @@ async def test_gzip_text_sitemap_parsed_by_extension_when_served_as_gzip() -> No urls = {f'{DEFAULT_URL}page-{i}' for i in range(3)} body = compress_gzip('\n'.join(urls)) - @asynccontextmanager - async def stream(_url: str, **_kwargs: Any) -> 'AsyncGenerator[HttpResponse]': + async def send_request(_url: str, **_kwargs: Any) -> HttpResponse: async def read_stream() -> 'AsyncIterator[bytes]': yield body @@ -639,16 +635,41 @@ async def read_stream() -> 'AsyncIterator[bytes]': response.status_code = 200 response.headers = {'content-type': 'application/gzip'} response.read_stream = read_stream - yield cast('HttpResponse', response) + return cast('HttpResponse', response) client = AsyncMock(spec=HttpClient) - client.stream = stream + client.send_request.side_effect = send_request items = [item async for item in parse_sitemap([{'type': 'url', 'url': f'{DEFAULT_URL}sitemap.txt.gz'}], client)] assert {item.loc for item in items} == urls +@pytest.mark.parametrize( + 'status_code', + [ + pytest.param(200, id='parsed'), + pytest.param(404, id='skipped'), + ], +) +async def test_sitemap_response_closed(status_code: int) -> None: + """Test that the response of a sitemap is closed, also when its status code skips reading the body.""" + + async def read_stream() -> 'AsyncIterator[bytes]': + yield get_basic_sitemap().encode() + + response = MagicMock(spec=HttpResponse) + response.status_code = status_code + response.headers = {'content-type': 'application/xml; charset=utf-8'} + response.read_stream = read_stream + client = AsyncMock(spec=HttpClient) + client.send_request.return_value = response + + [item async for item in parse_sitemap([{'type': 'url', 'url': f'{DEFAULT_URL}sitemap.xml'}], client)] + + response.__aexit__.assert_awaited_once() + + async def test_nested_sitemaps_followed_only_to_default_max_depth() -> None: """A chain of unique nested sitemap URLs is followed only up to the default max depth.""" chain_length = 3 * DEFAULT_MAX_DEPTH @@ -862,6 +883,20 @@ async def test_discover_sitemap_url_without_host_skipped() -> None: assert urls == [] +async def test_discover_closes_checked_responses() -> None: + """Test that sitemap discovery closes the response of every common path it checks.""" + response = MagicMock(spec=HttpResponse) + response.status_code = 404 + http_client = AsyncMock(spec=HttpClient) + http_client.send_request.return_value = response + + urls = [url async for url in discover_valid_sitemaps(['http://example.com/page'], http_client=http_client)] + + assert urls == [] + # The robots.txt request gets the same response, which it closes with `async with` instead of `aclose`. + assert response.aclose.await_count == len(COMMON_SITEMAP_PATHS) + + async def test_raw_sitemap_index_processes_nested_sitemaps() -> None: """Test that nested sitemap respects source url.""" raw_index = f""" diff --git a/tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py b/tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py index 6e2b212123..9d70f22aaa 100644 --- a/tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py +++ b/tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py @@ -32,7 +32,7 @@ ) from crawlee.crawlers._adaptive_playwright._adaptive_playwright_crawling_context import AdaptiveContextError from crawlee.errors import RequestThrottledError -from crawlee.http_clients import HttpClient, HttpCrawlingResult +from crawlee.http_clients import HttpClient, HttpResponse from crawlee.sessions import SessionPool from crawlee.statistics import Statistics from crawlee.storage_clients import SqlStorageClient @@ -977,10 +977,17 @@ async def handler(context: AdaptivePlaywrightCrawlingContext) -> None: async def test_throttled_static_429_is_deferred(monkeypatch: pytest.MonkeyPatch) -> None: """A throttled 429 from the static sub crawler defers the request without falling back to the browser.""" + url = 'https://throttled.placeholder.com/page' throttler = await open_throttler_held_until_reclaim(monkeypatch) http_client = AsyncMock(spec=HttpClient) - http_client.crawl.side_effect = [ - HttpCrawlingResult(http_response=Mock(status_code=status, headers={}, read=AsyncMock(return_value=b''))) + http_client.send_request.side_effect = [ + Mock( + spec=HttpResponse, + status_code=status, + headers={}, + url=url, + read=AsyncMock(return_value=b''), + ) for status in (429, 200) ] crawler = AdaptivePlaywrightCrawler.with_beautifulsoup_static_parser( @@ -1006,10 +1013,10 @@ async def track_browser_navigation(context: AdaptivePlaywrightPreNavCrawlingCont async def handler(context: AdaptivePlaywrightCrawlingContext) -> None: handled.append(context) - await crawler.run(['https://throttled.placeholder.com/page']) + await crawler.run([url]) browser_navigations.assert_not_called() - assert http_client.crawl.await_count == 2 + assert http_client.send_request.await_count == 2 assert len(handled) == 1 assert handled[0].request.retry_count == 0 @@ -1019,8 +1026,12 @@ async def test_static_handler_raised_throttled_error_is_deferred(monkeypatch: py url = 'https://throttled.placeholder.com/page' throttler = await open_throttler_held_until_reclaim(monkeypatch) http_client = AsyncMock(spec=HttpClient) - http_client.crawl.return_value = HttpCrawlingResult( - http_response=Mock(status_code=200, headers={}, read=AsyncMock(return_value=b'')) + http_client.send_request.return_value = Mock( + spec=HttpResponse, + status_code=200, + headers={}, + url=url, + read=AsyncMock(return_value=b''), ) crawler = AdaptivePlaywrightCrawler.with_beautifulsoup_static_parser( rendering_type_predictor=_SimpleRenderingTypePredictor( diff --git a/tests/unit/crawlers/_basic/test_basic_crawler.py b/tests/unit/crawlers/_basic/test_basic_crawler.py index 3b3e2d5fa6..a16deb5b7d 100644 --- a/tests/unit/crawlers/_basic/test_basic_crawler.py +++ b/tests/unit/crawlers/_basic/test_basic_crawler.py @@ -38,7 +38,7 @@ UserDefinedErrorHandlerError, ) from crawlee.events import Event, EventCrawlerStatusData, LocalEventManager -from crawlee.http_clients import HttpClient +from crawlee.http_clients import HttpClient, HttpResponse from crawlee.request_loaders import ( RequestList, RequestManagerTandem, @@ -645,7 +645,7 @@ async def handler(context: BasicCrawlingContext) -> None: async def test_send_request_forwards_timeout_to_http_client() -> None: http_client = AsyncMock(spec=HttpClient) - response = Mock() + response = Mock(spec=HttpResponse) http_client.send_request.return_value = response crawler = BasicCrawler(http_client=http_client, use_session_pool=False, max_requests_per_crawl=1) timeout = timedelta(seconds=12) @@ -676,6 +676,34 @@ async def handler(context: BasicCrawlingContext) -> None: ) +@pytest.mark.parametrize( + 'handler_fails', + [ + pytest.param(False, id='handled'), + pytest.param(True, id='failed'), + ], +) +async def test_send_request_response_closed_after_request(*, handler_fails: bool) -> None: + """Test that a response of `context.send_request` stays open in the handler and is closed after the request.""" + http_client = AsyncMock(spec=HttpClient) + response = Mock(spec=HttpResponse) + http_client.send_request.return_value = response + crawler = BasicCrawler(http_client=http_client, max_request_retries=0, max_requests_per_crawl=1) + handler_call = Mock() + + @crawler.router.default_handler + async def handler(context: BasicCrawlingContext) -> None: + await context.send_request('https://example.com') + handler_call(closed=response.aclose.called) + if handler_fails: + raise RuntimeError('Arbitrary crash for testing purposes') + + await crawler.run(['https://a.placeholder.com']) + + handler_call.assert_called_once_with(closed=False) + response.aclose.assert_awaited_once() + + async def test_send_request_respects_timeout(server_url: URL) -> None: request_timed_out = asyncio.Event() diff --git a/tests/unit/crawlers/_file_download/test_file_download.py b/tests/unit/crawlers/_file_download/test_file_download.py index 02d9fffc9e..55d83a049e 100644 --- a/tests/unit/crawlers/_file_download/test_file_download.py +++ b/tests/unit/crawlers/_file_download/test_file_download.py @@ -152,6 +152,29 @@ async def test_error_status_code(server_url: URL, *, stream: bool) -> None: handler.assert_not_called() +@pytest.mark.parametrize( + 'stream', + [ + pytest.param(False, id='buffered'), + pytest.param(True, id='stream'), + ], +) +async def test_loaded_url_after_redirect(server_url: URL, *, stream: bool) -> None: + """Test that `loaded_url` of a redirected request is the URL the file was downloaded from.""" + crawler = FileDownloadCrawler(stream=stream) + handler_call = Mock() + + @crawler.router.default_handler + async def request_handler(context: FileDownloadCrawlingContext) -> None: + handler_call(loaded_url=context.request.loaded_url) + + file_url = str(server_url.with_path('file').with_query(size=1024)) + stats = await crawler.run([str(server_url.with_path('redirect').with_query(url=file_url))]) + + assert stats.requests_failed == 0 + handler_call.assert_called_once_with(loaded_url=file_url) + + @pytest.mark.parametrize('content_type', ['application/pdf', 'image/png', 'video/mp4']) async def test_any_content_type(server_url: URL, content_type: str) -> None: """Any content type is downloaded as-is.""" diff --git a/tests/unit/crawlers/_http/test_http_crawler.py b/tests/unit/crawlers/_http/test_http_crawler.py index 0422b7456a..cd6306f6f4 100644 --- a/tests/unit/crawlers/_http/test_http_crawler.py +++ b/tests/unit/crawlers/_http/test_http_crawler.py @@ -1,10 +1,12 @@ from __future__ import annotations +import asyncio import json import logging +import time from datetime import UTC, datetime, timedelta from typing import TYPE_CHECKING, Any, cast -from unittest.mock import AsyncMock, Mock +from unittest.mock import AsyncMock, Mock, call from urllib.parse import parse_qs, urlencode import pytest @@ -12,7 +14,7 @@ from crawlee import ConcurrencySettings, Request, RequestState from crawlee.crawlers import HttpCrawler from crawlee.errors import PersistentRateLimitError -from crawlee.http_clients import HttpClient, HttpCrawlingResult +from crawlee.http_clients import HttpClient, HttpResponse from crawlee.request_loaders import ThrottlingRequestManager, _throttling_request_manager from crawlee.sessions import SessionPool from crawlee.statistics import Statistics @@ -693,9 +695,58 @@ async def failed_request_handler(context: BasicCrawlingContext, _error: Exceptio await queue.drop() -def crawl_result(status_code: int) -> HttpCrawlingResult: - """Build an HTTP client result with the given status code and an empty body.""" - return HttpCrawlingResult(http_response=Mock(status_code=status_code, headers={}, read=AsyncMock(return_value=b''))) +def mock_response(status_code: int, url: str) -> Mock: + """Build an HTTP response with the given status code and an empty body.""" + return Mock(spec=HttpResponse, status_code=status_code, headers={}, url=url, read=AsyncMock(return_value=b'')) + + +@pytest.mark.parametrize( + ('status_code', 'expected_handler_calls'), + [ + pytest.param(200, [call(closed=False)], id='handled'), + pytest.param(500, [], id='error_status'), + ], +) +async def test_navigation_response_closed_after_request(*, status_code: int, expected_handler_calls: list) -> None: + """Test that the response of the navigation request stays open in the handler and is closed after the request.""" + url = 'https://a.placeholder.com' + response = mock_response(status_code, url) + http_client = AsyncMock(spec=HttpClient) + http_client.send_request.return_value = response + crawler = HttpCrawler(http_client=http_client, max_request_retries=0) + handler_call = Mock() + + @crawler.router.default_handler + async def handler(_context: HttpCrawlingContext) -> None: + handler_call(closed=response.aclose.called) + + await crawler.run([url]) + + assert handler_call.call_args_list == expected_handler_calls + response.aclose.assert_awaited_once() + + +async def test_body_read_bound_by_navigation_timeout() -> None: + """Test that reading the body of the navigation response fails once the navigation timeout runs out.""" + url = 'https://a.placeholder.com' + response = mock_response(200, url) + response.read.side_effect = asyncio.Event().wait + http_client = AsyncMock(spec=HttpClient) + http_client.send_request.return_value = response + crawler = HttpCrawler( + http_client=http_client, + navigation_timeout=timedelta(milliseconds=100), + max_request_retries=0, + ) + handler = AsyncMock() + crawler.router.default_handler(handler) + failed_request_handler = AsyncMock() + crawler.failed_request_handler(failed_request_handler) + + await crawler.run([url]) + + handler.assert_not_called() + assert isinstance(failed_request_handler.call_args.args[1], TimeoutError) @pytest.mark.parametrize( @@ -710,7 +761,8 @@ async def test_throttled_429_is_deferred(crawler_kwargs: dict[str, Any], monkeyp """A throttled 429 is retried later without spending a retry, a session rotation or session reputation.""" throttler = await open_throttler_held_until_reclaim(monkeypatch) http_client = AsyncMock(spec=HttpClient) - http_client.crawl.side_effect = [crawl_result(429), crawl_result(200)] + url = 'https://throttled.placeholder.com/page' + http_client.send_request.side_effect = [mock_response(429, url), mock_response(200, url)] failed_request_handler = AsyncMock() crawler = HttpCrawler( http_client=http_client, @@ -727,7 +779,7 @@ async def test_throttled_429_is_deferred(crawler_kwargs: dict[str, Any], monkeyp async def handler(context: HttpCrawlingContext) -> None: handled.append(context) - await crawler.run(['https://throttled.placeholder.com/page']) + await crawler.run([url]) assert [context.http_response.status_code for context in handled] == [200] failed_request_handler.assert_not_called() @@ -797,3 +849,19 @@ async def is_finished_after_backoff() -> bool: # A failed run keeps the queue, and the stall clock still runs, so a re-run gives up again. with pytest.raises(PersistentRateLimitError): await crawler.run() + + +async def test_navigation_timeout_cancels_request(http_client: HttpClient, server_url: URL) -> None: + """Test that the navigation timeout cancels a request whose response doesn't arrive in time.""" + failed_request_handler = AsyncMock() + crawler = HttpCrawler( + http_client=http_client, navigation_timeout=timedelta(milliseconds=300), max_request_retries=0 + ) + crawler.failed_request_handler(failed_request_handler) + started = time.monotonic() + + await crawler.run([str((server_url / 'slow').with_query(delay=5))]) + + assert time.monotonic() - started < 5 + failed_request_handler.assert_called_once() + assert isinstance(failed_request_handler.call_args.args[1], TimeoutError) diff --git a/tests/unit/crawlers/_playwright/test_playwright_crawler.py b/tests/unit/crawlers/_playwright/test_playwright_crawler.py index aaeee53d5d..0c8578ce13 100644 --- a/tests/unit/crawlers/_playwright/test_playwright_crawler.py +++ b/tests/unit/crawlers/_playwright/test_playwright_crawler.py @@ -549,6 +549,43 @@ async def request_handler(context: PlaywrightCrawlingContext) -> None: assert session_cookies == {'check': 'test'} +async def test_send_request_sends_page_cookies(server_url: URL) -> None: + """Test that `context.send_request` sends the cookies set by the navigation and by the page in the handler.""" + crawler = PlaywrightCrawler() + sent_cookies: dict[str, str] = {} + + @crawler.router.default_handler + async def request_handler(context: PlaywrightCrawlingContext) -> None: + await context.page.context.add_cookies([{'name': 'check', 'value': 'test', 'url': str(server_url)}]) + # The redirect sets a cookie, so its second request carries a `Cookie` header. + response = await context.send_request(str(server_url.with_path('set_cookies').extend_query(b=2))) + sent_cookies.update(json.loads(await response.read())['cookies']) + + await crawler.run([str(server_url.with_path('set_cookies').extend_query(a=1))]) + + assert sent_cookies == {'a': '1', 'b': '2', 'check': 'test'} + + +async def test_send_request_skips_cookies_deleted_by_page(server_url: URL) -> None: + """Test that `context.send_request` doesn't send a session cookie that the page deleted in the handler.""" + crawler = PlaywrightCrawler(session_pool=SessionPool(max_pool_size=1)) + sent_cookies: list[dict[str, str]] = [] + + @crawler.router.default_handler + async def request_handler(context: PlaywrightCrawlingContext) -> None: + await context.add_requests([Request.from_url(str(server_url / 'cookies'), label='second')]) + + @crawler.router.handler('second') + async def second_handler(context: PlaywrightCrawlingContext) -> None: + await context.page.context.clear_cookies(name='a') + response = await context.send_request(str(server_url / 'cookies')) + sent_cookies.append(json.loads(await response.read())['cookies']) + + await crawler.run([str(server_url.with_path('set_cookies').extend_query(a=1))]) + + assert sent_cookies == [{}] + + async def test_read_write_cookies(server_url: URL) -> None: """Test that cookies are reloaded correctly.""" async with SessionPool(max_pool_size=1) as session_pool: diff --git a/tests/unit/crawlers/_playwright/test_playwright_http_client.py b/tests/unit/crawlers/_playwright/test_playwright_http_client.py index 2dad059799..18472fe1a1 100644 --- a/tests/unit/crawlers/_playwright/test_playwright_http_client.py +++ b/tests/unit/crawlers/_playwright/test_playwright_http_client.py @@ -4,16 +4,32 @@ from unittest.mock import AsyncMock, Mock, patch import pytest +from playwright.async_api import APIResponse from crawlee.crawlers._playwright._playwright_http_client import PlaywrightHttpClient, browser_page_context from crawlee.crawlers._playwright._types import PlaywrightHttpResponse +from crawlee.http_clients import HttpResponse + + +def mock_page() -> Mock: + """Build a browser page whose context has no cookies.""" + page = Mock() + page.context.cookies = AsyncMock(return_value=[]) + page.request.fetch = AsyncMock() + return page + + +def mock_response(status_code: int, url: str, *, location: str | None = None) -> Mock: + """Build a browser response with the given status code and an optional `Location` header.""" + headers = {'location': location} if location else {} + return Mock(spec=HttpResponse, status_code=status_code, headers=headers, url=url, headers_set_cookie=[]) async def test_send_request_converts_timeout_to_milliseconds() -> None: playwright_response = Mock() - expected_response = Mock() - page = Mock() - page.request.fetch = AsyncMock(return_value=playwright_response) + expected_response = mock_response(200, 'https://example.com/') + page = mock_page() + page.request.fetch.return_value = playwright_response client = PlaywrightHttpClient() with patch.object( @@ -30,15 +46,34 @@ async def test_send_request_converts_timeout_to_milliseconds() -> None: method='get', headers=None, data=None, - timeout=12_000, + timeout=pytest.approx(12_000, abs=100), + max_redirects=0, ) from_playwright_response.assert_awaited_once_with(playwright_response, protocol='') +async def test_send_request_follows_redirects_hop_by_hop() -> None: + """Test that `send_request` follows redirects itself and the browser sends every hop without following them.""" + redirect = mock_response(302, 'https://example.com/start', location='/final') + final = mock_response(200, 'https://example.com/final') + page = mock_page() + client = PlaywrightHttpClient() + + with patch.object(PlaywrightHttpResponse, 'from_playwright_response', new=AsyncMock(side_effect=[redirect, final])): + async with browser_page_context(page): + result = await client.send_request('https://example.com/start') + + assert result is final + assert [call.kwargs['url_or_request'] for call in page.request.fetch.await_args_list] == [ + 'https://example.com/start', + 'https://example.com/final', + ] + assert all(call.kwargs['max_redirects'] == 0 for call in page.request.fetch.await_args_list) + + async def test_send_request_zero_timeout_expires_immediately() -> None: """A `timedelta(0)` timeout must expire immediately rather than being forwarded as "disable timeout".""" - page = Mock() - page.request.fetch = AsyncMock() + page = mock_page() client = PlaywrightHttpClient() async with browser_page_context(page): @@ -46,3 +81,22 @@ async def test_send_request_zero_timeout_expires_immediately() -> None: await client.send_request('https://example.com', timeout=timedelta(0)) page.request.fetch.assert_not_awaited() + + +async def test_response_keeps_url_and_every_set_cookie_header() -> None: + """Test that a response of the browser keeps its URL and lists every `Set-Cookie` header.""" + api_response = Mock(spec=APIResponse) + api_response.url = 'https://example.com/final' + api_response.status = 200 + api_response.headers = {'set-cookie': 'a=1\nb=2', 'content-type': 'text/html'} + api_response.headers_array = [ + {'name': 'Set-Cookie', 'value': 'a=1'}, + {'name': 'set-cookie', 'value': 'b=2'}, + {'name': 'Content-Type', 'value': 'text/html'}, + ] + api_response.body.return_value = b'' + + response = await PlaywrightHttpResponse.from_playwright_response(api_response, protocol='') + + assert response.url == 'https://example.com/final' + assert response.headers_set_cookie == ['a=1', 'b=2'] diff --git a/tests/unit/http_clients/test_curl_impersonate.py b/tests/unit/http_clients/test_curl_impersonate.py index 446653e7db..6cfd0c5afc 100644 --- a/tests/unit/http_clients/test_curl_impersonate.py +++ b/tests/unit/http_clients/test_curl_impersonate.py @@ -1,8 +1,17 @@ from __future__ import annotations +import asyncio +from datetime import timedelta from typing import TYPE_CHECKING +from unittest.mock import AsyncMock, Mock +import pytest +from curl_cffi.requests import AsyncSession, Response +from curl_cffi.requests.exceptions import IncompleteRead + +from crawlee import HttpHeaders from crawlee.http_clients import CurlImpersonateHttpClient +from crawlee.http_clients._curl_impersonate import _CurlImpersonateResponse from crawlee.sessions import Session if TYPE_CHECKING: @@ -22,5 +31,112 @@ async def test_internal_session_cookie_jar_stays_empty(server_url: URL) -> None: curl_session = client._client_by_proxy_url[None] assert list(curl_session._cookies.jar) == [] - # The cookies are read from the `curl` handle, so they still reach the crawlee session. + # The cookies are stored by `send_request`, so they still reach the crawlee session. assert {cookie['name'] for cookie in session.cookies} == {f'c{index}' for index in range(5)} + + +async def test_read_after_failed_read_raises() -> None: + """Test that a body whose reading failed halfway can't be read again as an empty body.""" + curl_response = Mock(spec=Response) + curl_response.acontent.side_effect = [IncompleteRead('Connection dropped'), b''] + response = _CurlImpersonateResponse(curl_response, deadline=None) + + with pytest.raises(IncompleteRead): + await response.read() + + with pytest.raises(RuntimeError): + await response.read() + + +def mock_curl_session() -> Mock: + """Build a `curl_cffi` session whose requests return a mock response.""" + curl_session = Mock(spec=AsyncSession) + curl_session.request = AsyncMock(return_value=Mock(spec=Response)) + return curl_session + + +async def test_sub_millisecond_timeout_not_rounded_to_zero(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that a timeout under 1 ms reaches `curl_cffi` as 1 ms, since `libcurl` reads 0 ms as no timeout.""" + curl_session = mock_curl_session() + client = CurlImpersonateHttpClient() + monkeypatch.setattr(client, '_get_client', Mock(return_value=curl_session)) + + await client._fetch( + 'https://example.com/', + method='GET', + headers=HttpHeaders(), + payload=None, + proxy_info=None, + timeout=timedelta(microseconds=100), + ) + + assert curl_session.request.call_args.kwargs['timeout'] == 0.001 + + +async def test_no_timeout_uses_session_default(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that a request without a timeout gets the default timeout of the `curl_cffi` session, not none at all.""" + curl_session = mock_curl_session() + client = CurlImpersonateHttpClient() + monkeypatch.setattr(client, '_get_client', Mock(return_value=curl_session)) + + await client._fetch( + 'https://example.com/', + method='GET', + headers=HttpHeaders(), + payload=None, + proxy_info=None, + timeout=None, + ) + + assert 'timeout' not in curl_session.request.call_args.kwargs + + +async def test_timed_out_request_closes_connection() -> None: + """Test that a request that runs out of time before its response arrives closes the connection.""" + client_closed = asyncio.Event() + + async def handle(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + await reader.readuntil(b'\r\n\r\n') + # The server never answers, it only waits for the client to close the connection. + await reader.read() + client_closed.set() + writer.close() + + server = await asyncio.start_server(handle, '127.0.0.1', 0) + port = server.sockets[0].getsockname()[1] + + async with server, CurlImpersonateHttpClient() as client: + with pytest.raises(TimeoutError): + await client.send_request(f'http://127.0.0.1:{port}/', timeout=timedelta(seconds=0.2)) + + await asyncio.wait_for(client_closed.wait(), timeout=2) + + +async def test_closing_response_aborts_unread_body() -> None: + """Test that closing a response with an unread body closes the connection and leaves the body unreadable.""" + client_closed = asyncio.Event() + + async def handle(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + await reader.readuntil(b'\r\n\r\n') + writer.write(b'HTTP/1.1 200 OK\r\nContent-Length: 1000000\r\n\r\n') + try: + for _ in range(1000): + writer.write(b'x' * 1000) + await writer.drain() + await asyncio.sleep(0.01) + except ConnectionError: + client_closed.set() + finally: + writer.close() + + server = await asyncio.start_server(handle, '127.0.0.1', 0) + port = server.sockets[0].getsockname()[1] + + async with server, CurlImpersonateHttpClient() as client: + response = await client.send_request(f'http://127.0.0.1:{port}/') + await response.aclose() + + await asyncio.wait_for(client_closed.wait(), timeout=2) + + with pytest.raises(RuntimeError): + await response.read() diff --git a/tests/unit/http_clients/test_http_clients.py b/tests/unit/http_clients/test_http_clients.py index 56325f6108..36e53e0d5b 100644 --- a/tests/unit/http_clients/test_http_clients.py +++ b/tests/unit/http_clients/test_http_clients.py @@ -7,18 +7,20 @@ import sys import time from datetime import timedelta -from typing import TYPE_CHECKING -from unittest.mock import patch +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock, Mock, patch import pytest from curl_cffi import CurlHttpVersion from pydantic import ValidationError +from yarl import URL -from crawlee import Request from crawlee.errors import ProxyError -from crawlee.http_clients import CurlImpersonateHttpClient, HttpClient, HttpxHttpClient, ImpitHttpClient +from crawlee.http_clients import CurlImpersonateHttpClient, HttpClient, HttpResponse, HttpxHttpClient, ImpitHttpClient +from crawlee.http_clients._base import _is_cross_origin, _redirect_method +from crawlee.http_clients._httpx import _HttpxResponse +from crawlee.http_clients._impit import _ImpitResponse from crawlee.sessions import CookieParam, Session -from crawlee.statistics import Statistics from tests.unit.server import generate_file_content from tests.unit.server_endpoints import HELLO_WORLD @@ -26,12 +28,23 @@ from collections.abc import AsyncGenerator from _pytest.fixtures import SubRequest - from yarl import URL - from crawlee.http_clients import HttpResponse + from crawlee._types import HttpMethod from crawlee.proxy_configuration import ProxyInfo +@pytest.fixture( + params=[ + pytest.param(ImpitHttpClient, id='impit'), + pytest.param(HttpxHttpClient, id='httpx'), + pytest.param(CurlImpersonateHttpClient, id='curl'), + ] +) +def http_client_class(request: SubRequest) -> type[HttpClient]: + """Provide each HTTP client class, for tests that construct the client themselves.""" + return request.param + + @pytest.fixture async def custom_http_client(request: SubRequest) -> AsyncGenerator[HttpClient]: """Helper fixture to reduce code duplication. @@ -59,29 +72,6 @@ async def test_send_request_zero_timeout_expires_immediately(http_client: HttpCl assert time.monotonic() - start < 1 -async def test_crawl_zero_timeout_expires_immediately(http_client: HttpClient, server_url: URL) -> None: - """A `timedelta(0)` timeout must expire immediately rather than being silently treated as no timeout.""" - slow_url = str((server_url / 'slow').with_query(delay=2)) - start = time.monotonic() - - with pytest.raises(TimeoutError): - await http_client.crawl(Request.from_url(slow_url), timeout=timedelta(0)) - - assert time.monotonic() - start < 1 - - -async def test_stream_zero_timeout_expires_immediately(http_client: HttpClient, server_url: URL) -> None: - """A `timedelta(0)` timeout must expire immediately rather than being silently treated as no timeout.""" - slow_url = str((server_url / 'slow').with_query(delay=2)) - start = time.monotonic() - - with pytest.raises(TimeoutError): - async with http_client.stream(slow_url, timeout=timedelta(0)): - pass - - assert time.monotonic() - start < 1 - - async def test_http_1(http_client: HttpClient, server_url: URL) -> None: response = await http_client.send_request(str(server_url)) assert response.http_version == 'HTTP/1.1' @@ -107,34 +97,6 @@ async def test_http_2(custom_http_client: HttpClient) -> None: assert response.http_version == 'HTTP/2' -@pytest.mark.skipif(os.name == 'nt', reason='Skipped on Windows') -async def test_crawl_with_proxy( - http_client: HttpClient, - proxy: ProxyInfo, - server_url: URL, -) -> None: - url = str(server_url / 'status/222') - request = Request.from_url(url) - - async with Statistics.with_default_state() as statistics: - result = await http_client.crawl(request, proxy_info=proxy, statistics=statistics) - - assert result.http_response.status_code == 222 # 222 - authentication successful - - -@pytest.mark.skipif(os.name == 'nt', reason='Skipped on Windows') -async def test_crawl_with_proxy_disabled( - http_client: HttpClient, - disabled_proxy: ProxyInfo, -) -> None: - url = 'https://apify.com/' - request = Request.from_url(url) - - with pytest.raises(ProxyError): - async with Statistics.with_default_state() as statistics: - await http_client.crawl(request, proxy_info=disabled_proxy, statistics=statistics) - - @pytest.mark.skipif(os.name == 'nt', reason='Skipped on Windows') async def test_send_request_with_proxy( http_client: HttpClient, @@ -167,11 +129,6 @@ async def test_session_cookies_sent_through_proxy( """Test that requests going through a proxy carry the session cookies and stay isolated per session.""" session = Session(cookies=[CookieParam(name='jar', value='1', domain=server_url.host or '')]) - request = Request.from_url(str(server_url / 'cookies')) - crawling_result = await http_client.crawl(request, session=session, proxy_info=proxy) - - assert (await read_json(crawling_result.http_response))['cookies'] == {'jar': '1'} - response = await http_client.send_request(str(server_url / 'cookies'), session=session, proxy_info=proxy) assert (await read_json(response))['cookies'] == {'jar': '1'} @@ -194,140 +151,288 @@ async def test_session_cookies_sent_through_proxy( assert (await read_json(other_response))['cookies'] == {} -async def test_crawl_allow_redirects_by_default(http_client: HttpClient, server_url: URL) -> None: +async def test_response_url_after_redirect(http_client: HttpClient, server_url: URL) -> None: + """Test that the `url` of a redirected response is the URL of the last request in the chain.""" target_url = str(server_url / 'status/200') redirect_url = str((server_url / 'redirect').update_query(url=target_url)) - request = Request.from_url(redirect_url) - crawling_result = await http_client.crawl(request) - assert crawling_result.http_response.status_code == 200 - assert request.loaded_url == target_url + response = await http_client.send_request(redirect_url) + assert response.url == target_url -@pytest.mark.parametrize( - 'custom_http_client', - [ - pytest.param(CurlImpersonateHttpClient(allow_redirects=False), id='curl'), - pytest.param(HttpxHttpClient(follow_redirects=False), id='httpx'), - pytest.param(ImpitHttpClient(follow_redirects=False), id='impit'), - ], - indirect=['custom_http_client'], -) -async def test_crawl_allow_redirects_false(custom_http_client: HttpClient, server_url: URL) -> None: + +async def test_send_request_allow_redirects_by_default(http_client: HttpClient, server_url: URL) -> None: target_url = str(server_url / 'status/200') redirect_url = str((server_url / 'redirect').update_query(url=target_url)) - request = Request.from_url(redirect_url) - crawling_result = await custom_http_client.crawl(request) + response = await http_client.send_request(redirect_url) - assert crawling_result.http_response.status_code == 302 - assert crawling_result.http_response.headers['Location'] == target_url - assert request.loaded_url == redirect_url + assert response.status_code == 200 -async def test_send_request_allow_redirects_by_default(http_client: HttpClient, server_url: URL) -> None: - target_url = str(server_url / 'status/200') - redirect_url = str((server_url / 'redirect').update_query(url=target_url)) +async def test_ten_redirects_followed(http_client: HttpClient, server_url: URL) -> None: + """Test that a redirect chain of exactly 10 redirects is followed to its end.""" + url = server_url / 'status/200' + for _ in range(10): + url = (server_url / 'redirect').with_query(url=str(url)) - response = await http_client.send_request(redirect_url) + response = await http_client.send_request(str(url)) assert response.status_code == 200 +async def test_too_many_redirects(http_client: HttpClient, server_url: URL) -> None: + """Test that a redirect chain longer than 10 redirects raises an error.""" + with pytest.raises(RuntimeError, match=r'Too many redirects \(10\)'): + await http_client.send_request(str(server_url / 'redirect_loop')) + + +async def test_timeout_covers_redirect_chain(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that every request of a redirect chain gets what is left of the timeout, until it runs out.""" + url = 'https://example.com/' + redirect = Mock(spec=HttpResponse, status_code=302, headers={'location': '/next'}, url=url, headers_set_cookie=[]) + fetch = AsyncMock(return_value=redirect) + client = ImpitHttpClient() + monkeypatch.setattr(client, '_fetch', fetch) + # The deadline is set at 0, and the requests of the chain start at 0, 4 and 11 seconds. + monkeypatch.setattr('crawlee.http_clients._base.monotonic', Mock(side_effect=[0, 0, 4, 11])) + + with pytest.raises(TimeoutError): + await client.send_request(url, timeout=timedelta(seconds=10)) + + timeouts = [fetch_call.kwargs['timeout'] for fetch_call in fetch.call_args_list] + assert timeouts == [timedelta(seconds=10), timedelta(seconds=6)] + + +async def test_redirect_responses_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that every response of a redirect chain is closed except the returned one.""" + url = 'https://example.com/' + redirect = Mock(spec=HttpResponse, status_code=302, headers={'location': '/next'}, url=url, headers_set_cookie=[]) + final = Mock(spec=HttpResponse, status_code=200, headers={}, url=f'{url}next', headers_set_cookie=[]) + client = ImpitHttpClient() + monkeypatch.setattr(client, '_fetch', AsyncMock(side_effect=[redirect, final])) + + response = await client.send_request(url) + + assert response is final + redirect.aclose.assert_awaited_once() + final.aclose.assert_not_awaited() + + +async def test_redirect_response_closed_on_invalid_location(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that a redirect response whose `Location` can't be parsed is closed before the error propagates.""" + url = 'https://example.com/' + location = 'http://[invalid' + redirect = Mock(spec=HttpResponse, status_code=302, headers={'location': location}, url=url, headers_set_cookie=[]) + client = ImpitHttpClient() + monkeypatch.setattr(client, '_fetch', AsyncMock(return_value=redirect)) + + with pytest.raises(ValueError, match='IPv6'): + await client.send_request(url) + + redirect.aclose.assert_awaited_once() + + @pytest.mark.parametrize( - 'custom_http_client', + ('path', 'query', 'expected_status_code'), [ - pytest.param(CurlImpersonateHttpClient(allow_redirects=False), id='curl'), - pytest.param(HttpxHttpClient(follow_redirects=False), id='httpx'), - pytest.param(ImpitHttpClient(follow_redirects=False), id='impit'), + pytest.param('status/302', {}, 302, id='missing-location'), + pytest.param('redirect', {'url': 'ftp://127.0.0.1/file'}, 302, id='non-http-location'), + pytest.param('redirect', {'url': '/status/200', 'status': '300'}, 300, id='non-redirect-status'), ], - indirect=['custom_http_client'], ) -async def test_send_request_allow_redirects_false(custom_http_client: HttpClient, server_url: URL) -> None: - target_url = str(server_url / 'status/200') - redirect_url = str((server_url / 'redirect').update_query(url=target_url)) +async def test_redirect_not_followed( + http_client: HttpClient, server_url: URL, path: str, query: dict[str, str], expected_status_code: int +) -> None: + """Test that a 3xx without a `Location`, to a non-HTTP scheme or of a non-redirect status is returned as it is.""" + response = await http_client.send_request(str((server_url / path).with_query(query))) + + assert response.status_code == expected_status_code + + +@pytest.mark.parametrize( + ('status_code', 'method', 'expected_method', 'expected_body'), + [ + pytest.param(301, 'POST', 'GET', '', id='301-post'), + pytest.param(301, 'PUT', 'PUT', 'payload', id='301-put'), + pytest.param(302, 'POST', 'GET', '', id='302-post'), + pytest.param(302, 'PUT', 'PUT', 'payload', id='302-put'), + pytest.param(303, 'POST', 'GET', '', id='303-post'), + pytest.param(303, 'PUT', 'GET', '', id='303-put'), + pytest.param(307, 'POST', 'POST', 'payload', id='307-post'), + pytest.param(308, 'PUT', 'PUT', 'payload', id='308-put'), + ], +) +async def test_redirect_method( + http_client: HttpClient, + server_url: URL, + *, + status_code: int, + method: HttpMethod, + expected_method: str, + expected_body: str, +) -> None: + """Test that the method and the body of a redirected request follow the WHATWG Fetch algorithm.""" + redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'method'), status=status_code) + + response = await http_client.send_request( + str(redirect_url), + method=method, + payload=b'payload', + headers={'content-type': 'application/octet-stream'}, + ) + echo = await read_json(response) - response = await custom_http_client.send_request(redirect_url) + assert echo['method'] == expected_method + assert echo['body'] == expected_body - assert response.status_code == 302 - assert response.headers['Location'] == target_url +def test_see_other_keeps_head() -> None: + """Test that a `303` redirect of a `HEAD` request is followed with `HEAD`.""" + assert _redirect_method(303, 'HEAD') == 'HEAD' -async def test_stream(http_client: HttpClient, server_url: URL) -> None: + +async def test_body_headers_dropped(http_client: HttpClient, server_url: URL) -> None: + """Test that headers describing the request body are dropped once a redirect turns the request into a `GET`.""" + redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'headers'), status=302) + + response = await http_client.send_request( + str(redirect_url), + method='POST', + payload=b'payload', + headers={'content-type': 'application/json', 'content-language': 'uk', 'x-custom': 'kept'}, + ) + headers = await read_json(response) + + assert 'content-type' not in headers + assert 'content-language' not in headers + assert headers['x-custom'] == 'kept' + + +@pytest.mark.parametrize( + ('url', 'other', 'expected'), + [ + pytest.param('http://a.com/x', 'http://a.com/x', False, id='same'), + pytest.param('http://a.com/x', 'https://a.com/x', True, id='different-scheme'), + pytest.param('http://a.com/x', 'http://b.com/x', True, id='different-host'), + pytest.param('http://A.com/x', 'http://a.com/y', False, id='different-host-case'), + pytest.param('http://a.com/x', 'http://a.com:8080/x', True, id='different-port'), + pytest.param('http://a.com:80/x', 'http://a.com/y', False, id='explicit-default-port'), + pytest.param('http://a.com/x', 'http://a.com/y', False, id='different-path'), + ], +) +def test_is_cross_origin(url: str, other: str, *, expected: bool) -> None: + """Test that a redirect leaves the origin when the scheme, host or port changes.""" + assert _is_cross_origin(URL(url, encoded=True), URL(other, encoded=True)) is expected + + +async def test_read_stream(http_client: HttpClient, server_url: URL) -> None: + """Test that `read_stream` yields the whole body.""" content_body: bytes = b'' - async with http_client.stream(str(server_url)) as response: - assert response.status_code == 200 - async for chunk in response.read_stream(): - content_body += chunk + response = await http_client.send_request(str(server_url)) + assert response.status_code == 200 + async for chunk in response.read_stream(): + content_body += chunk assert content_body == HELLO_WORLD -async def test_stream_read_after_transfer_finished(http_client: HttpClient, server_url: URL) -> None: +async def test_read_stream_after_transfer_finished(http_client: HttpClient, server_url: URL) -> None: """A small body can be fully buffered before `read_stream` is called. It must still be readable.""" file_size = 16 content_body: bytes = b'' small_file_url = str((server_url / 'file').update_query(size=file_size)) - async with http_client.stream(small_file_url) as response: - # Guarantee at least one event-loop tick for the response to be buffered. - await asyncio.sleep(0) + response = await http_client.send_request(small_file_url) + # Guarantee at least one event-loop tick for the response to be buffered. + await asyncio.sleep(0) - async for chunk in response.read_stream(): - content_body += chunk + async for chunk in response.read_stream(): + content_body += chunk assert content_body == generate_file_content(file_size) -async def test_stream_with_empty_body(http_client: HttpClient, server_url: URL) -> None: - """A streamed response with an empty body must be readable and yield no chunks.""" +async def test_read_stream_with_empty_body(http_client: HttpClient, server_url: URL) -> None: + """A response with an empty body must be readable as a stream and yield no chunks.""" content_body: bytes = b'' - async with http_client.stream(str(server_url / 'status/200')) as response: - assert response.status_code == 200 - async for chunk in response.read_stream(): - content_body += chunk + response = await http_client.send_request(str(server_url / 'status/200')) + assert response.status_code == 200 + async for chunk in response.read_stream(): + content_body += chunk assert content_body == b'' -async def test_stream_error_double_read_stream(http_client: HttpClient, server_url: URL) -> None: - async with http_client.stream(str(server_url)) as response: - assert response.status_code == 200 - content_body_first: bytes = b'' - async for chunk in response.read_stream(): - content_body_first += chunk +async def test_read_stream_twice_raises(http_client: HttpClient, server_url: URL) -> None: + """Test that a body consumed by `read_stream` can't be streamed again.""" + response = await http_client.send_request(str(server_url)) + content_body_first: bytes = b'' + async for chunk in response.read_stream(): + content_body_first += chunk - with pytest.raises(RuntimeError): - [chunk async for chunk in response.read_stream()] + with pytest.raises(RuntimeError): + [chunk async for chunk in response.read_stream()] assert content_body_first == HELLO_WORLD -async def test_stream_error_for_read(http_client: HttpClient, server_url: URL) -> None: - async with http_client.stream(str(server_url)) as response: - assert response.status_code == 200 +async def test_read_after_read_stream_raises(http_client: HttpClient, server_url: URL) -> None: + """Test that a body consumed by `read_stream` can't be read.""" + response = await http_client.send_request(str(server_url)) + [chunk async for chunk in response.read_stream()] - with pytest.raises(RuntimeError): - await response.read() + with pytest.raises(RuntimeError): + await response.read() -async def test_send_request_error_for_read_stream(http_client: HttpClient, server_url: URL) -> None: +async def test_read_stream_after_read_raises(http_client: HttpClient, server_url: URL) -> None: + """Test that a body consumed by `read` can't be streamed.""" response = await http_client.send_request(str(server_url)) + await response.read() - assert response.status_code == 200 with pytest.raises(RuntimeError): - [item async for item in response.read_stream()] + [chunk async for chunk in response.read_stream()] -async def test_send_crawl_error_for_read_stream(http_client: HttpClient, server_url: URL) -> None: - response = await http_client.crawl(Request.from_url(str(server_url))) - http_response = response.http_response +async def test_response_context_manager_closes_response(http_client: HttpClient, server_url: URL) -> None: + """Test that leaving the context of a response closes it.""" + response = await http_client.send_request(str(server_url)) + + with patch.object(response, 'aclose', wraps=response.aclose) as aclose: + async with response as entered: + assert entered is response + aclose.assert_not_called() + + aclose.assert_awaited_once() + + +async def test_read_twice_returns_same_body(http_client: HttpClient, server_url: URL) -> None: + """Test that repeated `read` calls return the same body.""" + response = await http_client.send_request(str(server_url)) + + assert await response.read() == HELLO_WORLD + assert await response.read() == HELLO_WORLD - assert http_response.status_code == 200 - with pytest.raises(RuntimeError): - [item async for item in http_response.read_stream()] + +async def test_read_again_after_timeout(http_client: HttpClient, server_url: URL) -> None: + """Test that a body read within the timeout is returned again by `read` once the timeout has passed.""" + response = await http_client.send_request(str(server_url), timeout=timedelta(milliseconds=200)) + body = await response.read() + + await asyncio.sleep(0.3) + + assert await response.read() == body + + +async def test_aclose_is_idempotent(http_client: HttpClient, server_url: URL) -> None: + """Test that closing a response a second time has no effect.""" + response = await http_client.send_request(str(server_url)) + + await response.aclose() + await response.aclose() @pytest.mark.parametrize( @@ -361,13 +466,14 @@ async def test_work_after_cleanup(http_client: HttpClient, server_url: URL) -> N assert response.status_code == 200 -async def test_compressed_chunked_stream(http_client: HttpClient, server_url: URL) -> None: +async def test_compressed_chunked_read_stream(http_client: HttpClient, server_url: URL) -> None: + """Test that `read_stream` yields a compressed body decompressed.""" content_body: bytes = b'' - async with http_client.stream(str(server_url / 'get_compressed')) as response: - assert response.status_code == 200 - async for chunk in response.read_stream(): - content_body += chunk + response = await http_client.send_request(str(server_url / 'get_compressed')) + assert response.status_code == 200 + async for chunk in response.read_stream(): + content_body += chunk assert content_body == HELLO_WORLD * 1000 @@ -377,12 +483,6 @@ async def test_send_request_rejects_non_http_scheme(http_client: HttpClient) -> await http_client.send_request('gopher://127.0.0.1:6379/_PING') -async def test_stream_rejects_non_http_scheme(http_client: HttpClient) -> None: - with pytest.raises(ValidationError): - async with http_client.stream('gopher://127.0.0.1:6379/_PING'): - pass - - @pytest.mark.parametrize( ('optional_module_name', 'import_path'), [ @@ -439,6 +539,48 @@ async def test_cookies_collected_on_redirect(http_client: HttpClient, server_url assert (await read_json(response))['cookies'] == {'a': '1'} +async def test_cookies_kept_for_redirect_chain_without_session(http_client: HttpClient, server_url: URL) -> None: + """Test that without a session the cookies set during a redirect chain reach its next hops, but no other call.""" + response = await http_client.send_request(str((server_url / 'set_cookies').with_query(a='1'))) + other_response = await http_client.send_request(str(server_url / 'cookies')) + + assert (await read_json(response))['cookies'] == {'a': '1'} + assert (await read_json(other_response))['cookies'] == {} + + +async def test_headers_set_cookie_lists_every_header(http_client: HttpClient, server_url: URL) -> None: + """Test that `headers_set_cookie` holds every `Set-Cookie` header of the response.""" + response = await http_client.send_request(str(server_url / 'set_complex_cookies')) + + names = [header.partition('=')[0] for header in response.headers_set_cookie] + assert names == ['basic', 'withpath', 'strict', 'secure', 'short', 'domain'] + + +async def test_every_response_cookie_stored(http_client: HttpClient, server_url: URL) -> None: + """Test that every cookie of a response setting several cookies is stored in the session.""" + session = Session() + + await http_client.send_request(str((server_url / 'set_cookies').with_query(a='1', b='2', c='3')), session=session) + + assert {item['name'] for item in session.cookies.get_cookies_as_dicts()} == {'a', 'b', 'c'} + + +async def test_cookies_of_non_ascii_host_sent_back(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that a host with a non-ASCII name is requested in its IDNA form and gets its cookies back.""" + ascii_url = 'http://xn--bcher-kva.example/' + response = Mock(spec=HttpResponse, status_code=200, headers={}, url=ascii_url, headers_set_cookie=['sid=1']) + fetch = AsyncMock(return_value=response) + client = ImpitHttpClient() + monkeypatch.setattr(client, '_fetch', fetch) + session = Session() + + await client.send_request('http://b\u00fccher.example/', session=session) + await client.send_request('http://b\u00fccher.example/', session=session) + + assert fetch.call_args.args[0] == ascii_url + assert fetch.call_args.kwargs['headers']['cookie'] == 'sid=1' + + @pytest.mark.parametrize( 'custom_http_client', [ @@ -475,6 +617,18 @@ async def test_cookies_sent_when_not_persisted(custom_http_client: HttpClient, s assert (await read_json(response))['cookies'] == {'from_jar': '1'} +async def test_redirect_cookies_sent_when_not_persisted(server_url: URL, http_client_class: type[HttpClient]) -> None: + """Test that with `persist_cookies_per_session` off the cookies set during a redirect chain reach its next hops.""" + session = Session() + + async with http_client_class(persist_cookies_per_session=False) as client: + response = await client.send_request(str((server_url / 'set_cookies').with_query(a='1')), session=session) + cookies = (await read_json(response))['cookies'] + + assert cookies == {'a': '1'} + assert session.cookies.get_cookies_as_dicts() == [] + + async def test_cookie_header_rebuilt_per_hop(http_client: HttpClient, server_url: URL) -> None: """Test that the `Cookie` header of one hop does not reach a hop whose URL the cookie does not match.""" session = Session( @@ -501,6 +655,15 @@ async def test_cookie_header_wins_over_session(http_client: HttpClient, server_u assert (await read_json(response))['cookies'] == {'manual': 'value'} +async def test_cookie_header_not_stored_in_session(http_client: HttpClient, server_url: URL) -> None: + """Test that the cookies of the `Cookie` header of the caller are not stored in the session.""" + session = Session() + + await http_client.send_request(str(server_url / 'cookies'), session=session, headers={'cookie': 'manual=value'}) + + assert session.cookies.get_cookies_as_dicts() == [] + + async def test_cookie_header_kept_same_origin(http_client: HttpClient, server_url: URL) -> None: """Test that a `Cookie` header set by the caller survives a redirect within the origin.""" redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'cookies'), status=302) @@ -595,35 +758,211 @@ async def test_auth_dropped_cross_origin( assert headers['x-custom'] == 'kept' -async def test_stream_follows_redirects(http_client: HttpClient, server_url: URL) -> None: - """Test that streamed requests follow redirects and carry session cookies along.""" +async def test_send_request_keeps_cookies_and_encoding(http_client: HttpClient, server_url: URL) -> None: + """Test that session cookies survive a redirect and that signed URLs are sent without re-encoding.""" + session = Session(cookies=[CookieParam(name='preset', value='value', domain=server_url.host or '')]) + + signed_query = 'X-Amz-Credential=AKIA%2F20240101%2Fus-east-1&X-Amz-Date=2024-01-01T00%3A00%3A00Z' + target_url = f'{server_url / "cookies"}?{signed_query}' + + direct = await http_client.send_request(target_url, session=session) + + assert (await read_json(direct))['cookies'] == {'preset': 'value'} + assert direct.url == target_url + + redirect_url = str((server_url / 'redirect').with_query(url=target_url, status=302)) + redirected = await http_client.send_request(redirect_url, session=session) + + assert (await read_json(redirected))['cookies'] == {'preset': 'value'} + assert redirected.url == target_url + + +@pytest.mark.parametrize( + ('client_class', 'kwarg'), + [ + pytest.param(ImpitHttpClient, 'cookies', id='impit_cookies'), + pytest.param(ImpitHttpClient, 'cookie_jar', id='impit_cookie_jar'), + pytest.param(HttpxHttpClient, 'cookies', id='httpx_cookies'), + pytest.param(HttpxHttpClient, 'auth', id='httpx_auth'), + pytest.param(HttpxHttpClient, 'params', id='httpx_params'), + pytest.param(CurlImpersonateHttpClient, 'cookies', id='curl_cookies'), + pytest.param(CurlImpersonateHttpClient, 'auth', id='curl_auth'), + pytest.param(CurlImpersonateHttpClient, 'params', id='curl_params'), + ], +) +def test_ignored_argument_warns(client_class: type[HttpClient], kwarg: str) -> None: + """Test that an argument of the underlying library that the client ignores raises a warning.""" + with pytest.warns(UserWarning, match=f'`{kwarg}` argument'): + client_class(**{kwarg: Mock()}) + + +@pytest.mark.parametrize( + ('client_class', 'kwargs'), + [ + pytest.param(ImpitHttpClient, {'max_redirects': 0}, id='impit_max_redirects'), + pytest.param(HttpxHttpClient, {'max_redirects': 0}, id='httpx_max_redirects'), + pytest.param(CurlImpersonateHttpClient, {'max_redirects': 0}, id='curl_max_redirects'), + pytest.param(CurlImpersonateHttpClient, {'allow_redirects': False}, id='curl_allow_redirects'), + ], +) +async def test_library_redirect_options_ignored( + server_url: URL, + client_class: type[HttpClient], + kwargs: dict[str, Any], +) -> None: + """Test that the redirect options of the underlying library raise a warning and redirects are still followed.""" + with pytest.warns(UserWarning, match='argument'): + client = client_class(**kwargs) + + redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'cookies'), status=302) + + async with client: + response = await client.send_request(str(redirect_url)) + + assert response.status_code == 200 + + +@pytest.mark.parametrize( + ('follow_redirects', 'expected_status_code'), + [ + pytest.param(True, 200, id='followed'), + pytest.param(False, 302, id='not_followed'), + ], +) +async def test_follow_redirects( + server_url: URL, + http_client_class: type[HttpClient], + *, + follow_redirects: bool, + expected_status_code: int, +) -> None: + """Test that `follow_redirects` decides whether a redirect is followed, while its cookies are stored either way.""" session = Session() - stream_url = (server_url / 'set_cookies').with_query(a='1') - async with http_client.stream(str(stream_url), session=session) as response: - content = b'' - async for chunk in response.read_stream(): - content += chunk + async with http_client_class(follow_redirects=follow_redirects) as client: + response = await client.send_request(str((server_url / 'set_cookies').with_query(a='1')), session=session) + + assert response.status_code == expected_status_code - assert json.loads(content.decode())['cookies'] == {'a': '1'} assert {item['name'] for item in session.cookies.get_cookies_as_dicts()} == {'a'} -async def test_crawl_keeps_cookies_and_encoding(http_client: HttpClient, server_url: URL) -> None: - """Test that `crawl` carries session cookies through a redirect and sends signed URLs without re-encoding.""" - session = Session(cookies=[CookieParam(name='preset', value='value', domain=server_url.host or '')]) +async def test_closing_unread_response_reuses_connection(http_client_class: type[HttpClient]) -> None: + """Test that a redirect hop and a response closed unread keep their connection when their body is small.""" + connection_opened = Mock() + + async def handle(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + connection_opened() + try: + while True: + request_head = await reader.readuntil(b'\r\n\r\n') + if request_head.startswith(b'GET /start '): + writer.write(b'HTTP/1.1 302 Found\r\nLocation: /final\r\nContent-Length: 5\r\n\r\n') + body = b'moved' + else: + writer.write(b'HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n') + body = b'ok' + await writer.drain() + # The body arrives after the headers, so a response closed right away hasn't received it yet. + await asyncio.sleep(0.05) + writer.write(body) + await writer.drain() + except (asyncio.IncompleteReadError, ConnectionError): + pass + finally: + writer.close() - signed_query = 'X-Amz-Credential=AKIA%2F20240101%2Fus-east-1&X-Amz-Date=2024-01-01T00%3A00%3A00Z' - target_url = f'{server_url / "cookies"}?{signed_query}' + server = await asyncio.start_server(handle, '127.0.0.1', 0) + port = server.sockets[0].getsockname()[1] - direct_request = Request.from_url(target_url) - direct_result = await http_client.crawl(direct_request, session=session) + async with server, http_client_class() as client: + # `send_request` closes the redirect hop, and the final response is closed without reading it. + response = await client.send_request(f'http://127.0.0.1:{port}/start') + await response.aclose() + + response = await client.send_request(f'http://127.0.0.1:{port}/final') + async with response: + assert await response.read() == b'ok' + + connection_opened.assert_called_once() + + +@pytest.mark.parametrize( + 'response_class', + [ + pytest.param(_ImpitResponse, id='impit'), + pytest.param(_HttpxResponse, id='httpx'), + ], +) +async def test_aclose_closes_when_cancelled_while_reading( + response_class: type[_ImpitResponse | _HttpxResponse], +) -> None: + """Test that `aclose` closes the response of the library when it is cancelled while reading a small body.""" + library_response = Mock(headers={'content-length': '10'}) + library_response.aread = AsyncMock(side_effect=asyncio.CancelledError) + library_response.aclose = AsyncMock() + response = response_class(library_response, deadline=None) + + with pytest.raises(asyncio.CancelledError): + await response.aclose() + + library_response.aclose.assert_awaited_once() + + +async def test_client_headers_sent(server_url: URL, http_client_class: type[HttpClient]) -> None: + """Test that the headers given to the client are sent, and the headers of a request win over them.""" + async with http_client_class(headers={'x-client': 'client', 'x-both': 'client'}) as client: + response = await client.send_request(str(server_url / 'headers'), headers={'x-both': 'request'}) + headers = await read_json(response) + + assert headers['x-client'] == 'client' + assert headers['x-both'] == 'request' + + +async def test_client_auth_header_dropped_cross_origin( + server_url: URL, + redirect_server_url: URL, + http_client_class: type[HttpClient], +) -> None: + """Test that an `Authorization` header given to the client is dropped as soon as a redirect leaves the origin.""" + same_origin_url = (server_url / 'redirect').with_query(url=str(server_url / 'headers'), status=302) + cross_origin_url = (server_url / 'redirect').with_query(url=str(redirect_server_url / 'headers'), status=302) + + async with http_client_class(headers={'authorization': 'Bearer token'}) as client: + same_origin_headers = await read_json(await client.send_request(str(same_origin_url))) + cross_origin_headers = await read_json(await client.send_request(str(cross_origin_url))) + + assert same_origin_headers['authorization'] == 'Bearer token' + assert 'authorization' not in cross_origin_headers + + +async def test_timeout_bounds_waiting_for_headers(http_client: HttpClient, server_url: URL) -> None: + """Test that the timeout is a wall-clock deadline, even for a client that rounds its own timeout up.""" + started = time.monotonic() + + with pytest.raises(TimeoutError): + await http_client.send_request( + str((server_url / 'slow').with_query(delay=3)), + timeout=timedelta(milliseconds=200), + ) + + assert time.monotonic() - started < 0.8 + + +async def test_timeout_bounds_read(http_client: HttpClient, server_url: URL) -> None: + """Test that the timeout of `send_request` also bounds reading the body with `read`.""" + url = str((server_url / 'slow_body').with_query(chunks=50, delay=0.1)) + + async with await http_client.send_request(url, timeout=timedelta(milliseconds=500)) as response: + with pytest.raises(TimeoutError): + await response.read() - assert (await read_json(direct_result.http_response))['cookies'] == {'preset': 'value'} - assert direct_request.loaded_url == target_url - redirected_request = Request.from_url(str((server_url / 'redirect').with_query(url=target_url, status=302))) - redirected_result = await http_client.crawl(redirected_request, session=session) +async def test_timeout_bounds_read_stream(http_client: HttpClient, server_url: URL) -> None: + """Test that the timeout of `send_request` also bounds reading the body with `read_stream`.""" + url = str((server_url / 'slow_body').with_query(chunks=50, delay=0.1)) - assert (await read_json(redirected_result.http_response))['cookies'] == {'preset': 'value'} - assert redirected_request.loaded_url == target_url + async with await http_client.send_request(url, timeout=timedelta(milliseconds=500)) as response: + with pytest.raises(TimeoutError): + async for _ in response.read_stream(): + pass diff --git a/tests/unit/http_clients/test_httpx.py b/tests/unit/http_clients/test_httpx.py index 4bf8600570..d45c502b39 100644 --- a/tests/unit/http_clients/test_httpx.py +++ b/tests/unit/http_clients/test_httpx.py @@ -13,7 +13,6 @@ from crawlee.fingerprint_suite._browserforge_adapter import get_available_header_values from crawlee.fingerprint_suite._consts import COMMON_ACCEPT_LANGUAGE from crawlee.http_clients import HttpxHttpClient -from crawlee.http_clients._httpx import _HttpxTransport, _same_origin from crawlee.sessions import CookieParam, Session if TYPE_CHECKING: @@ -27,22 +26,6 @@ async def read_json(response: HttpResponse) -> dict: return json.loads((await response.read()).decode()) -@pytest.mark.parametrize( - ('url', 'other', 'expected'), - [ - pytest.param('http://a.com/x', 'http://a.com/x', True, id='same'), - pytest.param('http://a.com/x', 'https://a.com/x', False, id='different-scheme'), - pytest.param('http://a.com/x', 'http://b.com/x', False, id='different-host'), - pytest.param('http://a.com/x', 'http://a.com:8080/x', False, id='different-port'), - pytest.param('http://a.com:80/x', 'http://a.com/y', True, id='explicit-default-port'), - pytest.param('http://a.com/x', 'http://a.com/y', True, id='different-path'), - ], -) -def test_same_origin(url: str, other: str, *, expected: bool) -> None: - """Test that two URLs share an origin only when their scheme, host and port match.""" - assert _same_origin(httpx2.URL(url), httpx2.URL(other)) is expected - - @pytest.mark.parametrize( ('client_kwargs', 'expected_warning'), [ @@ -51,13 +34,13 @@ def test_same_origin(url: str, other: str, *, expected: bool) -> None: ], ) async def test_transport_kwargs_do_not_reach_the_client(client_kwargs: dict[str, Any], expected_warning: str) -> None: - """Test that kwargs mounting a transport of their own are rejected with a warning, so the cookies keep working.""" + """Test that kwargs mounting a transport of their own are rejected with a warning, so the proxy keeps working.""" with pytest.warns(UserWarning, match=expected_warning): client = HttpxHttpClient(**client_kwargs) async with client: assert client._get_client(None)._mounts == {} - assert isinstance(client._get_client(None)._transport, _HttpxTransport) + assert client._get_client(None)._transport is not client_kwargs.get('transport') async def test_proxy_kwarg_routes_requests(server_url: URL) -> None: @@ -134,26 +117,66 @@ async def test_headers_come_from_one_sample(server_url: URL) -> None: generator.get_specific_headers.assert_called_once_with(header_names={'Accept', 'Accept-Language', 'User-Agent'}) +async def test_headers_generated_once_per_redirect_chain(server_url: URL) -> None: + """Test that every request of a redirect chain carries the headers generated for the first one.""" + generator = Mock(spec=HeaderGenerator) + generator.get_specific_headers.side_effect = [ + HttpHeaders({'User-Agent': 'Mozilla/5.0 (First)'}), + HttpHeaders({'User-Agent': 'Mozilla/5.0 (Second)'}), + ] + redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'headers'), status=302) + + async with HttpxHttpClient(header_generator=generator) as client: + response = await client.send_request(str(redirect_url)) + headers = await read_json(response) + + assert headers['user-agent'] == 'Mozilla/5.0 (First)' + generator.get_specific_headers.assert_called_once() + + +async def test_client_headers_win_over_generated(server_url: URL) -> None: + """Test that the headers given to the client win over the generated browser headers.""" + async with HttpxHttpClient(headers={'user-agent': 'client'}) as client: + response = await client.send_request(str(server_url / 'headers')) + headers = await read_json(response) + + assert headers['user-agent'] == 'client' + + async def test_client_cookie_header_wins_over_session(server_url: URL) -> None: """Test that a `Cookie` header set on the underlying client replaces the cookies of the session.""" session = Session(cookies=[CookieParam(name='from_jar', value='1', domain=server_url.host or '')]) async with HttpxHttpClient(headers={'cookie': 'from_client=1'}) as client: response = await client.send_request(str(server_url / 'cookies'), session=session) + cookies = (await read_json(response))['cookies'] + + assert cookies == {'from_client': '1'} - assert (await read_json(response))['cookies'] == {'from_client': '1'} +async def test_cookies_kwarg_ignored(server_url: URL) -> None: + """Test that the `cookies` kwarg doesn't reach the requests, cookies come from sessions only.""" + with pytest.warns(UserWarning, match='`cookies` argument'): + client = HttpxHttpClient(cookies={'from_client': '1'}) -async def test_client_cookies_dropped_cross_origin(server_url: URL, redirect_server_url: URL) -> None: - """Test that cookies of the underlying client reach their origin but not the target of a cross-origin redirect.""" - redirect_url = (server_url / 'redirect').with_query(url=str(redirect_server_url / 'cookies'), status=302) + async with client: + response = await client.send_request(str(server_url / 'cookies')) + cookies = (await read_json(response))['cookies'] + + assert cookies == {} + + +async def test_own_cookie_jar_stays_empty(server_url: URL) -> None: + """Test that response cookies never accumulate in the jar of the underlying `httpx2` client.""" + session = Session() + client = HttpxHttpClient() + + async with client: + await client.send_request(str((server_url / 'set_cookies').with_query(a='1')), session=session) - async with HttpxHttpClient(cookies={'from_client': '1'}) as client: - direct = await client.send_request(str(server_url / 'cookies')) - redirected = await client.send_request(str(redirect_url)) + assert list(client._get_client(None).cookies.jar) == [] - assert (await read_json(direct))['cookies'] == {'from_client': '1'} - assert (await read_json(redirected))['cookies'] == {} + assert {item['name'] for item in session.cookies.get_cookies_as_dicts()} == {'a'} async def test_no_headers_without_generator(server_url: URL) -> None: diff --git a/tests/unit/http_clients/test_impit.py b/tests/unit/http_clients/test_impit.py index 9e2fd31c8b..5740e724e4 100644 --- a/tests/unit/http_clients/test_impit.py +++ b/tests/unit/http_clients/test_impit.py @@ -1,89 +1,31 @@ from __future__ import annotations -import json from typing import TYPE_CHECKING +from unittest.mock import AsyncMock, Mock -import pytest -from impit import TooManyRedirects +from impit import USE_CLIENT_DEFAULT, AsyncClient, Response +from crawlee import HttpHeaders from crawlee.http_clients import ImpitHttpClient if TYPE_CHECKING: - from collections.abc import AsyncGenerator + import pytest - from yarl import URL - from crawlee._types import HttpMethod - from crawlee.http_clients import HttpResponse - - -@pytest.fixture -async def http_client() -> AsyncGenerator[ImpitHttpClient]: +async def test_no_timeout_uses_client_default(monkeypatch: pytest.MonkeyPatch) -> None: + """Test that a request without a timeout gets the default timeout of the `impit` client, not none at all.""" + impit_client = Mock(spec=AsyncClient) + impit_client.request = AsyncMock(return_value=Mock(spec=Response)) client = ImpitHttpClient() - async with client: - yield client - - -async def read_json(response: HttpResponse) -> dict: - return json.loads((await response.read()).decode()) - - -@pytest.mark.parametrize( - ('status_code', 'method', 'expected_method', 'expected_body'), - [ - pytest.param(301, 'POST', 'GET', '', id='301-post'), - pytest.param(301, 'PUT', 'PUT', 'payload', id='301-put'), - pytest.param(302, 'POST', 'GET', '', id='302-post'), - pytest.param(302, 'PUT', 'PUT', 'payload', id='302-put'), - pytest.param(303, 'POST', 'GET', '', id='303-post'), - pytest.param(303, 'PUT', 'GET', '', id='303-put'), - pytest.param(307, 'POST', 'POST', 'payload', id='307-post'), - pytest.param(308, 'PUT', 'PUT', 'payload', id='308-put'), - ], -) -async def test_redirect_method( - http_client: ImpitHttpClient, - server_url: URL, - *, - status_code: int, - method: HttpMethod, - expected_method: str, - expected_body: str, -) -> None: - """Test that the method and the body of a redirected request follow the WHATWG Fetch algorithm.""" - redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'method'), status=status_code) - - response = await http_client.send_request( - str(redirect_url), - method=method, - payload=b'payload', - headers={'content-type': 'application/octet-stream'}, + monkeypatch.setattr(client, '_get_client', Mock(return_value=impit_client)) + + await client._fetch( + 'https://example.com/', + method='GET', + headers=HttpHeaders(), + payload=None, + proxy_info=None, + timeout=None, ) - echo = await read_json(response) - - assert echo['method'] == expected_method - assert echo['body'] == expected_body - - -async def test_body_headers_dropped(http_client: ImpitHttpClient, server_url: URL) -> None: - """Test that headers describing the request body are dropped once a redirect turns the request into a `GET`.""" - redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'headers'), status=302) - - response = await http_client.send_request( - str(redirect_url), - method='POST', - payload=b'payload', - headers={'content-type': 'application/json', 'content-language': 'uk', 'x-custom': 'kept'}, - ) - headers = await read_json(response) - - assert 'content-type' not in headers - assert 'content-language' not in headers - assert headers['x-custom'] == 'kept' - -async def test_too_many_redirects(server_url: URL) -> None: - """Test that an endless redirect chain is cut off by `max_redirects`.""" - async with ImpitHttpClient(max_redirects=2) as client: - with pytest.raises(TooManyRedirects, match='limit of 2 redirects'): - await client.send_request(str(server_url / 'redirect_loop')) + assert impit_client.request.call_args.kwargs['timeout'] is USE_CLIENT_DEFAULT diff --git a/tests/unit/request_loaders/test_sitemap_request_loader.py b/tests/unit/request_loaders/test_sitemap_request_loader.py index 121b61e565..7bc1b2be03 100644 --- a/tests/unit/request_loaders/test_sitemap_request_loader.py +++ b/tests/unit/request_loaders/test_sitemap_request_loader.py @@ -1,7 +1,6 @@ import asyncio import base64 import gzip -from contextlib import asynccontextmanager from typing import TYPE_CHECKING, Any, cast from unittest.mock import AsyncMock, MagicMock, patch @@ -25,7 +24,7 @@ ) if TYPE_CHECKING: - from collections.abc import AsyncGenerator, AsyncIterator + from collections.abc import AsyncIterator from crawlee._types import JsonSerializable @@ -49,8 +48,7 @@ async def test_nested_sitemap_chain_bounded_by_max_depth() -> None: """A malicious endless chain of unique nested sitemaps is followed only up to the default max depth.""" fetched: list[str] = [] - @asynccontextmanager - async def stream(url: str, **_kwargs: Any) -> 'AsyncGenerator[HttpResponse]': + async def send_request(url: str, **_kwargs: Any) -> HttpResponse: fetched.append(url) index = int(url.removesuffix('.xml').rsplit('-', 1)[-1]) # Every sitemap is an index pointing at the next unique URL, so only the depth cap can stop the chain. @@ -64,10 +62,10 @@ async def read_stream() -> 'AsyncIterator[bytes]': response.status_code = 200 response.headers = {'content-type': 'application/xml; charset=utf-8'} response.read_stream = read_stream - yield cast('HttpResponse', response) + return cast('HttpResponse', response) client = AsyncMock(spec=HttpClient) - client.stream = stream + client.send_request.side_effect = send_request loader = SitemapRequestLoader([f'{DEFAULT_URL}sitemap-0.xml'], http_client=client) assert finished(await poll_until_condition(loader.check_readiness, finished)) diff --git a/tests/unit/server.py b/tests/unit/server.py index ad617561ce..5c36d9af7c 100644 --- a/tests/unit/server.py +++ b/tests/unit/server.py @@ -152,6 +152,7 @@ async def app(scope: dict[str, Any], receive: Receive, send: Send) -> None: 'get_compressed': get_compressed, 'file': file_endpoint, 'slow': slow_response, + 'slow_body': slow_body, 'infinite_scroll': infinite_scroll_endpoint, 'resource_loading_page': resource_loading_endpoint, } @@ -547,6 +548,24 @@ async def slow_response(scope: dict[str, Any], _receive: Receive, send: Send) -> await send_html_response(send, HELLO_WORLD) +async def slow_body(scope: dict[str, Any], _receive: Receive, send: Send) -> None: + """Send the headers at once and then the body in small chunks with a configurable delay between them.""" + query_params = get_query_params(scope.get('query_string', b'')) + chunks = int(query_params.get('chunks', '50')) + delay = float(query_params.get('delay', '0.1')) + + await send( + { + 'type': 'http.response.start', + 'status': 200, + 'headers': [[b'content-type', b'text/plain'], [b'content-length', str(chunks).encode()]], + } + ) + for index in range(chunks): + await asyncio.sleep(delay) + await send({'type': 'http.response.body', 'body': b'x', 'more_body': index < chunks - 1}) + + async def infinite_scroll_endpoint(_scope: dict[str, Any], _receive: Receive, send: Send) -> None: """Handle requests for the infinite scroll page.""" await send_html_response( diff --git a/tests/unit/utils.py b/tests/unit/utils.py index 71ce08763c..87f596b541 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -4,7 +4,6 @@ import inspect import sys import time -from contextlib import asynccontextmanager from datetime import UTC, datetime, timedelta from typing import TYPE_CHECKING, Any, TypeVar, cast, overload from unittest.mock import AsyncMock, MagicMock, Mock @@ -16,7 +15,7 @@ from crawlee.storages import RequestQueue if TYPE_CHECKING: - from collections.abc import AsyncGenerator, AsyncIterator, Awaitable, Callable + from collections.abc import AsyncIterator, Awaitable, Callable from yarl import URL @@ -39,8 +38,7 @@ def make_status_stream_client( attempts: list[int] = [] indexes: dict[str, int] = {} - @asynccontextmanager - async def stream(url: str, **_kwargs: Any) -> AsyncGenerator[HttpResponse]: + async def send_request(url: str, **_kwargs: Any) -> HttpResponse: sequence = responses[url] index = indexes.get(url, 0) indexes[url] = index + 1 @@ -59,10 +57,10 @@ async def read_stream() -> AsyncIterator[bytes]: response.status_code = status response.headers = {'content-type': 'application/xml; charset=utf-8'} response.read_stream = read_stream - yield cast('HttpResponse', response) + return cast('HttpResponse', response) client = AsyncMock(spec=HttpClient) - client.stream = stream + client.send_request.side_effect = send_request return client, attempts