improve frappy.errors

- include all secop errors from spec
- add doc strings
- make conversion to and from error report nicer
- move all error classes to frappy.errors
- rename errors clashing with built-in errors

Change-Id: I4d882173b020cd4baf862c5891375b691e67e24a
Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/30721
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

frappy/client/__init__.py

@@ -593,8 +593,7 @@ class SecopClient(ProxyClient):
             raise ConnectionError('connection closed before reply')
         action, _, data = entry[2]  # pylint: disable=unpacking-non-sequence
         if action.startswith(ERRORPREFIX):
-            errcls = self.error_map(data[0])
+            raise frappy.errors.make_secop_error(*data[0:2])
-            raise errcls(data[1])
         return entry[2]  # reply
 
     def request(self, action, ident=None, data=None):
@@ -647,15 +646,9 @@ class SecopClient(ProxyClient):
 
     # the following attributes may be/are intended to be overwritten by a subclass
 
-    ERROR_MAP = frappy.errors.EXCEPTIONS
-    DEFAULT_EXCEPTION = frappy.errors.SECoPError
     PREDEFINED_NAMES = set(frappy.params.PREDEFINED_ACCESSIBLES)
     activate = True
 
-    def error_map(self, exc):
-        """how to convert SECoP and unknown exceptions"""
-        return self.ERROR_MAP.get(exc, self.DEFAULT_EXCEPTION)
-
     def internalize_name(self, name):
         """how to create internal names"""
         if name.startswith('_') and name[1:] not in self.PREDEFINED_NAMES:

frappy/datatypes.py

@@ -29,7 +29,7 @@ import sys
 from base64 import b64decode, b64encode
 
 from frappy.errors import WrongTypeError, RangeError, \
-    ConfigError, ProgrammingError, ProtocolError
+    ConfigError, ProgrammingError, ProtocolError, DiscouragedConversion
 from frappy.lib import clamp, generalConfig
 from frappy.lib.enum import Enum
 from frappy.parse import Parser
@@ -45,11 +45,6 @@ UNLIMITED = 1 << 64  # internal limit for integers, is probably high enough for
 Parser = Parser()
 
 
-class DiscouragedConversion(WrongTypeError):
-    """the discouraged conversion string - > float happened"""
-    log_message = True
-
-
 def shortrepr(value):
     """shortened repr for error messages
 

frappy/errors.py

@@ -20,14 +20,25 @@
 #   Markus Zolliker <markus.zolliker@psi.ch>
 #
 # *****************************************************************************
-"""Define (internal) SECoP Errors"""
+"""Define (internal) SECoP Errors
+
+all error classes inherited from SECoPError should be placed in this module,
+else they might not be registered and can therefore not be rebuilt on the client side
+"""
 
 import re
-from ast import literal_eval
 
 
 class SECoPError(RuntimeError):
     silent = False   # silent = True indicates that the error is already logged
+    clsname2class = {}  # needed to convert error reports back to classes
+    name = 'InternalError'
+    name2class = {}
+
+    def __init_subclass__(cls):
+        cls.clsname2class[cls.__name__] = cls
+        if 'name' in cls.__dict__:
+            cls.name2class[cls.name] = cls
 
     def __init__(self, *args, **kwds):
         super().__init__()
@@ -44,52 +55,67 @@ class SECoPError(RuntimeError):
             res.append(args)
         if kwds:
             res.append(kwds)
-        return '%s(%s)' % (self.name, ', '.join(res))
+        return '%s(%s)' % (self.name or type(self).__name__, ', '.join(res))
-
-    @property
-    def name(self):
-        return self.__class__.__name__[:-len('Error')]
-
-
-class SECoPServerError(SECoPError):
-    name = 'InternalError'
 
 
 class InternalError(SECoPError):
+    """uncatched error"""
     name = 'InternalError'
 
 
 class ProgrammingError(SECoPError):
-    name = 'InternalError'
+    """catchable programming error"""
 
 
 class ConfigError(SECoPError):
-    name = 'InternalError'
+    """invalid configuration"""
 
 
 class ProtocolError(SECoPError):
+    """A malformed request or on unspecified message was sent
+
+    This includes non-understood actions and malformed specifiers.
+    Also if the message exceeds an implementation defined maximum size.
+    """
     name = 'ProtocolError'
 
 
 class NoSuchModuleError(SECoPError):
+    """missing module
+
+    The action can not be performed as the specified module is non-existent"""
     name = 'NoSuchModule'
 
 
-# pylint: disable=redefined-builtin
+class NotImplementedSECoPError(NotImplementedError, SECoPError):
-class NotImplementedError(NotImplementedError, SECoPError):
+    """not (yet) implemented
-    pass
+
+    A (not yet) implemented action or combination of action and specifier
+    was requested. This should not be used in productive setups, but is very
+    helpful during development."""
+    name = 'NotImplemented'
 
 
 class NoSuchParameterError(SECoPError):
-    pass
+    """missing parameter
+
+    The action can not be performed as the specified parameter is non-existent.
+    Also raised when trying to use a command name in a 'read' or 'change' message.
+    """
+    name = 'NoSuchParameter'
 
 
 class NoSuchCommandError(SECoPError):
-    pass
+    """The specified command does not exist
+
+    Also raised when trying to use a parameter name in a 'do' message.
+    """
+    name = 'NoSuchCommand'
 
 
 class ReadOnlyError(SECoPError):
-    pass
+    """The requested write can not be performed on a readonly value"""
+    name = 'ReadOnly'
 
 
 class BadValueError(SECoPError):
@@ -97,42 +123,106 @@ class BadValueError(SECoPError):
 
 
 class RangeError(ValueError, BadValueError):
+    """data out of range
+
+    The requested parameter change or Command can not be performed as the
+    argument value is not in the allowed range specified by the datainfo
+    property. This also happens if an unspecified Enum variant is tried
+    to be used, the size of a Blob or String does not match the limits
+    given in the descriptive data, or if the number of elements in an
+    array does not match the limits given in the descriptive data."""
     name = 'RangeError'
 
 
+class BadJSONError(SECoPError):
+    """The data part of the message can not be parsed, i.e. the JSON-data is no valid JSON.
+
+    not used in Frappy, but might appear on the client side from a foreign SEC Node
+    """
+    # TODO: check whether this should not be removed from specs!
+    name = 'BadJSON'
+
+
 class WrongTypeError(TypeError, BadValueError):
-    pass
+    """Wrong data type
+
+    The requested parameter change or Command can not be performed as the
+    argument has the wrong type. (i.e. a string where a number is expected.)
+    It may also be used if an incomplete struct is sent, but a complete
+    struct is expected."""
+    name = 'WrongType'
+
+
+class DiscouragedConversion(ProgrammingError):
+    """the discouraged conversion string - > float happened"""
+    log_message = True
 
 
 class CommandFailedError(SECoPError):
-    pass
+    name = 'CommandFailed'
 
 
 class CommandRunningError(SECoPError):
-    pass
+    """The command is already executing.
+
+    request may be retried after the module is no longer BUSY
+    (retryable)"""
+    name = 'CommandRunning'
 
 
 class CommunicationFailedError(SECoPError):
-    pass
+    """Some communication (with hardware controlled by this SEC node) failed
+    (retryable)"""
+    name = 'CommunicationFailed'
+
+
+class SilentCommunicationFailedError(CommunicationFailedError):
+    silent = True
 
 
 class IsBusyError(SECoPError):
-    pass
+    """The requested action can not be performed while the module is Busy
+    or the command still running"""
+    name = 'IsBusy'
 
 
 class IsErrorError(SECoPError):
-    pass
+    """The requested action can not be performed while the module is in error state"""
+    name = 'IsError'
 
 
 class DisabledError(SECoPError):
-    pass
+    """The requested action can not be performed while the module is disabled"""
+    name = 'disabled'
+
+
+class ImpossibleError(SECoPError):
+    """The requested action can not be performed at the moment"""
+    name = 'Impossible'
+
+
+class ReadFailedError(SECoPError):
+    """The requested parameter can not be read just now"""
+    name = 'ReadFailed'
+
+
+class OutOfRangeError(SECoPError):
+    """The requested parameter can not be read just now"""
+    name = 'OutOfRange'
 
 
 class HardwareError(SECoPError):
+    """The connected hardware operates incorrect or may not operate at all
+    due to errors inside or in connected components."""
     name = 'HardwareError'
 
 
-FRAPPY_ERROR = re.compile(r'(.*)\(.*\)$')
+class TimeoutSECoPError(TimeoutError, SECoPError):
+    """Some initiated action took longer than the maximum allowed time (retryable)"""
+    name = 'TimeoutError'
+
+
+FRAPPY_ERROR = re.compile(r'(\w*): (.*)$')
 
 
 def make_secop_error(name, text):
@@ -142,47 +232,26 @@ def make_secop_error(name, text):
     :param text: the second item of a SECoP error report
     :return: the built instance of SECoPError
     """
-    try:
+    match = FRAPPY_ERROR.match(text)
-        # try to interprete the error text as a repr(<instance of SECoPError>)
+    if match:
-        # as it would be created by a Frappy server
+        clsname, errtext = match.groups()
-        cls, textarg = FRAPPY_ERROR.match(text).groups()
+        errcls = SECoPError.clsname2class.get(clsname)
-        errcls = locals()[cls]
+        if errcls:
-        if errcls.name == name:
+            return errcls(errtext)
-            # convert repr(<string>) to <string>
+    return SECoPError.name2class.get(name, InternalError)(text)
-            text = literal_eval(textarg)
-    except Exception:
-        # probably not a Frappy server, or running a different version
-        errcls = EXCEPTIONS.get(name, InternalError)
-    return errcls(text)
 
 
-def secop_error(exception):
+def secop_error(exc):
-    if isinstance(exception, SECoPError):
+    """turn into InternalError, if not already a SECoPError"""
-        return exception
+    if isinstance(exc, SECoPError):
-    return InternalError(repr(exception))
+        if SECoPError.name2class.get(exc.name) != type(exc):
+            return type(exc)('%s: %s' % (type(exc).__name__, exc))
+        return exc
+    return InternalError('%s: %s' % (type(exc).__name__, exc))
 
 
-EXCEPTIONS = {e().name: e for e in [
-    NoSuchModuleError,
-    NoSuchParameterError,
-    NoSuchCommandError,
-    CommandFailedError,
-    CommandRunningError,
-    ReadOnlyError,
-    BadValueError,
-    RangeError,
-    WrongTypeError,
-    CommunicationFailedError,
-    HardwareError,
-    IsBusyError,
-    IsErrorError,
-    DisabledError,
-    ProtocolError,
-    NotImplementedError,
-    InternalError]}
-
 # TODO: check if these are really needed:
-EXCEPTIONS.update(
+SECoPError.name2class.update(
     SyntaxError=ProtocolError,
     # internal short versions (candidates for spec)
     Protocol=ProtocolError,

frappy/io.py

@@ -31,7 +31,8 @@ import threading
 from frappy.lib.asynconn import AsynConn, ConnectionClosed
 from frappy.datatypes import ArrayOf, BLOBType, BoolType, FloatRange, IntRange, \
     StringType, TupleOf, ValueType
-from frappy.errors import CommunicationFailedError, ConfigError, ProgrammingError
+from frappy.errors import CommunicationFailedError, ConfigError, ProgrammingError, \
+    SilentCommunicationFailedError as SilentError
 from frappy.modules import Attached, Command, \
     Communicator, Module, Parameter, Property
 from frappy.lib import generalConfig
@@ -41,10 +42,6 @@ generalConfig.set_default('legacy_hasiodev', False)
 HEX_CODE = re.compile(r'[0-9a-fA-F][0-9a-fA-F]$')
 
 
-class SilentError(CommunicationFailedError):
-    silent = True
-
-
 class HasIO(Module):
     """Mixin for modules using a communicator"""
     io = Attached(mandatory=False)  # either io or uri must be given

frappy/protocol/dispatcher.py

@@ -43,7 +43,7 @@ from collections import OrderedDict
 from time import time as currenttime
 
 from frappy.errors import NoSuchCommandError, NoSuchModuleError, \
-    NoSuchParameterError, ProtocolError, ReadOnlyError, SECoPServerError
+    NoSuchParameterError, ProtocolError, ReadOnlyError
 from frappy.params import Parameter
 from frappy.protocol.messages import COMMANDREPLY, DESCRIPTIONREPLY, \
     DISABLEEVENTSREPLY, ENABLEEVENTSREPLY, ERRORPREFIX, EVENTREPLY, \
@@ -55,7 +55,7 @@ def make_update(modulename, pobj):
     if pobj.readerror:
         return (ERRORPREFIX + EVENTREPLY, '%s:%s' % (modulename, pobj.export),
                 # error-report !
-                [pobj.readerror.name, repr(pobj.readerror), {'t': pobj.timestamp}])
+                [pobj.readerror.name, str(pobj.readerror), {'t': pobj.timestamp}])
     return (EVENTREPLY, '%s:%s' % (modulename, pobj.export),
             [pobj.export_value(), {'t': pobj.timestamp}])
 
@@ -297,7 +297,7 @@ class Dispatcher:
 
             if handler:
                 return handler(conn, specifier, data)
-            raise SECoPServerError('unhandled message: %s' % repr(msg))
+            raise ProtocolError('unhandled message: %s' % repr(msg))
 
     # now the (defined) handlers for the different requests
     def handle_help(self, conn, specifier, data):

test/test_errors.py

@@ -21,16 +21,28 @@
 # *****************************************************************************
 """test data types."""
 
-import frappy.errors
+import pytest
-from frappy.errors import EXCEPTIONS, SECoPError
+from frappy.errors import RangeError, WrongTypeError, ProgrammingError, \
+    ConfigError, InternalError, DiscouragedConversion, secop_error, make_secop_error
 
 
-def test_errors():
+@pytest.mark.parametrize('exc, name, text, echk', [
-    """check consistence of frappy.errors.EXCEPTIONS"""
+    (RangeError('out of range'), 'RangeError', 'out of range', None),
-    for e in EXCEPTIONS.values():
+    (WrongTypeError('bad type'), 'WrongType', 'bad type', None),
-        assert EXCEPTIONS[e().name] == e
+    (ProgrammingError('x'), 'InternalError', 'ProgrammingError: x', None),
-    # check that all defined secop errors are in EXCEPTIONS
+    (ConfigError('y'), 'InternalError', 'ConfigError: y', None),
-    for cls in frappy.errors.__dict__.values():
+    (InternalError('z'), 'InternalError', 'z', None),
-        if isinstance(cls, type) and issubclass(cls, SECoPError):
+    (DiscouragedConversion('w'), 'InternalError', 'DiscouragedConversion: w', None),
-            if cls != SECoPError:
+    (ValueError('v'), 'InternalError', "ValueError: v", InternalError("ValueError: v")),
-                assert cls().name in EXCEPTIONS
+    (None, 'InternalError', "UnknownError: v", InternalError("UnknownError: v")),
+])
+def test_errors(exc, name, text, echk):
+    """check consistence of frappy.errors"""
+    if exc:
+        err = secop_error(exc)
+        assert err.name == name
+        assert str(err) == text
+    recheck = make_secop_error(name, text)
+    echk = echk or exc
+    assert type(recheck) == type(echk)
+    assert str(recheck) == str(echk)