hotfix: Cloud data sync upstash support for Tauri desktop app #6625
Conversation
WalkthroughIntroduces a Tauri-aware HTTP bridge (Rust commands: http_fetch, http_fetch_text, http_fetch_json) and a TypeScript httpFetch wrapper. Upstash client now routes requests via this bridge when Tauri is present and adjusts URL/path handling accordingly. Tauri main registers the new commands. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UpstashTS as Upstash client (TS)
participant httpFetchTS as httpFetch (TS)
participant TauriWin as window.__TAURI__.invoke
participant RustFetch as fetch::http_fetch (Rust)
participant Remote as Remote Endpoint
User->>UpstashTS: call check / redisGet / redisSet
UpstashTS->>httpFetchTS: httpFetch(method, url, headers, body)
alt Tauri environment
httpFetchTS->>TauriWin: invoke("http_fetch", {method,url,headers,body})
TauriWin->>RustFetch: http_fetch(...)
RustFetch->>Remote: reqwest request
Remote-->>RustFetch: HTTP response
RustFetch-->>TauriWin: {status,status_text,headers,body(bytes)}
TauriWin-->>httpFetchTS: response payload
httpFetchTS-->>UpstashTS: Fetch-like Response
else Browser environment
httpFetchTS->>Remote: native fetch(...)
Remote-->>httpFetchTS: response
httpFetchTS-->>UpstashTS: response
end
UpstashTS-->>User: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 |
Little-LittleProgrammer
changed the title
Little-LittleProgrammer
changed the title
Little-LittleProgrammer
changed the title
![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 (5)
src-tauri/src/fetch.rs (5)
1-4: Consider using English for code comments.While the Chinese comments are clear, using English would improve consistency with the rest of the codebase and make it more accessible to international contributors.
-// -// 普通 HTTP 请求处理模块 -// +// +// HTTP request handler module +//
60-60: Consider using English for inline comments.For consistency and international collaboration, consider using English for inline comments.
- // 解析 HTTP 方法 + // Parse HTTP methodApply similar changes to other Chinese comments in the file:
- Line 64:
// 创建客户端→// Create client- Line 73:
// 构建请求→// Build request- Line 80:
// 对于需要 body 的请求方法，添加请求体→// Add body for methods that require it- Line 91:
// 发送请求→// Send request- Line 100:
// 获取响应状态和头部→// Get response status and headers- Line 116:
// 读取响应体→// Read response body- Line 137:
// 将字符串 body 转换为字节→// Convert string body to bytes- Line 140:
// 调用主要的 fetch 方法→// Call main fetch method- Line 143:
// 将响应体转换为字符串→// Convert response body to string- Line 158:
// 将 JSON 转换为字符串再转换为字节→// Convert JSON to string then to bytes- Line 163:
// 确保设置了正确的 Content-Type→// Ensure correct Content-Type is set- Line 169:
// 调用主要的 fetch 方法→// Call main fetch method- Line 172:
// 将响应体解析为 JSON→// Parse response body as JSON
81-89: Consider supporting body for GET requests with caution.While most HTTP methods that typically have bodies are covered, some APIs might require bodies for GET requests (though this is non-standard). Consider whether to support this edge case or document that GET requests with bodies are not supported.
// Add body for methods that require it - if method == reqwest::Method::POST - || method == reqwest::Method::PUT - || method == reqwest::Method::PATCH - || method == reqwest::Method::DELETE { - if !body.is_empty() { - let body_bytes = bytes::Bytes::from(body); - request = request.body(body_bytes); - } + // Add body if provided (even for GET, though non-standard) + if !body.is_empty() { + let body_bytes = bytes::Bytes::from(body); + request = request.body(body_bytes); }
110-112: Consider preserving invalid UTF-8 headers more gracefully.When header values contain invalid UTF-8, replacing them with
"<invalid utf8>"might cause issues for binary headers. Consider using base64 encoding or preserving the raw bytes in a different format.std::str::from_utf8(value.as_bytes()) - .unwrap_or("<invalid utf8>") + .unwrap_or_else(|_| { + // Encode invalid UTF-8 as base64 + &format!("base64:{}", base64::encode(value.as_bytes())) + }) .to_string()Note: This would require adding
base64as a dependency inCargo.toml.
24-29: Remove unused structErrorResponse.The
ErrorResponsestruct is defined but never used in the code. Consider removing it to keep the codebase clean.-#[derive(Debug, Clone, serde::Serialize)] -pub struct ErrorResponse { - request_id: u32, - status: u16, - status_text: String, - error: String, -} -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/utils/cloud/upstash.ts(6 hunks)src-tauri/src/fetch.rs(1 hunks)src-tauri/src/main.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/utils/cloud/upstash.ts (3)
src-tauri/src/fetch.rs (3)
url(76-76)method(61-61)key(43-43)app/utils/cloud/webdav.ts (2)
headers(62-68)path(69-95)app/utils.ts (1)
fetch(353-361)
src-tauri/src/main.rs (2)
src-tauri/src/stream.rs (1)
stream_fetch(35-144)src-tauri/src/fetch.rs (3)
http_fetch(32-127)http_fetch_text(130-148)http_fetch_json(151-177)
🔇 Additional comments (4)
src-tauri/src/main.rs (2)
5-5: LGTM! Clean module addition.The fetch module is properly added to support HTTP requests in the Tauri environment.
9-14: Commands properly registered.The new fetch commands are correctly registered with the Tauri invoke handler, enabling the TypeScript side to make HTTP requests through the Rust backend.
app/utils/cloud/upstash.ts (2)
8-62: Well-implemented Tauri HTTP bridge with proper fallback.The
httpFetchfunction correctly handles the bridging between browser fetch and Tauri's HTTP commands. The implementation properly:
- Converts various header formats (Headers, arrays, objects)
- Handles different body types (string, ArrayBuffer, Uint8Array)
- Falls back to native fetch when not in Tauri environment
158-158: Good fix for URL construction.Adding
window.location.originas the base URL parameter ensures proper URL parsing whenproxyUrlis a relative path. This fixes the issue mentioned in the PR description where the code would always enter the catch block.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src-tauri/src/fetch.rs (5)
31-31: Consider using relaxed memory ordering for the counter.Since the request counter is only used for generating unique IDs and doesn't need strict synchronization guarantees, using
Ordering::Relaxedwould be more efficient thanSeqCst.- let request_id = REQUEST_COUNTER.fetch_add(1, Ordering::SeqCst); + let request_id = REQUEST_COUNTER.fetch_add(1, Ordering::Relaxed);
33-50: Improve header parsing with better error context.The current implementation loses context about which header caused the error. Consider providing more descriptive error messages that include the problematic header key/value pair.
let mut _headers = HeaderMap::new(); for (key, value) in &headers { match key.parse::<HeaderName>() { Ok(header_name) => { match value.parse() { Ok(header_value) => { _headers.insert(header_name, header_value); } Err(err) => { - return Err(format!("failed to parse header value '{}': {}", value, err)); + return Err(format!("failed to parse header '{}' with value '{}': {}", key, value, err)); } } } Err(err) => { return Err(format!("failed to parse header name '{}': {}", key, err)); } } }
86-90: Simplify error message extraction.The error source handling can be simplified using a more idiomatic approach.
let response = request.send().await .map_err(|err| { - let error_msg = err.source() - .map(|e| e.to_string()) - .unwrap_or_else(|| err.to_string()); - format!("request failed: {}", error_msg) + format!("request failed: {}", err) })?;
102-105: Consider preserving invalid UTF-8 headers differently.Using a placeholder like
<invalid utf8>loses information. Consider using lossy conversion or base64 encoding for non-UTF8 header values.response_headers.insert( name.as_str().to_string(), - std::str::from_utf8(value.as_bytes()) - .unwrap_or("<invalid utf8>") - .to_string() + String::from_utf8_lossy(value.as_bytes()).to_string() );
165-166: Provide more context for JSON parsing errors.When JSON parsing fails, it would be helpful to include a snippet of the response body in the error message for debugging purposes.
// Parse response body as JSON let response_json: serde_json::Value = serde_json::from_slice(&response.body) - .map_err(|err| format!("failed to parse response as JSON: {}", err))?; + .map_err(|err| { + let preview = String::from_utf8_lossy(&response.body[..response.body.len().min(100)]); + format!("failed to parse response as JSON: {}. Response preview: {}", err, preview) + })?;
| let client = Client::builder() | ||
| .default_headers(_headers) | ||
| .redirect(reqwest::redirect::Policy::limited(3)) | ||
| .connect_timeout(Duration::new(10, 0)) | ||
| .timeout(Duration::new(30, 0)) | ||
| .build() | ||
| .map_err(|err| format!("failed to create client: {}", err))?; |
There was a problem hiding this comment.
Headers should be added to the request, not as default headers.
Setting headers as default headers on the client affects all requests made by this client instance. Since we're only making one request, headers should be set on the request builder instead.
// Create client
let client = Client::builder()
- .default_headers(_headers)
.redirect(reqwest::redirect::Policy::limited(3))
.connect_timeout(Duration::new(10, 0))
.timeout(Duration::new(30, 0))
.build()
.map_err(|err| format!("failed to create client: {}", err))?;
// Build request
let mut request = client.request(
method.clone(),
url.parse::<reqwest::Url>()
.map_err(|err| format!("failed to parse url: {}", err))?
- );
+ ).headers(_headers);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let client = Client::builder() | |
| .default_headers(_headers) | |
| .redirect(reqwest::redirect::Policy::limited(3)) | |
| .connect_timeout(Duration::new(10, 0)) | |
| .timeout(Duration::new(30, 0)) | |
| .build() | |
| .map_err(|err| format!("failed to create client: {}", err))?; | |
| // Create client | |
| let client = Client::builder() | |
| .redirect(reqwest::redirect::Policy::limited(3)) | |
| .connect_timeout(Duration::new(10, 0)) | |
| .timeout(Duration::new(30, 0)) | |
| .build() | |
| .map_err(|err| format!("failed to create client: {}", err))?; | |
| // Build request | |
| let mut request = client.request( | |
| method.clone(), | |
| url.parse::<reqwest::Url>() | |
| .map_err(|err| format!("failed to parse url: {}", err))? | |
| ).headers(_headers); |
🤖 Prompt for AI Agents
In src-tauri/src/fetch.rs around lines 57 to 63, the code sets headers as
default headers on the Client which affects all requests; instead remove the
.default_headers(_headers) call from the Client builder and apply the headers to
the specific request before sending (use the request builder's
.headers(_headers) method or .header(key, value) calls). Ensure the headers
value is available/moved into the request builder (clone if needed) and keep the
client build without default headers, then attach the headers to the individual
GET/POST request prior to .send().
| if method == reqwest::Method::POST | ||
| || method == reqwest::Method::PUT | ||
| || method == reqwest::Method::PATCH | ||
| || method == reqwest::Method::DELETE { | ||
| if !body.is_empty() { | ||
| let body_bytes = bytes::Bytes::from(body); | ||
| request = request.body(body_bytes); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
DELETE requests typically don't have a body.
According to HTTP specifications, DELETE requests should not have a request body. While some servers may accept it, many will reject DELETE requests with bodies. Consider removing DELETE from this list or making it configurable.
// For request methods that need a body, add the request body
if method == reqwest::Method::POST
|| method == reqwest::Method::PUT
- || method == reqwest::Method::PATCH
- || method == reqwest::Method::DELETE {
+ || method == reqwest::Method::PATCH {
if !body.is_empty() {
let body_bytes = bytes::Bytes::from(body);
request = request.body(body_bytes);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if method == reqwest::Method::POST | |
| || method == reqwest::Method::PUT | |
| || method == reqwest::Method::PATCH | |
| || method == reqwest::Method::DELETE { | |
| if !body.is_empty() { | |
| let body_bytes = bytes::Bytes::from(body); | |
| request = request.body(body_bytes); | |
| } | |
| } | |
| // For request methods that need a body, add the request body | |
| if method == reqwest::Method::POST | |
| || method == reqwest::Method::PUT | |
| || method == reqwest::Method::PATCH { | |
| if !body.is_empty() { | |
| let body_bytes = bytes::Bytes::from(body); | |
| request = request.body(body_bytes); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src-tauri/src/fetch.rs around lines 73 to 81, the conditional that attaches a
body to requests currently includes reqwest::Method::DELETE which is
non-conformant; remove DELETE from the list (or make it configurable) so DELETE
requests do not get a request body by default. Update the condition to only
include POST, PUT, PATCH (or check an explicit allow_body flag passed into the
function/config), and adjust any callers/tests to reflect that DELETE no longer
carries a body unless explicitly permitted via the new flag.
| // Ensure the correct Content-Type is set | ||
| let mut json_headers = headers; | ||
| if !json_headers.contains_key("content-type") && !json_headers.contains_key("Content-Type") { | ||
| json_headers.insert("Content-Type".to_string(), "application/json".to_string()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use case-insensitive header lookup.
HTTP headers are case-insensitive, but the current check only handles two specific cases. Consider using a case-insensitive comparison or normalizing header names.
// Ensure the correct Content-Type is set
let mut json_headers = headers;
- if !json_headers.contains_key("content-type") && !json_headers.contains_key("Content-Type") {
+ let has_content_type = json_headers.keys()
+ .any(|k| k.to_lowercase() == "content-type");
+ if !has_content_type {
json_headers.insert("Content-Type".to_string(), "application/json".to_string());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Ensure the correct Content-Type is set | |
| let mut json_headers = headers; | |
| if !json_headers.contains_key("content-type") && !json_headers.contains_key("Content-Type") { | |
| json_headers.insert("Content-Type".to_string(), "application/json".to_string()); | |
| } | |
| // Ensure the correct Content-Type is set | |
| let mut json_headers = headers; | |
| let has_content_type = json_headers.keys() | |
| .any(|k| k.to_lowercase() == "content-type"); | |
| if !has_content_type { | |
| json_headers.insert("Content-Type".to_string(), "application/json".to_string()); | |
| } |
🤖 Prompt for AI Agents
In src-tauri/src/fetch.rs around lines 155 to 159, the code checks for
"content-type" and "Content-Type" explicitly which is brittle; change the check
to be case-insensitive by either normalizing header names to lowercase before
checking/inserting or by scanning the existing header keys and comparing
key.to_lowercase() == "content-type"; if not found insert the header using the
canonical "Content-Type" name and value "application/json".
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
目前的云端数据同步upstash 不支持 tauri app，所以进行了以下改动 fixes #5617
app/utils/cloud/upstash.ts文件，当检测是 tauri app, 请求路径不再走代理，直接调用 rust 请求app/utils/cloud/upstash.ts:158new URL 传参错误，导致永远进入 catchSummary by CodeRabbit
New Features
Bug Fixes