[Prudctual]

Overview

When a video-generation worker cannot download a user-supplied material URI (HTTP 403, expired signature, private bucket), Dynamo was answering HTTP 500 with error_type/error_class internal. That counted as a service failure even though the request material is the problem.

This change maps those client/material HTTP 4xx failures to HTTP 4xx and metrics validation, and leaves genuine internal failures as HTTP 500.

Details

urllib.error.HTTPError carries .code and .msg, not .message. extract_http_like_error now reads those attributes and never falls back to str(exc), so signed URLs stay out of the client body. Clippy needless_borrow on those attribute reads is fixed (value is already &Bound).

HTTP-like Python 4xx errors are classified through the DEP 14354 catalog. A material 403 becomes ErrorClass::PermissionDenied (reason request.permission_denied, metrics ErrorType::Validation), not Internal. Unclassified worker panics still become HTTP 500/internal.

Issue ai-dynamo#14911 asked for a distinct material-unavailable class. That is not a catalog class after DEP 14354; adding one would be a catalog/DEP change. PermissionDenied is the catalog mapping for HTTP 403 and already keeps these failures out of service-health internal metrics.

On /v1/videos streaming, the handler now waits UntilFirstEvent before committing SSE 200, so a first-event material 403 is HTTP 403 rather than 200/internal.

On the OSS SGLang video handler, material-fetch 4xx errors are re-raised so the frontend can map them.

Where should the reviewer start?

Start with extract_http_like_error in lib/bindings/python/rust/errors.rs, then the videos streaming pre-commit path in lib/llm/src/http/service/openai.rs. HTTP proof: test_videos_material_http_403_is_4xx_not_internal, test_videos_streaming_material_http_403_is_4xx_not_internal, and test_videos_internal_error_stays_500 in lib/llm/tests/http-service.rs.

Related Issues

• Closes [BUG] Video generation: user-material download failures (e.g. 403) are surfaced as HTTP 500 with error_type=internal, polluting service-side error metrics ai-dynamo/dynamo#14911
• Relates to DEP: Unified semantic error propagation across frontend and backends ai-dynamo/dynamo#14354

Summary

• Rebased onto latest ai-dynamo/dynamo main.
• Fixed rust-clippy (lib/bindings/python): clippy::needless_borrow in extract_http_like_error.
• Addressed dynamo-review-agent items: streaming pre-commit backend-error check; removed redundant envelope unit test; no restating test banners.
• Catalog classification for material 403 is PermissionDenied / Validation, not a new material-unavailable class.

Validation

• cargo clippy --no-deps --all-targets -- -D warnings in lib/bindings/python (the previously failing job).
• cargo test --lib http_like and http_statuses_map_to_semantic_classes in lib/bindings/python.
• cargo test -p dynamo-llm --test http-service -- test_videos_ (unary 403, streaming 403, internal 500).

  

---

Summary by cubic

Fixes /v1/videos so failed video material URI downloads (HTTP 403, expired signatures, private buckets) surface as HTTP 403 with Validation metrics instead of HTTP 500/internal; genuine worker errors stay 500.

• extract_http_like_error now reads urllib HTTPError's .code/.msg (plus .reason) and never falls back to str(exc), so signed URLs stay out of client bodies; 403 classifies as PermissionDenied.
• Streaming /v1/videos waits UntilFirstEvent before committing SSE 200, so a first-event 403 answers HTTP 403 rather than 200.
• The OSS SGLang video handler re-raises material-fetch 4xx so the frontend can map them, and a PermissionDenied diagnostic keeps the catalog message ("Permission denied") instead of the HTTP reason phrase.

Closes ai-dynamo#14911.

^{Written for commit e88f68c. Summary will update on new commits.}

[cubic-dev-ai]

2 issues found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="components/src/dynamo/sglang/request_handlers/video_generation/video_generation_handler.py">

<violation number="1" location="components/src/dynamo/sglang/request_handlers/video_generation/video_generation_handler.py:174">
P2: The 4xx re-raise keys on any exception carrying an int `.code`/`.status` attribute in 400-499, not on the exception actually being a material-fetch HTTP error, yet the log message always attributes it to material fetch. Any other stage in the video pipeline (DiffGenerator, imageio/ffmpeg, storage upload) that raises an exception with a 4xx-like `.status`/`.code` will now propagate out of `generate()` instead of returning the `status="failed"` response that was previously guaranteed, and will be mislabeled in the logs. Restrict the check to HTTP error types (e.g. `urllib.error.HTTPError`, `aiohttp` `ClientResponseError`/`HttpStatusError`) or otherwise confirm the exception is a material-fetch failure before re-raising, and align the log message with the actual matched type.</violation>
</file>

<file name="lib/bindings/python/rust/errors.rs">

<violation number="1" location="lib/bindings/python/rust/errors.rs:146">
P3: When an HTTP-like exception uses leading or trailing whitespace as part of its message, `py_attr_string` removes it before the client response is built. Preserve the original string and use a trimmed view only to reject whitespace-only messages.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The 4xx re-raise keys on any exception carrying an int .code/.status attribute in 400-499, not on the exception actually being a material-fetch HTTP error, yet the log message always attributes it to material fetch. Any other stage in the video pipeline (DiffGenerator, imageio/ffmpeg, storage upload) that raises an exception with a 4xx-like .status/.code will now propagate out of generate() instead of returning the status="failed" response that was previously guaranteed, and will be mislabeled in the logs. Restrict the check to HTTP error types (e.g. urllib.error.HTTPError, aiohttp ClientResponseError/HttpStatusError) or otherwise confirm the exception is a material-fetch failure before re-raising, and align the log message with the actual matched type.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At components/src/dynamo/sglang/request_handlers/video_generation/video_generation_handler.py, line 174:

<comment>The 4xx re-raise keys on any exception carrying an int `.code`/`.status` attribute in 400-499, not on the exception actually being a material-fetch HTTP error, yet the log message always attributes it to material fetch. Any other stage in the video pipeline (DiffGenerator, imageio/ffmpeg, storage upload) that raises an exception with a 4xx-like `.status`/`.code` will now propagate out of `generate()` instead of returning the `status="failed"` response that was previously guaranteed, and will be mislabeled in the logs. Restrict the check to HTTP error types (e.g. `urllib.error.HTTPError`, `aiohttp` `ClientResponseError`/`HttpStatusError`) or otherwise confirm the exception is a material-fetch failure before re-raising, and align the log message with the actual matched type.</comment>

<file context>
@@ -168,6 +168,15 @@ async def generate(
+            # Material fetch 4xx (urllib HTTPError, HttpStatusError, HttpError)
+            # must leave generate() as a typed exception so the frontend can
+            # answer HTTP 4xx instead of folding a failed payload into 200/500.
+            code = getattr(e, "code", None)
+            if not isinstance(code, int):
+                code = getattr(e, "status", None)
</file context>

[cubic-dev-ai]

P3: When an HTTP-like exception uses leading or trailing whitespace as part of its message, py_attr_string removes it before the client response is built. Preserve the original string and use a trimmed view only to reject whitespace-only messages.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At lib/bindings/python/rust/errors.rs, line 146:

<comment>When an HTTP-like exception uses leading or trailing whitespace as part of its message, `py_attr_string` removes it before the client response is built. Preserve the original string and use a trimmed view only to reject whitespace-only messages.</comment>

<file context>
@@ -134,27 +134,43 @@ define_dynamo_exceptions!(
+    }
+    attr.extract::<String>()
+        .ok()
+        .map(|s| s.trim().to_string())
+        .filter(|s| !s.is_empty())
+}
</file context>

Suggested change

	.map(|s| s.trim().to_string())
	.filter(|s| !s.is_empty())
	.filter(|s| !s.trim().is_empty())

[cursor]

Rebased onto ai-dynamo/dynamo main 517572309. Head is f1cc7d2dd. The only rebase conflict was in lib/bindings/python/rust/errors.rs (kept main's response-error helpers and this PR's urllib .msg/.reason extraction). Streaming /v1/videos already runs the UntilFirstEvent pre-commit check and marks the inflight guard before SSE 200. PermissionDenied {message, code} envelopes now render the catalog message "Permission denied", which is the sequential urllib 403 failure on 68258f1c. Local dynamo-llm envelope tests, videos HTTP tests, and clippy -D warnings (--no-default-features) passed. A maintainer still needs /ok to test f1cc7d2dd for trusted CI.

[cursor]

Review-thread replies for ai-dynamo#14930. The write credential on this run can push the fork branch and comment here, and GitHub returned 403 for comments on ai-dynamo/dynamo, so these are recorded on this PR.

• Streaming videos SSE (discussion_r4025708345): fixed on f1cc7d2dd. /v1/videos runs check_for_backend_error_info(..., BackendErrorCheck::UntilFirstEvent) before the SSE response. On that error the connection handle is disarmed and the inflight guard is marked from extract_error_type_from_response. An immediate typed material 4xx is HTTP 4xx, not SSE 200/internal. A PermissionDenied {message, code} envelope keeps the catalog message "Permission denied".
• Redundant unit test (discussion_r4025708356): removed test_backend_material_http_403_envelope_is_not_internal. Remaining coverage is permission_denied_http_envelope_uses_catalog_message, test_videos_material_http_403_is_4xx_not_internal, and test_videos_streaming_material_http_403_is_4xx_not_internal.
• Section banners (discussion_r4025708370): removed the videos section banner comments from lib/llm/tests/http-service.rs. The pre-existing images-route banner is unchanged.
• Test doc comments (discussion_r4025708379, discussion_r4025708390): removed. The videos cases are the test names above plus test_videos_internal_error_stays_500, without restating assertions in doc comments.