Fix 2247: AttributeError in deluge.error.DaemonRunningError

* Removed all the properties in error.py and added more tests
* Handle failure in client.py handling RPC_ERROR (From older daemons)

4 files changed, 101 insertions, 74 deletions

diff --git a/deluge/core/rpcserver.py b/deluge/core/rpcserver.py
index 649c2ce29..41cd7cf76 100644
--- a/deluge/core/rpcserver.py
+++ b/deluge/core/rpcserver.py
@@ -144,7 +144,7 @@ class DelugeRPCProtocol(DelugeTransferProtocol):
         only message that a client sends to the server is a RPC Request message.
         If the RPC Request message is valid, then the method is called in
         :meth:`dispatch`.
-        
+
         :param request: the request from the client.
         :type data: tuple
 
@@ -238,16 +238,17 @@ class DelugeRPCProtocol(DelugeTransferProtocol):
                     exceptionValue._kwargs,
                     formated_tb
                 ))
-            except Exception, err:
-                # This most likely not a deluge exception, let's wrap it
+            except AttributeError, err:
+                # This is not a deluge exception (object has no attribute '_args), let's wrap it
                 log.error("An exception occurred while sending RPC_ERROR to "
                           "client. Wrapping it and resending. Error to "
                           "send(causing exception goes next):\n%s", formated_tb)
-                log.exception(err)
                 try:
                     raise WrappedException(str(exceptionValue), exceptionType.__name__, formated_tb)
                 except:
                     sendError()
+            except Exception, err:
+                log.error("An exception occurred while sending RPC_ERROR to client: %s", err)
 
         if method == "daemon.info":
             # This is a special case and used in the initial connection process
diff --git a/deluge/error.py b/deluge/error.py
index 825163233..1850d2b06 100644
--- a/deluge/error.py
+++ b/deluge/error.py
@@ -36,15 +36,6 @@
 
 
 class DelugeError(Exception):
-    def _get_message(self):
-        return self._message
-    def _set_message(self, message):
-        self._message = message
-    message = property(_get_message, _set_message)
-    del _get_message, _set_message
-
-    def __str__(self):
-        return self.message
 
     def __new__(cls, *args, **kwargs):
         inst = super(DelugeError, cls).__new__(cls, *args, **kwargs)
@@ -52,80 +43,71 @@ class DelugeError(Exception):
         inst._kwargs = kwargs
         return inst
 
-class NoCoreError(DelugeError):
-    pass
+    def __init__(self, message=None):
+        self.message = message
+
+    def __str__(self):
+        return self.message
+
 
 class DaemonRunningError(DelugeError):
     pass
 
+
 class InvalidTorrentError(DelugeError):
     pass
 
+
 class InvalidPathError(DelugeError):
     pass
 
+
 class WrappedException(DelugeError):
-    def _get_traceback(self):
-        return self._traceback
-    def _set_traceback(self, traceback):
-        self._traceback = traceback
-    traceback = property(_get_traceback, _set_traceback)
-    del _get_traceback, _set_traceback
-
-    def _get_type(self):
-        return self._type
-    def _set_type(self, type):
-        self._type = type
-    type = property(_get_type, _set_type)
-    del _get_type, _set_type
 
     def __init__(self, message, exception_type, traceback):
         self.message = message
         self.type = exception_type
         self.traceback = traceback
 
+
 class _ClientSideRecreateError(DelugeError):
     pass
 
+
 class IncompatibleClient(_ClientSideRecreateError):
+
     def __init__(self, daemon_version):
         self.daemon_version = daemon_version
-        self.message = _(
-            "Your deluge client is not compatible with the daemon. "
-            "Please upgrade your client to %(daemon_version)s"
-        ) % dict(daemon_version=self.daemon_version)
+        msg = "Your deluge client is not compatible with the daemon. "\
+            "Please upgrade your client to %(daemon_version)s" % \
+            dict(daemon_version=self.daemon_version)
+        super(IncompatibleClient, self).__init__(message=msg)
+
 
 class NotAuthorizedError(_ClientSideRecreateError):
 
     def __init__(self, current_level, required_level):
-        self.message = _(
-            "Auth level too low: %(current_level)s < %(required_level)s" %
+        msg = "Auth level too low: %(current_level)s < %(required_level)s" % \
             dict(current_level=current_level, required_level=required_level)
-        )
+        super(NotAuthorizedError, self).__init__(message=msg)
         self.current_level = current_level
         self.required_level = required_level
 
 
 class _UsernameBasedPasstroughError(_ClientSideRecreateError):
 
-    def _get_username(self):
-        return self._username
-    def _set_username(self, username):
-        self._username = username
-    username = property(_get_username, _set_username)
-    del _get_username, _set_username
-
     def __init__(self, message, username):
         super(_UsernameBasedPasstroughError, self).__init__(message)
-        self.message = message
         self.username = username
 
 
 class BadLoginError(_UsernameBasedPasstroughError):
     pass
 
+
 class AuthenticationRequired(_UsernameBasedPasstroughError):
     pass
 
+
 class AuthManagerError(_UsernameBasedPasstroughError):
     pass
diff --git a/deluge/tests/test_error.py b/deluge/tests/test_error.py
new file mode 100644
index 000000000..1a57d1272
--- /dev/null
+++ b/deluge/tests/test_error.py
@@ -0,0 +1,35 @@
+from twisted.trial import unittest
+import deluge.error
+
+
+class ErrorTestCase(unittest.TestCase):
+    def setUp(self):
+        pass
+
+    def tearDown(self):
+        pass
+
+    def test_DelugeError(self):
+        msg = "Some message"
+        e = deluge.error.DelugeError(msg)
+        self.assertEquals(str(e), msg)
+        self.assertEquals(e._args, (msg,))
+        self.assertEquals(e._kwargs, {})
+
+    def test_IncompatibleClient(self):
+        version = "1.3.6"
+        e = deluge.error.IncompatibleClient(version)
+        self.assertEquals(str(e), "Your deluge client is not compatible with the daemon. \
+Please upgrade your client to %s" % version)
+
+    def test_NotAuthorizedError(self):
+        current_level = 5
+        required_level = 10
+        e = deluge.error.NotAuthorizedError(current_level, required_level)
+        self.assertEquals(str(e), "Auth level too low: %d < %d" % (current_level, required_level))
+
+    def test_BadLoginError(self):
+        message = "Login failed"
+        username = "deluge"
+        e = deluge.error.BadLoginError(message, username)
+        self.assertEquals(str(e), message)
diff --git a/deluge/ui/client.py b/deluge/ui/client.py
index 8e8b8a44f..650dcfa45 100644
--- a/deluge/ui/client.py
+++ b/deluge/ui/client.py
@@ -150,37 +150,46 @@ class DelugeRPCProtocol(DelugeTransferProtocol):
             d.callback(request[2])
         elif message_type == RPC_ERROR:
             # Recreate exception and errback'it
-            exception_cls = getattr(error, request[2])
-            exception = exception_cls(*request[3], **request[4])
-
-            # Ideally we would chain the deferreds instead of instance
-            # checking just to log them. But, that would mean that any
-            # errback on the fist deferred should returns it's failure
-            # so it could pass back to the 2nd deferred on the chain. But,
-            # that does not always happen.
-            # So, just do some instance checking and just log rpc error at
-            # diferent levels.
-            r = self.__rpc_requests[request_id]
-            msg = "RPCError Message Received!"
-            msg += "\n" + "-" * 80
-            msg += "\n" + "RPCRequest: " + r.__repr__()
-            msg += "\n" + "-" * 80
-            if isinstance(exception, error.WrappedException):
-                msg += "\n" + exception.type + "\n" + exception.message + ": "
-                msg += exception.traceback
-            else:
-                msg += "\n" + request[5] + "\n" + request[2] + ": "
-                msg += str(exception)
-            msg += "\n" + "-" * 80
-
-            if not isinstance(exception, error._ClientSideRecreateError):
-                # Let's log these as errors
-                log.error(msg)
-            else:
-                # The rest just get's logged in debug level, just to log
-                # what's happening
-                log.debug(msg)
-
+            try:
+                # The exception class is located in deluge.error
+                if hasattr(error, request[2]):
+                    exception_cls = getattr(error, request[2])
+                    exception = exception_cls(*request[3], **request[4])
+                else:
+                    # Shouldn't happen
+                    raise Exception("Received invalid exception: %s", request[2])
+
+                # Ideally we would chain the deferreds instead of instance
+                # checking just to log them. But, that would mean that any
+                # errback on the fist deferred should returns it's failure
+                # so it could pass back to the 2nd deferred on the chain. But,
+                # that does not always happen.
+                # So, just do some instance checking and just log rpc error at
+                # diferent levels.
+                r = self.__rpc_requests[request_id]
+                msg = "RPCError Message Received!"
+                msg += "\n" + "-" * 80
+                msg += "\n" + "RPCRequest: " + r.__repr__()
+                msg += "\n" + "-" * 80
+                if isinstance(exception, error.WrappedException):
+                    msg += "\n" + exception.type + "\n" + exception.message + ": "
+                    msg += exception.traceback
+                else:
+                    msg += "\n" + request[5] + "\n" + request[2] + ": "
+                    msg += str(exception)
+                msg += "\n" + "-" * 80
+
+                if not isinstance(exception, error._ClientSideRecreateError):
+                    # Let's log these as errors
+                    log.error(msg)
+                else:
+                    # The rest just get's logged in debug level, just to log
+                    # what's happening
+                    log.debug(msg)
+            except:
+                # Failed probably because of invalid data (old daemon)
+                import traceback
+                log.error("Failed to handle RPC_ERROR (Old daemon?): %s\nLocal error: %s", request[2], traceback.format_exc())
             d.errback(exception)
         del self.__rpc_requests[request_id]