Skip to content

fix(media): raise ToolError on redirect response missing Location header - #619

Closed
Preciousuche wants to merge 1 commit into
use-agent-os:mainfrom
Preciousuche:fix/tools-media-image-redirect-location
Closed

fix(media): raise ToolError on redirect response missing Location header#619
Preciousuche wants to merge 1 commit into
use-agent-os:mainfrom
Preciousuche:fix/tools-media-image-redirect-location

Conversation

@Preciousuche

@Preciousuche Preciousuche commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Description

closes #616
``### Summary
Fixes _fetch_image_url crashing with `httpx.StreamClosed` when a redirect response omits the `Location` header, and adds test coverage for redirect handling.

Problem

When receiving a redirect status code (301, 302, 303, 307, 308) missing a Location header, _fetch_image_url closed the response (await resp.aclose()) and breaked out of the redirect loop. Downstream, resp.raise_for_status() does not raise on 3xx, leading into async for chunk in resp.aiter_bytes() on the closed stream. This failed with an internal StreamClosed error rather than a clear explanation.

Solution

  • When location is missing on a redirect response, immediately raise an explicit ToolError naming the status code, the current URL, and the missing header.
  • Added comprehensive unit tests in tests/test_tools/test_media_image_download_cap.py:
    • Missing Location header raises clear ToolError.
    • Valid Location header is followed and the image downloaded.
    • Exceeding _MAX_REDIRECTS raises ToolError("Too many redirects (>5)").

Fixes #616

@andreapn

andreapn commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded. Reviewed alongside the other three PRs on issue #616; #773 was picked.

Your fix is correct and your tests are good — this was a close call. #773 edged it on test coverage (all five 3xx codes parametrized, a mid-chain redirect asserting the failing hop is named rather than the URL the caller passed, and both sides of the _MAX_REDIRECTS boundary) and it carries a CHANGELOG entry.

Thanks for the contribution — nothing wrong with this one.

@andreapn andreapn closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: media._fetch_image_url crashes with httpx.StreamClosed on redirect missing Location header

3 participants