Improve message propagation in exceptions.handle

In horizon/exceptions.py the handle(message="blah") function only
passes the message onto the user if the exception type is
HorizonException or a specifically handled class. In many cases,
including many forms, the except block calling this method is
catching the broad Exception and not more specific errors. Once
Exception is caught, we pass on an error message to
horizon/exceptions.handle(). That message is ignored in several cases
and the result is the underlying Django forms puts up a generic error
message instead: "Danger: There was an error submitting the form..." and
the redirect still happens.

This change treats exceptions where a message is passed in to handle as
recoverable as the exception has been caught and a user intended message
attached.

Closes-Bug: #1484723
Change-Id: I763f7606dde5df6cf09cbaf2c8e90158e0f256e3

horizon/exceptions.py

@@ -335,13 +335,14 @@ def handle(request, message=None, redirect=None, ignore=False,
     log_entry = encoding.force_text(exc_value)
 
     # We trust messages from our own exceptions
+    user_message = ""
     if issubclass(exc_type, HorizonException):
-        message = exc_value
+        user_message = exc_value
     # If the message has a placeholder for the exception, fill it in
     elif message and "%(exc)s" in message:
-        message = encoding.force_text(message) % {"exc": log_entry}
+        user_message = encoding.force_text(message) % {"exc": log_entry}
     if message:
-        message = encoding.force_text(message)
+        user_message = encoding.force_text(message)
 
     for exc_handler in HANDLE_EXC_METHODS:
         if issubclass(exc_type, exc_handler['exc']):
@@ -359,4 +360,13 @@ def handle(request, message=None, redirect=None, ignore=False,
     if wrap:
         raise HandledException([exc_type, exc_value, exc_traceback])
 
+    # assume exceptions handled in the code that pass in a message are already
+    # handled appropriately and treat as recoverable
+    if message:
+        ret = handle_recoverable(request, user_message, redirect, ignore,
+                                 escalate, handled, force_silence, force_log,
+                                 log_method, log_entry, log_level)
+        if ret:
+            return ret
+
     six.reraise(exc_type, exc_value, exc_traceback)

horizon/test/tests/exceptions.py

@@ -38,3 +38,25 @@ class HandleTests(test.TestCase):
         # UnicodeEncodeError, but making sure the message is correct could be
         # useful as well.
         self.assertItemsEqual(req.horizon['async_messages'], [expected])
+
+    def test_handle_message_as_recoverable(self):
+        # tests that if a message is passed to handle that it is treated
+        # like a recoverable exception
+
+        message = u"Couldn't make the thing"
+        exc_msg = u"Exception string"
+        req = self.request
+        req.META['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'
+
+        try:
+            raise Exception(exc_msg)
+        except Exception:
+            exceptions.handle(req, message)
+
+        # async_messages is a list of tuples, so [0][1] is getting to the
+        # message part of the first message. There should be only one message
+        # in this test case.
+        self.assertIn(message, req.horizon['async_messages'][0][1])
+        # verifying that the exec message which in this case is not trusted
+        # is not in the message content
+        self.assertNotIn(exc_msg, req.horizon['async_messages'][0][1])