From b76cdc2227f4e6aba9534b1d6ff3b7ea924ae1b5 Mon Sep 17 00:00:00 2001 From: bendikro Date: Thu, 23 May 2013 01:43:00 +0100 Subject: [PATCH] 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) --- deluge/core/rpcserver.py | 9 ++--- deluge/error.py | 60 ++++++++++++---------------------- deluge/tests/test_error.py | 35 ++++++++++++++++++++ deluge/ui/client.py | 67 +++++++++++++++++++++----------------- 4 files changed, 99 insertions(+), 72 deletions(-) create mode 100644 deluge/tests/test_error.py 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 +class WrappedException(DelugeError): 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]) + 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) + # 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]