feat(device): Add connection status to device cards #2970
Conversation
This commit introduces auto-generated TypeScript types for both the core and Tauri applications, enhancing type safety and consistency across the codebase. The generated types include various data structures and interfaces used in the applications, facilitating better integration and development experience. The types are generated using Specta and are intended to be maintained automatically, ensuring they remain up-to-date with the underlying data models.
This commit modifies the TypeScript types generation script to use the CARGO_MANIFEST_DIR environment variable, ensuring that the generated types are written to the correct directory within the core package structure. This change enhances the maintainability of the type generation process by preventing issues related to relative paths.
This commit deletes the auto-generated TypeScript types file from the project. The removal is part of a transition to a new type generation process that writes the types to a different location, ensuring better organization and maintainability. The previous types file is no longer needed as the generation script has been updated to reflect the new output path.
PR Summary
Written by Cursor Bugbot for commit 234e3f0. Configure here. |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| if let Some(library_manager) = library_manager_weak.upgrade() { | ||
| // Try to get device from first available library | ||
| // (device data should be consistent across libraries since it's synced) | ||
| let rt = tokio::runtime::Handle::try_current(); |
There was a problem hiding this comment.
Using block_on inside async code is problematic and can cause deadlocks if the runtime has only one thread or if the call happens on the runtime thread. Consider making emit_device_changed async and awaiting the database query instead.
| let rt = tokio::runtime::Handle::try_current(); | |
| async fn emit_device_changed(&self, device_id: Uuid, info: &DeviceInfo, is_connected: bool) { | |
| let Some(event_bus) = &self.event_bus else { | |
| return; | |
| }; | |
| // Try to query the full device from database to get hardware_model | |
| let device = self.query_device_from_database(device_id, info, is_connected).await; | |
| use crate::domain::resource::EventEmitter; | |
| if let Err(e) = device.emit_changed(event_bus) { |
Alternatively, spawn a separate task for the database query to avoid blocking.
| DeviceState::Connected { info, .. } => Some(info), | ||
| DeviceState::Disconnected { info, .. } => Some(info), | ||
| _ => None, | ||
| }; |
There was a problem hiding this comment.
The device variable is declared as mutable but never mutated after assignment. Remove the mut keyword.
| }; | |
| let device = Device::from_network_info(&info, is_actually_connected, connection_method); |
|
|
||
| // Find current device and infer destination device | ||
| const currentDevice = devices?.find(d => d.is_current); | ||
|
|
There was a problem hiding this comment.
This logic assumes there are only 2 devices in a cross-device transfer, but that may not always be true (e.g., multi-device environment). Consider using metadata from the job itself to identify the actual destination device ID rather than inferring it.
core/src/service/network/core/mod.rs
Outdated
| ) | ||
| } | ||
|
|
||
| /// Spawn a background task to watch for connection closure | ||
| /// | ||
| /// This provides instant reactivity when connections drop by waiting on | ||
| /// Iroh's Connection::closed() future, instead of relying on the 10-second | ||
| /// polling interval in update_connection_states(). | ||
| async fn spawn_connection_watcher(&self, conn: Connection, node_id: NodeId) { | ||
| // Check if we already have a watcher for this node | ||
| { | ||
| let mut watched = self.watched_nodes.write().await; | ||
| if watched.contains(&node_id) { | ||
| // Already watching this node, skip to prevent duplicates | ||
| return; | ||
| } | ||
| watched.insert(node_id); | ||
| } | ||
|
|
||
| let device_registry = self.device_registry.clone(); | ||
| let active_connections = self.active_connections.clone(); | ||
| let watched_nodes = self.watched_nodes.clone(); | ||
| let logger = self.logger.clone(); | ||
|
|
||
| tokio::spawn(async move { | ||
| // Wait for the connection to close | ||
| let close_reason = conn.closed().await; | ||
|
|
||
| logger | ||
| .info(&format!( | ||
| "Connection to {} closed instantly: {:?}", | ||
| node_id, close_reason | ||
| )) | ||
| .await; | ||
|
|
||
| // Remove from active connections | ||
| { | ||
| let mut connections = active_connections.write().await; | ||
| connections.retain(|(nid, _alpn), _conn| *nid != node_id); | ||
| } | ||
|
|
||
| // Remove from watched nodes set so future reconnections can spawn a new watcher | ||
| { | ||
| let mut watched = watched_nodes.write().await; | ||
| watched.remove(&node_id); | ||
| } | ||
|
|
||
| // Find the device ID for this node and update state | ||
| let mut registry = device_registry.write().await; | ||
| if let Some(device_id) = registry.get_device_by_node_id(node_id) { | ||
| // Use update_device_from_connection with ConnectionType::None | ||
| // This handles any current state and transitions appropriately | ||
| if let Err(e) = registry |
There was a problem hiding this comment.
There's significant code duplication between NetworkingService::spawn_connection_watcher and NetworkingEventLoop::spawn_connection_watcher. Consider extracting this into a shared helper function or trait to avoid maintaining two nearly identical implementations.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
| } | ||
| None | ||
| }); |
There was a problem hiding this comment.
block_on in async context will panic at runtime
High Severity
The emit_device_changed function uses Handle::block_on() to execute async database queries. This function is called from async methods like mark_connected, mark_disconnected, and update_device_from_connection, which run on the Tokio runtime. Calling Handle::block_on() from within an async context on the same runtime will panic with "Cannot block the current thread from within a runtime." The function needs to be made async or use a different approach to query the database.
Additional Locations (2)
| // (assuming only 2 devices in the transfer scenario) | ||
| const destinationDevice = isCrossDevice && currentDevice | ||
| ? devices?.find(d => !d.is_current) | ||
| : null; |
There was a problem hiding this comment.
Wrong destination device shown with multiple devices
Medium Severity
The logic to determine the destination device for cross-device transfers uses devices?.find(d => !d.is_current), which simply picks the first non-current device. When a user has more than 2 paired devices, this will display an incorrect device name in the job title. The CopyStrategyMetadata only contains is_cross_device but not the actual destination device ID, making accurate determination impossible with the current metadata structure.
| { | ||
| let mut connections = active_connections.write().await; | ||
| connections.retain(|(nid, _alpn), _conn| *nid != node_id); | ||
| } |
There was a problem hiding this comment.
Connection watcher removes all ALPN connections when one closes
Medium Severity
The connection watcher cleanup logic uses connections.retain(|(nid, _alpn), _conn| *nid != node_id) which removes ALL connections to a node regardless of ALPN protocol. Since active_connections is keyed by (NodeId, ALPN) and nodes can have multiple simultaneous connections (pairing, sync, file transfer, etc.), closing one connection incorrectly removes all others. This orphans still-active connections, marks the device as offline prematurely, and could cause redundant reconnection attempts while existing connections are still alive.
Additional Locations (1)
…ce state updates This commit refactors the connection watcher logic in the NetworkingService and NetworkingEventLoop by extracting the connection watcher task into a separate function. This change enhances code readability and maintainability. Additionally, it improves the handling of device state updates when connections close, ensuring accurate tracking of device connectivity status. Minor adjustments were made to the TypeScript types generation script for consistency.
…ab state This commit introduces a drag-and-drop feature for reordering tabs within the TabBar component, enhancing user experience by allowing intuitive tab management. The DndProvider is updated to handle tab reordering logic, while the TabManagerContext is modified to persist the state of tabs, including the active tab and explorer states, in local storage. This ensures that tab arrangements are maintained across sessions, improving usability and consistency.
No description provided.