Skip to content

fix(anthropic): stream tag duplication, 529 mapping, image fetch hardening - #3721

Closed
hpiclaranet wants to merge 3 commits into
langgenius:mainfrom
hpiclaranet:fix/anthropic-stream-errors-image-fetch
Closed

hpiclaranet wants to merge 3 commits into
langgenius:mainfrom
hpiclaranet:fix/anthropic-stream-errors-image-fetch

Conversation

@hpiclaranet

@hpiclaranet hpiclaranet commented Aug 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Three reliability fixes in the Anthropic provider, each confirmed by code-path analysis of the streaming state machine and the installed anthropic SDK (0.120.0):

  1. Duplicate closing think tag in streams — when a thinking block was followed directly by a tool_use block, the input_json_delta events matched none of the block-type branches, so current_block_type stayed "thinking". The block transition had already emitted the closing tag once, and the MessageStopEvent handler emitted it a second time, corrupting the thought text Dify stores. Tool-use deltas now mark the block as "tool_use".
  2. HTTP 529 misclassified as a client error — anthropic.OverloadedError is a direct subclass of APIStatusError (not InternalServerError), so it fell through the APIError catch-all into InvokeBadRequestError. It is now mapped to InvokeServerUnavailableError (server-side capacity, retryable). The mapping entry is ordered before the catch-all, which is what the SDK's first-match lookup requires.
  3. Unbounded image URL fetch — requests.get(data) had no timeout (a hung URL could block the worker indefinitely), no size cap, accepted any URL scheme, and followed redirects. It now uses a (5s connect, 15s read) timeout, streamed reads bounded to a 10 MB cap (early rejection via Content-Length or during iter_content, so a hostile server cannot push unbounded bytes into the process), http(s)-only URLs (surrounding whitespace tolerated), and no redirect following with explicit 3xx rejection (a 3xx with an image-like body is rejected, not consumed).

Release Notes

  • Fixed duplicated "thinking" end markers in streamed agent responses when Claude calls a tool directly after thinking
  • Fixed Anthropic 529 (overloaded) errors being shown as user errors; they are now treated as transient server issues
  • Hardened image URL fetching: bounded timeouts, 10 MB size limit, http(s) only, redirects rejected

Change Type

  • Documentation / non-plugin change
  • Non-LLM plugin (tools, extensions, datasource, etc.)
  • LLM plugin

Screenshots / Videos

Not applicable — streaming state machine, error mapping, and fetch hardening, covered by unit tests.

Before After
`...
...

` after a tool call | single closing marker |
| 529 → "bad request" | 529 → server unavailable (retryable) |

LLM Plugin Checklist

Areas affected by this change (check all that apply)
  • Message flow (system messages, user ↔ assistant turn-taking)
  • Tool interaction flow (multi-round usage, Agent App and Agent Node)
  • Multimodal input (images, PDFs, audio, video, etc.)
  • Multimodal output (images, audio, video, etc.)
  • Structured output (JSON, XML, etc.)
  • Token consumption metrics
  • Other LLM functionality (reasoning, grounding, prompt caching, etc.)
  • New models / model parameter fixes

Version

  • Bumped top-level version in manifest.yaml (not the one under meta) — 0.3.27 → 0.3.29
  • dify_plugin>=0.9.0 is declared in pyproject.toml and locked in uv.lock (locked at 0.10.0)

Testing

  • Local deployment — clean uv venv aligned with manifest.yaml, pyproject.toml, and uv.lock: 45 unit tests pass (18 new, built from real SDK stream event objects). The bug-target tests fail against the pre-fix code (verified by checking out the base commit): duplicate-tag, 529 mapping, timeout/no-redirect/stream kwargs, whitespace stripping, 3xx-with-valid-body, Content-Length cap, oversized body. ruff check shows zero new findings vs baseline.
  • SaaS (cloud.dify.ai)

Known limitations (pre-existing, out of scope here):

  • The fetch hardening does not restrict private-network hostnames (multi-tenant deployments should apply network-level egress controls); no upstream allowlist exists in this plugin's credential schema.
  • ServiceUnavailableError/DeadlineExceededError (503/504) still fall through to bad-request, but the installed SDK (0.120.0) does not instantiate those classes — generic 5xx map to InternalServerError, which is already mapped.
  • The redacted/omitted-thinking delta branch in the stream handler does not match the SDK 0.120 event model (pre-existing; encrypted thinking is intentionally not rendered).

…ening

Three reliability fixes in the Anthropic provider:

- Stream: a thinking block followed directly by a tool_use block left
  current_block_type as 'thinking' (input_json_delta matched no type branch),
  so the closing think tag was emitted a second time at MessageStopEvent.
  Tool-use deltas now mark the block type as 'tool_use'.
- Errors: HTTP 529 OverloadedError fell through the APIError catch-all into
  InvokeBadRequestError; it is now mapped to InvokeServerUnavailableError
  (server-side capacity, retryable).
- Image fetch: requests.get had no timeout (hung URLs could block the worker
  indefinitely), no size cap, accepted any URL scheme and followed redirects.
  It now uses a (5s connect, 15s read) timeout, a 10 MB cap, http(s)-only
  URLs, and no redirect following.

Tests use real SDK stream event objects; the five bug-target tests fail
against the pre-fix code and the seven behavior guards pass on both.
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Aug 21, 2026
Wrap the streamed fetch in the requests context manager so the connection
is released deterministically on early rejection (3xx / Content-Length),
mid-stream cap, error responses and success. Also cover the chunked
(no Content-Length) over-cap path with a deterministic test that asserts
iteration stops right after the cap, and make the redirect test assert
the body is never read.

Review findings from the second-opinion validation round of this PR.
The chunked-body test used a body of cap+1 bytes (161 chunks), so a
regression that drained the whole stream would have read the same chunk
count as a correct early abort. Use a 2x-cap body (320 chunks): a correct
abort reads 161, a full drain reads 320 and fails the bound.

Test-rigor finding from the second-opinion validation round of this PR.
@crazywoola

Copy link
Copy Markdown
Member

Please resolve the conflicts.

@hpiclaranet

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and reopened as #3757 (GitHub blocks reopening a closed PR after a force-push to its branch): #3757

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants