Merge lp:~rackspace-titan/nova/log_all_stack_traces into lp:~hudson-openstack/nova/trunk

[Dan Prince]

Update exception.wrap_exception so that all exceptions (not just Error and NovaException types) get logged correctly.

[Brian Lamar (blamar)]

I, too, support the removal of this block unless someone has a reason to keep. The attached bug has hit me a couple times and makes debugging difficult.

review: Approve

[Todd Willey (xtoddx)]

What is hard about debugging? You get the traceback from logger.exception, don't you? I'm not necessarily a fan of keeping it, but knowing where expected error cases are vs. brand new unanticipated errors has some value to developers.

review: Needs Information

[Dan Prince (dan-prince)]

> What is hard about debugging? You get the traceback from logger.exception,
> don't you? I'm not necessarily a fan of keeping it, but knowing where
> expected error cases are vs. brand new unanticipated errors has some value to
> developers.

Hey Todd,

Yes. I got a traceback that looked like this:

2011-09-15 12:37:02,599 ERROR nova.rpc [-] Exception during message handling
(nova.rpc): TRACE: Traceback (most recent call last):
(nova.rpc): TRACE: File "/usr/lib/pymodules/python2.6/nova/rpc/impl_kombu.py", line 620, in _process_data
(nova.rpc): TRACE: rval = node_func(context=ctxt, **node_args)
(nova.rpc): TRACE: File "/usr/lib/pymodules/python2.6/nova/exception.py", line 129, in wrapped
(nova.rpc): TRACE: raise Error(str(e))
(nova.rpc): TRACE: Error: list index out of range
(nova.rpc): TRACE:

---

Which pretty much tells me nothing! The issue I have with the code I removed is it effectively erases the full stack traces from runtime exceptions. Essentially anything that doesn't extend Error or NovaException won't get logged with a full stack trace.

The root cause of the exceptions getting wiped out is utils.to_primative which seems to have been added intentially but IMHO is somewhat of an evil method. Probably best not to get into that here however.

[Matt Dietz (cerberus)]

Rationale makes sense to me

review: Approve

=== modified file 'nova/exception.py'
--- nova/exception.py	2011-09-15 21:58:22 +0000
+++ nova/exception.py	2011-09-16 03:04:17 +0000
@@ -121,13 +121,6 @@
                     notifier.notify(publisher_id, temp_type, temp_level,
                                     payload)
 
-                if (not isinstance(e, Error) and
-                    not isinstance(e, NovaException)):
-                    #exc_type, exc_value, exc_traceback = sys.exc_info()
-                    LOG.exception(_('Uncaught exception'))
-                    #logging.error(traceback.extract_stack(exc_traceback))
-                    raise Error(str(e))
-
                 # re-raise original exception since it may have been clobbered
                 raise exc_info[0], exc_info[1], exc_info[2]
 

=== modified file 'nova/tests/test_exception.py'
--- nova/tests/test_exception.py	2011-07-01 14:38:17 +0000
+++ nova/tests/test_exception.py	2011-09-16 03:04:17 +0000
@@ -74,14 +74,13 @@
 
     def test_wrap_exception_throws_exception(self):
         wrapped = exception.wrap_exception()
-        # Note that Exception is converted to Error ...
-        self.assertRaises(exception.Error, wrapped(bad_function_exception))
+        self.assertRaises(Exception, wrapped(bad_function_exception))
 
     def test_wrap_exception_with_notifier(self):
         notifier = FakeNotifier()
         wrapped = exception.wrap_exception(notifier, "publisher", "event",
                                            "level")
-        self.assertRaises(exception.Error, wrapped(bad_function_exception))
+        self.assertRaises(Exception, wrapped(bad_function_exception))
         self.assertEquals(notifier.provided_publisher, "publisher")
         self.assertEquals(notifier.provided_event, "event")
         self.assertEquals(notifier.provided_priority, "level")
@@ -91,7 +90,7 @@
     def test_wrap_exception_with_notifier_defaults(self):
         notifier = FakeNotifier()
         wrapped = exception.wrap_exception(notifier)
-        self.assertRaises(exception.Error, wrapped(bad_function_exception))
+        self.assertRaises(Exception, wrapped(bad_function_exception))
         self.assertEquals(notifier.provided_publisher, None)
         self.assertEquals(notifier.provided_event, "bad_function_exception")
         self.assertEquals(notifier.provided_priority, notifier.ERROR)