diff --git a/horizon/exceptions.py b/horizon/exceptions.py index 9b523a6bca..069424cca3 100644 --- a/horizon/exceptions.py +++ b/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']): diff --git a/horizon/static/horizon/js/horizon.messages.js b/horizon/static/horizon/js/horizon.messages.js index 6d35418144..444d0ccecb 100644 --- a/horizon/static/horizon/js/horizon.messages.js +++ b/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); diff --git a/horizon/templates/horizon/_messages.html b/horizon/templates/horizon/_messages.html index d1c1ca77cd..7d19a48a21 100644 --- a/horizon/templates/horizon/_messages.html +++ b/horizon/templates/horizon/_messages.html @@ -1,4 +1,4 @@ -{% load i18n %} +{% load i18n splitfilter %}
{% for message in messages %} @@ -31,8 +31,19 @@ -

{% trans "Error: " %}{{ message }}

+

+ {% with message.message|split_message as messages %} + {% trans "Error: " %}{{ messages|first }} + {% if messages|length > 1 %} + Details +

+ {{ messages|last }} +
+ {% endif %} +

+ {% endwith %} {% endif %} {% endfor %} - \ No newline at end of file + diff --git a/horizon/templates/horizon/client_side/_alert_message.html b/horizon/templates/horizon/client_side/_alert_message.html index f63899d2c3..5f008ed5db 100644 --- a/horizon/templates/horizon/client_side/_alert_message.html +++ b/horizon/templates/horizon/client_side/_alert_message.html @@ -15,6 +15,10 @@ [[/safe]] [[^safe]] [[message]] + Details +
+ [[details]] +
[[/safe]]

diff --git a/horizon/templatetags/splitfilter.py b/horizon/templatetags/splitfilter.py new file mode 100644 index 0000000000..f4055a359f --- /dev/null +++ b/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') diff --git a/horizon/test/unit/test_exceptions.py b/horizon/test/unit/test_exceptions.py index f676f6bfa9..286abd78a2 100644 --- a/horizon/test/unit/test_exceptions.py +++ b/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]) diff --git a/releasenotes/notes/refactor-error-messages-26c53bb3fe57fe72.yaml b/releasenotes/notes/refactor-error-messages-26c53bb3fe57fe72.yaml new file mode 100644 index 0000000000..ebf5e60ab2 --- /dev/null +++ b/releasenotes/notes/refactor-error-messages-26c53bb3fe57fe72.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + [`blueprint 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.