-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use multipart upload for studio mode and instant mode re-uploads #1182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe video upload flow in apps/desktop/src-tauri/src/upload.rs was refactored from a single-part pre-signed PUT to an asynchronous multipart upload. It now initiates a multipart session, uploads file chunks via a multipart uploader with per-part presigning, collects part results, then proceeds to thumbnail upload. Stream-based single-part handling and total size logic were removed. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Desktop App
participant API as Backend API
participant MU as MultipartUploader
participant S3 as Object Storage
UI->>API: Initiate multipart upload (video_id, metadata)
API-->>UI: upload_id, per-part presign parameters
loop For each file chunk
UI->>MU: Provide next chunk
MU->>API: Request presigned PUT for part (upload_id, part_number)
API-->>MU: Presigned URL
MU->>S3: PUT part to URL
S3-->>MU: ETag/part result
MU-->>UI: Progress update (video_id)
end
UI->>MU: Finalize and collect part results
MU-->>UI: Vector of uploaded parts (part_number, ETag)
UI->>API: Complete multipart upload (upload_id, parts[])
API-->>UI: Completion ack
UI->>S3: Upload thumbnail (unchanged path)
S3-->>UI: Thumbnail upload result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/upload.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/upload.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/upload.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/upload.rs (1)
apps/desktop/src-tauri/src/api.rs (1)
upload_multipart_initiate(10-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
apps/desktop/src-tauri/src/upload.rs (1)
96-106: Allupload_videocalls use complete files Verified that all invocations in recording.rs and lib.rs operate on fully written files.
| let upload_id = api::upload_multipart_initiate(&app, &video_id).await?; | ||
|
|
||
| let stream = if let Some(channel) = channel { | ||
| tauri_progress(channel, stream).boxed() | ||
| } else { | ||
| stream.boxed() | ||
| }; | ||
|
|
||
| let video_fut = singlepart_uploader( | ||
| let video_fut = progress( | ||
| app.clone(), | ||
| PresignedS3PutRequest { | ||
| video_id: video_id.clone(), | ||
| subpath: "result.mp4".to_string(), | ||
| method: PresignedS3PutRequestMethod::Put, | ||
| meta: Some(meta), | ||
| }, | ||
| total_size, | ||
| stream.and_then(|(_, c)| async move { Ok(c) }), | ||
| ); | ||
| video_id.clone(), | ||
| multipart_uploader( | ||
| app.clone(), | ||
| video_id.clone(), | ||
| upload_id.clone(), | ||
| from_pending_file_to_chunks(file_path.clone(), None), | ||
| ), | ||
| ) | ||
| .try_collect::<Vec<_>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Critical: Missing multipart upload completion call.
The multipart upload flow is incomplete. After collecting the uploaded parts, you must call the completion API to finalize the S3 object. Without this, the uploaded parts remain uncommitted and the video won't be accessible.
Apply this diff to add the missing completion step:
let upload_id = api::upload_multipart_initiate(&app, &video_id).await?;
-let video_fut = progress(
+let parts_fut = progress(
app.clone(),
video_id.clone(),
multipart_uploader(
app.clone(),
video_id.clone(),
upload_id.clone(),
from_pending_file_to_chunks(file_path.clone(), None),
),
)
.try_collect::<Vec<_>>();
+let video_fut = async {
+ let parts = parts_fut.await?;
+ let metadata = build_video_meta(&file_path)
+ .map_err(|e| format!("Failed to get video metadata: {e}"))
+ .ok();
+ api::upload_multipart_complete(&app, &video_id, &upload_id, &parts, metadata).await
+};
+
// TODO: We don't report progress on image uploadNote: This mirrors the pattern used in InstantMultipartUpload::run at line 405.
Run the following script to confirm that api::upload_multipart_complete exists and verify there are no other callers that might be completing the upload on behalf of upload_video:
🏁 Script executed:
#!/bin/bash
# Verify the completion API exists and check usage patterns
# Find the completion function definition
echo "=== Checking for upload_multipart_complete definition ==="
ast-grep --pattern $'pub async fn upload_multipart_complete($$$) {
$$$
}'
# Search for all callers of upload_multipart_complete
echo -e "\n=== Finding all callers of upload_multipart_complete ==="
rg -n --type=rust 'upload_multipart_complete' -C 2Length of output: 1847
Missing multipart upload completion in upload_video
After collecting parts with .try_collect::<Vec<_>>(), await the parts future, build metadata, and invoke
api::upload_multipart_complete(&app, &video_id, &upload_id, &parts, metadata).await?
(as in InstantMultipartUpload::run) to finalize the S3 object.
🤖 Prompt for AI Agents
In apps/desktop/src-tauri/src/upload.rs around lines 94 to 106, the multipart
upload is initiated and parts are collected but never finalized; await the parts
future (e.g., let parts = video_fut.await? or similar), construct the metadata
required for the upload completion, and then call and await
api::upload_multipart_complete(&app, &video_id, &upload_id, &parts,
metadata).await? to complete the S3 multipart upload (ensure types match and
propagate errors).
Switches
upload_videoto use the multipart upload flow, since single-part upload can timeout.Summary by CodeRabbit