Files
DarkRoom/core/dr-sync/src/error.rs
T
dtourolle c102ba9df2 Treat a placeholder as the photograph, not as a one-byte file
The folder connector was pointed at a Nextcloud VFS tree and got three
things wrong, the first of which loses work.

**A dehydrated sidecar read as absent.** `a.drsc` does not exist when the
client has dehydrated it — only `a.drsc.nextcloud` does — so `get` missed,
`.ok()` swallowed the `NotFound`, and the sidecar writer took that for
"there is no sidecar yet" and wrote a fresh document over the existing
one. Every edit another device had put there went with it. That function's
own doc comment calls this the exact loss the format's unknown-key
preservation exists to prevent.

**A stub was catalogued as a 1-byte image**, and ARCH §9.0 measured this
machine at 121,785 placeholders against 10,267 real files — so a folder
library on a synced tree was ~92% broken rows.

**Identity changed on hydration**, so downloading a photograph looked like
a delete and an add, orphaning its thumbnail and its face rows.

Entries now carry the photograph's own name and a `materialised` flag;
`get` on a stub returns the new `RemoteError::NotMaterialised`, which is
distinct from `NotFound` precisely because the sidecar writer must treat
them differently — it fetches the sidecar and merges, or leaves the entry
queued.

Hydration is a **borrow**. `BorrowPool` records what was on disk before it
asked, so `release_all` dehydrates only what a pass brought and leaves
what the user already had. Reference counted: the thumbnail pass and the
face pass meet on the same RAW, and without counting the first to finish
dehydrates the file the second is reading. A borrow against a plain folder
or a server does nothing, so a pass written for VFS runs everywhere.

Releasing means asking the client to dehydrate and never deleting: a
deletion inside a synced tree propagates to the server and removes the
photograph from every device.

Not a second backend — the capability is per *connection*, not per type,
since the same folder hydrates only while the client runs. The convention
arrives through a detector the registry supplies, so `dr-sync-folder`
still knows nothing about any client's protocol.

ARCH §9.0a records this as an amendment: finding 3 rejected hydration
because it costs 100× a range read, and that comparison assumed a
connector was available. A folder library has none.
2026-08-29 09:57:52 +02:00

260 lines
11 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. A folder library whose directory has been unmounted,
/// or an account naming a backend a cut-down build was not compiled with,
/// 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.
#[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,
}
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(_))
}
}
#[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());
}
}