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 (2)
📝 WalkthroughWalkthroughImplements an IPv6-first staggered-parallel (Happy Eyeballs v2) TCP connector, adds a structured Cloudflare diagnostic logger invoked in three download paths, and applies small formatting refactors to DoH option handling and a Vue conditional. ChangesConnection Strategy and Diagnostic Improvements
Sequence DiagramsequenceDiagram
participant Resolver as DNS resolver
participant Interleaver as interleave_by_family
participant Connector as staggered_parallel_connector
participant Attempt as connect_one
participant Stream as TcpStream
Resolver->>Interleaver: resolved IPv6 & IPv4 addrs
Interleaver->>Connector: IPv6-first ordered list
Connector->>Attempt: start first attempt
Attempt-->>Connector: success (TcpStream) / error
alt success
Connector-->>Stream: return tuned TcpStream
else
Connector->>Attempt: start next attempt after 300ms
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 1219-1260: The function log_cloudflare_diagnostic currently logs
the full uri (potentially leaking tokens); update it to sanitize the uri before
logging by parsing the uri (use Url::parse) and removing query, fragment, and
any userinfo (username/password) or, if parsing fails, by stripping everything
after the first '?' or '#' as a fallback, assign that sanitized string to a new
variable (e.g., sanitized_uri) and use sanitized_uri in the tracing::warn call
instead of uri; keep all other logged fields (cookie_names, cookie_len,
has_cf_clearance, sent_ua, and resp_str values) unchanged.
In `@src-tauri/risuko-http/src/connector.rs`:
- Around line 257-300: The loop currently recreates stagger =
tokio::time::sleep(ATTEMPT_DELAY) on every iteration which resets the stagger
whenever an in-flight attempt finishes; instead, create a single Sleep before
the loop (e.g. let mut stagger = tokio::time::sleep(ATTEMPT_DELAY);
tokio::pin!(stagger);) and inside the select await &mut stagger so the timer
continues across iterations; when the timer completes push the next
connect_one(addr, timeout) and then reset the existing Sleep by assigning a new
sleep to the pinned variable (stagger.set(tokio::time::sleep(ATTEMPT_DELAY))) so
ATTEMPT_DELAY remains a steady cadence while still using in_flight, remaining,
connect_one, and ATTEMPT_DELAY as currently named.
🪄 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: ffe2ca0f-1dce-4708-9f98-5632e7541c11
📒 Files selected for processing (4)
src-tauri/risuko-engine/src/engine/http.rssrc-tauri/risuko-http/src/connector.rssrc-tauri/src/cli/commands.rssrc/renderer/components/Preference/Advanced.vue
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src-tauri/risuko-http/src/connector.rs">
<violation number="1" location="src-tauri/risuko-http/src/connector.rs:604">
P3: Using hardcoded "dead" ports (1/2) makes the new async tests environment-dependent and potentially flaky.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| async fn happy_eyeballs_fails_over_to_second_address() { | ||
| let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); | ||
| let live = listener.local_addr().unwrap(); | ||
| let dead: SocketAddr = "127.0.0.1:1".parse().unwrap(); |
There was a problem hiding this comment.
P3: Using hardcoded "dead" ports (1/2) makes the new async tests environment-dependent and potentially flaky.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/risuko-http/src/connector.rs, line 604:
<comment>Using hardcoded "dead" ports (1/2) makes the new async tests environment-dependent and potentially flaky.</comment>
<file context>
@@ -434,6 +528,98 @@ fn percent_decode_str(s: &str) -> String {
+ async fn happy_eyeballs_fails_over_to_second_address() {
+ let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+ let live = listener.local_addr().unwrap();
+ let dead: SocketAddr = "127.0.0.1:1".parse().unwrap();
+ let c = test_connector();
+ let stream = c
</file context>
There was a problem hiding this comment.
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)
1242-1255:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude cookie-jar cookies in this diagnostic.
req_headers.get(COOKIE)only reflects manually injected headers. This client also sends cookies viacookie_provider(jar), so acf_clearanceloaded fromload-cookiesor set by earlier responses will be logged as missing even when it was actually sent. That makes these Cloudflare diagnostics misleading on the main path this PR is trying to debug.🤖 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 1242 - 1255, The diagnostic currently inspects only req_headers.get(COOKIE) so cf_clearance cookies that come from the cookie jar (via cookie_provider(jar) or load-cookies) are missed; update the logic that builds cookie_names, cookie_len and has_cf_clearance to also read cookies from the cookie jar (the same jar used by cookie_provider), merge those cookie strings with the manual COOKIE header (req_headers.get(COOKIE)), parse the combined string into names, compute cookie_len from the combined header length, and set has_cf_clearance by checking the merged names for "cf_clearance" (keep using the existing variables cookie_names, cookie_len, has_cf_clearance so callers remain unchanged).
🤖 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.
Outside diff comments:
In `@src-tauri/risuko-engine/src/engine/http.rs`:
- Around line 1242-1255: The diagnostic currently inspects only
req_headers.get(COOKIE) so cf_clearance cookies that come from the cookie jar
(via cookie_provider(jar) or load-cookies) are missed; update the logic that
builds cookie_names, cookie_len and has_cf_clearance to also read cookies from
the cookie jar (the same jar used by cookie_provider), merge those cookie
strings with the manual COOKIE header (req_headers.get(COOKIE)), parse the
combined string into names, compute cookie_len from the combined header length,
and set has_cf_clearance by checking the merged names for "cf_clearance" (keep
using the existing variables cookie_names, cookie_len, has_cf_clearance so
callers remain unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b65ef71-0d0e-40f9-8d3f-c0696f66bf71
📒 Files selected for processing (2)
src-tauri/risuko-engine/src/engine/http.rssrc-tauri/risuko-http/src/connector.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Implements Happy Eyeballs v2 in the TCP connector for faster, more reliable IPv6/IPv4 connects, and adds clearer diagnostics when Cloudflare blocks a request to make failures easier to debug.
cf_clearancepresence), request UA, and response headers (cf-ray,cf-mitigated,cf-cache-status,server) across probe, piece-stream, and single-download paths.Written for commit 7b52af8. Summary will update on new commits.
Summary by CodeRabbit
Improvements
Chores