diff --git a/crates/pdfs-core/src/error.rs b/crates/pdfs-core/src/error.rs index 8bdd102..f1e580a 100644 --- a/crates/pdfs-core/src/error.rs +++ b/crates/pdfs-core/src/error.rs @@ -160,13 +160,14 @@ impl CoreError { /// telling the user their connection is down beats telling them the /// server refused. The SDK's other failures say what went wrong instead /// (see [`proton_kind`]). + /// + /// Callers also hand it this crate's [`Error`] from the cache and the + /// database, which is not a transport error at all. Taking those for + /// "offline" told the user their connection was down when a pin failed on + /// the database (`docs/BUGS.md` B266), so the source chain is searched for + /// either type before falling back. pub fn from_api(e: &(dyn std::error::Error + 'static), what: &str) -> Self { - let kind = match e.downcast_ref::() { - Some(proton) => proton_kind(proton), - // Not the SDK's at all: a timeout around the call, or the socket. - None => ErrorKind::Offline, - }; - Self::new(kind, format!("{what}: {e}")) + Self::new(api_kind_of(e), format!("{what}: {e}")) } /// Prefix the message while keeping the classification. @@ -208,19 +209,40 @@ impl From for CoreError { impl From for CoreError { fn from(e: Error) -> Self { - let kind = match &e { - Error::NotLoggedIn - | Error::ReloginRequired - | Error::Locked - | Error::WrongMailboxPassword - | Error::HumanVerificationRequired(_) => ErrorKind::Denied, - Error::Proton(proton) => proton_kind(proton), - _ => ErrorKind::Internal, - }; - Self::new(kind, e.to_string()) + Self::new(core_kind(&e), e.to_string()) } } +/// What this crate's [`Error`] means to a front-end. +fn core_kind(e: &Error) -> ErrorKind { + match e { + Error::NotLoggedIn + | Error::ReloginRequired + | Error::Locked + | Error::WrongMailboxPassword + | Error::HumanVerificationRequired(_) => ErrorKind::Denied, + Error::Proton(proton) => proton_kind(proton), + _ => ErrorKind::Internal, + } +} + +/// The kind for [`CoreError::from_api`]: the first SDK or crate error in `e`'s +/// source chain decides it. +fn api_kind_of(e: &(dyn std::error::Error + 'static)) -> ErrorKind { + let mut cause = Some(e); + while let Some(err) = cause { + if let Some(proton) = err.downcast_ref::() { + return proton_kind(proton); + } + if let Some(core) = err.downcast_ref::() { + return core_kind(core); + } + cause = err.source(); + } + // Not the SDK's at all: a timeout around the call, or the socket. + ErrorKind::Offline +} + /// What a failed SDK call means to a front-end. /// /// Only a transport failure is the connection. The rest happened on this @@ -414,6 +436,20 @@ mod tests { assert_ne!(e.kind, ErrorKind::Offline); } + /// A pin or a search that fails on the database is not the connection, so + /// it must not say "You're offline" (`docs/BUGS.md` B266). An SDK error + /// wrapped in this crate's type classifies as the SDK error does. + #[test] + fn a_database_error_handed_to_from_api_is_not_offline() { + let db = Error::Other("database is locked".into()); + assert_eq!(CoreError::from_api(&db, "pin").kind, ErrorKind::Internal); + let wrapped = Error::Proton(api(C::DoesNotExist)); + assert_eq!( + CoreError::from_api(&wrapped, "load node").kind, + ErrorKind::NotFound + ); + } + /// `Error` is the plumbing type; converting it must classify as sharply as /// the direct path, or the two disagree about the same failure. #[test] diff --git a/docs/BUGS.md b/docs/BUGS.md index 4f3da2e..e25adb4 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -74,7 +74,6 @@ original wording. | [B269](#b269--a-restore-that-skipped-every-folder-reports-success) | A restore that skipped every folder reports success | Not fixed | | [B268](#b268--pdfs-diagnose-says-no-problems-found-while-sync-is-stuck) | `pdfs diagnose` says "No problems found" while sync is stuck | Not fixed | | [B267](#b267--a-key-failure-at-a-start-from-a-warm-cache-shows-as-offline-for-good) | A key failure at a start from a warm cache shows as offline for good | Not fixed | -| [B266](#b266--pin-unpin-search-and-node-loads-report-a-database-error-as-offline) | Pin, unpin, search and node loads report a database error as "offline" | Not fixed | | [B265](#b265--address-keys-whose-token-is-for-an-inactive-account-key-never-open) | Address keys whose token is for an inactive account key never open | Needs an SDK change; not reproduced | | [B134](#b134--a-file-deleted-online-while-the-drain-checks-its-write-is-kept-as-a-conflict-copy) | A file deleted online while the drain checks its write is kept as a conflict copy | Only with `local_first` off | | [B91](#b91--the-daemon-hung-overnight-and-nothing-could-say-what-it-was-waiting-on) | The daemon hung overnight, and nothing could say what it was waiting on | The cause is not diagnosed; the daemon now reports where it is stuck and is restarted | @@ -111,6 +110,7 @@ a live account since. | Entry | Title | |---|---| +| [B266](#b266--pin-unpin-search-and-node-loads-report-a-database-error-as-offline) | Pin, unpin, search and node loads report a database error as "offline" | | [B264](#b264--a-mount-that-fails-after-starting-its-workers-leaves-them-running) | A mount that fails after starting its workers leaves them running | | [B261](#b261--a-folder-that-has-never-synced-reads-up-to-date) | A folder that has never synced reads "up to date" | | [B272](#b272--after-a-folder-is-removed-and-added-again-its-file-events-go-to-the-old-id) | After a folder is removed and added again, its file events go to the old id | @@ -525,7 +525,7 @@ report the others as their own state through `Status`. ### B266 — Pin, unpin, search and node loads report a database error as "offline" -**Status:** Open +**Status:** Fixed, not verified live **Found:** 2026-10-31, code audit after issue #38. B205 fixed the photo upload path only. **Where:** `CoreError::from_api` in `crates/pdfs-core/src/error.rs`; its callers in `crates/pdfs-fuse/src/lib.rs` (`pin`, `unpin`, the searches, `load node`) @@ -538,8 +538,11 @@ it was written for SDK calls, where that means a transport error. These callers `pdfs_core::Error` values from the cache and the database. A `ProtonError` wrapped in `pdfs_core::Error::Proton` also misses the downcast, since the source chain is not walked. -**Fix.** Not done. Use `CoreError::internal` at the database callers, and walk the source -chain in `from_api` before falling back. +**Fix.** `from_api` walks the source chain. The first `ProtonError` or `pdfs_core::Error` in it +decides the kind, the same way `From` does, and only an error with neither, a transport +error, reads as offline. That covers every caller at once, the database ones included. + +**Test:** `a_database_error_handed_to_from_api_is_not_offline` in `pdfs-core`. ---