Put fault message in the correct field

Currently when adding an instance fault, the message field looks
for kwargs['value'], which many exceptions do not have.  This
results in the message field being set to the name of the
exception class, which in some cases is not useful (see the bug
report for an example).

This change puts the exception message in the message field, and
removes it from the details field since there's no need to put it
in both places.  That leaves the details field for actual details,
like stack traces.

Fixes bug 1203880

Change-Id: Ic92a9bd9cb2e139ce7a9f3f1299b7a6ccae82dd1

nova/compute/utils.py

@@ -45,18 +45,28 @@ def add_instance_fault_from_exc(context, conductor,
     """Adds the specified fault to the database."""
 
     code = 500
-    message = fault.__class__.__name__
 
     if hasattr(fault, "kwargs"):
         code = fault.kwargs.get('code', 500)
-        # get the message from the exception that was thrown
-        # if that does not exist, use the name of the exception class itself
-        message = fault.kwargs.get('value', message)
 
-    details = unicode(fault)
+    # get the message from the exception that was thrown
+    # if that does not exist, use the name of the exception class itself
+    try:
+        message = fault.format_message()
+    # These exception handlers are broad so we don't fail to log the fault
+    # just because there is an unexpected error retrieving the message
+    except Exception:
+        try:
+            message = unicode(fault)
+        except Exception:
+            message = None
+    if not message:
+        message = fault.__class__.__name__
+    details = ''
+
     if exc_info and code == 500:
         tb = exc_info[2]
-        details += '\n' + ''.join(traceback.format_tb(tb))
+        details += ''.join(traceback.format_tb(tb))
 
     values = {
         'instance_uuid': instance['uuid'],

nova/tests/compute/test_compute.py

@@ -4261,13 +4261,12 @@ class ComputeTestCase(BaseTestCase):
         exc_info = None
 
         def fake_db_fault_create(ctxt, values):
-            self.assertTrue(values['details'].startswith('test'))
             self.assertTrue('raise NotImplementedError' in values['details'])
             del values['details']
 
             expected = {
                 'code': 500,
-                'message': 'NotImplementedError',
+                'message': 'test',
                 'instance_uuid': instance['uuid'],
                 'host': self.compute.host
             }
@@ -4292,7 +4291,6 @@ class ComputeTestCase(BaseTestCase):
         exc_info = None
 
         def fake_db_fault_create(ctxt, values):
-            self.assertTrue(values['details'].startswith('Remote error'))
             self.assertTrue('raise rpc_common.RemoteError'
                 in values['details'])
             del values['details']
@@ -4300,7 +4298,7 @@ class ComputeTestCase(BaseTestCase):
             expected = {
                 'code': 500,
                 'instance_uuid': instance['uuid'],
-                'message': 'My Test Message',
+                'message': 'Remote error: test My Test Message\nNone.',
                 'host': self.compute.host
             }
             self.assertEquals(expected, values)
@@ -4324,8 +4322,8 @@ class ComputeTestCase(BaseTestCase):
 
             expected = {
                 'code': 400,
-                'message': 'Invalid',
+                'message': 'fake details',
-                'details': 'fake details',
+                'details': '',
                 'instance_uuid': instance['uuid'],
                 'host': self.compute.host
             }
@@ -4350,8 +4348,8 @@ class ComputeTestCase(BaseTestCase):
         def fake_db_fault_create(ctxt, values):
             expected = {
                 'code': 500,
-                'message': 'NotImplementedError',
+                'message': 'test',
-                'details': 'test',
+                'details': '',
                 'instance_uuid': instance['uuid'],
                 'host': self.compute.host
             }