feat(videos): content-streaming proxy + OpenAI Sora as first consumer - #820
Conversation
Add a per-provider content-delivery mode to /v1/videos so providers whose
finished video requires the provider's own credential to download can be
served without ever exposing that credential to the client.
Part A — content-streaming proxy mechanism:
- `GET /v1/videos/:id/content` now branches on a ContentDelivery enum:
- Redirect(url): the existing 302 to a signed, credential-free provider
URL (Alibaba/Zhipu/Volcengine/Runway) — unchanged, zero relay bandwidth.
- Proxy { url }: the gateway GETs the provider's authenticated content
endpoint with the provider bearer injected and streams the bytes back.
- True streaming pass-through: reqwest `bytes_stream()` is bridged straight
into `axum::body::Body::from_stream` — the whole file is never buffered in
memory. (An established gateway buffers the entire file before returning;
we stream, for constant memory.)
- Upstream status is checked BEFORE any body is built: a non-2xx content GET
(403 expired/not-ready, 404) maps to a typed error and returns a JSON error
envelope — never a truncated `video/mp4` body.
- Relays upstream Content-Type / Content-Length (when present); sets
`Content-Disposition: attachment`.
- Connect + per-chunk reads bounded by the model streaming budget via the
existing #554 wrappers (send_with_deadline / with_read_timeout_bytes) — no
new timeout knob.
- Content route rate limiting stays `enforce(None)` (caller layers only).
Part B — OpenAI Sora (openai), first Proxy-mode consumer:
- submit `POST {base}/videos`, poll `GET {base}/videos/{id}`, content
`GET {base}/videos/{id}/content` (Proxy). Status enum maps 1:1 to the four
unified values; real `progress` percentage passed through (others stay
binary 0/100). seconds→string enum, size WIDTHxHEIGHT verbatim (near
identity — inbound is already OpenAI-shaped).
- First video provider WITH a default base: an openai video Model with no
api_base falls back to the same default the chat path uses; the other four
still require api_base.
- Field names/paths pinned against the openai-python SDK (resources/videos.py,
types/video.py, types/video_create_params.py, types/video_create_error.py).
Tests: Sora status map, submit body shape, default-base URL composition,
content-mode selection (openai→Proxy, others→Redirect), upstream-error→typed
error for Proxy content, and a full Sora submit→poll→content-proxy e2e (bearer
injected upstream, bytes streamed as video/mp4, key not leaked, upstream 403 →
JSON error). Existing 4-provider unit + e2e journeys stay green (still 302).
Refs api7/AISIX-Cloud#1118
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesOpenAI Sora provider integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant VideoGateway
participant OpenAI
Client->>VideoGateway: Submit video request
VideoGateway->>OpenAI: Create Sora video
OpenAI-->>VideoGateway: Video job
Client->>VideoGateway: Poll video status
VideoGateway->>OpenAI: Get video job
OpenAI-->>VideoGateway: Status and progress
Client->>VideoGateway: Fetch video content
VideoGateway->>OpenAI: Authenticated content request
OpenAI-->>VideoGateway: MP4 bytes or error
VideoGateway-->>Client: Streamed content or JSON error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/aisix-proxy/src/videos.rs (1)
996-1015: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHandle a string-valued
errorfield.Some vendors return
{"error": "human message"}.scopethen becomes a JSON string, both.get("code")and.get("message")yieldNone, and the caller sees the generic"upstream error"instead of the vendor detail.♻️ Suggested fallback
.and_then(|p| { + if let Some(s) = nonempty_str(p.get("error")) { + return Some(s.to_string()); + } let scope = if p.get("error").is_some() { p.get("error").cloned().unwrap_or_default() } else { p };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aisix-proxy/src/videos.rs` around lines 996 - 1015, Update parse_provider_error_message so a string-valued JSON error field is returned as the provider message instead of falling back to "upstream error". Preserve the existing code/message extraction for object-valued errors and the generic fallback when no usable error detail exists.tests/e2e/src/harness/upstream-openai.ts (1)
148-158: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBinary-streaming tests use a UTF-8-safe fixture, so they can't catch real byte-corruption bugs in the content-proxy path. The root cause is that
rawBodyis typed and encoded as a JSstring(Buffer.from(str)defaults to UTF-8), so neither the harness nor the test can represent arbitrary binary bytes as a real MP4 would contain.
tests/e2e/src/harness/upstream-openai.ts#L148-L158: accept the payload directly as aBuffer/Uint8Array(skipBuffer.fromre-encoding when already aBuffer) instead of always converting via UTF-8.tests/e2e/src/harness/upstream-openai.ts#L33-L43: widenOpenAiUpstreamOptions.rawBodytostring | Buffer.tests/e2e/src/harness/upstream-openai.ts#L68-L71: widenOpenAiUpstreamStep.rawBodytostring | Bufferto match.tests/e2e/src/cases/videos-sora-content-proxy-e2e.test.ts#L218-L279: defineMP4_BYTESas aBuffercontaining non-UTF-8-safe byte values, and assert via.arrayBuffer()/Buffer.compareinstead of.text(), to genuinely validate byte-for-byte, constant-memory streaming.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/src/harness/upstream-openai.ts` around lines 148 - 158, The binary-streaming fixture currently re-encodes string data as UTF-8, so it cannot test arbitrary bytes. In tests/e2e/src/harness/upstream-openai.ts at lines 33-43 and 68-71, widen OpenAiUpstreamOptions.rawBody and OpenAiUpstreamStep.rawBody to string | Buffer; at lines 148-158, send Buffer/Uint8Array payloads directly while retaining string conversion for strings. In tests/e2e/src/cases/videos-sora-content-proxy-e2e.test.ts at lines 218-279, define MP4_BYTES as a Buffer with non-UTF-8-safe bytes and compare the response bytes using arrayBuffer/Buffer.compare instead of text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-proxy/src/videos.rs`:
- Around line 1255-1261: Wrap the stream produced by
crate::stream_timeout::with_read_timeout_bytes in
crate::request_id::in_request_span before passing it to Body::from_stream, while
still within the handler. Preserve the existing timeout and response behavior so
each poll_next re-enters the current request span for chunk correlation.
---
Nitpick comments:
In `@crates/aisix-proxy/src/videos.rs`:
- Around line 996-1015: Update parse_provider_error_message so a string-valued
JSON error field is returned as the provider message instead of falling back to
"upstream error". Preserve the existing code/message extraction for
object-valued errors and the generic fallback when no usable error detail
exists.
In `@tests/e2e/src/harness/upstream-openai.ts`:
- Around line 148-158: The binary-streaming fixture currently re-encodes string
data as UTF-8, so it cannot test arbitrary bytes. In
tests/e2e/src/harness/upstream-openai.ts at lines 33-43 and 68-71, widen
OpenAiUpstreamOptions.rawBody and OpenAiUpstreamStep.rawBody to string | Buffer;
at lines 148-158, send Buffer/Uint8Array payloads directly while retaining
string conversion for strings. In
tests/e2e/src/cases/videos-sora-content-proxy-e2e.test.ts at lines 218-279,
define MP4_BYTES as a Buffer with non-UTF-8-safe bytes and compare the response
bytes using arrayBuffer/Buffer.compare instead of text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ddb4e2b-99e3-4bb3-b594-cc67fd4a1318
📒 Files selected for processing (3)
crates/aisix-proxy/src/videos.rstests/e2e/src/cases/videos-sora-content-proxy-e2e.test.tstests/e2e/src/harness/upstream-openai.ts
…tion notes Audit follow-ups on the content-streaming proxy: - the non-2xx content-proxy branch read the upstream error body with no time bound; a stalled error body could hang the handler (the success path is already per-chunk bounded). Wrap the read in the same per-request stream budget. - document that a mid-stream read timeout surfaces to the client as a short read against the relayed Content-Length (intended incomplete- download signal). - e2e now also asserts the client response carries no Authorization header (structural no-leak, complements the body-bytes assertion).
|
@coderabbitai review |
✅ Action performedReview finished.
|
Every other streaming path in the crate (chat, messages x2, responses, responses_bridge) wraps its stream in request_id::in_request_span so each poll_next re-enters the handler's request span. The new content proxy did not, so its per-chunk read-timeout warnings would lose request-id correlation. Wrap it the same way.
1 participant
What & why
Foundational piece for the
/v1/videossurface: a per-provider content-delivery mode so providers whose finished video requires the provider's own credential to download can be served without ever exposing that credential to the client. Ships the mechanism plus its first consumer (OpenAI Sora) as one e2e-able unit. Refs api7/AISIX-Cloud#1118 (binding design).The four existing providers (Alibaba / Zhipu / Volcengine / Runway) return signed, credential-free public URLs and stay exactly as they are (302 redirect).
Part A — content-delivery mode (
RedirectvsProxy)GET /v1/videos/:id/content, on a completed task, now dispatches on aContentDeliveryenum:Redirect(url)Proxy { url }GETs the provider's authenticated content endpoint with the provider bearer injected and streams the bytes back. Credential never reaches the client.Why streaming, not buffering: reqwest
bytes_stream()is bridged straight intoaxum::body::Body::from_stream— the video is never buffered whole in memory (constant memory). An established gateway buffers the entire file into memory before returning it; we stream. Stream-bridging API verified against the tree versions: reqwest 0.12 (streamfeature on) →Response::bytes_stream(), axum 0.7 →Body::from_stream, reusing the same#554wrappers (send_with_deadlinefor connect,with_read_timeout_bytesfor per-chunk reads, bounded by the model'sstream_timeout_effective()).Upstream-error correctness: the upstream content status is checked before any body is constructed. A non-2xx (403 expired/not-ready, 404) is read as a small body, mapped to a typed
BridgeError::upstream_status, and returned as a JSON error envelope — never streamed back labelledvideo/mp4.Relays upstream
Content-TypeandContent-Length(when present); setsContent-Disposition: attachment. Rate limiting on the content route staysenforce(state, auth, None)(caller layers only, exempt from model limits — a download is a bandwidth op). Decoded task id is charset-guarded before it's interpolated into the new content-proxy URL.Part B — OpenAI Sora (
openai), firstProxyconsumerAll facts pinned against the OpenAI videos API reference and the
openai-pythonSDK:POST {base}/videosresources/videos.py(self._post("/videos", …))GET {base}/videos/{id}resources/videos.py(self._get("/videos/{video_id}", …))GET {base}/videos/{id}/content→ Proxyresources/videos.py(self._get("/videos/{video_id}/content", …))variantquery paramvideo/thumbnail/spritesheet, defaultvideo(= MP4)resources/videos.pydownload_contentHttpxBinaryResponseContent, no Range paramqueued/in_progress/completed/failedtypes/video.pyprogresstypes/video.pysecondsLiteral["4","8","12"](string)seconds→ string enum (near-identity)types/video_create_params.pysizeWIDTHxHEIGHT(e.g.1280x720)types/video_create_params.pyerror{code, message}types/video_create_error.pymodelsora-2/sora-2-pro, defaultsora-2model_nametypes/video_create_params.pyhttps://api.openai.com/v1api_basefalls back to the same default the chat path uses (aisix_provider_openai::OPENAI_DEFAULT_BASE); the other four still requireapi_baseaisix-provider-openai/src/bridge.rsOpenAI composes its URLs via the shared
build_v1_url(owns the/v1prefix, tolerates both bare-host and…/v1bases), so no bespoke root-stripper was invented.Deferred (now unblocked by the proxy — each its own follow-up PR)
api-version.provider: openaialready exists in the catalog, so no newcp-admin.yamlenum is required for this DP change.Test evidence
cargo test -p aisix-proxy --lib): 722 passed. New: openai status map, near-identity submit body (string seconds),build_v1_url/default-base URL composition, content-mode selection (openai→Proxy, other four→Redirect + task-id path-traversal rejection), and the Proxy upstream-403→typed-error path. Full Sora submit→poll→content-proxy handler test asserts the bearer is injected on the upstream content GET, bytes stream back asvideo/mp4, and the provider key never appears in the client body.videos-sora-content-proxy-e2e.test.ts): full Sora journey against a mock upstream — (a) provider bearer injected on the upstream content GET, (b) MP4 bytes streamed to the client asvideo/mp4withContent-Disposition: attachment, (c) provider key absent from the client bytes, (d) upstream 403 on content → JSON error envelope (not a video body). Existing 4-provider 302 journeys (videos-providers-e2e.test.ts,videos-e2e.test.ts) stay green — still 302, not proxy.cargo fmt --checkclean,cargo clippy --workspace --tests0 warnings,cargo build --bin aisixok, videos + passthrough e2e families green (etcd on 127.0.0.1:2379).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests