From 8148bc9c8c113c41fcb18b397669b1f13447c653 Mon Sep 17 00:00:00 2001 From: kali Date: Thu, 6 Sep 2012 02:27:04 +0900 Subject: more generic error handler in EipConductorAppMixin documentation of the Exception Hierarchy and attributes. also a bit of general cleanup around error handling in conductor. Hopefully to be polished an abstracted to leap.base with time. not all errors are converted (and the old with_errors/ignoring errors) are still there, but we should be using this style of handlers from now on. wrapping up with this pseudo-feature for now. as we work on individual features we can mimick the exceptions that are working. --- src/leap/baseapp/constants.py | 5 ++ src/leap/baseapp/dialogs.py | 19 +++++- src/leap/baseapp/eip.py | 150 +++++++++++++++++++++-------------------- src/leap/baseapp/leap_app.py | 2 +- src/leap/baseapp/log.py | 3 +- src/leap/baseapp/mainwindow.py | 20 +++--- src/leap/baseapp/systray.py | 2 +- src/leap/eip/eipconnection.py | 4 +- src/leap/eip/exceptions.py | 97 +++++++++++++++++++------- 9 files changed, 190 insertions(+), 112 deletions(-) (limited to 'src') diff --git a/src/leap/baseapp/constants.py b/src/leap/baseapp/constants.py index 763df23b..e312be21 100644 --- a/src/leap/baseapp/constants.py +++ b/src/leap/baseapp/constants.py @@ -1 +1,6 @@ +# This timer used for polling vpn manager state. + +# XXX what is an optimum polling interval? +# too little will be overkill, too much will +# miss transition states. TIMER_MILLISECONDS = 250.0 diff --git a/src/leap/baseapp/dialogs.py b/src/leap/baseapp/dialogs.py index d37a234c..d4acb09d 100644 --- a/src/leap/baseapp/dialogs.py +++ b/src/leap/baseapp/dialogs.py @@ -1,14 +1,25 @@ +import logging + from PyQt4.QtGui import (QDialog, QFrame, QPushButton, QLabel, QMessageBox) +logger = logging.getLogger(name=__name__) + class ErrorDialog(QDialog): - def __init__(self, parent=None): + def __init__(self, parent=None, errtype=None, msg=None, label=None): super(ErrorDialog, self).__init__(parent) frameStyle = QFrame.Sunken | QFrame.Panel self.warningLabel = QLabel() self.warningLabel.setFrameStyle(frameStyle) self.warningButton = QPushButton("QMessageBox.&warning()") + if msg is not None: + self.msg = msg + if label is not None: + self.label = label + if errtype == "critical": + self.criticalMessage(self.msg, self.label) + def warningMessage(self, msg, label): msgBox = QMessageBox(QMessageBox.Warning, "QMessageBox.warning()", msg, @@ -26,5 +37,11 @@ class ErrorDialog(QDialog): QMessageBox.NoButton, self) msgBox.addButton("&Ok", QMessageBox.AcceptRole) msgBox.exec_() + + # It's critical, so we exit. + # We should better emit a signal and connect it + # with the proper shutdownAndQuit method, but + # this suffices for now. + logger.info('Quitting') import sys sys.exit() diff --git a/src/leap/baseapp/eip.py b/src/leap/baseapp/eip.py index dd88b7f5..afdb7adc 100644 --- a/src/leap/baseapp/eip.py +++ b/src/leap/baseapp/eip.py @@ -11,8 +11,7 @@ from leap.eip.eipconnection import EIPConnection logger = logging.getLogger(name=__name__) -class EIPConductorApp(object): - # XXX EIPConductorMixin ? +class EIPConductorAppMixin(object): """ initializes an instance of EIPConnection, gathers errors, and passes status-change signals @@ -52,86 +51,90 @@ class EIPConductorApp(object): lambda: self.start_or_stopVPN()) def error_check(self): + """ + consumes the conductor error queue. + pops errors, and acts accordingly (launching user dialogs). + """ logger.debug('error check') ##################################### # XXX refactor in progress (by #504) + errq = self.conductor.error_queue while errq.qsize() != 0: logger.debug('%s errors left in conductor queue', errq.qsize()) error = errq.get() + + # redundant log, debugging the loop. logger.error('%s: %s', error.__class__.__name__, error.message) if issubclass(error.__class__, eip_exceptions.EIPClientError): - if error.critical: - logger.critical(error.message) - logger.error('quitting') - - # XXX - # check headless = False before - # launching dialog. - # (for Qt tests) - - dialog = ErrorDialog() - if getattr(error, 'usermessage', None): - message = error.usermessage - else: - message = error.message - dialog.criticalMessage(message, 'error') - else: - logger.exception(error.message) + self.handle_eip_error(error) + else: + # This is not quite working. FIXME import traceback traceback.print_exc() raise error - if self.conductor.missing_definition is True: - dialog = ErrorDialog() - dialog.criticalMessage( - 'The default ' - 'definition.json file cannot be found', - 'error') + if error.failfirst is True: + break - if self.conductor.missing_provider is True: - dialog = ErrorDialog() - dialog.criticalMessage( - 'Missing provider. Add a remote_ip entry ' - 'under section [provider] in eip.cfg', - 'error') + ############################################# + # old errors to check + # write test for them and them remove + # their corpses from here. - if self.conductor.missing_vpn_keyfile is True: - dialog = ErrorDialog() - dialog.criticalMessage( - 'Could not find the vpn keys file', - 'error') + #if self.conductor.missing_vpn_keyfile is True: + #dialog = ErrorDialog() + #dialog.criticalMessage( + #'Could not find the vpn keys file', + #'error') - # ... btw, review pending. - # os.kill of subprocess fails if we have - # some of this errors. + #if self.conductor.bad_keyfile_perms is True: + #dialog = ErrorDialog() + #dialog.criticalMessage( + #'The vpn keys file has bad permissions', + #'error') - # deprecated. - # get something alike. - #if self.conductor.bad_provider is True: + # deprecated. configchecker takes care of that. + #if self.conductor.missing_definition is True: #dialog = ErrorDialog() #dialog.criticalMessage( - #'Bad provider entry. Check that remote_ip entry ' - #'has an IP under section [provider] in eip.cfg', + #'The default ' + #'definition.json file cannot be found', #'error') - if self.conductor.bad_keyfile_perms is True: - dialog = ErrorDialog() - dialog.criticalMessage( - 'The vpn keys file has bad permissions', - 'error') + def handle_eip_error(self, error): + """ + check severity and launches + dialogs informing user about the errors. + in the future we plan to derive errors to + our log viewer. + """ - if self.conductor.missing_pkexec is True: + if getattr(error, 'usermessage', None): + message = error.usermessage + else: + message = error.message + + # XXX + # check headless = False before + # launching dialog. + # (so Qt tests can assert stuff) + + if error.critical: + logger.critical(error.message) + #critical error (non recoverable), + #we give user some info and quit. + #(critical error dialog will exit app) + ErrorDialog(errtype="critical", + msg=message, + label="critical error") + + else: dialog = ErrorDialog() - dialog.warningMessage( - 'We could not find pkexec in your ' - 'system.
Do you want to try ' - 'setuid workaround? ' - '(DOES NOTHING YET)', - 'error') + dialog.warningMessage(message, 'error') @QtCore.pyqtSlot() def statusUpdate(self): @@ -188,29 +191,30 @@ class EIPConductorApp(object): """ stub for running child process with vpn """ + if self.conductor.has_errors(): + logger.debug('not starting vpn; conductor has errors') + if self.eip_service_started is False: try: self.conductor.connect() - # XXX move this to error queue - except eip_exceptions.EIPNoCommandError: - dialog = ErrorDialog() - dialog.warningMessage( - 'No suitable openvpn command found. ' - '
(Might be a permissions problem)', - 'error') - if self.debugmode: - self.startStopButton.setText('&Disconnect') - self.eip_service_started = True - # XXX what is optimum polling interval? - # too little is overkill, too much - # will miss transition states.. + except eip_exceptions.EIPNoCommandError as exc: + self.handle_eip_error(exc) + + except Exception as err: + # raise generic exception (Bad Thing Happened?) + logger.exception(err) + else: + # no errors, so go on. + if self.debugmode: + self.startStopButton.setText('&Disconnect') + self.eip_service_started = True - # XXX decouple! (timer is init by icons class). - # should bring it here? - # to its own class? + # XXX decouple! (timer is init by icons class). + # we could bring Timer Init to this Mixin + # or to its own Mixin. + self.timer.start(constants.TIMER_MILLISECONDS) - self.timer.start(constants.TIMER_MILLISECONDS) return if self.eip_service_started is True: diff --git a/src/leap/baseapp/leap_app.py b/src/leap/baseapp/leap_app.py index 85644360..f91b2329 100644 --- a/src/leap/baseapp/leap_app.py +++ b/src/leap/baseapp/leap_app.py @@ -7,7 +7,7 @@ from leap.gui import mainwindow_rc logger = logging.getLogger(name=__name__) -class MainWindow(object): +class MainWindowMixin(object): """ create the main window for leap app diff --git a/src/leap/baseapp/log.py b/src/leap/baseapp/log.py index 3580e987..8a7f81c3 100644 --- a/src/leap/baseapp/log.py +++ b/src/leap/baseapp/log.py @@ -6,7 +6,7 @@ from PyQt4 import QtCore vpnlogger = logging.getLogger('leap.openvpn') -class LogPane(object): +class LogPaneMixin(object): """ a simple log pane that writes new lines as they come @@ -22,7 +22,6 @@ class LogPane(object): self.logbrowser = QtGui.QTextBrowser() startStopButton = QtGui.QPushButton("&Connect") - #startStopButton.clicked.connect(self.start_or_stopVPN) self.startStopButton = startStopButton logging_layout.addWidget(self.logbrowser) diff --git a/src/leap/baseapp/mainwindow.py b/src/leap/baseapp/mainwindow.py index e87f5844..10b23d9a 100644 --- a/src/leap/baseapp/mainwindow.py +++ b/src/leap/baseapp/mainwindow.py @@ -5,18 +5,18 @@ import logging from PyQt4 import QtCore from PyQt4 import QtGui -from leap.baseapp.eip import EIPConductorApp -from leap.baseapp.log import LogPane -from leap.baseapp.systray import StatusAwareTrayIcon -from leap.baseapp.leap_app import MainWindow +from leap.baseapp.eip import EIPConductorAppMixin +from leap.baseapp.log import LogPaneMixin +from leap.baseapp.systray import StatusAwareTrayIconMixin +from leap.baseapp.leap_app import MainWindowMixin logger = logging.getLogger(name=__name__) class LeapWindow(QtGui.QMainWindow, - MainWindow, EIPConductorApp, - StatusAwareTrayIcon, - LogPane): + MainWindowMixin, EIPConductorAppMixin, + StatusAwareTrayIconMixin, + LogPaneMixin): """ main window for the leap app. Initializes all of its base classes @@ -34,9 +34,9 @@ class LeapWindow(QtGui.QMainWindow, super(LeapWindow, self).__init__() if self.debugmode: self.createLogBrowser() - EIPConductorApp.__init__(self, opts=opts) - StatusAwareTrayIcon.__init__(self) - MainWindow.__init__(self) + EIPConductorAppMixin.__init__(self, opts=opts) + StatusAwareTrayIconMixin.__init__(self) + MainWindowMixin.__init__(self) # bind signals # XXX move to parent classes init?? diff --git a/src/leap/baseapp/systray.py b/src/leap/baseapp/systray.py index f3832473..c696ee74 100644 --- a/src/leap/baseapp/systray.py +++ b/src/leap/baseapp/systray.py @@ -4,7 +4,7 @@ from PyQt4 import QtGui from leap.gui import mainwindow_rc -class StatusAwareTrayIcon(object): +class StatusAwareTrayIconMixin(object): """ a mix of several functions needed to create a systray and make it diff --git a/src/leap/eip/eipconnection.py b/src/leap/eip/eipconnection.py index e090f9a7..5c54a986 100644 --- a/src/leap/eip/eipconnection.py +++ b/src/leap/eip/eipconnection.py @@ -24,7 +24,6 @@ class EIPConnection(OpenVPNConnection): self.settingsfile = kwargs.get('settingsfile', None) self.logfile = kwargs.get('logfile', None) - # XXX USE THIS self.error_queue = Queue.Queue() status_signals = kwargs.pop('status_signals', None) @@ -33,6 +32,9 @@ class EIPConnection(OpenVPNConnection): super(EIPConnection, self).__init__(*args, **kwargs) + def has_errors(self): + return True if self.error_queue.qsize != 0 else True + def run_checks(self, skip_download=False): """ run all eip checks previous to attempting a connection diff --git a/src/leap/eip/exceptions.py b/src/leap/eip/exceptions.py index a30cd2a6..3c8f6afb 100644 --- a/src/leap/eip/exceptions.py +++ b/src/leap/eip/exceptions.py @@ -1,23 +1,60 @@ +""" +Generic error hierarchy +Leap/EIP exceptions used for exception handling, +logging, and notifying user of errors +during leap operation. + +Exception hierarchy +------------------- +All EIP Errors must inherit from EIPClientError (note: move that to +a more generic LEAPClientBaseError). + +Exception attributes and their meaning/uses +------------------------------------------- + +* critical: if True, will abort execution prematurely, + after attempting any cleaning + action. + +* failfirst: breaks any error_check loop that is examining + the error queue. + +* message: the message that will be used in the __repr__ of the exception. + +* usermessage: the message that will be passed to user in ErrorDialogs + in Qt-land. + +TODO: + +* EIPClientError: + Should inherit from LeapException + and move basic attrs there + +* gettext / i18n for user messages. + +""" + + class EIPClientError(Exception): """ base EIPClient exception """ - # Should inherit from LeapException - # and move basic attrs there critical = False - #def __str__(self): - #if len(self.args) >= 1: - #return repr(self.args[0]) - #else: - #return ConnectionError - class CriticalError(EIPClientError): """ we cannot do anything about it, sorry """ critical = True + failfirst = True + + +class Warning(EIPClientError): + """ + just that, warnings + """ + pass class EIPNoPolkitAuthAgentAvailable(CriticalError): @@ -28,33 +65,53 @@ class EIPNoPolkitAuthAgentAvailable(CriticalError): "polkit-gnome-authentication-agent-1 " "running and try again.") -# Errors needing some work +class EIPNoPkexecAvailable(Warning): + message = "No pkexec binary found" + usermessage = ("We could not find pkexec in your " + "system.
Do you want to try " + "setuid workaround? " + "(DOES NOTHING YET)") + failfirst = True -class EIPNoPkexecAvailable(Exception): - pass +class EIPNoCommandError(EIPClientError): + message = "no suitable openvpn command found" + usermessage = ("No suitable openvpn command found. " + "
(Might be a permissions problem)") -class EIPInitNoProviderError(Exception): - pass +# +# errors still needing some love +# + +class EIPInitNoKeyFileError(CriticalError): + message = "No vpn keys found in the expected path" + usermessage = "We could not find your eip certs in the expected path" -class EIPInitBadProviderError(Exception): + +class EIPInitBadKeyFilePermError(Warning): + # I don't know if we should be telling user or not, + # we try to fix permissions and should only re-raise + # if permission check failed. pass -class EIPInitNoKeyFileError(Exception): +class EIPInitNoProviderError(EIPClientError): pass -class EIPInitBadKeyFilePermError(Exception): +class EIPInitBadProviderError(EIPClientError): pass -class EIPNoCommandError(Exception): +class EIPConfigurationError(EIPClientError): pass +# # Errors that probably we don't need anymore +# chase down for them and check. +# class MissingSocketError(Exception): @@ -65,11 +122,5 @@ class ConnectionRefusedError(Exception): pass - - class EIPMissingDefaultProvider(Exception): pass - - -class EIPConfigurationError(Exception): - pass -- cgit v1.2.3