From 41c5a25dc8efc311b72812d84c884e9770ad5e4c Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 3 Apr 2021 16:00:15 +1000 Subject: [PATCH] simplify errors - use a flat enum instead of oneof messages, most of which were empty - tidy up the Python side --- pylib/.pylintrc | 3 +- pylib/anki/_backend/__init__.py | 75 ++++++++++++++++++++++++- pylib/anki/collection.py | 4 +- pylib/anki/errors.py | 98 ++++++++++----------------------- qt/aqt/errors.py | 2 +- qt/aqt/sync.py | 6 +- rslib/backend.proto | 65 +++++++--------------- rslib/src/backend/error.rs | 97 +++++++++++++------------------- rslib/src/backend/mod.rs | 6 +- 9 files changed, 169 insertions(+), 187 deletions(-) diff --git a/pylib/.pylintrc b/pylib/.pylintrc index bcd91244d..50ba2337b 100644 --- a/pylib/.pylintrc +++ b/pylib/.pylintrc @@ -9,7 +9,8 @@ ignored-classes= AnswerCardIn, UnburyCardsInCurrentDeckIn, BuryOrSuspendCardsIn, - NoteIsDuplicateOrEmptyOut + NoteIsDuplicateOrEmptyOut, + BackendError [REPORTS] output-format=colorized diff --git a/pylib/anki/_backend/__init__.py b/pylib/anki/_backend/__init__.py index f4665b8a4..b5f776de3 100644 --- a/pylib/anki/_backend/__init__.py +++ b/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) diff --git a/pylib/anki/collection.py b/pylib/anki/collection.py index 9a772340e..a400c9988 100644 --- a/pylib/anki/collection.py +++ b/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() diff --git a/pylib/anki/errors.py b/pylib/anki/errors.py index 5178d7643..1514916d4 100644 --- a/pylib/anki/errors.py +++ b/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 diff --git a/qt/aqt/errors.py b/qt/aqt/errors.py index 13fff133d..4cedf31d0 100644 --- a/qt/aqt/errors.py +++ b/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 diff --git a/qt/aqt/sync.py b/qt/aqt/sync.py index 2f3350633..c1e3b4573 100644 --- a/qt/aqt/sync.py +++ b/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: diff --git a/rslib/backend.proto b/rslib/backend.proto index 693549261..5227d4d41 100644 --- a/rslib/backend.proto +++ b/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 diff --git a/rslib/src/backend/error.rs b/rslib/src/backend/error.rs index b80498ae7..2e5c2a24c 100644 --- a/rslib/src/backend/error.rs +++ b/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 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 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 for Kind { + fn from(err: SyncErrorKind) -> Self { + match err { + SyncErrorKind::AuthFailed => Kind::SyncAuthError, + _ => Kind::SyncOtherError, + } } } diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index f04661175..38497c36b 100644 --- a/rslib/src/backend/mod.rs +++ b/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>>, 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> { 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