Skip to content

feat: 视频下载和界面预览统一使用OAI标准接口 - #2324

Merged
creamlike1024 merged 1 commit into
QuantumNous:mainfrom
feitianbubu:pr/video-download-oai
Nov 28, 2025
Merged

feat: 视频下载和界面预览统一使用OAI标准接口#2324
creamlike1024 merged 1 commit into
QuantumNous:mainfrom
feitianbubu:pr/video-download-oai

Conversation

@feitianbubu

@feitianbubu feitianbubu commented Nov 28, 2025

Copy link
Copy Markdown
Member

请求示例
curl http://localhost:3000/v1/videos/vira-891809323276095488/content
image
image

Summary by CodeRabbit

  • Bug Fixes
    • Improved video content delivery in task details with proper authorization handling and endpoint-based access.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Nov 28, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The pull request refactors video proxy channel-type handling to explicitly manage OpenAI and Sora video URLs with Authorization headers, relocates the video content route registration, and updates the frontend task logs UI to construct video endpoint URLs instead of using raw text values for modal display.

Changes

Cohort / File(s) Summary
Video Proxy Authorization & URL Handling
controller/video_proxy.go
Reworked channel-type switch statement: added explicit case for OpenAI and Sora that constructs video URL as baseURL/v1/videos/{taskID}/content with Authorization header; removed previous default logic that implicitly covered these types; other types now treated as direct FailReason URLs
Route Registration
router/video-router.go
Relocated GET route "/videos/:task_id/content" registration from before middleware Use(...) call to inside the route group block; reorders route registration without changing routing behavior
Frontend Video Modal Integration
web/src/components/table/task-logs/TaskLogsColumnDefs.jsx
Modified click handler for fail_reason column to construct videoUrl from task_id and call openVideoModal(videoUrl) for non-empty URLs on video-generating SUCCESS tasks, instead of passing raw text value

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Authorization header implementation in video_proxy.go: verify correct header format and token handling for OpenAI/Sora
  • Route registration impact: confirm middleware ordering is preserved and no routing conflicts introduced
  • URL construction consistency: ensure frontend-constructed URLs match backend expectations for /v1/videos/{taskID}/content endpoint
  • Task status filtering logic: validate that video modal is only triggered for SUCCESS video-generating tasks

Possibly related PRs

Suggested reviewers

  • seefs001
  • Calcium-Ion

Poem

🎥 A proxy refined with headers in place,
Routes reorganized, each endpoint in grace,
The frontend now weaves URLs with care,
OpenAI and Sora content flows fair,
Video streams dance through the UI air! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: unifying video download and UI preview to use the OAI standard interface, which is reflected across all three modified files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (1)

356-377: Decouple preview availability from fail_reason being a URL

Now that playback always uses /v1/videos/${record.task_id}/content, the isUrl check on fail_reason is only a heuristic gate. If you later stop storing upstream video URLs in fail_reason, the preview link will vanish even though the proxy endpoint is usable.

Consider basing the condition purely on task metadata (video type + SUCCESS + non‑empty task_id), e.g.:

-        const isUrl = typeof text === 'string' && /^https?:\/\//.test(text);
-        if (isSuccess && isVideoTask && isUrl) {
+        if (isSuccess && isVideoTask && record.task_id) {
           const videoUrl = `/v1/videos/${record.task_id}/content`;

This keeps the UI behavior aligned with the new unified OAI‑style endpoint instead of the internal fail_reason format.

controller/video_proxy.go (1)

120-125: Harden videoURL handling for OpenAI/Sora and the default FailReason path

The new OpenAI/Sora branch and the default FailReason branch work functionally, but you can make them more robust:

  • Default case: videoURL = task.FailReason is used directly. If it’s empty or has a non‑HTTP(S) scheme, the proxy will later fail in less obvious ways and also increases SSRF exposure.
    • Consider trimming, validating non‑emptiness, and enforcing http/https before proceeding. If validation fails, return a clear 4xx/5xx instead of attempting the upstream call.
  • OpenAI/Sora case: if channel.Key is missing, you currently send Authorization: Bearer and rely on upstream 401. Mirroring the Gemini branch, you might want to detect an empty key and return a clear server error immediately.

A possible direction for the default branch:

-   default:
-       // Video URL is directly in task.FailReason
-       videoURL = task.FailReason
+   default:
+       // Video URL is directly in task.FailReason
+       rawURL := strings.TrimSpace(task.FailReason)
+       if rawURL == "" {
+           logger.LogError(c.Request.Context(), fmt.Sprintf("Empty video URL for task %s", taskID))
+           c.JSON(http.StatusInternalServerError, gin.H{"error": gin.H{
+               "message": "Video URL not available for task",
+               "type":    "server_error",
+           }})
+           return
+       }
+       parsed, err := url.Parse(rawURL)
+       if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") {
+           logger.LogError(c.Request.Context(), fmt.Sprintf("Invalid video URL %s for task %s", rawURL, taskID))
+           c.JSON(http.StatusInternalServerError, gin.H{"error": gin.H{
+               "message": "Invalid video URL for task",
+               "type":    "server_error",
+           }})
+           return
+       }
+       videoURL = parsed.String()

(With an added strings import.) This aligns well with the existing effort to keep FailReason sanitized and avoids surprising failures at the HTTP layer. Based on learnings, this keeps binary/unsafe payloads out of the proxy path too.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b47cf4e and 2a77453.

📒 Files selected for processing (3)
  • controller/video_proxy.go (1 hunks)
  • router/video-router.go (1 hunks)
  • web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.

Applied to files:

  • controller/video_proxy.go
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, the redactVideoResponseBody function sanitizes video task responses by removing bytesBase64Encoded fields and truncating base64 strings to 256 characters to prevent large binary data from being stored in task.Data.

Applied to files:

  • controller/video_proxy.go
🧬 Code graph analysis (3)
controller/video_proxy.go (1)
constant/channel.go (2)
  • ChannelTypeOpenAI (5-5)
  • ChannelTypeSora (55-55)
web/src/components/table/task-logs/TaskLogsColumnDefs.jsx (1)
web/src/hooks/task-logs/useTaskLogsData.js (2)
  • videoUrl (72-72)
  • openVideoModal (265-268)
router/video-router.go (1)
controller/video_proxy.go (1)
  • VideoProxy (17-176)
🔇 Additional comments (1)
router/video-router.go (1)

14-17: Confirm that making /v1/videos/:task_id/content go through TokenAuth matches all callers

By moving this route into the block guarded by TokenAuth() and Distribute(), the content endpoint now requires the same authentication as other /v1 video routes. Please double‑check that:

  • The web video modal can still access this URL with whatever auth TokenAuth expects (headers vs cookies, <video> tag vs XHR), and
  • Any documented curl examples for this path include the required auth parameters.

@creamlike1024
creamlike1024 merged commit fa72a27 into QuantumNous:main Nov 28, 2025
1 check passed
ennnnny pushed a commit to ennnnny/new-api that referenced this pull request Mar 17, 2026
…d-oai

feat: 视频下载和界面预览统一使用OAI标准接口
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.

2 participants