use enums for some common errors

proto/backend.proto

@@ -55,9 +55,8 @@ message BackendError {
         TemplateParseError template_parse = 2;
         StringError io_error = 3;
         StringError db_error = 4;
-        StringError network_error = 5;
+        NetworkError network_error = 5;
-        Empty ankiweb_auth_failed = 6;
+        SyncError sync_error = 6;
-        StringError ankiweb_misc_error = 7;
         // user interrupted operation
         Empty interrupted = 8;
     }
@@ -78,6 +77,30 @@ message TemplateParseError {
     bool q_side = 2;
 }
 
+message NetworkError {
+    string info = 1;
+    enum NetworkErrorKind {
+        OTHER = 0;
+        OFFLINE = 1;
+        TIMEOUT = 2;
+        PROXY_AUTH = 3;
+    }
+    NetworkErrorKind kind = 2;
+}
+
+message SyncError {
+    string info = 1;
+    enum SyncErrorKind {
+        OTHER = 0;
+        CONFLICT = 1;
+        SERVER_ERROR = 2;
+        CLIENT_TOO_OLD = 3;
+        AUTH_FAILED = 4;
+        SERVER_MESSAGE = 5;
+    }
+    SyncErrorKind kind = 2;
+}
+
 message MediaSyncProgress {
     oneof value {
         uint32 downloaded_changes = 1;

pylib/anki/rsbackend.py

@@ -28,8 +28,12 @@ class StringError(Exception):
         return self.args[0]  # pylint: disable=unsubscriptable-object
 
 
+NetworkErrorKind = pb.NetworkError.NetworkErrorKind
+
+
 class NetworkError(StringError):
-    pass
+    def kind(self) -> NetworkErrorKind:
+        return self.args[1]
 
 
 class IOError(StringError):
@@ -45,12 +49,12 @@ class TemplateError(StringError):
         return self.args[1]
 
 
-class AnkiWebError(StringError):
+SyncErrorKind = pb.SyncError.SyncErrorKind
-    pass
 
 
-class AnkiWebAuthFailed(Exception):
+class SyncError(StringError):
-    pass
+    def kind(self) -> SyncErrorKind:
+        return self.args[1]
 
 
 def proto_exception_to_native(err: pb.BackendError) -> Exception:
@@ -58,7 +62,8 @@ def proto_exception_to_native(err: pb.BackendError) -> Exception:
     if val == "interrupted":
         return Interrupted()
     elif val == "network_error":
-        return NetworkError(err.network_error.info)
+        e = err.network_error
+        return NetworkError(e.info, e.kind)
     elif val == "io_error":
         return IOError(err.io_error.info)
     elif val == "db_error":
@@ -67,10 +72,9 @@ def proto_exception_to_native(err: pb.BackendError) -> Exception:
         return TemplateError(err.template_parse.info, err.template_parse.q_side)
     elif val == "invalid_input":
         return StringError(err.invalid_input.info)
-    elif val == "ankiweb_auth_failed":
+    elif val == "sync_error":
-        return AnkiWebAuthFailed()
+        e2 = err.sync_error
-    elif val == "ankiweb_misc_error":
+        return SyncError(e2.info, e2.kind)
-        return AnkiWebError(err.ankiweb_misc_error.info)
     else:
         assert_impossible_literal(val)
 

qt/aqt/mediasync.py

@@ -14,7 +14,6 @@ from anki import hooks
 from anki.lang import _
 from anki.media import media_paths_from_col_path
 from anki.rsbackend import (
-    AnkiWebAuthFailed,
     DBError,
     Interrupted,
     MediaSyncDownloadedChanges,
@@ -23,8 +22,11 @@ from anki.rsbackend import (
     MediaSyncRemovedFiles,
     MediaSyncUploaded,
     NetworkError,
+    NetworkErrorKind,
     Progress,
     ProgressKind,
+    SyncError,
+    SyncErrorKind,
 )
 from anki.types import assert_impossible
 from anki.utils import intTime
@@ -42,10 +44,11 @@ class MediaSyncState:
     removed_files: int = 0
 
 
+# fixme: sync.rs fixmes
+# fixme: maximum size when uploading
 # fixme: abort when closing collection/app
 # fixme: concurrent modifications during upload step
 # fixme: mediaSanity
-# fixme: corruptMediaDB
 # fixme: autosync
 #         elif evt == "mediaSanity":
 #         showWarning(
@@ -155,15 +158,31 @@ class MediaSyncer:
             return
 
         self._log_and_notify(_("Media sync failed."))
-        if isinstance(exc, AnkiWebAuthFailed):
+        if isinstance(exc, SyncError):
+            kind = exc.kind()
+            if kind == SyncErrorKind.AUTH_FAILED:
                 self.mw.pm.set_sync_key(None)
-            showWarning(_("AnkiWeb ID or password was incorrect; please try again."))
+                showWarning(
+                    _("AnkiWeb ID or password was incorrect; please try again.")
+                )
+            elif kind == SyncErrorKind.SERVER_ERROR:
+                showWarning(
+                    _(
+                        "AnkiWeb encountered a problem. Please try again in a few minutes."
+                    )
+                )
+            else:
+                showWarning(_("Unexpected error: {}").format(str(exc)))
         elif isinstance(exc, NetworkError):
+            nkind = exc.kind()
+            if nkind in (NetworkErrorKind.OFFLINE, NetworkErrorKind.TIMEOUT):
                 showWarning(
                     _("Syncing failed; please check your internet connection.")
                     + "\n\n"
                     + _("Detailed error: {}").format(str(exc))
                 )
+            else:
+                showWarning(_("Unexpected error: {}").format(str(exc)))
         elif isinstance(exc, DBError):
             showWarning(_("Problem accessing the media database: {}").format(str(exc)))
         else:

rslib/src/backend.rs

@@ -5,7 +5,7 @@ use crate::backend_proto as pt;
 use crate::backend_proto::backend_input::Value;
 use crate::backend_proto::{Empty, RenderedTemplateReplacement, SyncMediaIn};
 use crate::cloze::expand_clozes_to_reveal_latex;
-use crate::err::{AnkiError, Result};
+use crate::err::{AnkiError, NetworkErrorKind, Result, SyncErrorKind};
 use crate::media::sync::{sync_media, Progress as MediaSyncProgress};
 use crate::media::MediaManager;
 use crate::sched::{local_minutes_west_for_stamp, sched_timing_today};
@@ -41,12 +41,17 @@ impl std::convert::From<AnkiError> for pt::BackendError {
             AnkiError::InvalidInput { info } => V::InvalidInput(pt::StringError { info }),
             AnkiError::TemplateError { info, q_side } => {
                 V::TemplateParse(pt::TemplateParseError { info, q_side })
-            },
+            }
             AnkiError::IOError { info } => V::IoError(pt::StringError { info }),
             AnkiError::DBError { info } => V::DbError(pt::StringError { info }),
-            AnkiError::NetworkError { info } => V::NetworkError(pt::StringError { info }),
+            AnkiError::NetworkError { info, kind } => V::NetworkError(pt::NetworkError {
-            AnkiError::AnkiWebAuthenticationFailed => V::AnkiwebAuthFailed(Empty {}),
+                info,
-            AnkiError::AnkiWebMiscError { info } => V::AnkiwebMiscError(pt::StringError { info }),
+                kind: kind.into(),
+            }),
+            AnkiError::SyncError { info, kind } => V::SyncError(pt::SyncError {
+                info,
+                kind: kind.into(),
+            }),
             AnkiError::Interrupted => V::Interrupted(Empty {}),
         };
 
@@ -61,6 +66,32 @@ impl std::convert::From<AnkiError> for pt::backend_output::Value {
     }
 }
 
+impl std::convert::From<NetworkErrorKind> for i32 {
+    fn from(e: NetworkErrorKind) -> Self {
+        use pt::network_error::NetworkErrorKind as V;
+        (match e {
+            NetworkErrorKind::Offline => V::Offline,
+            NetworkErrorKind::Timeout => V::Timeout,
+            NetworkErrorKind::ProxyAuth => V::ProxyAuth,
+            NetworkErrorKind::Other => V::Other,
+        }) as i32
+    }
+}
+
+impl std::convert::From<SyncErrorKind> for i32 {
+    fn from(e: SyncErrorKind) -> Self {
+        use pt::sync_error::SyncErrorKind as V;
+        (match e {
+            SyncErrorKind::Conflict => V::Conflict,
+            SyncErrorKind::ServerError => V::ServerError,
+            SyncErrorKind::ClientTooOld => V::ClientTooOld,
+            SyncErrorKind::AuthFailed => V::AuthFailed,
+            SyncErrorKind::ServerMessage => V::ServerMessage,
+            SyncErrorKind::Other => V::Other,
+        }) as i32
+    }
+}
+
 pub fn init_backend(init_msg: &[u8]) -> std::result::Result<Backend, String> {
     let input: pt::BackendInit = match pt::BackendInit::decode(init_msg) {
         Ok(req) => req,

rslib/src/err.rs

@@ -2,6 +2,7 @@
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 pub use failure::{Error, Fail};
+use reqwest::StatusCode;
 use std::io;
 
 pub type Result<T> = std::result::Result<T, AnkiError>;
@@ -20,14 +21,14 @@ pub enum AnkiError {
     #[fail(display = "DB error: {}", info)]
     DBError { info: String },
 
-    #[fail(display = "Network error: {}", info)]
+    #[fail(display = "Network error: {:?} {}", kind, info)]
-    NetworkError { info: String },
+    NetworkError {
+        info: String,
+        kind: NetworkErrorKind,
+    },
 
-    #[fail(display = "AnkiWeb authentication failed.")]
+    #[fail(display = "Sync error: {:?}, {}", kind, info)]
-    AnkiWebAuthenticationFailed,
+    SyncError { info: String, kind: SyncErrorKind },
-
-    #[fail(display = "AnkiWeb error: {}", info)]
-    AnkiWebMiscError { info: String },
 
     #[fail(display = "The user interrupted the operation.")]
     Interrupted,
@@ -38,6 +39,20 @@ impl AnkiError {
     pub(crate) fn invalid_input<S: Into<String>>(s: S) -> AnkiError {
         AnkiError::InvalidInput { info: s.into() }
     }
+
+    pub(crate) fn server_message<S: Into<String>>(msg: S) -> AnkiError {
+        AnkiError::SyncError {
+            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,
+        }
+    }
 }
 
 #[derive(Debug, PartialEq)]
@@ -78,28 +93,93 @@ impl From<rusqlite::types::FromSqlError> 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 str_err = str_err.replace(url, "");
+        let info = str_err.replace(url, "");
-        AnkiError::NetworkError { info: str_err }
+
+        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,
+    Other,
+}
+
+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,
+            }
+        }
+        _ => AnkiError::NetworkError {
+            info,
+            kind: NetworkErrorKind::Other,
+        },
+    }
+}
+
+fn guess_reqwest_error(info: String) -> AnkiError {
+    let kind = if info.contains("unreachable") || info.contains("dns") {
+        NetworkErrorKind::Offline
+    } else {
+        NetworkErrorKind::Other
+    };
+    AnkiError::NetworkError { info, kind }
 }
 
 impl From<zip::result::ZipError> for AnkiError {
     fn from(err: zip::result::ZipError) -> Self {
-        AnkiError::AnkiWebMiscError {
+        AnkiError::sync_misc(err.to_string())
-            info: format!("{:?}", err),
-        }
     }
 }
 
 impl From<serde_json::Error> for AnkiError {
     fn from(err: serde_json::Error) -> Self {
-        AnkiError::AnkiWebMiscError {
+        AnkiError::sync_misc(err.to_string())
-            info: format!("{:?}", err),
-        }
     }
 }

rslib/src/media/sync.rs

@@ -10,7 +10,7 @@ use crate::media::{register_changes, MediaManager};
 use bytes::Bytes;
 use log::debug;
 use reqwest;
-use reqwest::{multipart, Client, Response, StatusCode};
+use reqwest::{multipart, Client, Response};
 use serde_derive::{Deserialize, Serialize};
 use serde_tuple::Serialize_tuple;
 use std::borrow::Cow;
@@ -79,15 +79,14 @@ where
             .query(&[("k", hkey), ("v", "ankidesktop,2.1.19,mac")])
             .send()
             .await?
-            .error_for_status()
+            .error_for_status()?;
-            .map_err(rewrite_forbidden)?;
 
         let reply: SyncBeginResult = resp.json().await?;
 
         if let Some(data) = reply.data {
             Ok((data.sync_key, data.usn))
         } else {
-            Err(AnkiError::AnkiWebMiscError { info: reply.err })
+            Err(AnkiError::server_message(reply.err))
         }
     }
 
@@ -184,15 +183,11 @@ where
             if data == "OK" {
                 Ok(())
             } else {
-                // fixme: force resync
+                // fixme: force resync, handle better
-                Err(AnkiError::AnkiWebMiscError {
+                Err(AnkiError::server_message("resync required"))
-                    info: "resync required ".into(),
-                })
             }
         } else {
-            Err(AnkiError::AnkiWebMiscError {
+            Err(AnkiError::server_message(resp.err))
-                info: format!("finalize failed: {}", resp.err),
-            })
         }
     }
 
@@ -214,7 +209,7 @@ where
         if let Some(batch) = res.data {
             Ok(batch)
         } else {
-            Err(AnkiError::AnkiWebMiscError { info: res.err })
+            Err(AnkiError::server_message(res.err))
         }
     }
 
@@ -298,14 +293,6 @@ struct SyncBeginResponse {
     usn: i32,
 }
 
-fn rewrite_forbidden(err: reqwest::Error) -> AnkiError {
-    if err.is_status() && err.status().unwrap() == StatusCode::FORBIDDEN {
-        AnkiError::AnkiWebAuthenticationFailed
-    } else {
-        err.into()
-    }
-}
-
 #[derive(Debug, Clone, Copy)]
 enum LocalState {
     NotInDB,
@@ -462,7 +449,7 @@ async fn ankiweb_request(
         .send()
         .await?
         .error_for_status()
-        .map_err(rewrite_forbidden)
+        .map_err(Into::into)
 }
 
 #[derive(Debug, Serialize)]
@@ -486,9 +473,9 @@ fn extract_into_media_folder(media_folder: &Path, zip: Bytes) -> Result<Vec<Adde
             continue;
         }
 
-        let real_name = fmap.get(name).ok_or(AnkiError::AnkiWebMiscError {
+        let real_name = fmap
-            info: "malformed zip received".into(),
+            .get(name)
-        })?;
+            .ok_or_else(|| AnkiError::sync_misc("malformed zip"))?;
 
         let mut data = Vec::with_capacity(file.size() as usize);
         file.read_to_end(&mut data)?;
@@ -571,9 +558,7 @@ fn zip_files(media_folder: &Path, files: &[MediaEntry]) -> Result<Vec<u8>> {
         let normalized = normalize_filename(&file.fname);
         if let Cow::Owned(_) = normalized {
             // fixme: non-string err, or should ignore instead
-            return Err(AnkiError::AnkiWebMiscError {
+            return Err(AnkiError::sync_misc("invalid file found"));
-                info: "Invalid filename found. Please use the Check Media function.".to_owned(),
-            });
         }
 
         let file_data = data_for_file(media_folder, &file.fname)?;
@@ -581,9 +566,7 @@ fn zip_files(media_folder: &Path, files: &[MediaEntry]) -> Result<Vec<u8>> {
         if let Some(data) = &file_data {
             if data.is_empty() {
                 // fixme: should ignore these, not error
-                return Err(AnkiError::AnkiWebMiscError {
+                return Err(AnkiError::sync_misc("0 byte file found"));
-                    info: "0 byte file found".to_owned(),
-                });
             }
             accumulated_size += data.len();
             zip.start_file(format!("{}", idx), options)?;