Files
DarkRoom/core/dr-sync/src/error.rs
T
dtourolleandClaude Opus 5 75fd5619ca Refuse a scan whose root has gone, instead of reporting it empty
FR-PLAT-AND-2, and a silent failure on both platforms. `dr_sync::scan`
stepped over a NotFound or PermissionDenied the way it does for a child
that vanished mid-walk -- correct for a child, wrong for the root, where
it ended the walk, returned Ok with nothing in it, and reported a
successful scan of a library that was no longer there.

A lost root is now its own error. The images under it are marked
Availability::Offline per FR-CAT-9 and no catalog row is deleted;
`library::persist` clears the mark per file as each one is listed again,
so a root that comes back needs no repair step.

Partly satisfied rather than closed, and the gap is worth stating.
The recovery half is real and reachable on Android today, because
`map_status` turns Nextcloud's 403 and 404 into it and Nextcloud is how
a phone actually gets a library in this build. The causes the
requirement names -- revocation, reinstall, a removed card -- are
properties of a persisted tree permission, and there is none: SAF does
not exist here, `SourceRef::Document` is constructed only in test
modules, and `LocalStorage` rejects the variant outright. When SAF
lands it becomes a third producer of this error and nothing above it
changes, which is why the discovery belongs in the connector.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 22:19:38 +02:00

314 lines
14 KiB
Rust

/// Failures from a remote backend.
#[derive(Debug, thiserror::Error)]
pub enum RemoteError {
#[error("not authenticated")]
Unauthenticated,
#[error("authentication rejected")]
AuthFailed,
/// Authenticated, but not permitted to do this.
///
/// **Distinct from [`AuthFailed`](Self::AuthFailed) on purpose.** Folding
/// 403 into 401 sends the user to re-check a credential that is working
/// perfectly: reads succeed, only the write is refused. On Nextcloud the
/// usual cause is an app password created without "Allow filesystem
/// access", or a read-only share — neither of which signing in again will
/// fix.
/// Authenticated, and refused anyway.
///
/// The message names the cause that has actually been observed, because
/// "permission denied" alone sends people to re-check a login that is
/// working. On a Nextcloud mount the folder's `oc:permissions` can carry
/// `C` (create) without `W` (update): a file may be written once and never
/// amended. A sidecar is rewritten on every rating and every edit, so the
/// first judgement on a photograph succeeds and every one after it is
/// refused — which reads as sync being broken rather than as a share
/// needing one more permission.
#[error(
"permission denied — authenticated, but this folder does not allow \
changing an existing file. A Nextcloud share or external mount set to \
create-only will accept a sidecar once and refuse every later edit; \
granting update (and delete) on it is the fix."
)]
PermissionDenied,
#[error("not found: {0}")]
NotFound(String),
/// TRACES: FR-NC-6c
/// The object exists, but its content is not on this device.
///
/// A virtual-filesystem placeholder: the sync client holds the name and a
/// stub, and the bytes are still on the server (ARCH §9.0).
///
/// **Emphatically not [`NotFound`](Self::NotFound), and the distinction is
/// what stops a silent data loss.** The sidecar writer reads before it
/// writes, and treats a miss as "there is no sidecar yet, create one" — so
/// a dehydrated sidecar reported as absent makes it write a fresh document
/// over an existing one, discarding every edit another device had put
/// there. It is also the difference between an error a user can act on
/// (fetch it) and one they cannot (it is gone).
#[error("not on this device: {0}")]
NotMaterialised(String),
/// The backend does not support this operation. Expected, not a bug —
/// callers check capabilities and adapt.
#[error("operation unsupported by this backend: {0}")]
Unsupported(&'static str),
/// The account is configured wrongly, or for a backend this build has no
/// connector for.
///
/// **Not a network failure and not an auth failure**, which is why it is
/// its own variant. An account naming a backend a cut-down build was not
/// compiled with, or a path that would leave the library folder, produces
/// a request that never leaves the process — reporting either as `Network`
/// would put the app into offline mode and tell the user their connection
/// is down, and reporting them as `AuthFailed` would send them to re-enter
/// a credential that is fine. The message names what is wrong with the
/// configuration, because that is the only thing that will fix it.
///
/// A folder library whose directory is not there was once reported here
/// too, and is now [`RootUnavailable`](Self::RootUnavailable): it is not
/// something wrong with the configuration, it is the library being gone,
/// and only the second of those has a catalog to protect.
#[error("account misconfigured: {0}")]
Configuration(String),
/// A conditional write failed: the remote changed underneath us. Triggers
/// the sidecar merge path (ARCH §8.5).
#[error("precondition failed — remote was modified")]
PreconditionFailed,
#[error("quota exceeded")]
QuotaExceeded,
#[error("network error: {0}")]
Network(String),
/// The TLS handshake was refused: an untrusted issuer, an expired
/// certificate, a hostname mismatch.
///
/// **Distinct from [`Network`](Self::Network) on purpose.** The server is
/// there and answering — the connection is refused on trust grounds, which
/// no amount of waiting repairs. Folding it into `Network` puts the app
/// into offline mode and tells the user their connection is down, sending
/// them to inspect a network that is working perfectly (observed
/// 2026-08-12: a Let's Encrypt chain anchored at ISRG Root YE, absent from
/// the compiled-in root store, reported as "you are offline" on a server
/// answering in 19 ms).
#[error("TLS error: {0}")]
Tls(String),
#[error("unexpected server response: {status} {detail}")]
Server { status: u16, detail: String },
#[error("malformed response: {0}")]
Protocol(String),
#[error("operation cancelled")]
Cancelled,
/// TRACES: FR-PLAT-AND-2 | FR-CAT-9
/// The library root itself could not be opened.
///
/// **The one failure that is about the library rather than about a file in
/// it**, and it is a separate variant because every other classification
/// of it is wrong in a way that costs the user something:
///
/// - As [`NotFound`](Self::NotFound) it is indistinguishable from a folder
/// deleted between listing its parent and reaching it, which the walk
/// correctly steps over — so a whole library going away is reported as a
/// successful scan that found nothing.
/// - As [`PermissionDenied`](Self::PermissionDenied) it inherits a message
/// about Nextcloud share permissions and sidecar writes, which is
/// accurate for the case it was written for and nonsense for a tree
/// grant the user revoked in system settings.
/// - As [`Network`](Self::Network) it would claim the connection is down,
/// which is a promise that waiting will fix it.
///
/// Today this is a Nextcloud root that answers 404 or 403 — deleted, or a
/// share withdrawn — or a folder library whose directory is not there. It
/// is also, exactly, the shape a revoked Android tree permission will have
/// when the Storage Access Framework connector FR-PLAT-AND-1 asks for
/// exists: the tree URI still stored, the permission behind it gone, every
/// read failing at the root and nowhere else. **That connector is not
/// built**, so no SAF grant can be lost yet; what this variant does is put
/// the recovery FR-PLAT-AND-2 requires in the one place all three causes
/// pass through, so the third needs no new handling above it.
///
/// The response is the same for all of them and is the point of the
/// variant: mark what the catalog holds as offline, keep every row, and
/// say which library and why (FR-CAT-9).
///
/// The string is the underlying failure, not a rewrite of it. What the
/// user is told is composed where the library's name is known.
#[error("the library folder could not be opened: {0}")]
RootUnavailable(String),
}
impl RemoteError {
/// Whether retrying might succeed.
pub fn is_transient(&self) -> bool {
match self {
RemoteError::Network(_) => true,
// Not transient: a rejected certificate is rejected identically on
// every retry. Retrying one buys nothing and hides the cause.
RemoteError::Tls(_) => false,
RemoteError::Server { status, .. } => {
// 5xx and 429 are worth retrying; other 4xx are not.
//
// 423 Locked is the exception, and it is not hypothetical:
// Nextcloud's file locking returns it on a plain *read* under
// concurrency, and the same range re-read seconds later
// succeeds. Treating it as permanent marks an image
// permanently undated over a lock that lasted moments.
*status >= 500 || *status == 429 || *status == 423
}
_ => false,
}
}
/// Whether this failure means *the server could not be reached*, as
/// opposed to the server answering and refusing.
///
/// The distinction is the whole basis of offline mode (FR-CAT-9). A 403
/// and a dead connection are both "the operation failed", but only one of
/// them is fixed by waiting, and only one of them should put the whole app
/// into a degraded mode. Signing the user out — or showing "you are
/// offline" — because a single file was forbidden would be a much worse
/// error than the one it reported.
///
/// A 5xx is deliberately **not** offline: the server is up and talking, it
/// is just failing, and a retry is the right response rather than a
/// mode change. 429 and 423 likewise — those are the server working
/// correctly under load.
///
/// [`Tls`](Self::Tls) is likewise not offline. The host resolved, the
/// socket connected, and the peer answered; only trust failed. Reporting
/// that as offline is what made a root-store gap look like a dead network.
pub fn indicates_offline(&self) -> bool {
matches!(self, RemoteError::Network(_))
}
/// TRACES: FR-PLAT-AND-2
/// Whether the *library* is gone, as opposed to the server or one file.
///
/// Kept beside [`Self::indicates_offline`] because the two answer the same
/// shape of question and must not be confused. Both put the app into a
/// degraded mode that keeps working from the catalog, but they differ in
/// what the user is told and in what would end it: an offline library
/// comes back when the network does, and an unavailable root comes back
/// only when someone grants access again.
pub fn indicates_lost_root(&self) -> bool {
matches!(self, RemoteError::RootUnavailable(_))
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn a_tls_failure_is_not_offline() {
// The regression this guards: a rejected certificate was reported as
// `Network`, which put the whole app into offline mode and told the
// user their connection was down — while the server answered in 19 ms.
let e = RemoteError::Tls("invalid peer certificate: UnknownIssuer".into());
assert!(
!e.indicates_offline(),
"trust failure is not unreachability"
);
assert!(
!e.is_transient(),
"a rejected certificate is rejected identically on every retry"
);
}
#[test]
fn a_dead_connection_is_still_offline() {
// The other side of the split: separating TLS out must not stop a
// genuine transport failure from reaching offline mode.
assert!(RemoteError::Network("dns error".into()).indicates_offline());
}
#[test]
fn transient_errors_are_retryable() {
assert!(RemoteError::Network("timeout".into()).is_transient());
assert!(RemoteError::Server {
status: 503,
detail: String::new()
}
.is_transient());
assert!(RemoteError::Server {
status: 429,
detail: String::new()
}
.is_transient());
}
#[test]
fn client_errors_are_not_retryable() {
assert!(!RemoteError::Server {
status: 404,
detail: String::new()
}
.is_transient());
assert!(!RemoteError::PreconditionFailed.is_transient());
assert!(!RemoteError::AuthFailed.is_transient());
// Neither is worth retrying, but they mean different things and a
// caller may want to say so.
assert!(!RemoteError::PermissionDenied.is_transient());
}
#[test]
fn permission_denied_is_not_an_auth_failure() {
// 403 folded into 401 sent a user to re-check a credential that was
// working: reads succeeded and only the write was refused (observed
// against a real server, 2026-08-09). The two must read differently.
let denied = RemoteError::PermissionDenied.to_string();
let rejected = RemoteError::AuthFailed.to_string();
assert_ne!(denied, rejected);
// Asserted on the intent rather than on a phrase: the message must
// send the reader to the folder's permissions and not to their
// credential. It gained the create-only detail after a real mount was
// observed accepting a sidecar once and refusing every later edit
// (2026-08-17), and pinning the old wording would have made that
// improvement look like a regression.
assert!(
denied.contains("folder") && denied.contains("permission"),
"the message must point at permissions, not the login: {denied}"
);
assert!(
!denied.contains("sign in") && !denied.contains("password"),
"it must not send the reader back to a working login: {denied}"
);
}
#[test]
fn a_lock_is_transient() {
// Observed against a real server: 12 concurrent range reads produced
// 423 on some files, and the identical request succeeded moments
// later. Classing it with the permanent 4xx left those images
// undated for good.
assert!(RemoteError::Server {
status: 423,
detail: String::new()
}
.is_transient());
// Still permanent, so the exception stays narrow.
assert!(!RemoteError::Server {
status: 404,
detail: String::new()
}
.is_transient());
assert!(!RemoteError::Server {
status: 400,
detail: String::new()
}
.is_transient());
}
}