Conversation
- Updated `axum` to version 0.8.8 and `axum-extra` to version 0.12, along with `tower-sessions` to version 0.15.
- Refactored route parameters from `:param` to `{param}` syntax for consistency across the application.
- Removed the `auth.rs` file, consolidating authentication logic elsewhere.
- Improved error handling and response formatting in WebSocket and oEmbed handlers.
There was a problem hiding this comment.
Pull request overview
This PR upgrades server dependencies (notably Axum and session-related crates) and updates server routing/WebSocket/DB code to align with the upgraded APIs.
Changes:
- Upgraded
axum,tower-sessions, andaxum-extradependency versions. - Updated route path parameter syntax across routers/handlers.
- Adjusted WebSocket message handling and improved Docker container spawn error handling; refactored parts of oEmbed DB querying.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
server/src/services/websocket.rs |
Updates WebSocket message API usage for newer Axum; refactors viewer container create/start flow and error handling. |
server/src/router.rs |
Updates WebSocket route path parameter syntax. |
server/src/handlers/project.rs |
Updates project-related route path parameter syntax. |
server/src/handlers/oembed.rs |
Refactors SQLx queries to query_as + bindings and adjusts response construction. |
server/src/handlers/admin.rs |
Updates admin route path parameter syntax. |
server/src/auth.rs |
Removes an unused/legacy auth module file. |
server/Cargo.toml |
Bumps versions for Axum and session/cookie dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/src/services/websocket.rs
Outdated
| "\r\n\x1b[31m[!] Failed to start viewer container: {}\x1b[0m\r\n", | ||
| e | ||
| ); | ||
| eprintln!("Viewer start error for session {}: {}", session_id, e); | ||
| let _ = socket.send(Message::Text(msg.into())).await; |
There was a problem hiding this comment.
The error message sent back over the WebSocket includes the raw Docker/bollard error (e). That can leak internal details (engine responses, container IDs, host config) to untrusted clients. Consider sending a generic client-facing message (optionally with a correlation ID) and keep the full error details only in server logs.
server/src/services/websocket.rs
Outdated
| "\r\n\x1b[31m[!] Failed to create viewer container: {}\x1b[0m\r\n", | ||
| e | ||
| ); | ||
| eprintln!("Viewer create error for session {}: {}", session_id, e); | ||
| let _ = socket.send(Message::Text(msg.into())).await; |
There was a problem hiding this comment.
The error message sent back over the WebSocket includes the raw Docker/bollard error (e). This risks exposing internal operational details to clients. Prefer returning a generic failure message to the client and logging the detailed error on the server side.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
The old already existing containers cannot be opened after the dependency upgrade. But any created after the upgrade are working fine . I want that without rolling back the dependency we save the pre existing containers. Check for that "Ok so I created a new session and it was a session that was created before the updates to the code that upgraded axum , , I want to access the old already existing images without having to roll back the dependency , also the code must follow all production level best practices." |
|
Refer to this and help in upgrading oauth2 to version 5 Upgrade GuideUpgrading from 4.x to 5.xThe 5.0 release includes breaking changes to address several long-standing API issues, along with Upgrade Rust to 1.65 or newerThe minimum supported Rust version (MSRV) is now 1.65. Going forward, this crate will maintain a Add typestate generic types to
|
- Add metadata labels to containers (session_id, owner_id, shell, etc.) - Restore sessions on server startup from existing containers - Attempt session restoration on WebSocket reconnection - Update container creation to include comprehensive labels - Fix pre-existing containers accessibility after upgrade Co-authored-by: Yashb404 <139128977+Yashb404@users.noreply.github.com>
- Replace detailed Docker/bollard errors with generic messages - Keep detailed error logging server-side for debugging - Prevent leaking internal operational details to clients Co-authored-by: Yashb404 <139128977+Yashb404@users.noreply.github.com>
- Fix timer reset issue: preserve container age on restoration - Replace println!/eprintln! with tracing::info!/error! - Use container creation timestamp for accurate session age tracking Co-authored-by: Yashb404 <139128977+Yashb404@users.noreply.github.com>
Co-authored-by: Yashb404 <139128977+Yashb404@users.noreply.github.com>
Restore sessions from Docker container metadata after server restart
Upgrade oauth2 to v5.0 with typestate endpoint safety
|
the issue is still there , for the containers created before the dependency upgrade they are unable to connect , they do not load in the window. in the terminal , an attempting to connect and then it fails with [!] Failed to start viewer container. Please try again later. [!] Connection Closed. meanwhile the container was created in background but never connected to the screen Server listening on port 3000... |
- Enhanced restore_sessions_from_containers to handle legacy containers - Legacy containers are identified by name pattern (trycli-studio-viewer-* or trycli-studio-session-*) - UUID from container name is used as session_id for legacy containers - Added restore_specific_session fallback for legacy containers by name matching - Added find_existing_viewer_container helper to reuse existing containers in get_project - Modified get_project to check for existing viewer containers before creating new sessions - Prevents creating duplicate sessions for pre-existing containers after dependency upgrade Co-authored-by: Yashb404 <139128977+Yashb404@users.noreply.github.com>
- Extract timestamp calculation into helper function calculate_session_created_at - Remove unused container_type variable - Reduce code duplication across restoration functions Co-authored-by: Yashb404 <139128977+Yashb404@users.noreply.github.com>
…-one Fix legacy container restoration after dependency upgrade
No description provided.