mirror of
https://github.com/langchain-ai/deepagents.git
synced 2026-07-21 17:25:26 -04:00
fix(code): handle recursive fetch_url conversion (#4257)
`fetch_url` no longer crashes when HTML-to-markdown conversion exceeds recursion depth. --- `fetch_url` can fail when `markdownify` hits Python recursion depth on deeply nested HTML. This keeps the existing markdown path but falls back to stdlib text extraction so dcode returns usable page content instead of crashing. Made by [Open SWE](https://openswe.vercel.app/agents/79d44275-aab0-bf62-abde-8242f55e0648) --------- Co-authored-by: Nick Hollon <274035459+nick-hollon-lc@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> Co-authored-by: Mason Daugherty <github@mdrxy.com>
This commit is contained in:
@@ -7,6 +7,7 @@ import ipaddress
|
||||
import logging
|
||||
import socket
|
||||
import threading
|
||||
from html.parser import HTMLParser
|
||||
from typing import TYPE_CHECKING, Any, Literal
|
||||
from urllib.parse import urljoin, urlparse
|
||||
|
||||
@@ -14,7 +15,7 @@ from langchain_core.runnables import RunnableConfig # noqa: TC002 # runtime hi
|
||||
from langchain_core.tools import tool
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Iterator
|
||||
from collections.abc import Callable, Iterator
|
||||
|
||||
from tavily import TavilyClient
|
||||
|
||||
@@ -200,6 +201,86 @@ def _pinned_dns(hostname: str, allowed_ips: list[str]) -> Iterator[None]:
|
||||
urllib3_connection.create_connection = original
|
||||
|
||||
|
||||
class _TextExtractor(HTMLParser):
|
||||
"""Extract text content from HTML as a markdownify fallback.
|
||||
|
||||
The character data inside raw-text elements (`script`, `style`,
|
||||
`noscript`, `template`) is skipped so the fallback never emits
|
||||
JavaScript or CSS source from the fetched (untrusted) page as page
|
||||
content.
|
||||
"""
|
||||
|
||||
# Tags whose character data is never page content. Suppressed via an
|
||||
# explicit allowlist of skipped tags rather than trying to detect script
|
||||
# payloads after the fact.
|
||||
_SKIP_TAGS = frozenset({"script", "style", "noscript", "template"})
|
||||
|
||||
def __init__(self) -> None:
|
||||
super().__init__(convert_charrefs=True)
|
||||
self.parts: list[str] = []
|
||||
self._skip_depth = 0
|
||||
|
||||
def handle_starttag(
|
||||
self,
|
||||
tag: str,
|
||||
attrs: list[tuple[str, str | None]], # noqa: ARG002 # required by HTMLParser override
|
||||
) -> None:
|
||||
"""Enter a raw-text element so its data is skipped."""
|
||||
if tag in self._SKIP_TAGS:
|
||||
self._skip_depth += 1
|
||||
|
||||
def handle_endtag(self, tag: str) -> None:
|
||||
"""Leave a raw-text element."""
|
||||
if tag in self._SKIP_TAGS and self._skip_depth:
|
||||
self._skip_depth -= 1
|
||||
|
||||
def handle_data(self, data: str) -> None:
|
||||
"""Collect non-empty, whitespace-collapsed text outside skipped tags."""
|
||||
if self._skip_depth:
|
||||
return
|
||||
text = " ".join(data.split())
|
||||
if text:
|
||||
self.parts.append(text)
|
||||
|
||||
def get_text(self) -> str:
|
||||
"""Return extracted text fragments separated by blank lines."""
|
||||
return "\n\n".join(self.parts)
|
||||
|
||||
|
||||
def _html_to_markdown_content(html: str, markdownify: Callable[[str], str]) -> str:
|
||||
"""Convert HTML to markdown, falling back to plain text on recursion.
|
||||
|
||||
Args:
|
||||
html: Raw HTML to convert.
|
||||
markdownify: The `markdownify.markdownify` callable, injected so this
|
||||
module avoids an eager top-level import of the optional dependency.
|
||||
|
||||
Returns:
|
||||
Markdown content, or text extracted from the HTML if markdown
|
||||
conversion exceeds the recursion limit. Returns an empty string if
|
||||
the text-extraction fallback itself fails.
|
||||
"""
|
||||
try:
|
||||
return markdownify(html)
|
||||
except RecursionError:
|
||||
logger.warning(
|
||||
"markdownify hit recursion depth; falling back to text extraction",
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
# Best-effort plain-text extraction. Guard it so a failure here (e.g. the
|
||||
# same pathological input that exhausted markdownify's recursion) cannot
|
||||
# re-introduce the uncaught crash this fallback exists to prevent.
|
||||
try:
|
||||
parser = _TextExtractor()
|
||||
parser.feed(html)
|
||||
parser.close()
|
||||
except Exception: # fallback is best-effort; must never propagate
|
||||
logger.warning("text-extraction fallback failed", exc_info=True)
|
||||
return ""
|
||||
return parser.get_text()
|
||||
|
||||
|
||||
def _get_tavily_client() -> TavilyClient | None:
|
||||
"""Get or initialize the lazy Tavily client singleton.
|
||||
|
||||
@@ -356,7 +437,13 @@ def fetch_url(url: str, timeout: int = 30) -> dict[str, Any]:
|
||||
except requests.exceptions.RequestException as e:
|
||||
return {"error": f"Fetch URL error: {e!s}", "url": url, "category": "network"}
|
||||
|
||||
markdown_content = markdownify(response.text)
|
||||
markdown_content = _html_to_markdown_content(response.text, markdownify)
|
||||
if not markdown_content:
|
||||
logger.warning(
|
||||
"fetch_url produced empty content for %s (status %s)",
|
||||
response.url,
|
||||
response.status_code,
|
||||
)
|
||||
return {
|
||||
"url": str(response.url),
|
||||
"markdown_content": markdown_content,
|
||||
|
||||
@@ -4,12 +4,21 @@ from __future__ import annotations
|
||||
|
||||
import socket
|
||||
from typing import TYPE_CHECKING, Any
|
||||
from unittest import mock
|
||||
|
||||
import markdownify as markdownify_module
|
||||
import pytest
|
||||
import requests
|
||||
import responses
|
||||
|
||||
from deepagents_code.tools import _UrlValidationError, _validate_url, fetch_url
|
||||
from deepagents_code import tools
|
||||
from deepagents_code.tools import (
|
||||
_html_to_markdown_content,
|
||||
_TextExtractor,
|
||||
_UrlValidationError,
|
||||
_validate_url,
|
||||
fetch_url,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable
|
||||
@@ -98,6 +107,128 @@ def test_fetch_url_success() -> None:
|
||||
assert result["content_length"] > 0
|
||||
|
||||
|
||||
@responses.activate
|
||||
@pytest.mark.usefixtures("resolve_public")
|
||||
def test_fetch_url_falls_back_when_markdownify_recurses(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Recursive markdown conversion falls back to extracted text."""
|
||||
body = "<html><body><p>Hello <strong>world</strong></p><p>Second</p></body></html>"
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"http://example.com/deep",
|
||||
body=body,
|
||||
status=200,
|
||||
)
|
||||
|
||||
def recursive_markdownify(_html: str) -> str:
|
||||
msg = "maximum recursion depth exceeded"
|
||||
raise RecursionError(msg)
|
||||
|
||||
monkeypatch.setattr(markdownify_module, "markdownify", recursive_markdownify)
|
||||
|
||||
result = fetch_url("http://example.com/deep")
|
||||
|
||||
assert result["status_code"] == 200
|
||||
content = result["markdown_content"]
|
||||
assert "Hello" in content
|
||||
assert "world" in content
|
||||
assert "Second" in content
|
||||
# The fallback extracts text rather than passing raw HTML through: the
|
||||
# markup that was present in the body must be gone. Substring assertions
|
||||
# alone would also pass if the raw HTML leaked, so check tags are stripped.
|
||||
assert "<strong>" not in content
|
||||
assert "<p>" not in content
|
||||
|
||||
|
||||
@responses.activate
|
||||
@pytest.mark.usefixtures("resolve_public")
|
||||
def test_fetch_url_handles_deeply_nested_html() -> None:
|
||||
"""Deeply nested real HTML degrades gracefully instead of crashing.
|
||||
|
||||
Exercises the real `markdownify` (no monkeypatch) against markup deep
|
||||
enough to exhaust its recursion limit, proving the premise of the fix:
|
||||
`fetch_url` returns the inner text instead of propagating a crash.
|
||||
"""
|
||||
depth = 6000
|
||||
inner = "deep content here"
|
||||
body = "<div>" * depth + inner + "</div>" * depth
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"http://example.com/nested",
|
||||
body=body,
|
||||
status=200,
|
||||
)
|
||||
|
||||
result = fetch_url("http://example.com/nested")
|
||||
|
||||
assert result["status_code"] == 200
|
||||
assert inner in result["markdown_content"]
|
||||
|
||||
|
||||
def test_text_extractor_skips_script_and_style() -> None:
|
||||
"""Script and style source is not emitted as extracted text."""
|
||||
parser = _TextExtractor()
|
||||
parser.feed(
|
||||
"<style>.a{color:red}</style>"
|
||||
"<script>var x = 1; alert('hi');</script>"
|
||||
"<p>Hello</p>"
|
||||
)
|
||||
parser.close()
|
||||
|
||||
text = parser.get_text()
|
||||
assert text == "Hello"
|
||||
assert "color" not in text
|
||||
assert "alert" not in text
|
||||
|
||||
|
||||
def test_text_extractor_collapses_whitespace_and_drops_empty() -> None:
|
||||
"""Internal whitespace is collapsed and empty fragments are dropped."""
|
||||
parser = _TextExtractor()
|
||||
parser.feed("<p> Hello world </p><p> </p>")
|
||||
parser.close()
|
||||
|
||||
assert parser.get_text() == "Hello world"
|
||||
|
||||
|
||||
def test_text_extractor_empty_input_returns_empty_string() -> None:
|
||||
"""An empty document yields an empty string, not an error."""
|
||||
parser = _TextExtractor()
|
||||
parser.feed("")
|
||||
parser.close()
|
||||
|
||||
assert parser.get_text() == ""
|
||||
|
||||
|
||||
def test_html_to_markdown_content_returns_markdownify_output() -> None:
|
||||
"""The happy path returns the injected converter's output verbatim."""
|
||||
|
||||
def fake_markdownify(html: str) -> str:
|
||||
return f"converted:{html}"
|
||||
|
||||
assert (
|
||||
_html_to_markdown_content("<p>x</p>", fake_markdownify) == "converted:<p>x</p>"
|
||||
)
|
||||
|
||||
|
||||
def test_html_to_markdown_content_returns_empty_when_fallback_fails() -> None:
|
||||
"""If text extraction itself fails, the helper returns an empty string."""
|
||||
|
||||
def recursive_markdownify(_html: str) -> str:
|
||||
msg = "maximum recursion depth exceeded"
|
||||
raise RecursionError(msg)
|
||||
|
||||
class _BoomExtractor(_TextExtractor):
|
||||
def feed(self, data: str) -> None: # noqa: ARG002 # override signature
|
||||
msg = "fallback parser blew up"
|
||||
raise ValueError(msg)
|
||||
|
||||
with mock.patch.object(tools, "_TextExtractor", _BoomExtractor):
|
||||
result = _html_to_markdown_content("<p>x</p>", recursive_markdownify)
|
||||
|
||||
assert result == ""
|
||||
|
||||
|
||||
@responses.activate
|
||||
@pytest.mark.usefixtures("resolve_public")
|
||||
def test_fetch_url_http_error() -> None:
|
||||
|
||||
Reference in New Issue
Block a user