move sync/network errors into separate file

rslib/src/backend/err.rs

@@ -16,10 +16,12 @@ pub(super) fn anki_error_to_proto_error(err: AnkiError, tr: &I18n) -> pb::Backen
         AnkiError::TemplateError { .. } => V::TemplateParse(pb::Empty {}),
         AnkiError::IoError { .. } => V::IoError(pb::Empty {}),
         AnkiError::DbError { .. } => V::DbError(pb::Empty {}),
-        AnkiError::NetworkError { kind, .. } => {
+        AnkiError::NetworkError(err) => V::NetworkError(pb::NetworkError {
-            V::NetworkError(pb::NetworkError { kind: kind.into() })
+            kind: err.kind.into(),
-        }
+        }),
-        AnkiError::SyncError { kind, .. } => V::SyncError(pb::SyncError { kind: kind.into() }),
+        AnkiError::SyncError(err) => V::SyncError(pb::SyncError {
+            kind: err.kind.into(),
+        }),
         AnkiError::Interrupted => V::Interrupted(pb::Empty {}),
         AnkiError::CollectionNotOpen => V::InvalidInput(pb::Empty {}),
         AnkiError::CollectionAlreadyOpen => V::InvalidInput(pb::Empty {}),

rslib/src/backend/sync/server.rs

@@ -24,12 +24,13 @@ impl Backend {
         F: FnOnce(&mut LocalServer) -> Result<T>,
     {
         let mut state_guard = self.state.lock().unwrap();
-        let out = func(state_guard.sync.http_sync_server.as_mut().ok_or_else(|| {
+        let out = func(
-            AnkiError::SyncError {
+            state_guard
-                kind: SyncErrorKind::SyncNotStarted,
+                .sync
-                info: Default::default(),
+                .http_sync_server
-            }
+                .as_mut()
-        })?);
+                .ok_or_else(|| AnkiError::sync_error("", SyncErrorKind::SyncNotStarted))?,
+        );
         if out.is_err() {
             self.abort_and_restore_collection(Some(state_guard))
         }
@@ -81,10 +82,7 @@ impl Backend {
             .sync
             .http_sync_server
             .take()
-            .ok_or_else(|| AnkiError::SyncError {
+            .ok_or_else(|| AnkiError::sync_error("", SyncErrorKind::SyncNotStarted))
-                kind: SyncErrorKind::SyncNotStarted,
-                info: String::new(),
-            })
     }
 
     fn start(&self, input: StartIn) -> Result<Graves> {

rslib/src/error/mod.rs

@@ -2,16 +2,17 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 mod db;
+mod network;
 mod search;
 
 pub use {
     db::DbErrorKind,
+    network::{NetworkError, NetworkErrorKind, SyncError, SyncErrorKind},
     search::{ParseError, SearchErrorKind},
 };
 
 use crate::i18n::I18n;
 pub use failure::{Error, Fail};
-use reqwest::StatusCode;
 use std::io;
 use tempfile::PathPersistError;
 
@@ -34,14 +35,11 @@ pub enum AnkiError {
     #[fail(display = "DB error: {}", info)]
     DbError { info: String, kind: DbErrorKind },
 
-    #[fail(display = "Network error: {:?} {}", kind, info)]
+    #[fail(display = "Network error: {:?}", _0)]
-    NetworkError {
+    NetworkError(NetworkError),
-        info: String,
-        kind: NetworkErrorKind,
-    },
 
-    #[fail(display = "Sync error: {:?}, {}", kind, info)]
+    #[fail(display = "Sync error: {:?}", _0)]
-    SyncError { info: String, kind: SyncErrorKind },
+    SyncError(SyncError),
 
     #[fail(display = "JSON encode/decode error: {}", info)]
     JsonError { info: String },
@@ -87,45 +85,13 @@ impl AnkiError {
     }
 
     pub(crate) fn server_message<S: Into<String>>(msg: S) -> AnkiError {
-        AnkiError::SyncError {
+        AnkiError::sync_error(msg, SyncErrorKind::ServerMessage)
-            info: msg.into(),
-            kind: SyncErrorKind::ServerMessage,
-        }
-    }
-
-    pub(crate) fn sync_misc<S: Into<String>>(msg: S) -> AnkiError {
-        AnkiError::SyncError {
-            info: msg.into(),
-            kind: SyncErrorKind::Other,
-        }
     }
 
     pub fn localized_description(&self, tr: &I18n) -> String {
         match self {
-            AnkiError::SyncError { info, kind } => match kind {
+            AnkiError::SyncError(err) => err.localized_description(tr),
-                SyncErrorKind::ServerMessage => info.into(),
+            AnkiError::NetworkError(err) => err.localized_description(tr),
-                SyncErrorKind::Other => info.into(),
-                SyncErrorKind::Conflict => tr.sync_conflict(),
-                SyncErrorKind::ServerError => tr.sync_server_error(),
-                SyncErrorKind::ClientTooOld => tr.sync_client_too_old(),
-                SyncErrorKind::AuthFailed => tr.sync_wrong_pass(),
-                SyncErrorKind::ResyncRequired => tr.sync_resync_required(),
-                SyncErrorKind::ClockIncorrect => tr.sync_clock_off(),
-                SyncErrorKind::DatabaseCheckRequired => tr.sync_sanity_check_failed(),
-                // server message
-                SyncErrorKind::SyncNotStarted => "sync not started".into(),
-            }
-            .into(),
-            AnkiError::NetworkError { kind, info } => {
-                let summary = match kind {
-                    NetworkErrorKind::Offline => tr.network_offline(),
-                    NetworkErrorKind::Timeout => tr.network_timeout(),
-                    NetworkErrorKind::ProxyAuth => tr.network_proxy_auth(),
-                    NetworkErrorKind::Other => tr.network_other(),
-                };
-                let details = tr.network_details(info.as_str());
-                format!("{}\n\n{}", summary, details)
-            }
             AnkiError::TemplateError { info } => {
                 // already localized
                 info.into()
@@ -176,114 +142,6 @@ impl From<io::Error> for AnkiError {
     }
 }
 
-#[derive(Debug, PartialEq)]
-pub enum NetworkErrorKind {
-    Offline,
-    Timeout,
-    ProxyAuth,
-    Other,
-}
-
-impl From<reqwest::Error> for AnkiError {
-    fn from(err: reqwest::Error) -> Self {
-        let url = err.url().map(|url| url.as_str()).unwrap_or("");
-        let str_err = format!("{}", err);
-        // strip url from error to avoid exposing keys
-        let info = str_err.replace(url, "");
-
-        if err.is_timeout() {
-            AnkiError::NetworkError {
-                info,
-                kind: NetworkErrorKind::Timeout,
-            }
-        } else if err.is_status() {
-            error_for_status_code(info, err.status().unwrap())
-        } else {
-            guess_reqwest_error(info)
-        }
-    }
-}
-
-#[derive(Debug, PartialEq)]
-pub enum SyncErrorKind {
-    Conflict,
-    ServerError,
-    ClientTooOld,
-    AuthFailed,
-    ServerMessage,
-    ClockIncorrect,
-    Other,
-    ResyncRequired,
-    DatabaseCheckRequired,
-    SyncNotStarted,
-}
-
-fn error_for_status_code(info: String, code: StatusCode) -> AnkiError {
-    use reqwest::StatusCode as S;
-    match code {
-        S::PROXY_AUTHENTICATION_REQUIRED => AnkiError::NetworkError {
-            info,
-            kind: NetworkErrorKind::ProxyAuth,
-        },
-        S::CONFLICT => AnkiError::SyncError {
-            info,
-            kind: SyncErrorKind::Conflict,
-        },
-        S::FORBIDDEN => AnkiError::SyncError {
-            info,
-            kind: SyncErrorKind::AuthFailed,
-        },
-        S::NOT_IMPLEMENTED => AnkiError::SyncError {
-            info,
-            kind: SyncErrorKind::ClientTooOld,
-        },
-        S::INTERNAL_SERVER_ERROR | S::BAD_GATEWAY | S::GATEWAY_TIMEOUT | S::SERVICE_UNAVAILABLE => {
-            AnkiError::SyncError {
-                info,
-                kind: SyncErrorKind::ServerError,
-            }
-        }
-        S::BAD_REQUEST => AnkiError::SyncError {
-            info,
-            kind: SyncErrorKind::DatabaseCheckRequired,
-        },
-        _ => AnkiError::NetworkError {
-            info,
-            kind: NetworkErrorKind::Other,
-        },
-    }
-}
-
-fn guess_reqwest_error(mut info: String) -> AnkiError {
-    if info.contains("dns error: cancelled") {
-        return AnkiError::Interrupted;
-    }
-    let kind = if info.contains("unreachable") || info.contains("dns") {
-        NetworkErrorKind::Offline
-    } else if info.contains("timed out") {
-        NetworkErrorKind::Timeout
-    } else {
-        if info.contains("invalid type") {
-            info = format!(
-                "{} {} {}\n\n{}",
-                "Please force a full sync in the Preferences screen to bring your devices into sync.",
-                "Then, please use the Check Database feature, and sync to your other devices.",
-                "If problems persist, please post on the support forum.",
-                info,
-            );
-        }
-
-        NetworkErrorKind::Other
-    };
-    AnkiError::NetworkError { info, kind }
-}
-
-impl From<zip::result::ZipError> for AnkiError {
-    fn from(err: zip::result::ZipError) -> Self {
-        AnkiError::sync_misc(err.to_string())
-    }
-}
-
 impl From<serde_json::Error> for AnkiError {
     fn from(err: serde_json::Error) -> Self {
         AnkiError::JsonError {

rslib/src/error/network.rs

@@ -0,0 +1,167 @@
+// Copyright: Ankitects Pty Ltd and contributors
+// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
+
+use super::AnkiError;
+
+use anki_i18n::I18n;
+use reqwest::StatusCode;
+
+#[derive(Debug, PartialEq)]
+pub struct NetworkError {
+    pub info: String,
+    pub kind: NetworkErrorKind,
+}
+
+#[derive(Debug, PartialEq)]
+pub enum NetworkErrorKind {
+    Offline,
+    Timeout,
+    ProxyAuth,
+    Other,
+}
+
+#[derive(Debug, PartialEq)]
+pub struct SyncError {
+    pub info: String,
+    pub kind: SyncErrorKind,
+}
+
+#[derive(Debug, PartialEq)]
+pub enum SyncErrorKind {
+    Conflict,
+    ServerError,
+    ClientTooOld,
+    AuthFailed,
+    ServerMessage,
+    ClockIncorrect,
+    Other,
+    ResyncRequired,
+    DatabaseCheckRequired,
+    SyncNotStarted,
+}
+
+impl AnkiError {
+    pub(crate) fn sync_error(info: impl Into<String>, kind: SyncErrorKind) -> Self {
+        AnkiError::SyncError(SyncError {
+            info: info.into(),
+            kind,
+        })
+    }
+}
+
+impl From<reqwest::Error> for AnkiError {
+    fn from(err: reqwest::Error) -> Self {
+        let url = err.url().map(|url| url.as_str()).unwrap_or("");
+        let str_err = format!("{}", err);
+        // strip url from error to avoid exposing keys
+        let info = str_err.replace(url, "");
+
+        if err.is_timeout() {
+            AnkiError::NetworkError(NetworkError {
+                info,
+                kind: NetworkErrorKind::Timeout,
+            })
+        } else if err.is_status() {
+            error_for_status_code(info, err.status().unwrap())
+        } else {
+            guess_reqwest_error(info)
+        }
+    }
+}
+
+fn error_for_status_code(info: String, code: StatusCode) -> AnkiError {
+    use reqwest::StatusCode as S;
+    match code {
+        S::PROXY_AUTHENTICATION_REQUIRED => AnkiError::NetworkError(NetworkError {
+            info,
+            kind: NetworkErrorKind::ProxyAuth,
+        }),
+        S::CONFLICT => AnkiError::SyncError(SyncError {
+            info,
+            kind: SyncErrorKind::Conflict,
+        }),
+        S::FORBIDDEN => AnkiError::SyncError(SyncError {
+            info,
+            kind: SyncErrorKind::AuthFailed,
+        }),
+        S::NOT_IMPLEMENTED => AnkiError::SyncError(SyncError {
+            info,
+            kind: SyncErrorKind::ClientTooOld,
+        }),
+        S::INTERNAL_SERVER_ERROR | S::BAD_GATEWAY | S::GATEWAY_TIMEOUT | S::SERVICE_UNAVAILABLE => {
+            AnkiError::SyncError(SyncError {
+                info,
+                kind: SyncErrorKind::ServerError,
+            })
+        }
+        S::BAD_REQUEST => AnkiError::SyncError(SyncError {
+            info,
+            kind: SyncErrorKind::DatabaseCheckRequired,
+        }),
+        _ => AnkiError::NetworkError(NetworkError {
+            info,
+            kind: NetworkErrorKind::Other,
+        }),
+    }
+}
+
+fn guess_reqwest_error(mut info: String) -> AnkiError {
+    if info.contains("dns error: cancelled") {
+        return AnkiError::Interrupted;
+    }
+    let kind = if info.contains("unreachable") || info.contains("dns") {
+        NetworkErrorKind::Offline
+    } else if info.contains("timed out") {
+        NetworkErrorKind::Timeout
+    } else {
+        if info.contains("invalid type") {
+            info = format!(
+                "{} {} {}\n\n{}",
+                "Please force a full sync in the Preferences screen to bring your devices into sync.",
+                "Then, please use the Check Database feature, and sync to your other devices.",
+                "If problems persist, please post on the support forum.",
+                info,
+            );
+        }
+
+        NetworkErrorKind::Other
+    };
+    AnkiError::NetworkError(NetworkError { info, kind })
+}
+
+impl From<zip::result::ZipError> for AnkiError {
+    fn from(err: zip::result::ZipError) -> Self {
+        AnkiError::sync_error(err.to_string(), SyncErrorKind::Other)
+    }
+}
+
+impl SyncError {
+    pub fn localized_description(&self, tr: &I18n) -> String {
+        match self.kind {
+            SyncErrorKind::ServerMessage => self.info.clone().into(),
+            SyncErrorKind::Other => self.info.clone().into(),
+            SyncErrorKind::Conflict => tr.sync_conflict(),
+            SyncErrorKind::ServerError => tr.sync_server_error(),
+            SyncErrorKind::ClientTooOld => tr.sync_client_too_old(),
+            SyncErrorKind::AuthFailed => tr.sync_wrong_pass(),
+            SyncErrorKind::ResyncRequired => tr.sync_resync_required(),
+            SyncErrorKind::ClockIncorrect => tr.sync_clock_off(),
+            SyncErrorKind::DatabaseCheckRequired => tr.sync_sanity_check_failed(),
+            SyncErrorKind::SyncNotStarted => "sync not started".into(),
+        }
+        .into()
+    }
+}
+
+impl NetworkError {
+    pub fn localized_description(&self, tr: &I18n) -> String {
+        let summary = match self.kind {
+            NetworkErrorKind::Offline => tr.network_offline(),
+            NetworkErrorKind::Timeout => tr.network_timeout(),
+            NetworkErrorKind::ProxyAuth => tr.network_proxy_auth(),
+            NetworkErrorKind::Other => tr.network_other(),
+        };
+        let details = tr.network_details(self.info.as_str());
+        format!("{}\n\n{}", summary, details)
+    }
+}

rslib/src/media/sync.rs

@@ -402,10 +402,7 @@ where
                 Ok(())
             } else {
                 self.ctx.transact(|ctx| ctx.force_resync())?;
-                Err(AnkiError::SyncError {
+                Err(AnkiError::sync_error("", SyncErrorKind::ResyncRequired))
-                    info: "".into(),
-                    kind: SyncErrorKind::ResyncRequired,
-                })
             }
         } else {
             Err(AnkiError::server_message(resp.err))
@@ -608,7 +605,7 @@ fn extract_into_media_folder(
 
         let real_name = fmap
             .get(name)
-            .ok_or_else(|| AnkiError::sync_misc("malformed zip"))?;
+            .ok_or_else(|| AnkiError::sync_error("malformed zip", SyncErrorKind::Other))?;
 
         let mut data = Vec::with_capacity(file.size() as usize);
         file.read_to_end(&mut data)?;

rslib/src/notes/mod.rs

@@ -570,8 +570,8 @@ fn note_differs_from_db(existing_note: &mut Note, note: &mut Note) -> bool {
 mod test {
     use super::{anki_base91, field_checksum};
     use crate::{
-        collection::open_test_collection, config::BoolKey, decks::DeckId, error::Result, prelude::*,
+        collection::open_test_collection, config::BoolKey, decks::DeckId, error::Result,
-        search::SortMode,
+        prelude::*, search::SortMode,
     };
 
     #[test]

rslib/src/storage/sync_check.rs

@@ -39,10 +39,10 @@ impl SqliteStorage {
             "notetypes",
         ] {
             if self.table_has_usn(table)? {
-                return Err(AnkiError::SyncError {
+                return Err(AnkiError::sync_error(
-                    info: format!("table had usn=-1: {}", table),
+                    format!("table had usn=-1: {}", table),
-                    kind: SyncErrorKind::Other,
+                    SyncErrorKind::Other,
-                });
+                ));
             }
         }
 

rslib/src/sync/http_client.rs

@@ -281,10 +281,7 @@ impl HttpSyncClient {
         resp.error_for_status_ref()?;
         let text = resp.text().await?;
         if text != "OK" {
-            Err(AnkiError::SyncError {
+            Err(AnkiError::sync_error(text, SyncErrorKind::Other))
-                info: text,
-                kind: SyncErrorKind::Other,
-            })
         } else {
             Ok(())
         }
@@ -349,7 +346,10 @@ fn sync_endpoint(host_number: u32) -> String {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::{error::SyncErrorKind, sync::SanityCheckDueCounts};
+    use crate::{
+        error::{SyncError, SyncErrorKind},
+        sync::SanityCheckDueCounts,
+    };
     use tokio::runtime::Runtime;
 
     async fn http_client_inner(username: String, password: String) -> Result<()> {
@@ -357,10 +357,10 @@ mod test {
 
         assert!(matches!(
             syncer.login("nosuchuser", "nosuchpass").await,
-            Err(AnkiError::SyncError {
+            Err(AnkiError::SyncError(SyncError {
                 kind: SyncErrorKind::AuthFailed,
                 ..
-            })
+            }))
         ));
 
         assert!(syncer.login(&username, &password).await.is_ok());
@@ -370,10 +370,10 @@ mod test {
         // aborting before a start is a conflict
         assert!(matches!(
             syncer.abort().await,
-            Err(AnkiError::SyncError {
+            Err(AnkiError::SyncError(SyncError {
                 kind: SyncErrorKind::Conflict,
                 ..
-            })
+            }))
         ));
 
         let _graves = syncer.start(Usn(1), true, None).await?;

rslib/src/sync/mod.rs

@@ -10,6 +10,7 @@ use crate::{
     card::{Card, CardQueue, CardType},
     deckconf::DeckConfSchema11,
     decks::DeckSchema11,
+    error::SyncError,
     error::SyncErrorKind,
     notes::Note,
     notetype::{Notetype, NotetypeSchema11},
@@ -339,10 +340,10 @@ where
                         self.col.storage.rollback_trx()?;
                         let _ = self.remote.abort().await;
 
-                        if let AnkiError::SyncError {
+                        if let AnkiError::SyncError(SyncError {
-                            kind: SyncErrorKind::DatabaseCheckRequired,
                             info,
-                        } = &e
+                            kind: SyncErrorKind::DatabaseCheckRequired,
+                        }) = &e
                         {
                             debug!(self.col.log, "sanity check failed:\n{}", info);
                         }
@@ -359,10 +360,10 @@ where
         debug!(self.col.log, "remote {:?}", &remote);
         if !remote.should_continue {
             debug!(self.col.log, "server says abort"; "message"=>&remote.server_message);
-            return Err(AnkiError::SyncError {
+            return Err(AnkiError::sync_error(
-                info: remote.server_message,
+                remote.server_message,
-                kind: SyncErrorKind::ServerMessage,
+                SyncErrorKind::ServerMessage,
-            });
+            ));
         }
 
         let local = self.col.sync_meta()?;
@@ -370,11 +371,7 @@ where
         let delta = remote.current_time.0 - local.current_time.0;
         if delta.abs() > 300 {
             debug!(self.col.log, "clock off"; "delta"=>delta);
-            return Err(AnkiError::SyncError {
+            return Err(AnkiError::sync_error("", SyncErrorKind::ClockIncorrect));
-                // fixme: need to rethink error handling; defer translation and pass in time difference
-                info: "".into(),
-                kind: SyncErrorKind::ClockIncorrect,
-            });
         }
 
         Ok(local.compared_to_remote(remote))
@@ -551,10 +548,10 @@ where
         let out: SanityCheckOut = self.remote.sanity_check(local_counts).await?;
         debug!(self.col.log, "got server reply");
         if out.status != SanityCheckStatus::Ok {
-            Err(AnkiError::SyncError {
+            Err(AnkiError::sync_error(
-                info: format!("local {:?}\nremote {:?}", out.client, out.server),
+                format!("local {:?}\nremote {:?}", out.client, out.server),
-                kind: SyncErrorKind::DatabaseCheckRequired,
+                SyncErrorKind::DatabaseCheckRequired,
-            })
+            ))
         } else {
             Ok(())
         }
@@ -852,10 +849,10 @@ impl Collection {
                     if (existing_nt.fields.len() != nt.fields.len())
                         || (existing_nt.templates.len() != nt.templates.len())
                     {
-                        return Err(AnkiError::SyncError {
+                        return Err(AnkiError::sync_error(
-                            info: "notetype schema changed".into(),
+                            "notetype schema changed",
-                            kind: SyncErrorKind::ResyncRequired,
+                            SyncErrorKind::ResyncRequired,
-                        });
+                        ));
                     }
                     true
                 } else {