Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/core/account_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ pub fn outcome_log_line(outcome: &crate::core::scheduler::SyncOutcome) -> &'stat
t("Synchronization blocked: password keyring is locked")
}
crate::core::scheduler::SyncOutcome::Failed => t("Synchronization failed — view the log"),
crate::core::scheduler::SyncOutcome::NetworkError => {
t("Synchronization blocked: the server is unreachable")
}
}
}

Expand Down Expand Up @@ -1941,8 +1944,10 @@ mod tests {
SyncOutcome::Success,
SyncOutcome::Conflict,
SyncOutcome::AuthFailed,
SyncOutcome::NoCredentials,
SyncOutcome::KeyringLocked,
SyncOutcome::Failed,
SyncOutcome::NetworkError,
] {
let line = outcome_log_line(&outcome);
assert!(!line.is_empty(), "English label for {outcome:?}");
Expand Down
6 changes: 5 additions & 1 deletion src/core/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ pub fn failure_notification(outcome: &crate::core::scheduler::SyncOutcome) -> Op
"No credentials are saved for this account.",
)),
SyncOutcome::Failed => Some(crate::util::i18n::t("A synchronization failed.")),
SyncOutcome::Success | SyncOutcome::Conflict => None,
// A transport failure is a transient network condition (the server is
// unreachable), not a problem the account needs a notification for:
// it resolves on the next automatic trigger once the server answers.
// Silence it like the healthy outcomes (issue #162).
SyncOutcome::Success | SyncOutcome::Conflict | SyncOutcome::NetworkError => None,
}
}

Expand Down
41 changes: 41 additions & 0 deletions src/core/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ pub enum SyncOutcome {
NoCredentials,
/// The synchronization failed (any other error).
Failed,
/// The account's server is unreachable (transport failure). Treated as
/// Offline for that account only: the machine has a network link but the
/// specific server does not answer (issue #162).
NetworkError,
}

/// Executes a reconciliation. Implemented by the sync engine in Task 2.3.
Expand Down Expand Up @@ -696,6 +700,19 @@ impl SchedulerInner {
.set(AppState::Error, t("Synchronization failed — view the log"));
true
}
SyncOutcome::NetworkError => {
// Issue #162: the server itself is unreachable even though the
// machine has a network link. Mark this folder Offline (with
// its own message) so the account no longer reads as Connected
// and stops spinning in a wait loop. The periodic/module
// triggers retry on the next tick, so it clears once the
// server answers again. This only applies to the failing
// account/folder, never to other accounts.
self.keyring_locked = false;
self.state
.set(AppState::Offline, t("Waiting for a network connection"));
false
}
};
// Fase 3: after a successful run the guard baseline is refreshed so
// it reflects what nextcloudcmd just reconciled (Python: only for
Expand Down Expand Up @@ -1539,6 +1556,30 @@ mod tests {
assert_eq!(scheduler.state().snapshot().state, AppState::Error);
}

/// Issue #162: an unreachable server (transport failure) marks the folder
/// Offline instead of Connected, and does not arm the credential gate or
/// leave the account spinning. The scheduler keeps retrying on the next
/// automatic trigger, so it recovers once the server answers again.
#[test]
fn network_error_sets_offline_and_recovers_on_retry() {
let (scheduler, source, runner) = make_scheduler(None);
scheduler.request(Trigger::Manual);
run_idle(&source);
finish(&runner, SyncOutcome::NetworkError);
assert_eq!(scheduler.state().snapshot().state, AppState::Offline);
assert!(!scheduler.auth_required());
assert!(!scheduler.keyring_locked());

// A later automatic trigger retries (unlike the credential gate, which
// defers): the run starts again and, when the server answers, the
// folder is no longer Offline.
scheduler.request(Trigger::RemoteInterval);
run_idle(&source);
assert_eq!(runner.0.borrow().start_calls, 2);
finish(&runner, SyncOutcome::Success);
assert_eq!(scheduler.state().snapshot().state, AppState::IdleOk);
}

#[test]
fn auth_failure_sets_auth_required() {
let (scheduler, source, runner) = make_scheduler(None);
Expand Down
36 changes: 34 additions & 2 deletions src/nextcloud/sync_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,16 @@ fn engine_thread(
// `nextcloudcmd` exits 1 with no output when the remote folder does not
// exist; create it (and its parents) first. Auth rejection surfaces as
// such; anything else falls through and lets nextcloudcmd report.
// Issue #162: a transport failure (the server itself does not answer)
// must not launch nextcloudcmd against an unreachable host. Return a
// NetworkError outcome so the scheduler marks this folder Offline and the
// account stops reading as Connected.
if let Some(ensure) = inputs.remote_ensurer.as_ref() {
if let Err(ApiError::AuthRejected) = ensure(&inputs.account, &inputs.folder, &password) {
return EngineRun::Direct(SyncOutcome::AuthFailed);
match ensure(&inputs.account, &inputs.folder, &password) {
Ok(()) => {}
Err(ApiError::AuthRejected) => return EngineRun::Direct(SyncOutcome::AuthFailed),
Err(ApiError::Transport) => return EngineRun::Direct(SyncOutcome::NetworkError),
Err(_) => {}
}
}
let driver = driver_for(inputs.account.provider);
Expand Down Expand Up @@ -934,6 +941,31 @@ mod tests {
assert_eq!(outcome, SyncOutcome::AuthFailed);
}

/// Issue #162: a transport failure (the server itself does not answer)
/// must short-circuit to NetworkError without spawning nextcloudcmd. The
/// fake binary is /bin/false, so if the engine spawned it the run would
/// end Failed instead; Transport must prevent the spawn entirely.
#[test]
fn remote_ensurer_transport_failure_short_circuits_to_network_error() {
let (progress_tx, _progress_rx) = async_channel::unbounded();
let engine = SyncEngine::new(
account(),
folder(),
NetworkConfig::default(),
None,
Some(PathBuf::from("/bin/false")),
progress_tx,
)
.with_credentials(Arc::new(FakeCredentials(CredentialLookup::Found(
"secret".to_string(),
))))
.with_remote_ensurer(Arc::new(|_account, _folder, _password| {
Err(ApiError::Transport)
}));
let (outcome, _) = run_engine(engine, &async_channel::unbounded().1);
assert_eq!(outcome, SyncOutcome::NetworkError);
}

#[test]
fn remote_ensurer_non_auth_error_does_not_block_the_run() {
let dir = tempfile::tempdir().unwrap();
Expand Down
1 change: 1 addition & 0 deletions src/util/translations/es.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ pub static CATALOG: &[(&str, &str)] = &[
("Synchronization blocked", "Sincronización bloqueada"),
("Synchronization blocked: no saved credentials", "Sincronización bloqueada: no hay credenciales guardadas"),
("Synchronization blocked: password keyring is locked", "Sincronización bloqueada: el almacén de contraseñas está bloqueado"),
("Synchronization blocked: the server is unreachable", "Sincronización bloqueada: el servidor no está disponible"),
("Synchronization completed", "Sincronización completada"),
("Synchronization completed with conflicts", "Sincronización completada con conflictos"),
("Synchronization failed", "Error de sincronización"),
Expand Down
Loading