simplify errors

- use a flat enum instead of oneof messages, most of which were empty
- tidy up the Python side

pylib/.pylintrc

@@ -9,7 +9,8 @@ ignored-classes=
   AnswerCardIn,
   UnburyCardsInCurrentDeckIn,
   BuryOrSuspendCardsIn,
-  NoteIsDuplicateOrEmptyOut
+  NoteIsDuplicateOrEmptyOut,
+  BackendError
 
 [REPORTS]
 output-format=colorized

pylib/anki/_backend/__init__.py

@@ -3,24 +3,39 @@
 
 from __future__ import annotations
 
-import os
 import sys
 import traceback
 from typing import Any, Dict, List, Optional, Sequence, Tuple, Union
 from weakref import ref
 
+from markdown import markdown
+
 import anki.buildinfo
 from anki._backend.generated import RustBackendGenerated
 from anki.dbproxy import Row as DBRow
 from anki.dbproxy import ValueForDB
-from anki.errors import backend_exception_to_pylib
 from anki.utils import from_json_bytes, to_json_bytes
 
+from ..errors import (
+    BackendIOError,
+    DBError,
+    ExistsError,
+    FilteredDeckError,
+    Interrupted,
+    InvalidInput,
+    LocalizedError,
+    NetworkError,
+    NotFoundError,
+    SearchError,
+    SyncError,
+    SyncErrorKind,
+    TemplateError,
+    UndoEmpty,
+)
 from . import backend_pb2 as pb
 from . import rsbridge
 from .fluent import GeneratedTranslations, LegacyTranslationEnum
 
-# pylint: disable=c-extension-no-member
 assert rsbridge.buildhash() == anki.buildinfo.buildhash
 
 
@@ -147,3 +162,57 @@ class Translations(GeneratedTranslations):
         return self.backend().translate(
             module_index=module, message_index=message, **args
         )
+
+
+def backend_exception_to_pylib(err: pb.BackendError) -> Exception:
+    kind = pb.BackendError
+    val = err.kind
+    if val == kind.INTERRUPTED:
+        return Interrupted()
+
+    elif val == kind.NETWORK_ERROR:
+        return NetworkError(err.localized)
+
+    elif val == kind.SYNC_AUTH_ERROR:
+        return SyncError(err.localized, SyncErrorKind.AUTH)
+
+    elif val == kind.SYNC_OTHER_ERROR:
+        return SyncError(err.localized, SyncErrorKind.OTHER)
+
+    elif val == kind.IO_ERROR:
+        return BackendIOError(err.localized)
+
+    elif val == kind.DB_ERROR:
+        return DBError(err.localized)
+
+    elif val == kind.TEMPLATE_PARSE:
+        return TemplateError(err.localized)
+
+    elif val == kind.INVALID_INPUT:
+        return InvalidInput(err.localized)
+
+    elif val == kind.JSON_ERROR:
+        return LocalizedError(err.localized)
+
+    elif val == kind.NOT_FOUND_ERROR:
+        return NotFoundError()
+
+    elif val == kind.EXISTS:
+        return ExistsError()
+
+    elif val == kind.FILTERED_DECK_ERROR:
+        return FilteredDeckError(err.localized)
+
+    elif val == kind.PROTO_ERROR:
+        return LocalizedError(err.localized)
+
+    elif val == kind.SEARCH_ERROR:
+        return SearchError(markdown(err.localized))
+
+    elif val == kind.UNDO_EMPTY:
+        return UndoEmpty()
+
+    else:
+        # sadly we can't do exhaustiveness checking on protobuf enums
+        # assert_exhaustive(val)
+        return LocalizedError(err.localized)

pylib/anki/collection.py

@@ -40,7 +40,7 @@ from anki.config import Config, ConfigManager
 from anki.consts import *
 from anki.dbproxy import DBProxy
 from anki.decks import DeckId, DeckManager
-from anki.errors import AnkiError, DBError
+from anki.errors import AbortSchemaModification, DBError
 from anki.lang import FormatTimeSpan
 from anki.media import MediaManager, media_paths_from_col_path
 from anki.models import ModelManager, NotetypeDict, NotetypeId
@@ -290,7 +290,7 @@ class Collection:
         "Mark schema modified. Call this first so user can abort if necessary."
         if not self.schemaChanged():
             if check and not hooks.schema_will_change(proceed=True):
-                raise AnkiError("abortSchemaMod")
+                raise AbortSchemaModification()
         self.db.execute("update col set scm=?", intTime(1000))
         self.save()
 

pylib/anki/errors.py

@@ -3,41 +3,48 @@
 
 from __future__ import annotations
 
-from markdown import markdown
-
-import anki._backend.backend_pb2 as _pb
-
-from anki.types import assert_exhaustive
+from enum import Enum
 
 
-class StringError(Exception):
+class LocalizedError(Exception):
+    "An error with a localized description."
+
+    def __init__(self, localized: str) -> None:
+        self._localized = localized
+        super().__init__()
+
     def __str__(self) -> str:
-        return self.args[0]  # pylint: disable=unsubscriptable-object
+        return self._localized
 
 
 class Interrupted(Exception):
     pass
 
 
-class NetworkError(StringError):
+class NetworkError(LocalizedError):
     pass
 
 
-class SyncError(StringError):
-    # pylint: disable=no-member
-    def is_auth_error(self) -> bool:
-        return self.args[1] == _pb.SyncError.SyncErrorKind.AUTH_FAILED
+class SyncErrorKind(Enum):
+    AUTH = 1
+    OTHER = 2
 
 
-class IOError(StringError):
+class SyncError(LocalizedError):
+    def __init__(self, localized: str, kind: SyncErrorKind):
+        self.kind = kind
+        super().__init__(localized)
+
+
+class BackendIOError(LocalizedError):
     pass
 
 
-class DBError(StringError):
+class DBError(LocalizedError):
     pass
 
 
-class TemplateError(StringError):
+class TemplateError(LocalizedError):
     pass
 
 
@@ -53,67 +60,22 @@ class UndoEmpty(Exception):
     pass
 
 
-class DeckRenameError(Exception):
-    """Legacy error, use FilteredDeckError instead."""
-
-    def __init__(self, description: str, *args: object) -> None:
-        super().__init__(description, *args)
-        self.description = description
-
-
-class FilteredDeckError(StringError, DeckRenameError):
+class FilteredDeckError(LocalizedError):
     pass
 
 
-class InvalidInput(StringError):
+class InvalidInput(LocalizedError):
     pass
 
 
-class SearchError(StringError):
+class SearchError(LocalizedError):
     pass
 
 
-def backend_exception_to_pylib(err: _pb.BackendError) -> Exception:
-    val = err.WhichOneof("value")
-    if val == "interrupted":
-        return Interrupted()
-    elif val == "network_error":
-        return NetworkError(err.localized, err.network_error.kind)
-    elif val == "sync_error":
-        return SyncError(err.localized, err.sync_error.kind)
-    elif val == "io_error":
-        return IOError(err.localized)
-    elif val == "db_error":
-        return DBError(err.localized)
-    elif val == "template_parse":
-        return TemplateError(err.localized)
-    elif val == "invalid_input":
-        return InvalidInput(err.localized)
-    elif val == "json_error":
-        return StringError(err.localized)
-    elif val == "not_found_error":
-        return NotFoundError()
-    elif val == "exists":
-        return ExistsError()
-    elif val == "filtered_deck_error":
-        return FilteredDeckError(err.localized)
-    elif val == "proto_error":
-        return StringError(err.localized)
-    elif val == "search_error":
-        return SearchError(markdown(err.localized))
-    elif val == "undo_empty":
-        return UndoEmpty()
-    else:
-        assert_exhaustive(val)
-        return StringError(err.localized)
+class AbortSchemaModification(Exception):
+    pass
 
 
-# FIXME: this is only used with "abortSchemaMod", but currently some
-# add-ons depend on it
-class AnkiError(Exception):
-    def __init__(self, type: str) -> None:
-        super().__init__()
-        self.type = type
-
-    def __str__(self) -> str:
-        return self.type
+# legacy
+DeckRenameError = FilteredDeckError
+AnkiError = AbortSchemaModification

qt/aqt/errors.py

@@ -71,7 +71,7 @@ class ErrorHandler(QObject):
         error = html.escape(self.pool)
         self.pool = ""
         self.mw.progress.clear()
-        if "abortSchemaMod" in error:
+        if "AbortSchemaModification" in error:
             return
         if "DeprecationWarning" in error:
             return

qt/aqt/sync.py

@@ -9,7 +9,7 @@ from concurrent.futures import Future
 from typing import Callable, Tuple
 
 import aqt
-from anki.errors import Interrupted, SyncError
+from anki.errors import Interrupted, SyncError, SyncErrorKind
 from anki.lang import without_unicode_isolation
 from anki.sync import SyncOutput, SyncStatus
 from anki.utils import platDesc
@@ -62,7 +62,7 @@ def get_sync_status(
 
 def handle_sync_error(mw: aqt.main.AnkiQt, err: Exception) -> None:
     if isinstance(err, SyncError):
-        if err.is_auth_error():
+        if err.kind is SyncErrorKind.AUTH:
             mw.pm.clear_sync_auth()
     elif isinstance(err, Interrupted):
         # no message to show
@@ -247,7 +247,7 @@ def sync_login(
         try:
             auth = fut.result()
         except SyncError as e:
-            if e.is_auth_error():
+            if e.kind is SyncErrorKind.AUTH:
                 showWarning(str(e))
                 sync_login(mw, on_success, username, password)
             else:

rslib/backend.proto

@@ -553,53 +553,28 @@ message I18nBackendInit {
 ///////////////////////////////////////////////////////////
 
 message BackendError {
+  enum Kind {
+    INVALID_INPUT = 0;
+    UNDO_EMPTY = 1;
+    INTERRUPTED = 2;
+    TEMPLATE_PARSE = 3;
+    IO_ERROR = 4;
+    DB_ERROR = 5;
+    NETWORK_ERROR = 6;
+    SYNC_AUTH_ERROR = 7;
+    SYNC_OTHER_ERROR = 8;
+    JSON_ERROR = 9;
+    PROTO_ERROR = 10;
+    NOT_FOUND_ERROR = 11;
+    EXISTS = 12;
+    FILTERED_DECK_ERROR = 13;
+    SEARCH_ERROR = 14;
+  }
+
   // localized error description suitable for displaying to the user
   string localized = 1;
-  // error specifics
-  oneof value {
-    Empty invalid_input = 2;
-    Empty template_parse = 3;
-    Empty io_error = 4;
-    Empty db_error = 5;
-    NetworkError network_error = 6;
-    SyncError sync_error = 7;
-    // user interrupted operation
-    Empty interrupted = 8;
-    string json_error = 9;
-    string proto_error = 10;
-    Empty not_found_error = 11;
-    Empty exists = 12;
-    Empty filtered_deck_error = 13;
-    Empty search_error = 14;
-    Empty undo_empty = 15;
-  }
-}
-
-message NetworkError {
-  enum NetworkErrorKind {
-    OTHER = 0;
-    OFFLINE = 1;
-    TIMEOUT = 2;
-    PROXY_AUTH = 3;
-  }
-  NetworkErrorKind kind = 1;
-}
-
-message SyncError {
-  enum SyncErrorKind {
-    OTHER = 0;
-    CONFLICT = 1;
-    SERVER_ERROR = 2;
-    CLIENT_TOO_OLD = 3;
-    AUTH_FAILED = 4;
-    SERVER_MESSAGE = 5;
-    MEDIA_CHECK_REQUIRED = 6;
-    RESYNC_REQUIRED = 7;
-    CLOCK_INCORRECT = 8;
-    DATABASE_CHECK_REQUIRED = 9;
-    SYNC_NOT_STARTED = 10;
-  }
-  SyncErrorKind kind = 1;
+  // the error subtype
+  Kind kind = 2;
 }
 
 // Progress

rslib/src/backend/error.rs

@@ -3,72 +3,49 @@
 
 use crate::{
     backend_proto as pb,
-    error::{AnkiError, NetworkErrorKind, SyncErrorKind},
+    error::{AnkiError, SyncErrorKind},
     prelude::*,
 };
 
-/// Convert an Anki error to a protobuf error.
-pub(super) fn anki_error_to_proto_error(err: AnkiError, tr: &I18n) -> pb::BackendError {
-    use pb::backend_error::Value as V;
-    let localized = err.localized_description(tr);
-    let value = match err {
-        AnkiError::InvalidInput { .. } => V::InvalidInput(pb::Empty {}),
-        AnkiError::TemplateError { .. } => V::TemplateParse(pb::Empty {}),
-        AnkiError::IoError { .. } => V::IoError(pb::Empty {}),
-        AnkiError::DbError { .. } => V::DbError(pb::Empty {}),
-        AnkiError::NetworkError(err) => V::NetworkError(pb::NetworkError {
-            kind: err.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 {}),
-        AnkiError::JsonError(info) => V::JsonError(info),
-        AnkiError::ProtoError(info) => V::ProtoError(info),
-        AnkiError::NotFound => V::NotFoundError(pb::Empty {}),
-        AnkiError::Existing => V::Exists(pb::Empty {}),
-        AnkiError::FilteredDeckError(_) => V::FilteredDeckError(pb::Empty {}),
-        AnkiError::SearchError(_) => V::SearchError(pb::Empty {}),
-        AnkiError::TemplateSaveError { .. } => V::TemplateParse(pb::Empty {}),
-        AnkiError::ParseNumError => V::InvalidInput(pb::Empty {}),
-        AnkiError::InvalidRegex(_) => V::InvalidInput(pb::Empty {}),
-        AnkiError::UndoEmpty => V::UndoEmpty(pb::Empty {}),
-    };
+use pb::backend_error::Kind;
 
-    pb::BackendError {
-        value: Some(value),
-        localized,
+impl AnkiError {
+    pub(super) fn into_protobuf(self, tr: &I18n) -> pb::BackendError {
+        let localized = self.localized_description(tr);
+        let kind = match self {
+            AnkiError::InvalidInput(_) => Kind::InvalidInput,
+            AnkiError::TemplateError(_) => Kind::TemplateParse,
+            AnkiError::IoError(_) => Kind::IoError,
+            AnkiError::DbError(_) => Kind::DbError,
+            AnkiError::NetworkError(_) => Kind::NetworkError,
+            AnkiError::SyncError(err) => err.kind.into(),
+            AnkiError::Interrupted => Kind::Interrupted,
+            AnkiError::CollectionNotOpen => Kind::InvalidInput,
+            AnkiError::CollectionAlreadyOpen => Kind::InvalidInput,
+            AnkiError::JsonError(_) => Kind::JsonError,
+            AnkiError::ProtoError(_) => Kind::ProtoError,
+            AnkiError::NotFound => Kind::NotFoundError,
+            AnkiError::Existing => Kind::Exists,
+            AnkiError::FilteredDeckError(_) => Kind::FilteredDeckError,
+            AnkiError::SearchError(_) => Kind::SearchError,
+            AnkiError::TemplateSaveError(_) => Kind::TemplateParse,
+            AnkiError::ParseNumError => Kind::InvalidInput,
+            AnkiError::InvalidRegex(_) => Kind::InvalidInput,
+            AnkiError::UndoEmpty => Kind::UndoEmpty,
+        };
+
+        pb::BackendError {
+            kind: kind as i32,
+            localized,
+        }
     }
 }
 
-impl std::convert::From<NetworkErrorKind> for i32 {
-    fn from(e: NetworkErrorKind) -> Self {
-        use pb::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 pb::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::ResyncRequired => V::ResyncRequired,
-            SyncErrorKind::DatabaseCheckRequired => V::DatabaseCheckRequired,
-            SyncErrorKind::Other => V::Other,
-            SyncErrorKind::ClockIncorrect => V::ClockIncorrect,
-            SyncErrorKind::SyncNotStarted => V::SyncNotStarted,
-        }) as i32
+impl From<SyncErrorKind> for Kind {
+    fn from(err: SyncErrorKind) -> Self {
+        match err {
+            SyncErrorKind::AuthFailed => Kind::SyncAuthError,
+            _ => Kind::SyncOtherError,
+        }
     }
 }

rslib/src/backend/mod.rs

@@ -61,8 +61,6 @@ use std::{
 };
 use tokio::runtime::{self, Runtime};
 
-use self::error::anki_error_to_proto_error;
-
 pub struct Backend {
     col: Arc<Mutex<Option<Collection>>>,
     tr: I18n,
@@ -137,7 +135,7 @@ impl Backend {
                 pb::ServiceIndex::Cards => CardsService::run_method(self, method, input),
             })
             .map_err(|err| {
-                let backend_err = anki_error_to_proto_error(err, &self.tr);
+                let backend_err = err.into_protobuf(&self.tr);
                 let mut bytes = Vec::new();
                 backend_err.encode(&mut bytes).unwrap();
                 bytes
@@ -146,7 +144,7 @@ impl Backend {
 
     pub fn run_db_command_bytes(&self, input: &[u8]) -> std::result::Result<Vec<u8>, Vec<u8>> {
         self.db_command(input).map_err(|err| {
-            let backend_err = anki_error_to_proto_error(err, &self.tr);
+            let backend_err = err.into_protobuf(&self.tr);
             let mut bytes = Vec::new();
             backend_err.encode(&mut bytes).unwrap();
             bytes