fix(core): read database errors handed to from_api as internal, not offline

from_api took every error that was not a ProtonError for a transport
failure. Pin, unpin, the searches and node loads hand it pdfs_core
errors from the cache and the database, so a failure there said
"You're offline" (B266, the rest of B205). The source chain now
decides, and only an error with neither type reads as offline.
This commit is contained in:
nvrl committed 2026-10-31 10:53:25 +01:00
1 parent 9460c93b50
commit bdbfc0bfd4
2 files changed
+59 -20

No files matched your search

+52 -16
View File
@@ -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::<proton_sdk::ProtonError>() {
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<serde_json::Error> for CoreError {
impl From<Error> 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::<proton_sdk::ProtonError>() {
return proton_kind(proton);
}
if let Some(core) = err.downcast_ref::<Error>() {
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]
+7 -4
View File
@@ -75,7 +75,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 |
| [B264](#b264--a-mount-that-fails-after-starting-its-workers-leaves-them-running) | A mount that fails after starting its workers leaves them running | Not fixed |
| [B263](#b263--a-mount-that-can-never-succeed-retries-every-5-s-and-tells-only-the-journal) | A mount that can never succeed retries every 5 s and tells only the journal | Not fixed |
@@ -116,6 +115,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" |
| [B256](#b256--pdfs-copy-right-after-a-save-copies-the-old-version) | `pdfs copy` right after a save copies the old version |
| [B255](#b255--a-synced-folder-whose-files-are-all-gone-here-is-stuck-for-good) | A synced folder whose files are all gone here is stuck for good |
| [B254](#b254--an-edit-in-the-same-second-as-a-new-upload-never-arrives) | An edit in the same second as a new upload never arrives |
@@ -523,7 +523,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`)
@@ -536,8 +536,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<Error>` 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`.
---