diff --git a/docs/architecture/02-svelte-frontend.md b/docs/architecture/02-svelte-frontend.md index d685a57..757b84f 100644 --- a/docs/architecture/02-svelte-frontend.md +++ b/docs/architecture/02-svelte-frontend.md @@ -176,7 +176,10 @@ classDiagram -server_url: String -user_id: String -access_token: String + -connectivity: Option~Arc~ConnectivityMonitor~~ +new() + +with_connectivity() + -report_outcome() } class OfflineRepository { @@ -192,10 +195,9 @@ classDiagram class HybridRepository { -online: Arc~OnlineRepository~ -offline: Arc~OfflineRepository~ - -connectivity: Arc~ConnectivityMonitor~ +new() - -parallel_query() - -has_meaningful_content() + -parallel_race() + -cache_with_timeout() } MediaRepository <|.. OnlineRepository @@ -214,6 +216,7 @@ classDiagram - Returns cache result if it has meaningful content - Falls back to server result otherwise - Background cache updates planned + - **Connectivity feedback**: `OnlineRepository` reports the outcome of every server request to the `ConnectivityMonitor` (classified via `RepoError`). This is the source of truth for the offline/online banner — see [07-connectivity.md](07-connectivity.md). The frontend `connectivity` store is a pure reflection of the resulting events; `navigator.onLine` is only an advisory hint that triggers an immediate recheck. 2. **Handle-Based Resource Management** (`repository.rs` commands): ```rust diff --git a/docs/architecture/03-data-flow.md b/docs/architecture/03-data-flow.md index f44ab76..4e07663 100644 --- a/docs/architecture/03-data-flow.md +++ b/docs/architecture/03-data-flow.md @@ -10,6 +10,7 @@ sequenceDiagram participant Hybrid as HybridRepository participant Cache as OfflineRepository (SQLite) participant Server as OnlineRepository (HTTP) + participant Conn as ConnectivityMonitor UI->>Client: getItems(parentId) Client->>Rust: invoke("repository_get_items", {handle, parentId}) @@ -20,6 +21,13 @@ sequenceDiagram Hybrid->>Server: get_items() (no timeout) end + Note over Server,Conn: Every server request reports its outcome + alt Server succeeds (or answers with 4xx/5xx) + Server->>Conn: mark_reachable() (server is up) + else Network failure / timeout + Server->>Conn: mark_unreachable() (debounced) + end + alt Cache returns with content Cache-->>Hybrid: Result with items Hybrid-->>Rust: Return cache result @@ -39,6 +47,7 @@ sequenceDiagram - Cache wins if it has meaningful content - Automatic fallback to server if cache is empty/stale - Background cache updates (planned) +- **Connectivity side-effect**: each server request feeds the `ConnectivityMonitor`, which is the source of truth for the offline/online banner (see [07-connectivity.md](07-connectivity.md)). A server-answered error (401/404/5xx) still counts as *reachable* — only network failures, sustained past a debounce window, flip the app to offline. ## Playback Initiation Flow diff --git a/docs/architecture/07-connectivity.md b/docs/architecture/07-connectivity.md index fef2999..248fb18 100644 --- a/docs/architecture/07-connectivity.md +++ b/docs/architecture/07-connectivity.md @@ -13,12 +13,13 @@ pub struct HttpClient { } pub struct HttpConfig { - pub base_url: String, - pub timeout: Duration, // Default: 10s + pub timeout: Duration, // Default: 30s (large library queries can be slow) pub max_retries: u32, // Default: 3 } ``` +> Note: ordinary requests use the 30s timeout above. The connectivity recovery probe (`ping`) uses a shorter, dedicated 5s timeout so an unreachable server is detected quickly while offline. + **Retry Strategy:** - Retry delays: 1s, 2s, 4s (exponential backoff) - Retries on: Network errors, 5xx server errors @@ -38,39 +39,71 @@ pub enum ErrorKind { **Location**: `src-tauri/src/connectivity/mod.rs` -The connectivity monitor tracks server reachability with adaptive polling: +The connectivity monitor is the **single source of truth** for server reachability. Its primary signal is the outcome of *real repository traffic* — every server request the user actually makes. A standalone `/System/Info/Public` probe is kept only as an offline recovery detector. + +### Source of truth: repository traffic + +`OnlineRepository` reports the result of each server request to the monitor, classified via `RepoError`: + +| Repository outcome | Meaning | Effect on reachability | +|--------------------|---------|------------------------| +| `Ok(_)` | Server answered successfully | Mark **reachable** (instant recovery) | +| `Err(Authentication)` | Server answered with 401/403 | Mark **reachable** (server is up; request was rejected) | +| `Err(NotFound)` | Server answered with 404 | Mark **reachable** (server is up) | +| `Err(Server)` | Server answered with 5xx / bad body | Mark **reachable** (server is up) | +| `Err(Network)` | Connection failure / timeout / DNS | **Candidate for offline** (see debounce) | +| `Err(Database)` | Local cache error only | No effect (not a server signal) | + +This classification fixes the previous bug where a successful `/System/Info/Public` ping reported "online" even while the user's authenticated data calls were failing — and vice versa. + +### Time-window debounce (offline) + instant recovery (online) + +To stop the banner from flapping on a single dropped request, the transition to **offline** is debounced over a time window: + +- On the **first** `Network` failure, the monitor records `first_failure_at`. +- It flips `is_server_reachable = false` only once `Network` failures have persisted continuously for `OFFLINE_CONFIRM_WINDOW` (5s) with no intervening success. +- **Any** success (or server-answered error) clears `first_failure_at` and immediately marks reachable. + +Recovery is therefore instant and asymmetric: one good response brings the app back online, but a brief blip never trips the banner. + +### Offline-only recovery probe ```mermaid flowchart TB - Monitor["ConnectivityMonitor"] --> Poller["Background Task"] - Poller --> Check{"Server
Reachable?"} - Check -->|"Yes"| Online["30s Interval"] - Check -->|"No"| Offline["5s Interval"] - Online --> Emit["Emit Events"] - Offline --> Emit - Emit --> Frontend["Frontend Store"] + Repo["OnlineRepository"] -->|"success / RepoError"| Monitor["ConnectivityMonitor"] + Monitor --> State{"is_server_reachable?"} + State -->|"Online"| NoProbe["No background polling
(real traffic is the signal)"] + State -->|"Offline"| Probe["5s /System/Info/Public probe
(recovery detector)"] + Probe -->|"reachable again"| Monitor + Monitor -->|"on change"| Emit["Emit connectivity:changed
+ connectivity:reconnected"] + Emit --> Frontend["Frontend Store → banner"] ``` +While **online**, there is no background polling — real requests keep the state fresh. While **offline**, the fast 5s probe runs so an idle app still detects the server returning even when no user traffic is flowing. + **Features:** -- **Adaptive Polling**: 30s when online, 5s when offline (for quick reconnection detection) -- **Event Emission**: Emits `connectivity:changed` and `connectivity:reconnected` events -- **Manual Marking**: Can mark reachable/unreachable based on API call results -- **Thread-Safe**: Uses Arc> for shared state +- **Traffic-driven**: Reachability follows the requests the user actually makes. +- **Time-window debounce**: Offline declared only after `OFFLINE_CONFIRM_WINDOW` (5s) of sustained network failure; recovery is instant. +- **Offline-only probe**: 5s `/System/Info/Public` probe runs only while offline. +- **Event Emission**: Emits `connectivity:changed` and `connectivity:reconnected` events. +- **Thread-Safe**: Uses `Arc>` for shared state. **Tauri Commands:** | Command | Description | |---------|-------------| -| `connectivity_check_server` | Manual reachability check | +| `connectivity_check_server` | Manual reachability check (also used by the frontend's advisory `navigator.onLine` hint) | | `connectivity_set_server_url` | Update monitored server URL | | `connectivity_get_status` | Get current connectivity status | -| `connectivity_start_monitoring` | Start background monitoring | -| `connectivity_stop_monitoring` | Stop monitoring | -| `connectivity_mark_reachable` | Mark server as reachable (after successful API call) | -| `connectivity_mark_unreachable` | Mark server as unreachable (after failed API call) | +| `connectivity_start_monitoring` | Start the offline recovery probe | +| `connectivity_stop_monitoring` | Stop the probe | +| `connectivity_mark_reachable` | Mark reachable — driven by `OnlineRepository` on every server success | +| `connectivity_mark_unreachable` | Mark unreachable — driven by `OnlineRepository` on `RepoError::Network` (subject to debounce) | **Frontend Integration:** ```typescript -// TypeScript store listens to Rust events +// The store is a pure reflection of backend events — it no longer decides +// reachability itself. navigator.onLine is advisory: it triggers an immediate +// recheck rather than forcing the offline state. listen<{ isReachable: boolean }>("connectivity:changed", (event) => { updateConnectivityState(event.payload.isReachable); }); @@ -81,12 +114,14 @@ listen<{ isReachable: boolean }>("connectivity:changed", (event) => { The connectivity system provides resilience through multiple layers: 1. **HTTP Client Layer**: Automatic retry with exponential backoff -2. **Connectivity Monitoring**: Background reachability checks -3. **Frontend Integration**: Offline mode detection and UI updates +2. **Connectivity Monitoring**: Reachability derived from real repository traffic, with an offline-only recovery probe +3. **Frontend Integration**: Offline mode detection and UI updates (a pure reflection of backend events) 4. **Sync Queue**: Offline mutations queued for later (see [06-downloads-and-offline.md](06-downloads-and-offline.md)) **Design Principles:** -- **Fail Fast**: Don't retry 4xx errors (client errors, authentication) -- **Fail Slow**: Retry network and 5xx errors with increasing delays -- **Adaptive Polling**: Reduce polling frequency when online, increase when offline -- **Event-Driven**: Frontend reacts to connectivity changes via events +- **Single source of truth**: Reachability follows the outcome of real requests, classified via `RepoError`; the frontend store and the probe never compete to decide it. +- **Fail Fast**: Don't retry 4xx errors (client errors, authentication). +- **Fail Slow**: Retry network and 5xx errors with increasing delays. +- **Debounced offline, instant online**: Declare offline only after a sustained failure window; recover on the first success. +- **Probe only when needed**: Background polling runs only while offline, as a recovery detector. +- **Event-Driven**: Frontend reacts to connectivity changes via events. diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 05cd5e2..f8279e4 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -15,6 +15,7 @@ JellyTau uses a client-server architecture: business logic lives in a comprehens - **Events + Polling hybrid**: Rust emits events the frontend listens to, and the UI also polls status on short intervals in a few hot spots (e.g. queue status in `library/+layout.svelte`, playback progress in `VideoPlayer.svelte`). - **Handle-Based Resources**: UUID handles for stateful Rust objects. - **Cache-First**: Parallel queries with intelligent fallback. +- **Single source of truth for reachability**: Server reachability is derived from the outcome of *real repository traffic*, not a side-channel poller. The `OnlineRepository` reports each server result to the `ConnectivityMonitor` (classified via `RepoError`), which applies a time-window debounce before declaring the server offline and recovers instantly on the first success. The standalone `/System/Info/Public` probe runs *only while offline*, as a recovery detector for idle sessions. - **Poison-tolerant locking**: Shared `std::sync` state is accessed via the `MutexSafe`/`RwLockSafe` helpers in `utils/lock.rs`, which recover a poisoned lock instead of cascading a panic across the player. - **Graceful backend init**: If a native player backend (MPV/ExoPlayer) fails to initialize, the app falls back to a no-op backend and emits a `backend-init-failed` event rather than crashing. @@ -79,9 +80,12 @@ flowchart TB Core --> Storage Repository --> HttpClient Repository --> DatabaseService + Repository -->|"reports server outcome
(success / RepoError)"| ConnectivityMonitor end ``` +> The `Repository --> ConnectivityMonitor` edge is the source of truth for the offline/online banner: every server request the user actually makes updates reachability. The monitor's own polling is now an offline-only recovery probe (see [07-connectivity.md](07-connectivity.md)). + --- ## Detailed Documentation @@ -187,7 +191,7 @@ src/lib/ **What moved to Rust (~3,500 lines of business logic):** 1. **HTTP Client** (338 lines) - Retry logic with exponential backoff -2. **Connectivity Monitor** (301 lines) - Adaptive polling, event emission +2. **Connectivity Monitor** (301 lines) - Reachability derived from real repository traffic, time-window debounce, offline-only recovery probe, event emission 3. **Repository Pattern** (1061 lines) - Cache-first hybrid with parallel racing 4. **Database Service** - Async wrapper preventing UI freezing 5. **Playback Mode** (303 lines) - Local/remote transfer coordination diff --git a/src-tauri/src/commands/repository.rs b/src-tauri/src/commands/repository.rs index a5b00eb..843bd96 100644 --- a/src-tauri/src/commands/repository.rs +++ b/src-tauri/src/commands/repository.rs @@ -52,6 +52,7 @@ pub struct RepositoryManagerWrapper(pub RepositoryManager); pub async fn repository_create( manager: State<'_, RepositoryManagerWrapper>, db: State<'_, crate::commands::storage::DatabaseWrapper>, + connectivity: State<'_, crate::commands::connectivity::ConnectivityMonitorWrapper>, server_url: String, user_id: String, access_token: String, @@ -68,9 +69,18 @@ pub async fn repository_create( })?; debug!("[REPO] HTTP client created successfully"); - // Create online repository + // Grab a connectivity reporter so the online repository's server outcomes + // drive the reachability state the UI observes (source of truth for the + // offline/online banner). See docs/architecture/07-connectivity.md. + let connectivity_reporter = { + let monitor = connectivity.0.lock().await; + monitor.reporter() + }; + + // Create online repository wired to connectivity reporting debug!("[REPO] Creating online repository..."); - let online = OnlineRepository::new(Arc::new(http_client), server_url, user_id.clone(), access_token); + let online = OnlineRepository::new(Arc::new(http_client), server_url, user_id.clone(), access_token) + .with_connectivity(connectivity_reporter); debug!("[REPO] Online repository created"); // Create offline repository with async-safe database service diff --git a/src-tauri/src/connectivity/mod.rs b/src-tauri/src/connectivity/mod.rs index be6e941..fe50d59 100644 --- a/src-tauri/src/connectivity/mod.rs +++ b/src-tauri/src/connectivity/mod.rs @@ -1,15 +1,23 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; use tokio::sync::RwLock; use tauri::{AppHandle, Emitter}; use serde::{Serialize, Deserialize}; use crate::jellyfin::http_client::HttpClient; -// Adaptive polling intervals (matches TypeScript) -const AUTO_CHECK_INTERVAL_MS: u64 = 30000; // 30 seconds when online -const RETRY_CHECK_INTERVAL_MS: u64 = 5000; // 5 seconds when offline +// Offline recovery probe interval. +// Reachability while online is driven by real repository traffic, so there is +// no online polling. While offline we probe quickly to detect the server +// returning even when no user traffic is flowing. +const RETRY_CHECK_INTERVAL_MS: u64 = 5000; // 5 seconds when offline + +// Time-window debounce for declaring the server offline. +// A single dropped request must not trip the banner: we only flip to offline +// once network failures have persisted continuously for this window with no +// intervening success. Recovery (online) is instant on the first success. +const OFFLINE_CONFIRM_WINDOW: Duration = Duration::from_secs(5); /// Connectivity status #[derive(specta::Type, Debug, Clone, Serialize, Deserialize)] @@ -45,210 +53,115 @@ struct ConnectivityChangeEvent { is_reachable: bool, } -/// Connectivity monitor for tracking server reachability -pub struct ConnectivityMonitor { - server_url: Arc>>, - http_client: Arc, +/// Shared reachability state and transition logic. +/// +/// This is the single place that mutates reachability and emits events. It is +/// cheap to clone (all fields are `Arc`/`Option`) and is shared by: +/// - the `ConnectivityMonitor` (commands, offline recovery probe), and +/// - `OnlineRepository`, which reports the outcome of every server request. +/// +/// Reachability is therefore driven by real traffic; the probe only fills the +/// gap while offline. +#[derive(Clone)] +pub struct ConnectivityReporter { status: Arc>, - is_monitoring: Arc, + /// Timestamp of the first network failure in the current failure streak. + /// Used to debounce the transition to offline (see `OFFLINE_CONFIRM_WINDOW`). + first_failure_at: Arc>>, app_handle: Option, } -impl ConnectivityMonitor { - /// Create a new connectivity monitor - pub fn new(http_client: HttpClient) -> Self { +impl ConnectivityReporter { + fn new(status: Arc>, app_handle: Option) -> Self { Self { - server_url: Arc::new(RwLock::new(None)), - http_client: Arc::new(http_client), - status: Arc::new(RwLock::new(ConnectivityStatus::default())), - is_monitoring: Arc::new(AtomicBool::new(false)), - app_handle: None, + status, + first_failure_at: Arc::new(RwLock::new(None)), + app_handle, } } - /// Set the Tauri app handle for event emission - pub fn set_app_handle(&mut self, app_handle: AppHandle) { - self.app_handle = Some(app_handle); + /// Current reachability as seen by this reporter (shared with the monitor + /// and the UI). Useful for callers that want to branch on connectivity. + pub async fn is_reachable(&self) -> bool { + self.status.read().await.is_server_reachable } - /// Update the server URL - pub async fn set_server_url(&self, url: String) { - log::info!("[ConnectivityMonitor] Setting server URL: {}", url); - let mut server_url = self.server_url.write().await; - *server_url = Some(url.clone()); - drop(server_url); - - // Check new server immediately - log::info!("[ConnectivityMonitor] Checking reachability of new server..."); - let is_reachable = self.check_reachability().await; - log::info!("[ConnectivityMonitor] New server is {}", if is_reachable { "REACHABLE" } else { "UNREACHABLE" }); + /// Test-only: force the reporter into the offline state without going through + /// the debounce, so other modules' tests can set up an "offline" precondition. + #[cfg(test)] + pub async fn mark_unreachable_for_test(&self) { + self.apply_probe_result(false, Some("forced offline (test)".to_string())) + .await; } - /// Get current connectivity status - pub async fn get_status(&self) -> ConnectivityStatus { - self.status.read().await.clone() + /// Report that a real server request succeeded (or that the server answered + /// at all, e.g. with 401/404/5xx). The server is up — recover instantly. + pub async fn report_success(&self) { + *self.first_failure_at.write().await = None; + self.set_reachable(true, None).await; } - /// Check if the Jellyfin server is reachable - pub async fn check_reachability(&self) -> bool { - // Mark as checking - { - let mut status = self.status.write().await; - status.is_checking = true; + /// Report that a real server request failed with a network-level error + /// (connection refused, timeout, DNS). Subject to the time-window debounce: + /// we only flip to offline once failures have persisted for + /// `OFFLINE_CONFIRM_WINDOW` with no intervening success. + pub async fn report_network_failure(&self, error: Option) { + // If already offline, nothing to debounce. + if !self.status.read().await.is_server_reachable { + return; } - let server_url = self.server_url.read().await.clone(); - - if server_url.is_none() { - log::warn!("[ConnectivityMonitor] Cannot check reachability: No server URL configured"); - let mut status = self.status.write().await; - status.is_server_reachable = false; - status.connection_error = Some("No server URL configured".to_string()); - status.is_checking = false; - return false; - } - - let url = server_url.unwrap(); - let ping_url = format!("{}/System/Info/Public", url); - - // Store previous reachability state - let was_reachable = { - let status = self.status.read().await; - status.is_server_reachable + let now = Instant::now(); + let streak_start = { + let mut first = self.first_failure_at.write().await; + *first.get_or_insert(now) }; - log::debug!("[ConnectivityMonitor] Pinging server: {}", ping_url); + if now.duration_since(streak_start) >= OFFLINE_CONFIRM_WINDOW { + log::warn!( + "[ConnectivityMonitor] Network failures sustained for {:?}; declaring offline", + OFFLINE_CONFIRM_WINDOW + ); + self.set_reachable(false, error).await; + } else { + log::debug!( + "[ConnectivityMonitor] Network failure within debounce window; not yet offline" + ); + } + } - // Attempt to ping the server - let is_reachable = self.http_client.ping(&ping_url).await; + /// Apply a deliberate reachability probe result (offline recovery probe or a + /// manual check). Unlike `report_network_failure`, a probe is an explicit + /// reachability test, so its result is applied immediately without debounce. + async fn apply_probe_result(&self, is_reachable: bool, error: Option) { + if is_reachable { + *self.first_failure_at.write().await = None; + } + self.set_reachable(is_reachable, error).await; + } - log::debug!( - "[ConnectivityMonitor] Ping result: {} (was: {})", - if is_reachable { "SUCCESS" } else { "FAILED" }, - if was_reachable { "reachable" } else { "unreachable" } - ); - - // Update status - { + /// Core transition: update status and emit events only on an actual change. + async fn set_reachable(&self, is_reachable: bool, error: Option) { + let was_reachable = { let mut status = self.status.write().await; + let was = status.is_server_reachable; status.is_server_reachable = is_reachable; status.last_checked = Some(chrono::Utc::now().to_rfc3339()); status.connection_error = if is_reachable { None } else { - Some("Server unreachable".to_string()) + Some(error.unwrap_or_else(|| "Server unreachable".to_string())) }; status.is_checking = false; - } + was + }; - // Emit events if reachability changed if is_reachable != was_reachable { self.emit_connectivity_change(is_reachable).await; - } - - // Emit reconnection event - if is_reachable && !was_reachable { - self.emit_server_reconnected().await; - } - - is_reachable - } - - /// Mark server as reachable (called after successful API call) - pub async fn mark_reachable(&self) { - let mut status = self.status.write().await; - let was_reachable = status.is_server_reachable; - - status.is_server_reachable = true; - status.last_checked = Some(chrono::Utc::now().to_rfc3339()); - status.connection_error = None; - - drop(status); - - if !was_reachable { - log::info!("[ConnectivityMonitor] Server marked as reachable (was unreachable)"); - self.emit_connectivity_change(true).await; - self.emit_server_reconnected().await; - } - } - - /// Mark server as unreachable (called after failed API call) - pub async fn mark_unreachable(&self, error: Option) { - let mut status = self.status.write().await; - let was_reachable = status.is_server_reachable; - - status.is_server_reachable = false; - status.last_checked = Some(chrono::Utc::now().to_rfc3339()); - status.connection_error = error.or_else(|| Some("Server unreachable".to_string())); - - let error_msg = status.connection_error.clone().unwrap_or_default(); - drop(status); - - if was_reachable { - log::warn!("[ConnectivityMonitor] Server marked as unreachable (was reachable): {}", error_msg); - self.emit_connectivity_change(false).await; - } - } - - /// Start monitoring connectivity with adaptive polling - pub async fn start_monitoring(&self) { - if self.is_monitoring.swap(true, Ordering::SeqCst) { - log::info!("[ConnectivityMonitor] Already monitoring"); - return; - } - - log::info!("[ConnectivityMonitor] Starting connectivity monitoring"); - - // Perform immediate check before starting background task - // This ensures we get an accurate state right away instead of assuming offline - let is_reachable = self.check_reachability().await; - log::info!("[ConnectivityMonitor] Initial connectivity check: {}", if is_reachable { "ONLINE" } else { "OFFLINE" }); - - // Clone Arc references for the background task - let status = Arc::clone(&self.status); - let is_monitoring = Arc::clone(&self.is_monitoring); - let server_url = Arc::clone(&self.server_url); - let http_client = Arc::clone(&self.http_client); - let self_clone = Arc::new(ConnectivityMonitorHandle { - server_url, - http_client, - status, - app_handle: self.app_handle.clone(), - }); - - // Spawn background monitoring task - tokio::spawn(async move { - while is_monitoring.load(Ordering::SeqCst) { - // Determine interval based on current reachability - let interval_ms = { - let status = self_clone.status.read().await; - if status.is_server_reachable { - AUTO_CHECK_INTERVAL_MS - } else { - RETRY_CHECK_INTERVAL_MS - } - }; - - // Wait for the interval - tokio::time::sleep(Duration::from_millis(interval_ms)).await; - - // Check if still monitoring - if !is_monitoring.load(Ordering::SeqCst) { - break; - } - - // Perform connectivity check - let _ = self_clone.check_reachability().await; + if is_reachable { + self.emit_server_reconnected().await; } - - log::info!("[ConnectivityMonitor] Stopped monitoring"); - }); - } - - /// Stop monitoring connectivity - pub fn stop_monitoring(&self) { - log::info!("[ConnectivityMonitor] Stopping connectivity monitoring"); - self.is_monitoring.store(false, Ordering::SeqCst); + } } /// Emit connectivity change event to frontend @@ -275,74 +188,160 @@ impl ConnectivityMonitor { } } -/// Handle for the background monitoring task -struct ConnectivityMonitorHandle { +/// Connectivity monitor for tracking server reachability. +/// +/// Reachability is driven primarily by real repository traffic via the shared +/// [`ConnectivityReporter`]. The monitor itself only runs an offline recovery +/// probe (see `start_monitoring`) and serves the connectivity Tauri commands. +pub struct ConnectivityMonitor { server_url: Arc>>, http_client: Arc, - status: Arc>, - app_handle: Option, + reporter: ConnectivityReporter, + is_monitoring: Arc, } -impl ConnectivityMonitorHandle { - async fn check_reachability(&self) -> bool { +impl ConnectivityMonitor { + /// Create a new connectivity monitor + pub fn new(http_client: HttpClient) -> Self { + let status = Arc::new(RwLock::new(ConnectivityStatus::default())); + Self { + server_url: Arc::new(RwLock::new(None)), + http_client: Arc::new(http_client), + reporter: ConnectivityReporter::new(status, None), + is_monitoring: Arc::new(AtomicBool::new(false)), + } + } + + /// Set the Tauri app handle for event emission. + /// Must be called before the reporter is shared with the repository. + pub fn set_app_handle(&mut self, app_handle: AppHandle) { + self.reporter.app_handle = Some(app_handle); + } + + /// Get a cheap, cloneable reporter so the repository can feed server + /// outcomes into the same reachability state the UI observes. + pub fn reporter(&self) -> ConnectivityReporter { + self.reporter.clone() + } + + /// Update the server URL + pub async fn set_server_url(&self, url: String) { + log::info!("[ConnectivityMonitor] Setting server URL: {}", url); + *self.server_url.write().await = Some(url); + + // Check new server immediately + log::info!("[ConnectivityMonitor] Checking reachability of new server..."); + let is_reachable = self.check_reachability().await; + log::info!("[ConnectivityMonitor] New server is {}", if is_reachable { "REACHABLE" } else { "UNREACHABLE" }); + } + + /// Get current connectivity status + pub async fn get_status(&self) -> ConnectivityStatus { + self.reporter.status.read().await.clone() + } + + /// Deliberately probe the server's reachability (manual check / recovery probe). + /// The result is applied immediately (no debounce) since this is an explicit test. + pub async fn check_reachability(&self) -> bool { + { + let mut status = self.reporter.status.write().await; + status.is_checking = true; + } + let server_url = self.server_url.read().await.clone(); - if server_url.is_none() { + let Some(url) = server_url else { + log::warn!("[ConnectivityMonitor] Cannot check reachability: No server URL configured"); + self.reporter + .apply_probe_result(false, Some("No server URL configured".to_string())) + .await; return false; - } - - let url = server_url.unwrap(); - let ping_url = format!("{}/System/Info/Public", url); - - // Store previous reachability state - let was_reachable = { - let status = self.status.read().await; - status.is_server_reachable }; - // Attempt to ping the server + let ping_url = format!("{}/System/Info/Public", url); + log::debug!("[ConnectivityMonitor] Pinging server: {}", ping_url); + let is_reachable = self.http_client.ping(&ping_url).await; + log::debug!( + "[ConnectivityMonitor] Ping result: {}", + if is_reachable { "SUCCESS" } else { "FAILED" } + ); - // Update status - { - let mut status = self.status.write().await; - status.is_server_reachable = is_reachable; - status.last_checked = Some(chrono::Utc::now().to_rfc3339()); - status.connection_error = if is_reachable { - None - } else { - Some("Server unreachable".to_string()) - }; - } - - // Emit events if reachability changed - if is_reachable != was_reachable { - self.emit_connectivity_change(is_reachable).await; - } - - // Emit reconnection event - if is_reachable && !was_reachable { - self.emit_server_reconnected().await; - } - + self.reporter.apply_probe_result(is_reachable, None).await; is_reachable } - async fn emit_connectivity_change(&self, is_reachable: bool) { - if let Some(app_handle) = &self.app_handle { - let event = ConnectivityChangeEvent { is_reachable }; - if let Err(e) = app_handle.emit("connectivity:changed", event) { - log::error!("[ConnectivityMonitor] Failed to emit connectivity change event: {}", e); - } - } + /// Mark server as reachable (called after successful API call / login) + pub async fn mark_reachable(&self) { + self.reporter.report_success().await; } - async fn emit_server_reconnected(&self) { - if let Some(app_handle) = &self.app_handle { - if let Err(e) = app_handle.emit("connectivity:reconnected", ()) { - log::error!("[ConnectivityMonitor] Failed to emit reconnection event: {}", e); - } + /// Mark server as unreachable directly. + /// + /// Used by deliberate signals (e.g. a failed login/connect) where the caller + /// knows the server is unreachable now. Repository traffic should prefer + /// `reporter().report_network_failure()` so the debounce applies. + pub async fn mark_unreachable(&self, error: Option) { + self.reporter.apply_probe_result(false, error).await; + } + + /// Start the offline recovery probe. + /// + /// While **online**, reachability is kept fresh by real traffic, so the probe + /// idles. While **offline**, it polls `/System/Info/Public` every + /// `RETRY_CHECK_INTERVAL_MS` to detect the server returning even when no user + /// traffic is flowing. + pub async fn start_monitoring(&self) { + if self.is_monitoring.swap(true, Ordering::SeqCst) { + log::info!("[ConnectivityMonitor] Already monitoring"); + return; } + + log::info!("[ConnectivityMonitor] Starting connectivity monitoring (offline recovery probe)"); + + // Perform an immediate check so startup reflects reality quickly. + let is_reachable = self.check_reachability().await; + log::info!("[ConnectivityMonitor] Initial connectivity check: {}", if is_reachable { "ONLINE" } else { "OFFLINE" }); + + let is_monitoring = Arc::clone(&self.is_monitoring); + let server_url = Arc::clone(&self.server_url); + let http_client = Arc::clone(&self.http_client); + let reporter = self.reporter.clone(); + + tokio::spawn(async move { + while is_monitoring.load(Ordering::SeqCst) { + tokio::time::sleep(Duration::from_millis(RETRY_CHECK_INTERVAL_MS)).await; + + if !is_monitoring.load(Ordering::SeqCst) { + break; + } + + // Only probe while offline — real traffic is the signal when online. + if reporter.status.read().await.is_server_reachable { + continue; + } + + let Some(url) = server_url.read().await.clone() else { + continue; + }; + let ping_url = format!("{}/System/Info/Public", url); + let is_reachable = http_client.ping(&ping_url).await; + + // Probe only ever recovers us to online; a failed probe leaves us + // offline without re-emitting (no change). + if is_reachable { + reporter.apply_probe_result(true, None).await; + } + } + + log::info!("[ConnectivityMonitor] Stopped monitoring"); + }); + } + + /// Stop monitoring connectivity + pub fn stop_monitoring(&self) { + log::info!("[ConnectivityMonitor] Stopping connectivity monitoring"); + self.is_monitoring.store(false, Ordering::SeqCst); } } @@ -350,11 +349,22 @@ impl ConnectivityMonitorHandle { mod tests { use super::*; + /// Build a reporter backed by a fresh (optimistic) status, with no app handle. + /// Event emission is a no-op without a handle, which is exactly what we want + /// for unit-testing the reachability state transitions. + fn test_reporter() -> ConnectivityReporter { + ConnectivityReporter::new(Arc::new(RwLock::new(ConnectivityStatus::default())), None) + } + + async fn is_reachable(reporter: &ConnectivityReporter) -> bool { + reporter.status.read().await.is_server_reachable + } + #[test] fn test_intervals() { - // Verify intervals match TypeScript - assert_eq!(AUTO_CHECK_INTERVAL_MS, 30000); + // Offline recovery probe interval (online has no polling). assert_eq!(RETRY_CHECK_INTERVAL_MS, 5000); + assert_eq!(OFFLINE_CONFIRM_WINDOW, Duration::from_secs(5)); } #[tokio::test] @@ -366,4 +376,120 @@ mod tests { assert!(status.connection_error.is_none()); assert!(!status.is_checking); } + + /// A single (or brief) network failure must NOT flip the app offline: + /// the time-window debounce keeps us online until the failure persists. + /// + /// @req-test: UR-002 - Access media when online or offline + #[tokio::test] + async fn test_single_network_failure_does_not_go_offline() { + let reporter = test_reporter(); + assert!(is_reachable(&reporter).await, "starts online"); + + reporter + .report_network_failure(Some("timeout".to_string())) + .await; + + assert!( + is_reachable(&reporter).await, + "one network failure within the debounce window stays online" + ); + // But the failure streak is now being tracked. + assert!(reporter.first_failure_at.read().await.is_some()); + } + + /// Once failures persist past OFFLINE_CONFIRM_WINDOW, we flip offline. + /// We simulate elapsed time by backdating the streak start. + /// + /// @req-test: UR-002 - Access media when online or offline + #[tokio::test] + async fn test_sustained_network_failure_goes_offline() { + let reporter = test_reporter(); + + // First failure starts the streak. + reporter.report_network_failure(None).await; + assert!(is_reachable(&reporter).await); + + // Backdate the streak start to before the window. + { + let mut first = reporter.first_failure_at.write().await; + *first = Some(Instant::now() - OFFLINE_CONFIRM_WINDOW - Duration::from_secs(1)); + } + + // Next failure now exceeds the window → offline. + reporter + .report_network_failure(Some("connection refused".to_string())) + .await; + assert!( + !is_reachable(&reporter).await, + "sustained network failure flips to offline" + ); + } + + /// A success during a failure streak clears the streak and keeps us online — + /// recovery is instant and never trips the banner. + #[tokio::test] + async fn test_success_clears_failure_streak() { + let reporter = test_reporter(); + + reporter.report_network_failure(None).await; + assert!(reporter.first_failure_at.read().await.is_some()); + + reporter.report_success().await; + + assert!(is_reachable(&reporter).await); + assert!( + reporter.first_failure_at.read().await.is_none(), + "success resets the debounce streak" + ); + } + + /// First success after being offline recovers instantly (no debounce on the + /// way back up). + #[tokio::test] + async fn test_recovery_is_instant() { + let reporter = test_reporter(); + + // Force offline. + reporter.apply_probe_result(false, Some("down".to_string())).await; + assert!(!is_reachable(&reporter).await); + + // A single success brings us straight back online. + reporter.report_success().await; + assert!(is_reachable(&reporter).await); + let status = reporter.status.read().await; + assert!(status.connection_error.is_none()); + } + + /// Server-answered errors (401/404/5xx) are reported via report_success + /// by the repository, because the server is demonstrably reachable. This + /// test documents that contract: report_success means "server is up". + #[tokio::test] + async fn test_server_answered_error_counts_as_reachable() { + let reporter = test_reporter(); + + // Simulate being offline, then the server answers (even with an error). + reporter.apply_probe_result(false, None).await; + assert!(!is_reachable(&reporter).await); + + // Repository maps Authentication/NotFound/Server errors to report_success. + reporter.report_success().await; + assert!( + is_reachable(&reporter).await, + "a server that answers (even with 4xx/5xx) is reachable" + ); + } + + /// report_network_failure is a no-op once already offline (nothing to debounce, + /// no duplicate events). + #[tokio::test] + async fn test_network_failure_noop_when_already_offline() { + let reporter = test_reporter(); + reporter.apply_probe_result(false, None).await; + assert!(!is_reachable(&reporter).await); + + // Should not panic or change state. + reporter.report_network_failure(Some("still down".to_string())).await; + assert!(!is_reachable(&reporter).await); + } } diff --git a/src-tauri/src/repository/online.rs b/src-tauri/src/repository/online.rs index b87aeb7..269d6f9 100644 --- a/src-tauri/src/repository/online.rs +++ b/src-tauri/src/repository/online.rs @@ -7,6 +7,7 @@ use log::{debug, error, info}; use log::warn; use serde::{Deserialize, Serialize}; +use crate::connectivity::ConnectivityReporter; use crate::jellyfin::HttpClient; use super::{MediaRepository, types::*}; @@ -16,6 +17,10 @@ pub struct OnlineRepository { server_url: String, user_id: String, access_token: String, + /// Reports the outcome of every server request to the connectivity monitor. + /// This is the source of truth for the offline/online banner. `None` in + /// tests / contexts where connectivity tracking isn't wired up. + connectivity: Option, } impl OnlineRepository { @@ -30,6 +35,44 @@ impl OnlineRepository { server_url, user_id, access_token, + connectivity: None, + } + } + + /// Attach a connectivity reporter so server outcomes drive the reachability + /// state observed by the UI. See `report_outcome`. + pub fn with_connectivity(mut self, reporter: ConnectivityReporter) -> Self { + self.connectivity = Some(reporter); + self + } + + /// Feed a request outcome into the connectivity monitor. + /// + /// Classification (matches docs/architecture/07-connectivity.md): + /// - `Ok` / `Authentication` / `NotFound` / `Server` → the server answered, + /// so it is reachable → `report_success` (instant recovery). + /// - `Network` → connection-level failure → `report_network_failure` + /// (subject to the time-window debounce before going offline). + /// - `Database` → not a server signal → ignored. + async fn report_outcome(&self, result: &Result) { + let Some(reporter) = &self.connectivity else { + return; + }; + + match result { + Ok(_) + | Err(RepoError::Authentication { .. }) + | Err(RepoError::NotFound { .. }) + | Err(RepoError::Server { .. }) => { + reporter.report_success().await; + } + Err(RepoError::Network { message }) => { + reporter.report_network_failure(Some(message.clone())).await; + } + Err(RepoError::Database { .. }) | Err(RepoError::Offline) => { + // Local-side errors (cache failure / already-offline) — not a + // statement about the server's reachability, so ignore them. + } } } @@ -63,6 +106,12 @@ impl OnlineRepository { /// Make authenticated GET request async fn get_json Deserialize<'de>>(&self, endpoint: &str) -> Result { + let result = self.get_json_inner(endpoint).await; + self.report_outcome(&result).await; + result + } + + async fn get_json_inner Deserialize<'de>>(&self, endpoint: &str) -> Result { let url = format!("{}{}", self.server_url, endpoint); let request = self.http_client.client.get(&url) @@ -110,6 +159,12 @@ impl OnlineRepository { /// Make authenticated POST request async fn post_json(&self, endpoint: &str, body: &T) -> Result<(), RepoError> { + let result = self.post_json_inner(endpoint, body).await; + self.report_outcome(&result).await; + result + } + + async fn post_json_inner(&self, endpoint: &str, body: &T) -> Result<(), RepoError> { let url = format!("{}{}", self.server_url, endpoint); let request = self.http_client.client.post(&url) @@ -145,6 +200,16 @@ impl OnlineRepository { &self, endpoint: &str, body: &T, + ) -> Result { + let result = self.post_json_response_inner(endpoint, body).await; + self.report_outcome(&result).await; + result + } + + async fn post_json_response_inner Deserialize<'de>>( + &self, + endpoint: &str, + body: &T, ) -> Result { let url = format!("{}{}", self.server_url, endpoint); @@ -1156,23 +1221,29 @@ impl MediaRepository for OnlineRepository { let endpoint = format!("/Users/{}/FavoriteItems/{}", self.user_id, item_id); let url = format!("{}{}", self.server_url, endpoint); - let request = self.http_client.client.delete(&url) - .header("X-Emby-Authorization", self.auth_header()) - .build() - .map_err(|e| RepoError::Network { - message: format!("Failed to build request: {}", e), - })?; + let result = async { + let request = self.http_client.client.delete(&url) + .header("X-Emby-Authorization", self.auth_header()) + .build() + .map_err(|e| RepoError::Network { + message: format!("Failed to build request: {}", e), + })?; - let response = self.http_client.request_with_retry(request).await - .map_err(|e| RepoError::Network { message: e.to_string() })?; + let response = self.http_client.request_with_retry(request).await + .map_err(|e| RepoError::Network { message: e.to_string() })?; - if !response.status().is_success() { - return Err(RepoError::Server { - message: format!("HTTP {}", response.status()), - }); + if !response.status().is_success() { + return Err(RepoError::Server { + message: format!("HTTP {}", response.status()), + }); + } + + Ok(()) } + .await; - Ok(()) + self.report_outcome(&result).await; + result } async fn get_person(&self, person_id: &str) -> Result { @@ -1397,6 +1468,91 @@ mod tests { ) } + /// Build a repository wired to a real ConnectivityReporter so we can assert + /// how `report_outcome` classifies each `RepoError` into reachability. + /// (No app handle → event emission is a harmless no-op.) + fn create_test_repository_with_connectivity( + ) -> (OnlineRepository, crate::connectivity::ConnectivityReporter) { + let monitor_http = HttpClient::new(crate::jellyfin::HttpConfig::default()) + .expect("Failed to create HTTP client for monitor"); + let monitor = crate::connectivity::ConnectivityMonitor::new(monitor_http); + let reporter = monitor.reporter(); + let repo = create_test_repository().with_connectivity(reporter.clone()); + (repo, reporter) + } + + /// `report_outcome` is the seam between repository traffic and the + /// connectivity monitor. Verify each `RepoError` variant routes correctly: + /// - the server answering at all (Ok / 401 / 404 / 5xx) ⇒ reachable + /// - a network-level failure ⇒ marked unreachable (debounce reduced for test) + /// - local-side errors (Database / Offline) ⇒ no effect on reachability + /// + /// @req-test: UR-002 - Access media when online or offline + /// @req-test: DR-013 - Repository pattern for online/offline data access + #[tokio::test] + async fn test_report_outcome_classifies_server_answered_as_reachable() { + let (repo, reporter) = create_test_repository_with_connectivity(); + + // Drive offline first so we can observe "recover to reachable". + for err in [ + RepoError::Authentication { message: "401".into() }, + RepoError::NotFound { message: "404".into() }, + RepoError::Server { message: "500".into() }, + ] { + reporter.mark_unreachable_for_test().await; + assert!(!reporter.is_reachable().await, "precondition: offline"); + + let result: Result<(), RepoError> = Err(err); + repo.report_outcome(&result).await; + + assert!( + reporter.is_reachable().await, + "a server that answers should be reported reachable" + ); + } + + // Ok should also report reachable. + reporter.mark_unreachable_for_test().await; + let ok: Result<(), RepoError> = Ok(()); + repo.report_outcome(&ok).await; + assert!(reporter.is_reachable().await, "Ok ⇒ reachable"); + } + + /// Local-side errors must NOT flip reachability — they say nothing about the + /// server. + #[tokio::test] + async fn test_report_outcome_ignores_local_errors() { + let (repo, reporter) = create_test_repository_with_connectivity(); + + // Force offline, then a Database/Offline error must leave it offline + // (not falsely report reachable). + reporter.mark_unreachable_for_test().await; + for err in [RepoError::Database { message: "cache".into() }, RepoError::Offline] { + let result: Result<(), RepoError> = Err(err); + repo.report_outcome(&result).await; + assert!( + !reporter.is_reachable().await, + "local-side error must not change reachability" + ); + } + } + + /// A network error routes through the debounced path. A single failure stays + /// online (debounce window not yet elapsed). + #[tokio::test] + async fn test_report_outcome_network_error_is_debounced() { + let (repo, reporter) = create_test_repository_with_connectivity(); + assert!(reporter.is_reachable().await, "starts online"); + + let result: Result<(), RepoError> = Err(RepoError::Network { message: "timeout".into() }); + repo.report_outcome(&result).await; + + assert!( + reporter.is_reachable().await, + "a single network failure stays online (debounced)" + ); + } + #[tokio::test] async fn test_get_audio_stream_url_formats_correctly() { let repo = create_test_repository(); diff --git a/src/lib/stores/connectivity.ts b/src/lib/stores/connectivity.ts index 2afb4f9..1c852cb 100644 --- a/src/lib/stores/connectivity.ts +++ b/src/lib/stores/connectivity.ts @@ -1,7 +1,12 @@ // Connectivity state store for offline support // -// Simplified wrapper over Rust connectivity monitor. -// The Rust backend handles all polling, reachability checks, and adaptive intervals. +// Pure reflection of the Rust ConnectivityMonitor. Reachability is the single +// source of truth in Rust, derived from real repository traffic (success/ +// RepoError), with a time-window debounce before going offline and instant +// recovery. This store only listens for `connectivity:changed` events and +// mirrors status; it does not decide reachability itself. navigator.onLine is +// advisory and triggers a recheck rather than forcing offline. +// See docs/architecture/07-connectivity.md. // TRACES: UR-002 | DR-013 import { writable, derived } from "svelte/store"; @@ -61,22 +66,27 @@ function createConnectivityStore() { } }); - // Listen to browser online/offline events and update state + // Listen to browser online/offline events. These are ADVISORY only — the + // Rust ConnectivityMonitor (fed by real repository traffic) is the source of + // truth for server reachability. navigator.onLine can be wrong (e.g. a + // LAN-only server is still reachable while the browser reports "offline"), + // so we use these events to trigger an immediate recheck rather than forcing + // the offline state. See docs/architecture/07-connectivity.md. window.addEventListener("online", () => { update((s) => ({ ...s, isOnline: true })); + // Device regained network — ask the backend to re-verify the server now. + checkServerReachable().catch((err) => { + console.debug("[ConnectivityStore] Recheck after 'online' failed:", err); + }); }); window.addEventListener("offline", () => { - update((s) => ({ - ...s, - isOnline: false, - isServerReachable: false, - connectionError: "Device is offline", - })); - - if (eventHandlers.onConnectivityChange) { - eventHandlers.onConnectivityChange(false); - } + // Reflect the browser's view of the device link, but let the backend + // decide whether the server is actually reachable. + update((s) => ({ ...s, isOnline: false })); + checkServerReachable().catch((err) => { + console.debug("[ConnectivityStore] Recheck after 'offline' failed:", err); + }); }); } @@ -199,43 +209,11 @@ function createConnectivityStore() { return checkServerReachable(); } - /** - * Mark server as reachable (e.g., after successful API call) - */ - async function markReachable(): Promise { - try { - await commands.connectivityMarkReachable(); - - // Update local state - update((s) => ({ - ...s, - isServerReachable: true, - lastChecked: new Date(), - connectionError: null, - })); - } catch (error) { - console.error("[ConnectivityStore] Failed to mark reachable:", error); - } - } - - /** - * Mark server as unreachable (e.g., after failed API call) - */ - async function markUnreachable(error?: string): Promise { - try { - await commands.connectivityMarkUnreachable(error ?? null); - - // Update local state - update((s) => ({ - ...s, - isServerReachable: false, - lastChecked: new Date(), - connectionError: error || "Server unreachable", - })); - } catch (err) { - console.error("[ConnectivityStore] Failed to mark unreachable:", err); - } - } + // Reachability is now driven by the Rust backend from real repository + // traffic (see docs/architecture/07-connectivity.md). The frontend no longer + // mutates reachability itself, so the former markReachable/markUnreachable + // helpers have been removed. The underlying Rust commands remain available + // for deliberate signals if ever needed. return { subscribe, @@ -244,8 +222,6 @@ function createConnectivityStore() { setServerUrl, forceCheck, checkServerReachable, - markReachable, - markUnreachable, isNetworkError, }; }