Repository navigation
fix(anthropic): stream tag duplication, 529 mapping, image fetch hardening - #3721
Closed
hpiclaranet wants to merge 3 commits into
Closed
hpiclaranet wants to merge 3 commits into
hpiclaranet wants to merge 3 commits into
Conversation
…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.
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.
Member
|
Please resolve the conflicts. |
7 of 15 tasks
Contributor
Author
7 of 15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three reliability fixes in the Anthropic provider, each confirmed by code-path analysis of the streaming state machine and the installed
anthropicSDK (0.120.0):thinkingblock was followed directly by atool_useblock, theinput_json_deltaevents matched none of the block-type branches, socurrent_block_typestayed"thinking". The block transition had already emitted the closing tag once, and theMessageStopEventhandler emitted it a second time, corrupting the thought text Dify stores. Tool-use deltas now mark the block as"tool_use".anthropic.OverloadedErroris a direct subclass ofAPIStatusError(notInternalServerError), so it fell through theAPIErrorcatch-all intoInvokeBadRequestError. It is now mapped toInvokeServerUnavailableError(server-side capacity, retryable). The mapping entry is ordered before the catch-all, which is what the SDK's first-match lookup requires.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 viaContent-Lengthor duringiter_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
Change Type
Screenshots / Videos
Not applicable — streaming state machine, error mapping, and fetch hardening, covered by unit tests.
` 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)
Version
versioninmanifest.yaml(not the one undermeta) — 0.3.27 → 0.3.29dify_plugin>=0.9.0is declared inpyproject.tomland locked inuv.lock(locked at 0.10.0)Testing
uvvenv aligned withmanifest.yaml,pyproject.toml, anduv.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 checkshows zero new findings vs baseline.Known limitations (pre-existing, out of scope here):
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 toInternalServerError, which is already mapped.