Merge branch 'feature/error-handling' into develop

Closes #504: design generic error handling solution.

* app-wide logging config.
* --logfile command line argument.
* created basic exception hierarchy
* conductor pushes exceptions to error queue
* in Qt, error dialogs are created from exception attributes

5 files changed, 127 insertions, 65 deletions

diff --git a/src/leap/eip/checks.py b/src/leap/eip/checks.py
index 1b7c2e1b..4a2a9599 100644
--- a/src/leap/eip/checks.py
+++ b/src/leap/eip/checks.py
@@ -1,12 +1,7 @@
-#import json
 import logging
 import ssl
 import os
 
-logging.basicConfig()
-logger = logging.getLogger(name=__name__)
-logger.setLevel(logging.DEBUG)
-
 import requests
 
 from leap.base import constants as baseconstants
@@ -17,6 +12,8 @@ from leap.eip import exceptions as eipexceptions
 from leap.eip import specs as eipspecs
 from leap.util.fileutil import mkdir_p
 
+logger = logging.getLogger(name=__name__)
+
 """
 EIPConfigChecker
 ----------
diff --git a/src/leap/eip/config.py b/src/leap/eip/config.py
index 810a5a8d..f4b979ce 100644
--- a/src/leap/eip/config.py
+++ b/src/leap/eip/config.py
@@ -10,9 +10,7 @@ from leap.baseapp.permcheck import (is_pkexec_in_system,
 from leap.eip import exceptions as eip_exceptions
 from leap.eip import specs as eipspecs
 
-logging.basicConfig()
 logger = logging.getLogger(name=__name__)
-logger.setLevel('DEBUG')
 
 
 class EIPConfig(baseconfig.JSONLeapConfig):
diff --git a/src/leap/eip/eipconnection.py b/src/leap/eip/eipconnection.py
index 386b71be..5c54a986 100644
--- a/src/leap/eip/eipconnection.py
+++ b/src/leap/eip/eipconnection.py
@@ -3,15 +3,14 @@ EIP Connection Class
 """
 from __future__ import (absolute_import,)
 import logging
-
-logging.basicConfig()
-logger = logging.getLogger(name=__name__)
-logger.setLevel(logging.DEBUG)
+import Queue
 
 from leap.eip.checks import EIPConfigChecker
 from leap.eip import exceptions as eip_exceptions
 from leap.eip.openvpnconnection import OpenVPNConnection
 
+logger = logging.getLogger(name=__name__)
+
 
 class EIPConnection(OpenVPNConnection):
     """
@@ -25,8 +24,7 @@ class EIPConnection(OpenVPNConnection):
         self.settingsfile = kwargs.get('settingsfile', None)
         self.logfile = kwargs.get('logfile', None)
 
-        # not used atm. but should.
-        self.error_queue = []
+        self.error_queue = Queue.Queue()
 
         status_signals = kwargs.pop('status_signals', None)
         self.status = EIPConnectionStatus(callbacks=status_signals)
@@ -34,11 +32,19 @@ 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
         """
-        self.config_checker.run_all(skip_download=skip_download)
+        logger.debug('running conductor checks')
+        try:
+            self.config_checker.run_all(skip_download=skip_download)
+            self.run_openvpn_checks()
+        except Exception as exc:
+            self.error_queue.put(exc)
 
     def connect(self):
         """
@@ -46,7 +52,6 @@ class EIPConnection(OpenVPNConnection):
         """
         self.forget_errors()
         self._try_connection()
-        # XXX should capture errors?
 
     def disconnect(self):
         """
@@ -67,11 +72,11 @@ class EIPConnection(OpenVPNConnection):
         """
         return self.status.current
 
-    def desired_connection_state(self):
-        """
-        returns the desired_connection state
-        """
-        return self.desired_con_state
+    #def desired_connection_state(self):
+        #"""
+        #returns the desired_connection state
+        #"""
+        #return self.desired_con_state
 
     def poll_connection_state(self):
         """
@@ -109,26 +114,27 @@ class EIPConnection(OpenVPNConnection):
         private method for disconnecting
         """
         if self.subp is not None:
+            logger.debug('disconnecting...')
             self.subp.terminate()
             self.subp = None
-        # XXX signal state changes! :)
 
-    def _is_alive(self):
-        """
-        don't know yet
-        """
-        pass
+    #def _is_alive(self):
+        #"""
+        #don't know yet
+        #"""
+        #pass
 
     def _connect(self):
         """
         entry point for connection cascade methods.
         """
-        #conn_result = ConState.DISCONNECTED
         try:
             conn_result = self._try_connection()
         except eip_exceptions.UnrecoverableError as except_msg:
             logger.error("FATAL: %s" % unicode(except_msg))
             conn_result = self.status.UNRECOVERABLE
+
+        # XXX enqueue exceptions themselves instead?
         except Exception as except_msg:
             self.error_queue.append(except_msg)
             logger.error("Failed Connection: %s" %
diff --git a/src/leap/eip/exceptions.py b/src/leap/eip/exceptions.py
index 19a0e707..3c8f6afb 100644
--- a/src/leap/eip/exceptions.py
+++ b/src/leap/eip/exceptions.py
@@ -1,71 +1,126 @@
-class EIPNoCommandError(Exception):
-    pass
+"""
+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).
 
-class ConnectionError(Exception):
-    """
-    generic connection error
-    """
-    pass
+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
     """
-    def __str__(self):
-        if len(self.args) >= 1:
-            return repr(self.args[0])
-        else:
-            return ConnectionError
+    critical = False
 
 
-class UnrecoverableError(EIPClientError):
+class CriticalError(EIPClientError):
     """
     we cannot do anything about it, sorry
     """
-    # XXX we should catch this and raise
-    # to qtland, so we emit signal
-    # to translate whatever kind of error
-    # to user-friendly msg in dialog.
-    pass
+    critical = True
+    failfirst = True
 
 
-class MissingSocketError(Exception):
+class Warning(EIPClientError):
+    """
+    just that, warnings
+    """
     pass
 
 
-class ConnectionRefusedError(Exception):
-    pass
+class EIPNoPolkitAuthAgentAvailable(CriticalError):
+    message = "No polkit authentication agent could be found"
+    usermessage = ("We could not find any authentication "
+                   "agent in your system.<br/>"
+                   "Make sure you have "
+                   "<b>polkit-gnome-authentication-agent-1</b> "
+                   "running and try again.")
 
 
-class EIPNoPkexecAvailable(Exception):
-    pass
+class EIPNoPkexecAvailable(Warning):
+    message = "No pkexec binary found"
+    usermessage = ("We could not find <b>pkexec</b> in your "
+                   "system.<br/> Do you want to try "
+                   "<b>setuid workaround</b>? "
+                   "(<i>DOES NOTHING YET</i>)")
+    failfirst = True
+
+
+class EIPNoCommandError(EIPClientError):
+    message = "no suitable openvpn command found"
+    usermessage = ("No suitable openvpn command found. "
+                   "<br/>(Might be a permissions problem)")
+
 
+#
+# errors still needing some love
+#
 
-class EIPNoPolkitAuthAgentAvailable(Exception):
+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 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 EIPInitNoProviderError(Exception):
+class EIPInitNoProviderError(EIPClientError):
     pass
 
 
-class EIPInitBadProviderError(Exception):
+class EIPInitBadProviderError(EIPClientError):
     pass
 
 
-class EIPInitNoKeyFileError(Exception):
+class EIPConfigurationError(EIPClientError):
     pass
 
+#
+# Errors that probably we don't need anymore
+# chase down for them and check.
+#
 
-class EIPInitBadKeyFilePermError(Exception):
+
+class MissingSocketError(Exception):
     pass
 
 
-class EIPMissingDefaultProvider(Exception):
+class ConnectionRefusedError(Exception):
     pass
 
 
-class EIPConfigurationError(Exception):
+class EIPMissingDefaultProvider(Exception):
     pass
diff --git a/src/leap/eip/openvpnconnection.py b/src/leap/eip/openvpnconnection.py
index 32fa55b1..4a6a495a 100644
--- a/src/leap/eip/openvpnconnection.py
+++ b/src/leap/eip/openvpnconnection.py
@@ -7,9 +7,7 @@ import socket
 import time
 from functools import partial
 
-logging.basicConfig()
 logger = logging.getLogger(name=__name__)
-logger.setLevel(logging.DEBUG)
 
 from leap.base.connection import Connection
 from leap.util.coroutines import spawn_and_watch_process
@@ -24,7 +22,6 @@ class OpenVPNConnection(Connection):
     All related to invocation
     of the openvpn binary
     """
-    # Connection Methods
 
     def __init__(self, config_file=None,
                  watcher_cb=None,
@@ -45,8 +42,8 @@ to be triggered for each one of them.
         :type watcher_cb: function
         :type signal_map: dict
         """
+        logger.debug('init openvpn connection')
         self.debug = debug
-        #print('conductor:%s' % debug)
 
         self.config_file = config_file
         self.watcher_cb = watcher_cb
@@ -59,15 +56,18 @@ to be triggered for each one of them.
         self.port = None
         self.proto = None
 
+        ##################################
         # XXX move all error messages
         # into a more encapsulated object.
         self.missing_pkexec = False
         self.missing_auth_agent = False
+
         self.bad_keyfile_perms = False
         self.missing_vpn_keyfile = False
         self.missing_provider = False
         self.missing_definition = False
         self.bad_provider = False
+        #################################
 
         #XXX workaround for signaling
         #the ui that we don't know how to
@@ -79,9 +79,6 @@ to be triggered for each one of them.
 
         # XXX get autostart from config
         self.autostart = True
-        #self._get_or_create_config()
-        self._set_ovpn_command()
-        self._check_vpn_keys()
 
         #
         # management init methods
@@ -97,6 +94,11 @@ to be triggered for each one of them.
         self.port = port
         self.password = password
 
+    def run_openvpn_checks(self):
+        logger.debug('running openvpn checks')
+        self._set_ovpn_command()
+        self._check_vpn_keys()
+
     def _set_ovpn_command(self):
         # XXX check also for command-line --command flag
         try:
@@ -104,10 +106,13 @@ to be triggered for each one of them.
                 debug=self.debug)
         except eip_exceptions.EIPNoPolkitAuthAgentAvailable:
             command = args = None
+            # XXX deprecate
             self.missing_auth_agent = True
+            raise
         except eip_exceptions.EIPNoPkexecAvailable:
             command = args = None
             self.missing_pkexec = True
+            raise
 
         # XXX if not command, signal error.
         self.command = command
@@ -192,7 +197,8 @@ to be triggered for each one of them.
     #
 
     def forget_errors(self):
-        print('forgetting errors')
+        #print('forgetting errors')
+        logger.debug('forgetting errors')
         self.with_errors = False
 
     def connect_to_management(self):