Skip to content

[Bugfix] Classify Inkling media errors as client errors (HTTP 400) instead of server faults (500) - #40931

Closed
apex-mochen wants to merge 1 commit into
sgl-project:mainfrom
apex-mochen:fix/inkling-media-client-errors
Closed

apex-mochen wants to merge 1 commit into
sgl-project:mainfrom
apex-mochen:fix/inkling-media-client-errors

Conversation

@apex-mochen

@apex-mochen apex-mochen commented Sep 23, 2026 •

Copy link
Copy Markdown

Motivation

Fixes #40897. When an Inkling model receives invalid client-supplied media — an empty image_url, an unreachable media host, or undecodable image bytes — the server responds HTTP 500 instead of 400. Clients cannot distinguish their own bad request from a genuine server fault, and client input errors pollute server-error monitoring.

Every other VLM processor routes media through BaseMultimodalProcessor._load_single_item, where #31417 added the CLIENT_MEDIA_EXCEPTIONS classification (ValueError / UnidentifiedImageError / requests exceptions → ValueError). Inkling bypasses that path: it resolves request media in processors/inkling.py::_resolve_media_item and decodes image bytes in inkling/image_processing.py::_encode_image_bytes, so the classification never applies. Raw requests.exceptions.ConnectionError, UnidentifiedImageError, and the later FileNotFoundError from an empty URL escape unclassified and land on the server-fault path.

Modifications

  • _resolve_media_item:
    • An empty/whitespace media URL now raises ValueError instead of returning "" (which previously died as FileNotFoundError further down the line → 500).
    • download_remote_media calls are wrapped: any CLIENT_MEDIA_EXCEPTIONS (DNS/connection failure, timeout, redirect loop, oversized payload, bad URL) is re-raised as ValueError with the original exception chained, mirroring BaseMultimodalProcessor._load_single_item and the llava processor.
  • _encode_image_bytes: PIL UnidentifiedImageError from Image.open on undecodable client bytes is wrapped into ValueError("Could not decode image bytes: ..."), cause chained.
  • Added test/registered/unit/multimodal/test_inkling_media_errors.py covering: empty-URL variants; ConnectionError/Timeout/TooManyRedirects wrapping with cause preserved; invalid data: base64; valid data: round-trip; local path / raw bytes passthrough; undecodable image bytes → ValueError with PIL cause; and a real small PNG still encodes (no regression on the happy path).

Accuracy Tests

N/A — this only changes error-path exception classification; no model forward or output computation is touched. Happy paths were verified unchanged locally: valid data: URLs round-trip byte-exact, local paths and raw bytes pass through, and a real 14x14 PNG still produces the expected patch tensor.

Speed Tests and Profiling

N/A — no hot-path code: one .strip() on URLs and exception wrappers around already-remote IO / decode steps.

Checklist

  • Format code according to the pre-commit guide (ruff check + ruff format on the changed files; pre-existing import blocks left untouched).
  • Add unit tests.
  • N/A — documentation unchanged.
  • N/A — no accuracy/speed impact.
  • Follows SGLang's existing multimodal error-handling conventions (CLIENT_MEDIA_EXCEPTIONS → ValueError).

Local CPU verification (heavy deps stubbed, real modules loaded):


CI States

Latest PR Test (Base): ❌ Run #35880343869
Latest PR Test (Extra): ❌ Run #35880343693
Latest PR Test (AMD ROCm 10): ❌ Run #35880343734

…stead of server faults (500)

Fixes sgl-project#40897. (AI-assisted)

Co-authored-by: AI assistant
@apex-mochen

Copy link
Copy Markdown
Author

Closing this as a duplicate of #40899, which I should have found before writing it.

What I checked before opening it: nothing beyond the issue. I did not search for open PRs touching these files, so I did not see that #40899 was already up — opened 2026-09-23T10:45Z, four and a half hours before this one, fixing the same issue (#40897) with the same three files (image_processing.py, inkling.py, test_inkling_media_errors.py) and the same approach: route Inkling's media resolution and decode errors through the client-error classification so they surface as HTTP 400 rather than 500.

That is the process failure worth recording, not the code: the diagnosis here was fine, but a technically correct patch that duplicates an earlier one has no merge path, and mine is also the worse patch of the two — +693/-569 against their +159/-7, because my branch rewrites image_processing.py wholesale (@@ -1,252 +1,259 @@, every line replaced). Even without #40899, that diff would have been the first thing a reviewer asked me to fix, since the behavioural change is small.

So: no changes requested here, nothing to rebase. #40899 is the one to review. If anything in this branch turns out to be covered there but untested — the _resolve_media_item fetch path and _encode_image_bytes decode path are the two call sites that bypass BaseMultimodalProcessor._load_single_item — I would be glad to add edge-case tests to that PR instead, which is where the value is now.

Sorry for the duplicate. Closing.

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] Inkling multimodal returns HTTP 500 instead of 400 for invalid image/audio input

1 participant