From 80cb6404b61c50546381e16db8728d86c3d89167 Mon Sep 17 00:00:00 2001 From: Heo Hao <130041734+Shelden-Hao@users.noreply.github.com> Date: Wed, 11 Nov 2026 19:43:53 +0800 Subject: [PATCH] fix(browserless): reject timeouts httpx cannot use (#8025) * fix(browserless): reject timeouts httpx cannot use `_coerce_timeout` forwarded any int, float or numeric string to `httpx.AsyncClient(timeout=...)` unchanged, so a configured `timeout_s: 0` or a negative value failed every fetch and capture with `ConnectTimeout` against a healthy backend, `nan` raised `ValueError: deadline must not be NaN`, and `inf` left the request without a deadline so it never completed. Each of those silently disabled the provider. Fall back to the documented 30s default with a warning when the number is not a usable deadline. This matches `crawl4ai._coerce_timeout` after #7481; `jina_ai._coerce_timeout` still forwards non-positive values. * fix(browserless): address timeout review and format tests Explain how HTTPX handles NaN and positive infinity. Document that Browserless accepts positive finite timeouts without an upper limit, and pin that behavior with a 1e308 test case. Crawl4AI and Jina timeout validation are tracked in #7481 and #7823. Remove the Browserless docstring's claim that Crawl4AI shares its coercion behavior. Format the test file for the backend lint check. --------- Co-authored-by: Willem Jiang --- backend/docs/CONFIGURATION.md | 8 ++++ .../deerflow/community/browserless/tools.py | 24 ++++++----- backend/tests/test_browserless_client.py | 41 +++++++++++++++++++ docs/guide/configuration.md | 2 + 4 files changed, 64 insertions(+), 11 deletions(-) diff --git a/backend/docs/CONFIGURATION.md b/backend/docs/CONFIGURATION.md index 1fac382419..a795ead967 100644 --- a/backend/docs/CONFIGURATION.md +++ b/backend/docs/CONFIGURATION.md @@ -1372,6 +1372,14 @@ shared (`100.64.0.0/10`, used by CGNAT and Tailscale), other non-global, or cloud-metadata addresses; set `allow_private_addresses: true` only when you intentionally point the tool at an internal target. +Each Browserless tool entry reads its request timeout from `timeout_s`, with +`timeout` as an alias when `timeout_s` is absent. The default is 30 seconds. +Zero, negative, and non-finite values fall back to the default with a warning. +Strings that cannot be parsed as numbers also trigger this fallback. HTTPX can +time out immediately for non-positive values. It raises an error for NaN and +applies no deadline for positive infinity. Browserless preserves positive finite +values in seconds, including very large values. + Both `web_fetch` (Browserless provider) and `web_capture` need a running Browserless instance. You can point `base_url` at [Browserless Cloud](https://www.browserless.io/) (set `BROWSERLESS_TOKEN`) or run one locally with Docker: diff --git a/backend/packages/harness/deerflow/community/browserless/tools.py b/backend/packages/harness/deerflow/community/browserless/tools.py index 3184395840..16042fe731 100644 --- a/backend/packages/harness/deerflow/community/browserless/tools.py +++ b/backend/packages/harness/deerflow/community/browserless/tools.py @@ -1,5 +1,6 @@ import asyncio import logging +import math import os import re from datetime import UTC, datetime @@ -48,22 +49,23 @@ def _get_tool_config(tool_name: str) -> dict | None: def _coerce_timeout(value: object, default: float) -> float: - """Coerce a config timeout into seconds, falling back to ``default`` on bad input. + """Parse Browserless timeout seconds, falling back to ``default`` on bad input. - Mirrors ``crawl4ai._coerce_timeout`` / ``jina_ai._coerce_timeout``: booleans and - non-numeric strings fall back to the default, so ``timeout_s: off`` (YAML ``False``) - does not become ``0.0`` and time out every request against a healthy server, and a - typo'd value does not raise out of tool construction. + Reject booleans before conversion so YAML ``timeout_s: off`` does not + become an immediate timeout. HTTPX treats non-positive deadlines as expired, + raises for NaN and treats positive infinity as an unlimited deadline. + Preserve positive finite values without imposing an upper limit. """ if isinstance(value, bool): return default - if isinstance(value, (int, float)): - return float(value) - if isinstance(value, str): + if isinstance(value, (int, float, str)): try: - return float(value) - except ValueError: - logger.warning("Browserless: invalid timeout %r in config; using %ss", value, default) + timeout = float(value) + except (TypeError, ValueError, OverflowError): + timeout = 0.0 + if math.isfinite(timeout) and timeout > 0: + return timeout + logger.warning("Browserless: invalid timeout %r in config; using %ss", value, default) return default diff --git a/backend/tests/test_browserless_client.py b/backend/tests/test_browserless_client.py index a402ca2719..8ca535d14f 100644 --- a/backend/tests/test_browserless_client.py +++ b/backend/tests/test_browserless_client.py @@ -442,6 +442,47 @@ class TestBrowserlessTools: """YAML `timeout_s: off` parses as False; 0.0 would time out every request.""" assert self._client_with_config({"timeout_s": False}).timeout_s == 30.0 + @pytest.mark.parametrize( + "value", + [ + 0, + 0.0, + -1, + -0.5, + float("inf"), + float("-inf"), + float("nan"), + "0", + "-1", + "inf", + "-inf", + "nan", + "1e400", + pytest.param(10**400, id="int-overflow"), + ], + ) + async def test_unusable_timeout_falls_back_to_default(self, value): + """A value `httpx` cannot use as a deadline must never reach it. + + Measured with httpx 0.28.1 against a backend that accepts the connection + and then never answers: `0` and the negatives fail every fetch with + `ConnectTimeout`, `nan` raises `ValueError: deadline must not be NaN`, + and `inf` leaves the request without a deadline so it never completes. + Either way a healthy Browserless backend looks broken. + """ + assert self._client_with_config({"timeout_s": value}).timeout_s == 30.0 + + @pytest.mark.parametrize("value", [1, 0.001, 45, 12.5, "1", "45", "12.5", pytest.param(1e308, id="large-finite")]) + async def test_usable_timeout_is_kept(self, value): + """Preserve positive finite timeout values without an upper limit.""" + assert self._client_with_config({"timeout_s": value}).timeout_s == float(value) + + async def test_unusable_timeout_warns(self, caplog): + """Swapping in the default silently would hide the operator's typo.""" + with caplog.at_level("WARNING", logger="deerflow.community.browserless.tools"): + assert self._client_with_config({"timeout_s": 0}).timeout_s == 30.0 + assert "invalid timeout 0" in caplog.text + async def test_env_token_is_stripped(self): """A token read from a file keeps its trailing newline. diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 12065235ed..ed9af164b2 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -149,6 +149,8 @@ fences, including unclosed fences, in complete and streamed replies. Direct Python calls to `web_fetch_tool` return `(content, artifact)` so Browserless can keep the target page's HTTP status out of page prose. Callers that previously expected a string should unpack the tuple; Agent tool calls continue to expose content and artifact through LangChain's tool result. + Browserless fetch and capture accept positive finite request timeouts in seconds. Invalid values fall back to 30 seconds. See [Browserless configuration](../../backend/docs/CONFIGURATION.md#delegated-fetch-backend-isolation). + Self-hosted Browserless, Crawl4AI, Firecrawl, and fastCRW fetch backends require outbound isolation from private, loopback, link-local, and cloud-metadata networks before setting `network_isolation_confirmed: true` on each tool. DeerFlow rejects private or unverifiable backends by default because those services handle redirects, DNS resolution, and subresources themselves. `allow_private_addresses` permits intentional internal target URLs and does not bypass the backend check. See [delegated fetch deployment](../../backend/docs/CONFIGURATION.md#delegated-fetch-backend-isolation); `make doctor` reports backend configurations that would be refused. Jina fetches support opt-in bounded retries via `max_retries` (default `0`) and `retry_budget_seconds` (default `30`) in the tool configuration. Valid `Retry-After` hints set a minimum wait for HTTP 429/503; 429 without a valid hint stays terminal. Hints that cannot fit the remaining budget stop retries. Local backoff remains randomized. Retries may increase upstream requests and cost; see [Jina fetch retries](../../backend/docs/CONFIGURATION.md#jina-fetch-retries).