-
Notifications
You must be signed in to change notification settings - Fork 59.8k
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
Tauri v2 and mobile platforms #5927
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@04041b is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces several updates across multiple files, primarily focusing on modifying the structure and retrieval of configuration and API methods. Key changes include the update of the version retrieval logic in Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 7
🧹 Outside diff range and nitpick comments (7)
src-tauri/capabilities/migrated.json (1)
8-51
: Review permission scope for mobile platformsThe current permission set is migrated from desktop and may be too broad for mobile platforms. Consider:
- Creating separate capability files for mobile vs desktop
- Limiting file system access on mobile
- Reviewing window management permissions that may not apply to mobile
Would you like assistance in creating platform-specific capability files?
src-tauri/Cargo.toml (1)
28-36
: Review dependency versions and featuresSeveral observations about the dependencies:
- The reqwest upgrade to 0.12.9 with specific features is good for security
- All tauri plugins are on version 2, maintaining consistency
- Consider adding version constraints (^2.0.0) for better dependency management
-tauri-plugin-fs = "2" +tauri-plugin-fs = "^2.0.0" -tauri-plugin-dialog = "2" +tauri-plugin-dialog = "^2.0.0" -tauri-plugin-notification = "2" +tauri-plugin-notification = "^2.0.0" -tauri-plugin-shell = "2" +tauri-plugin-shell = "^2.0.0" -tauri-plugin-http = "2" +tauri-plugin-http = "^2.0.0"src-tauri/src/lib.rs (1)
15-16
: Improve error handling messageThe current error message is generic. Consider providing more context about what failed.
- .expect("error while running tauri application"); + .expect("Failed to start Tauri application. Please check the logs for more details");app/utils/stream.ts (3)
Line range hint
77-86
: Implement FormData handlingThere's a TODO comment for FormData implementation which needs to be addressed.
Would you like me to help implement the FormData handling or create an issue to track this task?
Line range hint
92-101
: Enhance error handling and type safetyThe error handling could be improved in several ways:
- The catch block suppresses the original error
- Status 599 is non-standard
}) - .catch((e) => { - console.error("stream error", e); - // throw e; - return new Response("", { status: 599 }); + .catch((e: Error) => { + console.error("[Tauri Stream Error]", e); + return new Response( + JSON.stringify({ error: e.message }), + { + status: 502, + headers: { 'Content-Type': 'application/json' } + } + ); });
Update remaining Tauri API calls to use the core namespace
The verification revealed several instances where the Tauri API is used without the
core
namespace:
app/utils.ts
:writeText
,dialog.save
,fs.writeTextFile
app/utils/stream.ts
:event.listen
app/components/exporter.tsx
:dialog.save
,fs.writeBinaryFile
These APIs need to be updated to use the appropriate namespace according to Tauri v2 API changes:
writeText
→clipboard.writeText
dialog.save
→dialog.save
fs.writeTextFile
→fs.writeTextFile
event.listen
→event.listen
fs.writeBinaryFile
→fs.writeBinaryFile
🔗 Analysis chain
Line range hint
77-89
: Verify Tauri v2 API usage across the codebaseLet's ensure all Tauri API calls have been updated to use the new core namespace.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for old Tauri API usage rg "window\.__TAURI__\." --type ts -A 2 -B 2 | grep -v "window\.__TAURI__\.core\." # Search for potential missing core namespace updates rg "invoke\(" --type tsLength of output: 2516
src-tauri/src/stream.rs (1)
59-64
: Review timeout and redirect settings for mobile networksThe current settings might need adjustment for mobile networks:
- 3-second connect timeout might be too short for mobile networks
- 3 redirects might not be sufficient for some APIs
Consider:
- Making timeouts configurable
- Increasing default timeout for mobile platforms
- Adding read timeout in addition to connect timeout
- .connect_timeout(Duration::new(3, 0)) + .connect_timeout(Duration::new(10, 0)) + .timeout(Duration::new(30, 0))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (48)
src-tauri/Cargo.lock
is excluded by!**/*.lock
src-tauri/gen/android/.editorconfig
is excluded by!**/gen/**
src-tauri/gen/android/.gitignore
is excluded by!**/gen/**
src-tauri/gen/android/app/.gitignore
is excluded by!**/gen/**
src-tauri/gen/android/app/build.gradle.kts
is excluded by!**/gen/**
src-tauri/gen/android/app/proguard-rules.pro
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/AndroidManifest.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/java/com/yida/chatgpt/next/web/MainActivity.kt
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/drawable-v24/ic_launcher_foreground.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/drawable/ic_launcher_background.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/layout/activity_main.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-hdpi/ic_launcher.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-hdpi/ic_launcher_foreground.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-hdpi/ic_launcher_round.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-mdpi/ic_launcher.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-mdpi/ic_launcher_foreground.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-mdpi/ic_launcher_round.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xhdpi/ic_launcher.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xhdpi/ic_launcher_foreground.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xhdpi/ic_launcher_round.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_foreground.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_round.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_foreground.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_round.png
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/android/app/src/main/res/values-night/themes.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/values/colors.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/values/strings.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/values/themes.xml
is excluded by!**/gen/**
src-tauri/gen/android/app/src/main/res/xml/file_paths.xml
is excluded by!**/gen/**
src-tauri/gen/android/build.gradle.kts
is excluded by!**/gen/**
src-tauri/gen/android/buildSrc/build.gradle.kts
is excluded by!**/gen/**
src-tauri/gen/android/buildSrc/src/main/java/com/yida/chatgpt/next/web/kotlin/BuildTask.kt
is excluded by!**/gen/**
src-tauri/gen/android/buildSrc/src/main/java/com/yida/chatgpt/next/web/kotlin/RustPlugin.kt
is excluded by!**/gen/**
src-tauri/gen/android/gradle.properties
is excluded by!**/gen/**
src-tauri/gen/android/gradle/wrapper/gradle-wrapper.jar
is excluded by!**/*.jar
,!**/gen/**
src-tauri/gen/android/gradle/wrapper/gradle-wrapper.properties
is excluded by!**/gen/**
src-tauri/gen/android/gradlew
is excluded by!**/gen/**
src-tauri/gen/android/gradlew.bat
is excluded by!**/gen/**
src-tauri/gen/android/settings.gradle
is excluded by!**/gen/**
src-tauri/gen/schemas/acl-manifests.json
is excluded by!**/gen/**
src-tauri/gen/schemas/android-schema.json
is excluded by!**/gen/**
src-tauri/gen/schemas/capabilities.json
is excluded by!**/gen/**
src-tauri/gen/schemas/desktop-schema.json
is excluded by!**/gen/**
src-tauri/gen/schemas/mobile-schema.json
is excluded by!**/gen/**
src-tauri/gen/schemas/windows-schema.json
is excluded by!**/gen/**
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (12)
app/config/build.ts
(1 hunks)app/global.d.ts
(1 hunks)app/utils/stream.ts
(1 hunks)package.json
(2 hunks)src-tauri/Cargo.toml
(1 hunks)src-tauri/build.rs
(1 hunks)src-tauri/capabilities/desktop.json
(1 hunks)src-tauri/capabilities/migrated.json
(1 hunks)src-tauri/src/lib.rs
(1 hunks)src-tauri/src/main.rs
(1 hunks)src-tauri/src/stream.rs
(1 hunks)src-tauri/tauri.conf.json
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src-tauri/build.rs
🔇 Additional comments (12)
src-tauri/src/main.rs (1)
4-4
: Verify error handling in app_lib::run()
While the simplified main function improves code organization, we should ensure proper error handling is maintained.
Consider adding platform-specific logging initialization before app_lib::run()
for better debugging on mobile platforms.
✅ Verification successful
Error handling and mobile support are properly implemented
The simplified main.rs correctly delegates to app_lib::run() which:
- Maintains proper error handling via .expect() for critical application failures
- Includes mobile support through the #[cfg_attr(mobile, tauri::mobile_entry_point)] attribute
- Removes plugins incompatible with mobile (updater, window_state) as shown in the commented code
- Retains all essential cross-platform plugins with proper initialization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for error handling in app_lib implementation
ast-grep --pattern 'fn run() {
$$$
.expect($$$)
$$$
}'
# Check for platform-specific handling
rg -A 5 "cfg\(target_os"
Length of output: 89
Script:
#!/bin/bash
# Let's try a broader search for the app_lib::run implementation
rg -l "fn run"
# Then search for error handling patterns more broadly
rg "Result|Error|expect|unwrap|panic" --type rust
# Check for any mobile/platform specific code
rg "target_os|cfg\(" --type rust
Length of output: 944
Script:
#!/bin/bash
# Let's look at the implementation of run() in lib.rs
cat src-tauri/src/lib.rs
# And check the main.rs file for context
cat src-tauri/src/main.rs
Length of output: 1608
src-tauri/capabilities/desktop.json (1)
1-14
: Ensure proper capability separation for mobile platforms
The desktop capability configuration looks good with appropriate platform restrictions. However, since the PR adds mobile support:
Consider adding explicit documentation in this file about why certain permissions (like updater) are desktop-only, to prevent future confusion when implementing mobile features.
✅ Verification successful
Mobile capability separation is properly maintained
The desktop capability configuration correctly isolates the updater permission to desktop platforms only. The repository structure shows:
- Only
desktop.json
andmigrated.json
exist in the capabilities directory - Mobile-specific configurations are properly separated in the
gen/android
directory - No updater references exist in non-desktop capability configurations
The current implementation maintains proper platform separation, with the updater functionality correctly restricted to desktop platforms only.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for mobile-specific capability files
fd -g 'android.json' -g 'ios.json' src-tauri/capabilities/
# Verify no updater references in mobile contexts
rg -g '!desktop.json' "updater" src-tauri/capabilities/
Length of output: 167
Script:
#!/bin/bash
# Let's check the entire capabilities directory structure and content
ls -la src-tauri/capabilities/
# Check for any mobile-related files or configurations
rg -g '!desktop.json' -l "android|ios|mobile" src-tauri/
# Check tauri.conf.json for mobile configurations
cat src-tauri/tauri.conf.json
Length of output: 3448
app/config/build.ts (2)
13-13
: Verify version format compatibility across platforms
The version retrieval has been updated for Tauri v2, but we should ensure consistency across platforms.
Consider adding platform-specific version suffixes (e.g., "-android") for mobile builds:
- const version = "v" + tauriConfig.version;
+ const version = "v" + tauriConfig.version + (process.env.BUILD_MOBILE ? "-mobile" : "");
✅ Verification successful
Version format change is compatible and no platform-specific handling needed
The version format change from tauriConfig.package.version
to tauriConfig.version
is correct and consistent with Tauri v2's configuration structure. The search results show:
- Only one instance of version handling in build configuration
- No existing mobile-specific build configurations
- Version format is consistently prefixed with "v"
- No evidence of platform-specific version handling needed
The suggestion to add mobile-specific suffixes is unnecessary as there's no precedent or requirement for platform-specific version handling in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check version format in tauri.conf.json
jq '.version' src-tauri/tauri.conf.json
# Look for other version usages that might need updates
rg "tauriConfig\.package\.version"
Length of output: 85
Script:
#!/bin/bash
# Search for version-related patterns in the codebase
rg -A 2 "tauriConfig.*version|version\s*=\s*[\"']v"
# Check if there are any mobile-specific build configurations
rg -i "mobile.*build|build.*mobile"
# Look for platform-specific version handling
ast-grep --pattern 'const version = $_'
Length of output: 395
Line range hint 15-33
: Handle git operations gracefully on mobile platforms
The git commit info retrieval might not work as expected in mobile environments.
Consider platform-specific handling for commit info:
const commitInfo = (() => {
try {
+ // Skip git operations on mobile platforms
+ if (process.env.BUILD_MOBILE) {
+ return {
+ commitDate: "mobile-build",
+ commitHash: tauriConfig.version,
+ };
+ }
const childProcess = require("child_process");
app/global.d.ts (1)
15-42
: Verify all Tauri API calls are updated for v2
The restructuring of Tauri APIs under the core
namespace is correct for v2, but this change requires updates to all existing API calls throughout the codebase.
Let's verify that all Tauri API calls have been updated:
src-tauri/Cargo.toml (2)
12-14
: Verify library configuration for mobile support
The new library configuration with multiple crate types is correct for supporting mobile platforms. Ensure that the mobile build pipeline is properly configured to handle these crate types.
38-39
: Verify mobile platform exclusions
Good use of target-specific dependencies to exclude the updater plugin on mobile platforms. Consider:
- Testing the conditional compilation on both Android and iOS
- Documenting the mobile platform limitations
✅ Verification successful
Let me search for any documentation about mobile platform limitations.
Let me check for any mobile-specific code or configuration in the project.
Based on the search results, I can now provide a final response:
Mobile platform exclusions are properly configured and documented
The project has appropriate mobile platform handling:
- The updater plugin is correctly excluded for mobile platforms using target-specific dependencies in Cargo.toml
- Mobile platform support is properly configured with Android/iOS specific code in the
gen/
directory - The codebase includes mobile-specific entry points and configurations marked with
#[cfg_attr(mobile, ...)]
- Comments in the code document mobile platform limitations (e.g. "not compatible with mobile")
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other potential platform-specific code
rg "cfg\(.*target_os.*android.*ios" src-tauri/
Length of output: 153
Script:
#!/bin/bash
# Check for documentation about mobile platforms
rg -i "android|ios|mobile" README.md docs/ 2>/dev/null
# Check if there's a docs directory
fd -t d "^docs$"
# Look for any other potential documentation files
fd -e md
Length of output: 67508
Script:
#!/bin/bash
# Check for mobile-specific code and configuration
rg -i "android|ios|mobile" src-tauri/
rg "cfg\(.*target_os" src-tauri/
Length of output: 26837
src-tauri/src/lib.rs (1)
5-14
: Verify mobile platform compatibility for remaining plugins
The plugins have been correctly configured for mobile support, but let's verify their compatibility.
✅ Verification successful
Let me try to find the correct path for Cargo.toml and capabilities files.
All plugins are using mobile-compatible v2.x versions
The verification shows that all Tauri plugins used in the application (clipboard-manager
, fs
, dialog
, notification
, shell
, and http
) are using version 2.x, which is the mobile-compatible version series. These versions align with Tauri's mobile support requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Cargo.toml for mobile-compatible versions of plugins
rg -A 1 "tauri-plugin-\w+" Cargo.toml
# Check if plugins are properly configured in capabilities
fd capabilities --exec cat {} \; | jq '.plugins'
Length of output: 193
Script:
#!/bin/bash
# Find and check Cargo.toml files
fd Cargo.toml --exec rg -A 1 "tauri-plugin-\w+" {}
# Find and check capabilities.json
fd capabilities.json --exec cat {} \; | jq '.plugins'
Length of output: 384
src-tauri/tauri.conf.json (1)
69-70
: Verify HTTPS scheme configuration for mobile platforms
useHttpsScheme: false
might cause issues with certain mobile platform features.
package.json (3)
60-60
: Verify rt-client dependency source
The rt-client dependency is being fetched from a GitHub release. Consider:
- Hosting the package in a controlled environment
- Adding integrity hash for security
Let's verify the package source:
67-68
: Verify Tauri v2 API compatibility
The upgrade to Tauri v2 API (^2.1.1
) and CLI (^2.1.0
) is significant. Ensure all Tauri API calls in the codebase have been updated to match v2 patterns.
Let's scan for potential v1 API usage that needs updating:
27-34
: 🛠️ Refactor suggestion
Standardize version constraints for Tauri plugins
The Tauri plugins use inconsistent version constraints:
- Some use
~2
(patches only) - Others use
^2.x.x
(minor + patches)
This could lead to compatibility issues. Consider standardizing to ~2
for all plugins to ensure consistent behavior across updates.
Let's verify the compatibility of these plugins with mobile platforms:
"fs:allow-remove", | ||
"fs:allow-remove", |
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.
Remove duplicate permission entry
The permission fs:allow-remove
is listed twice consecutively.
"fs:allow-mkdir",
"fs:allow-remove",
- "fs:allow-remove",
"fs:allow-rename",
📝 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.
"fs:allow-remove", | |
"fs:allow-remove", | |
"fs:allow-remove", | |
"fs:allow-rename", |
"allow": [ | ||
{ | ||
"url": "https://*/" | ||
}, | ||
{ | ||
"url": "http://*/" | ||
} | ||
] |
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.
Review HTTP permissions for security
The current HTTP permissions allow unrestricted access to all domains over both HTTP and HTTPS. Consider:
- Restricting to specific domains needed by the application
- Enforcing HTTPS-only unless HTTP is specifically required
- Adding appropriate CSP headers for mobile security
{
"identifier": "http:default",
"allow": [
{
- "url": "https://*/"
+ "url": "https://<your-specific-domains>/"
},
- {
- "url": "http://*/"
- }
]
},
Committable suggestion skipped: line range outside the PR's diff.
tauri = { version = "2", features = [ | ||
] } |
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.
💡 Codebase verification
Based on the code analysis, I can now provide a final response about the required Tauri features:
Add required Tauri features based on plugin usage
The application uses several Tauri plugins that require specific features to be enabled. Based on the dependencies and usage in lib.rs, you should add the following features:
clipboard-all
- for clipboard-manager pluginfs-all
- for fs plugin operationsdialog-all
- for dialog pluginnotification-all
- for notification pluginshell-all
- for shell pluginhttp-all
- for http pluginwindow-all
- for window state management
🔗 Analysis chain
Review empty feature list for tauri dependency
The tauri dependency features list is empty. Consider which features are required for both desktop and mobile platforms.
Let me check if there are any other relevant files that might indicate which Tauri features are needed.
Let me check the actual usage of Tauri APIs in the source files to determine which features are needed.
Let me check the Tauri configuration file to understand which features are needed based on the app's configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check which features were previously enabled
git show HEAD^:src-tauri/Cargo.toml | rg -A 5 'tauri.*features'
Length of output: 434
Script:
#!/bin/bash
# Check main.rs for Tauri API usage
rg -l "tauri::" src-tauri/src/
# Check if there's any documentation about required features
fd README -e md | xargs rg -i "tauri.*features"
# Look for any mobile-specific configurations
fd "mobile|capacitor" -e json -e toml
Length of output: 206
Script:
#!/bin/bash
# Check the content of files using Tauri APIs
cat src-tauri/src/stream.rs src-tauri/src/lib.rs
# Check mobile schema for any hints about required features
cat src-tauri/gen/schemas/mobile-schema.json
Length of output: 72538
Script:
#!/bin/bash
# Check tauri.conf.json for enabled features and capabilities
fd "tauri.conf" -e json | xargs cat
# Check if there are any other Cargo.toml files that might show feature usage
fd Cargo.toml | xargs cat
Length of output: 3481
"security": { | ||
"csp": null | ||
} |
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.
Security Configuration Needs Review
Setting CSP to null could expose the application to security vulnerabilities. Consider defining appropriate Content Security Policy rules.
Consider implementing a CSP that restricts:
- Script sources
- Connect sources
- Frame sources
- Media sources
let mut _headers = HeaderMap::new(); | ||
for (key, value) in &headers { | ||
_headers.insert(key.parse::<HeaderName>().unwrap(), value.parse().unwrap()); | ||
} |
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.
Improve header parsing error handling
The current header parsing uses unwrap()
which could panic:
_headers.insert(key.parse::<HeaderName>().unwrap(), value.parse().unwrap());
Replace with proper error handling:
- _headers.insert(key.parse::<HeaderName>().unwrap(), value.parse().unwrap());
+ match (key.parse::<HeaderName>(), value.parse()) {
+ (Ok(name), Ok(val)) => _headers.insert(name, val),
+ (Err(e), _) => return Err(format!("Invalid header name '{}': {}", key, e)),
+ (_, Err(e)) => return Err(format!("Invalid header value for '{}': {}", key, e)),
+ };
let error: String = err | ||
.source() | ||
.map(|e| e.to_string()) | ||
.unwrap_or_else(|| "Unknown error occurred".to_string()); | ||
println!("Error response: {:?}", error); |
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.
Secure error handling in response
The current error handling might expose sensitive information to clients:
let error: String = err
.source()
.map(|e| e.to_string())
.unwrap_or_else(|| "Unknown error occurred".to_string());
Consider sanitizing error messages:
- .map(|e| e.to_string())
- .unwrap_or_else(|| "Unknown error occurred".to_string());
+ .map(|e| {
+ // Log full error for debugging
+ println!("Full error: {:?}", e);
+ // Return sanitized message to client
+ "Request failed. Please try again later.".to_string()
+ })
+ .unwrap_or_else(|| "Request failed. Please try again later.".to_string());
tauri::async_runtime::spawn(async move { | ||
let mut stream = res.bytes_stream(); | ||
|
||
while let Some(chunk) = stream.next().await { | ||
match chunk { | ||
Ok(bytes) => { | ||
// println!("chunk: {:?}", bytes); | ||
if let Err(e) = window.emit( | ||
event_name, | ||
ChunkPayload { | ||
request_id, | ||
chunk: bytes, | ||
}, | ||
) { | ||
println!("Failed to emit chunk payload: {:?}", e); | ||
} | ||
} | ||
Err(err) => { | ||
println!("Error chunk: {:?}", err); | ||
} | ||
} | ||
} | ||
if let Err(e) = window.emit( | ||
event_name, | ||
EndPayload { | ||
request_id, | ||
status: 0, | ||
}, | ||
) { | ||
println!("Failed to emit end payload: {:?}", e); | ||
} | ||
}); |
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.
🛠️ Refactor suggestion
Optimize stream handling for mobile devices
The current stream handling could be improved for mobile devices:
- No backpressure handling
- Unbounded memory usage
- No cancellation support
Consider:
- Adding chunk size limits
- Implementing backpressure
- Supporting cancellation
+ let max_chunk_size = 1024 * 16; // 16KB chunks
tauri::async_runtime::spawn(async move {
let mut stream = res.bytes_stream();
while let Some(chunk) = stream.next().await {
match chunk {
Ok(bytes) => {
+ if bytes.len() > max_chunk_size {
+ println!("Chunk size exceeds limit");
+ break;
+ }
if let Err(e) = window.emit(
Committable suggestion skipped: line range outside the PR's diff.
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
🧹 Outside diff range and nitpick comments (1)
src-tauri/src/lib.rs (1)
17-28
: Consider reorganizing plugin initialization order.For better maintainability and potential initialization dependencies, consider grouping related plugins together:
- Core system plugins (http, shell)
- UI/Window plugins (window_state)
- File system and clipboard plugins
- Update and notification plugins
tauri::Builder::default() .plugin(tauri_plugin_http::init()) .plugin(tauri_plugin_shell::init()) + .plugin(tauri_plugin_window_state::Builder::default().build()) .plugin(tauri_plugin_fs::init()) .plugin(tauri_plugin_clipboard_manager::init()) .plugin(tauri_plugin_notification::init()) .plugin(tauri_plugin_dialog::init()) - .plugin(tauri_plugin_window_state::Builder::default().build()) .plugin(tauri_plugin_updater::Builder::new().build())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src-tauri/gen/schemas/acl-manifests.json
is excluded by!**/gen/**
📒 Files selected for processing (1)
src-tauri/src/lib.rs
(1 hunks)
🔇 Additional comments (3)
src-tauri/src/lib.rs (3)
1-3
: LGTM! Entry point configuration is correct.
The module declaration and entry point setup properly handles both mobile and desktop platforms using conditional compilation.
12-12
: Verify stream_fetch compatibility with mobile platforms.
The stream_fetch
handler is being used across all platforms. We should verify its implementation is compatible with mobile platforms.
✅ Verification successful
The stream_fetch implementation is compatible with mobile platforms
The implementation of stream_fetch
uses platform-agnostic components:
- Uses standard Rust libraries (
std
) - Relies on
reqwest
for HTTP operations which supports all platforms - Uses
tauri
event system which is cross-platform - No platform-specific code or conditionals in the implementation
- No use of filesystem or other OS-specific APIs
The mobile configuration in lib.rs
properly handles the mobile entry point, indicating the project is already set up for cross-platform support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check stream_fetch implementation for mobile-specific code
# Look for any platform-specific handling in the stream module
# Check the stream module implementation
ast-grep --pattern 'fn stream_fetch' -f src-tauri/src/stream.rs
# Look for any cfg attributes or platform-specific code
rg "cfg\(.*android.*\)|cfg\(.*ios.*\)" src-tauri/src/stream.rs
Length of output: 308
Script:
#!/bin/bash
# Let's check the stream_fetch implementation and any platform-specific code
# First, check if the stream.rs file exists and view its content
fd "stream.rs" --type f
# If found, let's examine its content
cat src-tauri/src/stream.rs
# Also check for any mobile platform configurations in the entire project
rg "cfg.*(?:android|ios|mobile)" --type rust
Length of output: 5778
22-22
:
Verify plugin compatibility with Tauri v2.
The PR objectives mentioned that tauri_plugin_window_state
and tauri_plugin_updater
were disabled due to compatibility issues, but they are still included in the desktop configuration.
Also applies to: 26-26
.plugin(tauri_plugin_clipboard_manager::init()) | ||
.invoke_handler(tauri::generate_handler![stream::stream_fetch]) | ||
.run(tauri::generate_context!()) | ||
.expect("error while running tauri application"); |
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.
🛠️ Refactor suggestion
Consider improving error handling for mobile platforms.
Using expect()
for error handling in a mobile application might lead to abrupt termination. Consider implementing a more graceful error handling approach.
- .expect("error while running tauri application");
+ .map_err(|err| {
+ eprintln!("Error running application: {}", err);
+ // Handle cleanup or recovery steps here
+ })
+ .expect("Critical error: could not recover");
Committable suggestion skipped: line range outside the PR's diff.
This reverts commit cdec56e.
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: 0
🧹 Outside diff range and nitpick comments (1)
app/store/update.ts (1)
Line range hint
92-123
: LGTM! Tauri v2 notification API migration is correct.The migration from
window.__TAURI__?.notification
towindow.__TAURI__?.core.notification
aligns with Tauri v2's API structure.Consider extracting the common notification configuration to reduce duplication:
if (window.__TAURI__?.core.notification && isApp) { + const sendNotification = (body: string) => { + window.__TAURI__?.core.notification.sendNotification({ + title: "NextChat", + body, + icon: `${ChatGptIcon.src}`, + sound: "Default", + }); + }; await window.__TAURI__?.core.notification .isPermissionGranted() .then((granted) => { if (!granted) { return; } else { window.__TAURI__?.core.notification .requestPermission() .then((permission) => { if (permission === "granted") { if (version === remoteId) { - window.__TAURI__?.core.notification.sendNotification({ - title: "NextChat", - body: `${Locale.Settings.Update.IsLatest}`, - icon: `${ChatGptIcon.src}`, - sound: "Default", - }); + sendNotification(Locale.Settings.Update.IsLatest); } else { const updateMessage = Locale.Settings.Update.FoundUpdate(`${remoteId}`); - window.__TAURI__?.core.notification.sendNotification({ - title: "NextChat", - body: updateMessage, - icon: `${ChatGptIcon.src}`, - sound: "Default", - }); + sendNotification(updateMessage); clientUpdate(); } } }); } }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/components/exporter.tsx
(2 hunks)app/store/update.ts
(2 hunks)app/utils.ts
(4 hunks)
🔇 Additional comments (2)
app/utils.ts (1)
24-24
: LGTM! Tauri v2 API migrations are correctly implemented.
The following Tauri API calls have been properly migrated to use the core namespace:
writeText
for clipboard operationsdialog.save
andfs.writeTextFile
for file operationsupdater
for application updates
Also applies to: 48-65, 399-403
app/components/exporter.tsx (1)
Line range hint 476-494
: LGTM! Tauri v2 file system API migration is correct.
The migration of dialog and fs APIs to use the core namespace is properly implemented for image export functionality:
core.dialog.save
for file selectioncore.fs.writeBinaryFile
for saving the PNG data
quick update, it's stable now. I need some time for ios. |
Thanks for your contribution! This update is very important. If you think it's stable, tag me and I'll review the code. |
Currently, it's stable, the only feature that doesn't work on mobile is the export config feature (the import works though). |
@Leizhenpeng ready to merge |
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: 0
🧹 Nitpick comments (4)
docs/mobile-build-instruction.md (3)
4-7
: Add language specifier to the code blockThe code block should specify the shell language for better syntax highlighting and clarity.
-``` +```shell yarn yarn tauri android build<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.37.0)</summary> 4-4: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `9-9`: **Fix grammar in iOS requirements** Add the missing article "a" before "Mac". ```diff -IOS build requires Mac and a developer account or ios certificate. +iOS build requires a Mac and a developer account or iOS certificate.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~9-~9: You might be missing the article “a” here.
Context: ...oid build ``` ## IOS IOS build requires Mac and a developer account or ios certific...(AI_EN_LECTOR_MISSING_DETERMINER_A)
12-15
: Add language specifier to the code blockThe code block should specify the shell language for better syntax highlighting and clarity.
-``` +```shell yarn yarn tauri ios build<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.37.0)</summary> 12-12: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `71-71`: **Fix capitalization of Apple operating systems** The operating systems from Apple should be written as "macOS" and "iOS" respectively. ```diff -Compact client (~5MB) on Linux/Windows/MacOS/IOS/Android +Compact client (~5MB) on Linux/Windows/macOS/iOS/Android
And in the Chinese section:
-的跨平台客户端(Linux/Windows/MacOS/IOS/Android) +的跨平台客户端(Linux/Windows/macOS/iOS/Android)Also applies to: 116-116
🧰 Tools
🪛 LanguageTool
[grammar] ~71-~71: The operating system from Apple is written “macOS”.
Context: ... Compact client (~5MB) on Linux/Windows/MacOS/IOS/Android, [download it now](https://...(MAC_OS)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (33)
src-tauri/gen/apple/.gitignore
is excluded by!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/[email protected]
is excluded by!**/*.png
,!**/gen/**
src-tauri/gen/apple/Assets.xcassets/AppIcon.appiconset/Contents.json
is excluded by!**/gen/**
src-tauri/gen/apple/Assets.xcassets/Contents.json
is excluded by!**/gen/**
src-tauri/gen/apple/ExportOptions.plist
is excluded by!**/gen/**
src-tauri/gen/apple/LaunchScreen.storyboard
is excluded by!**/gen/**
src-tauri/gen/apple/Podfile
is excluded by!**/gen/**
src-tauri/gen/apple/Sources/nextchat/bindings/bindings.h
is excluded by!**/gen/**
src-tauri/gen/apple/Sources/nextchat/main.mm
is excluded by!**/gen/**
src-tauri/gen/apple/nextchat.xcodeproj/project.pbxproj
is excluded by!**/gen/**
src-tauri/gen/apple/nextchat.xcodeproj/project.xcworkspace/contents.xcworkspacedata
is excluded by!**/gen/**
src-tauri/gen/apple/nextchat.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings
is excluded by!**/gen/**
src-tauri/gen/apple/nextchat.xcodeproj/xcshareddata/xcschemes/nextchat_iOS.xcscheme
is excluded by!**/gen/**
src-tauri/gen/apple/nextchat_iOS/Info.plist
is excluded by!**/gen/**
src-tauri/gen/apple/nextchat_iOS/nextchat_iOS.entitlements
is excluded by!**/gen/**
src-tauri/gen/apple/project.yml
is excluded by!**/gen/**
📒 Files selected for processing (2)
README.md
(2 hunks)docs/mobile-build-instruction.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/mobile-build-instruction.md
[duplication] ~1-~1: Possible typo: you repeated a word
Context: # Build Instruction for IOS and Android ## Android Run command ``` yarn yarn tauri android...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~8-~8: Possible typo: you repeated a word
Context: ...`` yarn yarn tauri android build ``` ## IOS IOS build requires Mac and a developer acco...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~9-~9: You might be missing the article “a” here.
Context: ...oid build ``` ## IOS IOS build requires Mac and a developer account or ios certific...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
README.md
[grammar] ~71-~71: The operating system from Apple is written “macOS”.
Context: ... Compact client (~5MB) on Linux/Windows/MacOS/IOS/Android, [download it now](https://...
(MAC_OS)
[grammar] ~116-~116: The operating system from Apple is written “macOS”.
Context: ...署** - 提供体积极小(~5MB)的跨平台客户端(Linux/Windows/MacOS/IOS/Android), [下载地址](https://github.com...
(MAC_OS)
🪛 Markdownlint (0.37.0)
docs/mobile-build-instruction.md
4-4: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
12-12: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Your build has completed! |
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
Upgrade to Tauri v2 and add support for android.
Have not yet test ios.Need more test!!! might be unstable.📝 补充信息 | Additional Information
tauri_plugin_window_state and tauri_plugin_updater disabled, not compatible with mobile
Summary by CodeRabbit
Release Notes
New Features
Improvements
core
property.Dependencies
Configuration Changes
tauri.conf.json
structure for clarity and updated key properties.Code Quality