Log additional details of suds faults

When a suds requests results in a fault response, the current
code loses details on that fault in the sequence of exceptions
propagated through the call stack. A NoPermission fault will contain
additional metadata on the privilegeId and object type which
needs to be logged.  The fault string will be propagated with this
fix, along with details of a NoPermission fault.

It is possible 'details' to contain unicode values which is handled by
implementing __unicode__() for VimFaultException.

This functionality was added in Nova with commit
62cb0dc6257daac5ec9fd1a90ee5721e6543dd76 and we need the same thing in
oslo.vmware

Change-Id: Ia7ca46bb3f263211ab1fe37585f183edf00461b1

4 files changed, 47 insertions, 16 deletions

diff --git a/oslo/vmware/api.py b/oslo/vmware/api.py
index 2705c8a..e4d08f1 100644
--- a/oslo/vmware/api.py
+++ b/oslo/vmware/api.py
@@ -325,7 +325,8 @@ class VMwareAPISession(object):
                     # Raise specific exceptions here if possible
                     if excep.fault_list:
                         LOG.debug("Fault list: %s", excep.fault_list)
-                        raise exceptions.get_fault_class(excep.fault_list[0])
+                        fault = excep.fault_list[0]
+                        raise exceptions.get_fault_class(fault)(unicode(excep))
                     raise
 
             except exceptions.VimConnectionException:
diff --git a/oslo/vmware/exceptions.py b/oslo/vmware/exceptions.py
index 3db8802..cfd330d 100644
--- a/oslo/vmware/exceptions.py
+++ b/oslo/vmware/exceptions.py
@@ -79,15 +79,24 @@ class VimFaultException(VimException):
         super(VimFaultException, self).__init__(message, cause)
         if not isinstance(fault_list, list):
             raise ValueError(_("fault_list must be a list"))
+        if details is not None and not isinstance(details, dict):
+            raise ValueError(_("details must be a dict"))
         self.fault_list = fault_list
         self.details = details
 
     def __str__(self):
-        descr = VimException.__str__(self)
+        return unicode(self).encode('utf8')
+
+    def __unicode__(self):
+        descr = unicode(VimException.__str__(self))
         if self.fault_list:
+            # fault_list doesn't contain non-ASCII chars, we can use str()
             descr += '\nFaults: ' + str(self.fault_list)
         if self.details:
-            descr += '\nDetails: ' + str(self.details)
+            # details may contain non-ASCII values
+            details = '{%s}' % ', '.join(["'%s': '%s'" % (k, v)
+                                         for k, v in self.details.iteritems()])
+            descr += '\nDetails: ' + details
         return descr
 
 
diff --git a/oslo/vmware/service.py b/oslo/vmware/service.py
index 92fadb8..7e3b8ad 100644
--- a/oslo/vmware/service.py
+++ b/oslo/vmware/service.py
@@ -27,7 +27,6 @@ import suds
 
 from oslo.vmware import exceptions
 from oslo.vmware.openstack.common.gettextutils import _
-from oslo.vmware.openstack.common.gettextutils import _LE
 from oslo.vmware import vim_util
 
 
@@ -102,13 +101,9 @@ class Service(object):
         :param response: response from RetrievePropertiesEx API call
         :raises: VimFaultException
         """
-        # TODO(rgerganov) this method doesn't belong here because this fault
-        # checking is specific to the Vim service. We should come up with
-        # a new design that allows extensible fault checking and have the
-        # service specific parts in the corresponding child classes
-
         LOG.debug("Checking RetrievePropertiesEx API response for faults.")
         fault_list = []
+        details = {}
         if not response:
             # This is the case when the session has timed out. ESX SOAP
             # server sends an empty RetrievePropertiesExResponse. Normally
@@ -125,15 +120,19 @@ class Service(object):
             for obj_cont in response.objects:
                 if hasattr(obj_cont, 'missingSet'):
                     for missing_elem in obj_cont.missingSet:
-                        fault_type = missing_elem.fault.fault.__class__
-                        fault_list.append(fault_type.__name__)
+                        f_type = missing_elem.fault.fault
+                        f_name = f_type.__class__.__name__
+                        fault_list.append(f_name)
+                        if f_name == exceptions.NO_PERMISSION:
+                            details['object'] = f_type.object.value
+                            details['privilegeId'] = f_type.privilegeId
+
         if fault_list:
-            LOG.error(_LE("Faults %s found in RetrievePropertiesEx API "
-                          "response."),
-                      fault_list)
+            fault_string = _("Error occurred while calling "
+                             "RetrievePropertiesEx.")
             raise exceptions.VimFaultException(fault_list,
-                                               _("Error occurred while calling"
-                                                 " RetrievePropertiesEx."))
+                                               fault_string,
+                                               details=details)
         LOG.debug("No faults found in RetrievePropertiesEx API response.")
 
     @property
diff --git a/tests/test_api.py b/tests/test_api.py
index ec5a15c..6c02051 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1,3 +1,4 @@
+# coding=utf-8
 # Copyright (c) 2014 VMware, Inc.
 # All Rights Reserved.
 #
@@ -19,6 +20,7 @@ Unit tests for session management and API invocation classes.
 
 from eventlet import greenthread
 import mock
+import suds
 
 from oslo.vmware import api
 from oslo.vmware import exceptions
@@ -257,6 +259,26 @@ class VMwareAPISessionTest(base.TestCase):
                           module,
                           'api')
 
+    def test_invoke_api_with_vim_fault_exception_details(self):
+        api_session = self._create_api_session(True)
+        fault_string = 'Invalid property.'
+        fault_list = [exceptions.INVALID_PROPERTY]
+        details = {u'name': suds.sax.text.Text(u'фира')}
+
+        module = mock.Mock()
+        module.api.side_effect = exceptions.VimFaultException(fault_list,
+                                                              fault_string,
+                                                              details=details)
+        e = self.assertRaises(exceptions.InvalidPropertyException,
+                              api_session.invoke_api,
+                              module,
+                              'api')
+        details_str = u"{'name': 'фира'}"
+        expected_str = "%s\nFaults: %s\nDetails: %s" % (fault_string,
+                                                        fault_list,
+                                                        details_str)
+        self.assertEqual(expected_str, unicode(e))
+
     def test_invoke_api_with_empty_response(self):
         api_session = self._create_api_session(True)
         vim_obj = api_session.vim