mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-12 14:00:35 +00:00
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 <willem.jiang@gmail.com>
This commit is contained in:
parent
218c64180e
commit
80cb6404b6
@ -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:
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
|
||||
@ -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.
|
||||
|
||||
|
||||
@ -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).
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user