Merge "Refactor error messages."

horizon/exceptions.py

@@ -260,8 +260,13 @@ HANDLE_EXC_METHODS = [
 ]
 
 
+def _append_detail(message, details):
+    return encoding.force_text(message) + '\u2026' + \
+        encoding.force_text(details)
+
+
 def handle(request, message=None, redirect=None, ignore=False,
-           escalate=False, log_level=None, force_log=None):
+           escalate=False, log_level=None, force_log=None, details=None):
     """Centralized error handling for Horizon.
 
     Because Horizon consumes so many different APIs with completely
@@ -288,6 +293,9 @@ def handle(request, message=None, redirect=None, ignore=False,
     If the exception is not re-raised, an appropriate wrapper exception
     class indicating the type of exception that was encountered will be
     returned.
+    If details is None (default), take it from exception sys.exc_info.
+    If details is other string, then use that string explicitly or if details
+    is empty then suppress it.
     """
     exc_type, exc_value, exc_traceback = sys.exc_info()
     log_method = getattr(LOG, log_level or "exception")
@@ -316,6 +324,10 @@ def handle(request, message=None, redirect=None, ignore=False,
         user_message = encoding.force_text(message) % {"exc": log_entry}
     elif message:
         user_message = encoding.force_text(message)
+    if details is None:
+        user_message = _append_detail(user_message, exc_value)
+    elif details:
+        user_message = _append_detail(user_message, details)
 
     for exc_handler in HANDLE_EXC_METHODS:
         if issubclass(exc_type, exc_handler['exc']):

horizon/static/horizon/js/horizon.messages.js

@@ -12,8 +12,11 @@
  * under the License.
  */
 
-horizon.alert = function (type, message, extra_tags) {
+horizon.alert = function (type, message, extra_tags, details) {
   var safe = false;
+  var arr = extractDetail(message);
+  var message = arr[0];
+  var details = arr[1];
   // Check if the message is tagged as safe.
   if (typeof(extra_tags) !== "undefined" && $.inArray('safe', extra_tags.split(' ')) !== -1) {
     safe = true;
@@ -32,12 +35,17 @@ horizon.alert = function (type, message, extra_tags) {
     type = 'danger';
   }
 
+  function extractDetail(str) {
+    return str.split('\u2026');
+  }
+
   var template = horizon.templates.compiled_templates["#alert_message_template"],
     params = {
       "type": type || 'default',
       "type_display": type_display,
       "message": message,
-      "safe": safe
+      "safe": safe,
+      "details": details
     };
   var this_alert = $(template.render(params)).hide().prependTo("#main_content .messages").fadeIn(100);
   horizon.autoDismissAlert(this_alert);

horizon/templates/horizon/_messages.html

@@ -1,4 +1,4 @@
-{% load i18n %}
+{% load i18n splitfilter %}
 <div class="messages">
 <toast></toast>
 {% for message in messages %}
@@ -31,8 +31,19 @@
       <a class="close" data-dismiss="alert" href="#">
         <span class="fa fa-times"></span>
       </a>
-      <p><strong>{% trans "Error: " %}</strong>{{ message }}</p>
+      <p>
+        {% with message.message|split_message as messages %}
+        <strong>{% trans "Error: " %}</strong>{{ messages|first }}
+          {% if messages|length > 1 %}
+            <a href="#message_details"  data-toggle="collapse"
+            data-target="#message_details">Details</a>
+            <div id="message_details" class="collapse">
+              {{ messages|last }}
+            </div>
+          {% endif %}
+      </p>
     </div>
+    {% endwith %}
   {% endif %}
 {% endfor %}
-</div>
+</div>

horizon/templates/horizon/client_side/_alert_message.html

@@ -15,6 +15,10 @@
     [[/safe]]
     [[^safe]]
       [[message]]
+      <a href="#message_details"  data-toggle="collapse" data-target="#message_details">Details</a>
+      <div id="message_details" class="collapse">
+      [[details]]
+      </div>
     [[/safe]]
   </p>
 </div>

horizon/templatetags/splitfilter.py

@@ -0,0 +1,20 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from django import template
+
+register = template.Library()
+
+
+@register.filter(name='split_message')
+def split_message(value):
+    return value.split('\u2026')

horizon/test/unit/test_exceptions.py

@@ -25,7 +25,8 @@ class HandleTests(test.TestCase):
         # Japanese translation of:
         # 'Because the container is not empty, it can not be deleted.'
 
-        expected = ['error', force_text(translated_unicode), '']
+        expected = ['error', force_text(translated_unicode +
+                                        '\u2026' + translated_unicode), '']
 
         req = self.request
         req.META['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'
@@ -57,6 +58,35 @@ class HandleTests(test.TestCase):
         # 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])
-        # verifies 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])
+        # verifies that the exec message which in this case is in the
+        # message content.
+        self.assertIn(exc_msg, req.horizon['async_messages'][0][1])
+
+    def test_handle_exception_with_empty_details(self):
+        message = u"Couldn't make the thing"
+        details = ""
+        expected = ['error', message, '']
+        req = self.request
+        req.META['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'
+
+        try:
+            raise Exception(message)
+        except Exception:
+            exceptions.handle(req, message, details=details)
+
+        self.assertCountEqual(req.horizon['async_messages'], [expected])
+
+    def test_handle_exception_with_details(self):
+        message = u"Couldn't make the thing"
+        exc_msg = u"Exception string"
+        details = "custom detail message"
+        expected = ['error', message + '\u2026' + details, '']
+        req = self.request
+        req.META['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest'
+
+        try:
+            raise Exception(exc_msg)
+        except Exception:
+            exceptions.handle(req, message, details=details)
+
+        self.assertCountEqual(req.horizon['async_messages'], [expected])

releasenotes/notes/refactor-error-messages-26c53bb3fe57fe72.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    [`blueprint refactor-error-messages <https://blueprints.launchpad.net/horizon/+spec/refactor-error-messages>`_]
+    User can see detailed error message on the horizon UI.
+    This blueprint adds a hyperlink "details" in the alert box.
+    So now when an exception occurs a user can click on this hyperlink
+    "details" which shows original error message included in a corresponding
+    error. This may help a user to understand what happens in detail.