Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds automatic in-place retry and resume for the single-connection fallback, reuses range-probe request shape for small files via a new ChangesSingle-Connection Download Retry and Range Reuse
Sequence Diagram(s)sequenceDiagram
participant Engine
participant RangeProbe
participant SingleFallback
participant RetryLoop
participant Server
Engine->>RangeProbe: probe_range_support()
RangeProbe->>Server: HEAD/GET Range probe
Server-->>RangeProbe: 206 + headers
RangeProbe-->>Engine: Range supported
Engine->>SingleFallback: decide single-fallback (size < threshold)
SingleFallback->>RetryLoop: start (force_range = true)
loop up to SINGLE_MAX_RETRIES
RetryLoop->>Server: GET (Range: bytes=0- + Accept-Encoding: identity)
alt transient failure
Server-->>RetryLoop: connection reset / truncated
RetryLoop->>SingleFallback: resume from .part and retry
else success
Server-->>RetryLoop: 206/200 + data
RetryLoop-->>Engine: download complete
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/risuko-engine/src/engine/http.rs (1)
1169-1187:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve range-request shape on stale-
.partretry.The stale-file recovery path falls back to
&clientwithforce_range=false, which can reintroduce theHTTP 412failure this PR is fixing for Quark-style CDNs.Suggested fix
- run_single_download( - &client, + run_single_download( + single_client, uri, &part_path, &headers, total, completed, speed, cancelled, cancel_token, &filename, dir_path, global_limiter, task_limiter, stall, filename_was_url_derived, - false, + probe_confirmed_range, ) .await🤖 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 `@src-tauri/risuko-engine/src/engine/http.rs` around lines 1169 - 1187, The stale-.part recovery path is calling run_single_download(&client, ..., false) which forces force_range=false and can reintroduce HTTP 412 errors; change the retry call in the stale-file branch to preserve the original range-request behavior by forwarding the same force_range boolean used in the initial attempt (or set it to true if the initial attempt used range requests) instead of hardcoding false, and ensure you still pass the same client (&client) and all other parameters to run_single_download so the retry maintains the same range/request shape.
🤖 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 `@src-tauri/risuko-engine/src/engine/http.rs`:
- Around line 1892-1899: The warning log in the piece request handler currently
emits full response headers and a 256-char body snippet (variables header_dump,
body, snippet and the tracing::warn! call), which may leak sensitive data;
change this to avoid logging sensitive headers and body at warn level: redact or
filter out headers like "set-cookie", "authorization" and any cookies when
building the header summary (only include safe header names/values or a count),
do not include raw body snippets (instead log body length or a safe
hash/placeholder), and demote the message to debug/trace level (or keep only
status and range at warn). Update the tracing call that uses
range.to_range_header_value() accordingly so only non-sensitive, minimal info is
emitted.
In `@src-tauri/risuko-engine/tests/http_features.rs`:
- Around line 720-736: The test currently treats a missing Range header as
start.unwrap_or(0) so a retry with no resume range (start==None) still appears
as start=0 and passes; change the flaky-server handler around the start variable
(the code that sets let start = start.unwrap_or(0) and builds head) to detect a
missing Range explicitly and fail the request (return a non-resume/invalid
response) when a retry does not include a resume Range: i.e., do not default to
0—require start.is_some() for resume behavior and return an error or non-206
response when it's None so the test fails if the client does not send a Range on
retry; apply the same change in the other mirror block around lines handling
head (the second start/Content-Range block).
---
Outside diff comments:
In `@src-tauri/risuko-engine/src/engine/http.rs`:
- Around line 1169-1187: The stale-.part recovery path is calling
run_single_download(&client, ..., false) which forces force_range=false and can
reintroduce HTTP 412 errors; change the retry call in the stale-file branch to
preserve the original range-request behavior by forwarding the same force_range
boolean used in the initial attempt (or set it to true if the initial attempt
used range requests) instead of hardcoding false, and ensure you still pass the
same client (&client) and all other parameters to run_single_download so the
retry maintains the same range/request shape.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a8be329c-bf43-4b14-81ba-a0330506f227
📒 Files selected for processing (3)
src-tauri/risuko-engine/src/engine/error_code.rssrc-tauri/risuko-engine/src/engine/http.rssrc-tauri/risuko-engine/tests/http_features.rs
There was a problem hiding this comment.
4 issues found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Summary by cubic
Fixes failures on HTTP 412 and mid-stream connection resets. Small single-connection downloads reuse the successful range probe and auto-retry transient errors by resuming the
.partfile.Range: bytes=0-with identity encoding and the range client for small files when a probe confirmed range support; avoids 412 on signed-URL CDNs (e.g. Quark).http/m3u8.Written for commit 67e9199. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Improvements
Tests