x/tuskar-ui
Code Issues Proposed changes

Centralized error handling.

Browse Source 
Implements blueprint exception-handling.

This takes care of a first-pass implementation at reusable,
consistent, centralized error handling.

It is known to be incomplete as far as the "recognized" exception
types. It needs to be expanded over time as it is further
put into real use and tested. This is only a starting point.

Change-Id: If19e7c1414456f1be69ad867995f46368749a9e9
This commit is contained in:
Gabriel Hurley 2012-01-12 19:37:15 -08:00
parent 61650cec97
commit 7fe56c5aa8
12 changed files with 260 additions and 156 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
horizon/horizon/dashboards/nova/access_and_security/security_groups/tests.py
View File

@ -211,7 +211,7 @@ class SecurityGroupsViewTests(test.BaseViewTests):
self.assertRedirectsNoFollow(res, SG_EDIT_RULE_URL)
def test_edit_rules_delete_rule(self):
RULE_ID = '1'
RULE_ID = 1
self.mox.StubOutWithMock(api, 'security_group_rule_delete')
api.security_group_rule_delete(IsA(http.HttpRequest), RULE_ID)
@ -227,7 +227,7 @@ class SecurityGroupsViewTests(test.BaseViewTests):
SG_EDIT_RULE_URL)
def test_edit_rules_delete_rule_exception(self):
RULE_ID = '1'
RULE_ID = 1
self.mox.StubOutWithMock(api, 'security_group_rule_delete')
@ -248,7 +248,7 @@ class SecurityGroupsViewTests(test.BaseViewTests):
def test_delete_group(self):
self.mox.StubOutWithMock(api, 'security_group_delete')
api.security_group_delete(IsA(http.HttpRequest), '2')
api.security_group_delete(IsA(http.HttpRequest), 2)
self.mox.ReplayAll()
@ -264,7 +264,7 @@ class SecurityGroupsViewTests(test.BaseViewTests):
self.mox.StubOutWithMock(api, 'security_group_delete')
exception = novaclient_exceptions.ClientException('ClientException',
message='ClientException')
api.security_group_delete(IsA(http.HttpRequest), '2').\
api.security_group_delete(IsA(http.HttpRequest), 2).\
AndRaise(exception)
self.mox.ReplayAll()

9
horizon/horizon/dashboards/nova/images_and_snapshots/images/forms.py
View File

@ -29,9 +29,9 @@ from django.shortcuts import redirect
from django.utils.text import normalize_newlines
from django.utils.translation import ugettext as _
from glance.common import exception as glance_exception
from openstackx.api import exceptions as api_exceptions
from horizon import api
from horizon import exceptions
from horizon import forms
LOG = logging.getLogger(__name__)
@ -166,11 +166,8 @@ class LaunchForm(forms.SelfHandlingForm):
return redirect(
'horizon:nova:instances_and_volumes:instances:index')
except api_exceptions.ApiException, e:
LOG.exception('ApiException while creating instances of image "%s"'
% image_id)
messages.error(request,
_('Unable to launch instance: %s') % e.message)
except:
exceptions.handle(request, _('Unable to launch instance.'))
class DeleteImage(forms.SelfHandlingForm):

35
horizon/horizon/dashboards/nova/images_and_snapshots/images/tests.py
View File

@ -22,7 +22,7 @@ from django import http
from django.contrib import messages
from django.core.urlresolvers import reverse
from glance.common import exception as glance_exception
from openstackx.api import exceptions as api_exceptions
from keystoneclient import exceptions as keystone_exceptions
from mox import IgnoreArg, IsA
from horizon import api
@ -182,7 +182,7 @@ class ImageViewTests(test.BaseViewTests):
api.tenant_quota_get(IsA(http.HttpRequest),
self.TEST_TENANT).AndReturn(FakeQuota)
exception = api_exceptions.ApiException('apiException')
exception = keystone_exceptions.ClientException('Failed.')
self.mox.StubOutWithMock(api, 'flavor_list')
api.flavor_list(IsA(http.HttpRequest)).AndRaise(exception)
@ -221,7 +221,7 @@ class ImageViewTests(test.BaseViewTests):
self.mox.StubOutWithMock(api, 'flavor_list')
api.flavor_list(IsA(http.HttpRequest)).AndReturn(self.flavors)
exception = api_exceptions.ApiException('apiException')
exception = keystone_exceptions.ClientException('Failed.')
self.mox.StubOutWithMock(api, 'keypair_list')
api.keypair_list(IsA(http.HttpRequest)).AndRaise(exception)
@ -243,13 +243,21 @@ class ImageViewTests(test.BaseViewTests):
form_keyfield = form.fields['key_name']
self.assertEqual(len(form_keyfield.choices), 0)
def test_launch_form_apiexception(self):
def test_launch_form_keystone_exception(self):
FLAVOR_ID = self.flavors[0].id
IMAGE_ID = '1'
KEY_NAME = self.keypairs[0].name
SERVER_NAME = 'serverName'
USER_DATA = 'userData'
self.mox.StubOutWithMock(api, 'image_get_meta')
self.mox.StubOutWithMock(api, 'tenant_quota_get')
self.mox.StubOutWithMock(api, 'flavor_list')
self.mox.StubOutWithMock(api, 'keypair_list')
self.mox.StubOutWithMock(api, 'security_group_list')
self.mox.StubOutWithMock(api, 'flavor_get')
self.mox.StubOutWithMock(api, 'server_create')
form_data = {'method': 'LaunchForm',
'flavor': FLAVOR_ID,
'image_id': IMAGE_ID,
@ -260,39 +268,26 @@ class ImageViewTests(test.BaseViewTests):
'security_groups': 'default',
}
self.mox.StubOutWithMock(api, 'image_get_meta')
api.image_get_meta(IgnoreArg(),
IMAGE_ID).AndReturn(self.visibleImage)
self.mox.StubOutWithMock(api, 'tenant_quota_get')
api.tenant_quota_get(IsA(http.HttpRequest),
self.TEST_TENANT).AndReturn(FakeQuota)
self.mox.StubOutWithMock(api, 'flavor_list')
api.flavor_list(IgnoreArg()).AndReturn(self.flavors)
self.mox.StubOutWithMock(api, 'keypair_list')
api.keypair_list(IgnoreArg()).AndReturn(self.keypairs)
self.mox.StubOutWithMock(api, 'security_group_list')
api.security_group_list(IsA(http.HttpRequest)).AndReturn(
self.security_groups)
# called again by the form
api.image_get_meta(IgnoreArg(),
IMAGE_ID).AndReturn(self.visibleImage)
self.mox.StubOutWithMock(api, 'flavor_get')
api.flavor_get(IgnoreArg(),
IsA(unicode)).AndReturn(self.flavors[0])
self.mox.StubOutWithMock(api, 'server_create')
exception = api_exceptions.ApiException('apiException')
exception = keystone_exceptions.ClientException('Failed')
api.server_create(IsA(http.HttpRequest), SERVER_NAME,
self.visibleImage, self.flavors[0],
KEY_NAME, USER_DATA,
self.security_groups).AndRaise(exception)
[group.name for group in self.security_groups]) \
.AndRaise(exception)
self.mox.StubOutWithMock(messages, 'error')
messages.error(IsA(http.HttpRequest), IsA(basestring))

15
horizon/horizon/dashboards/nova/images_and_snapshots/images/views.py
View File

@ -33,8 +33,8 @@ from novaclient import exceptions as novaclient_exceptions
from openstackx.api import exceptions as api_exceptions
from horizon import api
from horizon.dashboards.nova.images_and_snapshots.images.forms import \
(UpdateImageForm, LaunchForm, DeleteImage)
from horizon import exceptions
from .forms import UpdateImageForm, LaunchForm, DeleteImage
LOG = logging.getLogger(__name__)
@ -76,7 +76,6 @@ def index(request):
@login_required
def launch(request, image_id):
def flavorlist():
try:
fl = api.flavor_list(request)
@ -85,8 +84,9 @@ def launch(request, image_id):
sel = [(f.id, '%s (%svcpu / %sGB Disk / %sMB Ram )' %
(f.name, f.vcpus, f.disk, f.ram)) for f in fl]
return sorted(sel)
except api_exceptions.ApiException:
LOG.exception('Unable to retrieve list of instance types')
except:
exceptions.handle(request,
_('Unable to retrieve list of instance types'))
return [(1, 'm1.tiny')]
def keynamelist():
@ -94,8 +94,9 @@ def launch(request, image_id):
fl = api.keypair_list(request)
sel = [(f.name, f.name) for f in fl]
return sel
except api_exceptions.ApiException:
LOG.exception('Unable to retrieve list of keypairs')
except:
exceptions.handle(request,
_('Unable to retrieve list of keypairs'))
return []
def securitygrouplist():

125
horizon/horizon/exceptions.py
View File

@ -14,7 +14,48 @@
# License for the specific language governing permissions and limitations
# under the License.
""" Exceptions raised by the Horizon code. """
"""
Exceptions raised by the Horizon code and the machinery for handling them.
"""
from __future__ import absolute_import
import logging
import sys
from django.contrib import messages
from keystoneclient import exceptions as keystoneclient
from novaclient import exceptions as novaclient
from openstackx.api import exceptions as openstackx
LOG = logging.getLogger(__name__)
UNAUTHORIZED = (openstackx.Unauthorized,
openstackx.Unauthorized,
keystoneclient.Unauthorized,
keystoneclient.Forbidden,
novaclient.Unauthorized,
novaclient.Forbidden)
NOT_FOUND = (keystoneclient.NotFound,
novaclient.NotFound,
openstackx.NotFound)
# NOTE(gabriel): This is very broad, and may need to be dialed in.
RECOVERABLE = (keystoneclient.ClientException,
novaclient.ClientException,
openstackx.ApiException)
class Http302(Exception):
"""
Error class which can be raised from within a handler to cause an
early bailout and redirect at the middleware level.
"""
def __init__(self, location):
self.location = location
class NotAuthorized(Exception):
@ -38,3 +79,85 @@ class ServiceCatalogException(Exception):
def __init__(self, service_name):
message = 'Invalid service catalog service: %s' % service_name
super(ServiceCatalogException, self).__init__(message)
class HandledException(Exception):
"""
Used internally to track exceptions that have gone through
:func:`horizon.exceptions.handle` more than once.
"""
def __init__(self, wrapped):
self.wrapped = wrapped
def handle(request, message=None, redirect=None, ignore=False, escalate=False):
""" Centralized error handling for Horizon.
Because Horizon consumes so many different APIs with completely
different ``Exception`` types, it's necessary to have a centralized
place for handling exceptions which may be raised.
Exceptions are roughly divided into 3 types:
#. ``UNAUTHORIZED``: Errors resulting from authentication or authorization
problems. These result in being logged out and sent to the login screen.
#. ``NOT_FOUND``: Errors resulting from objects which could not be
located via the API. These generally result in a user-facing error
message, but are otherwise returned to the normal code flow. Optionally
a redirect value may be passed to the error handler so users are
returned to a different view than the one requested in addition to the
error message.
#. RECOVERABLE: Generic API errors which generate a user-facing message
but drop directly back to the regular code flow.
All other exceptions bubble the stack as normal unless the ``ignore``
argument is passed in as ``True``, in which case only unrecognized
errors are
"""
exc_type, exc_value, exc_traceback = sys.exc_info()
# Because the same exception may travel through this method more than
# once (if it's re-raised) we may want to treat it differently
# the second time (e.g. no user messages/logging).
handled = issubclass(exc_type, HandledException)
wrap = False
# Restore our original exception information, but re-wrap it at the end
if handled:
exc_type, exc_value, exc_traceback = exc_value.wrapped
wrap = True
if issubclass(exc_type, UNAUTHORIZED):
if ignore:
return
request.session.clear()
if not handled:
LOG.debug("Unauthorized: %s" % exc_value)
# We get some pretty useless error messages back from
# some clients, so let's define our own fallback.
fallback = _("Unauthorized. Please try logging in again.")
messages.error(request, message or fallback, extra_tags="login")
raise NotAuthorized # Redirect handled in middleware
if issubclass(exc_type, NOT_FOUND):
if not ignore and not handled:
LOG.debug("Not Found: %s" % exc_value)
messages.error(request, message or exc_value)
if redirect:
raise Http302(redirect)
wrap = True
if not escalate:
return # return to normal code flow
if issubclass(exc_type, RECOVERABLE) and not ignore:
if not ignore and not handled:
LOG.debug("Recoverable error: %s" % exc_value)
messages.error(request, message or exc_value)
wrap = True
if not escalate:
return # return to normal code flow
# If we've gotten here, time to wrap and/or raise our exception.
if wrap:
raise HandledException([exc_type, exc_value, exc_traceback])
raise exc_type, exc_value, exc_traceback

10
horizon/horizon/forms/base.py
View File

@ -209,14 +209,8 @@ class SelfHandlingForm(Form):
try:
return form, form.handle(request, data)
except Exception as e:
LOG.exception('Error while handling form "%s".' % cls.__name__)
if issubclass(e.__class__, exceptions.NotAuthorized):
# Let the middleware handle it as intended.
raise
if not hasattr(e, 'message'):
e.message = str(e)
messages.error(request, _('%s') % e.message)
except:
exceptions.handle(request)
return form, None

21
horizon/horizon/middleware.py
View File

@ -23,9 +23,6 @@ Middleware provided and used by Horizon.
from django.contrib import messages
from django import shortcuts
from django.utils.translation import ugettext as _
import openstackx
from horizon import exceptions
from horizon import users
@ -46,16 +43,10 @@ class HorizonMiddleware(object):
request.horizon = {'dashboard': None, 'panel': None}
def process_exception(self, request, exception):
""" Catch NotAuthorized and handle it gracefully. """
if issubclass(exception.__class__, exceptions.NotAuthorized):
messages.error(request, _(unicode(exception)))
return shortcuts.redirect('/auth/logout')
""" Catch NotAuthorized and Http302 and handle them gracefully. """
if isinstance(exception, exceptions.NotAuthorized):
messages.error(request, unicode(exception))
return shortcuts.redirect('/auth/login')
if type(exception) == openstackx.api.exceptions.Forbidden:
# flush other error messages, which are collateral damage
# when our token expires
for message in messages.get_messages(request):
pass
messages.error(request,
_('Your token has expired. Please log in again'))
return shortcuts.redirect('/auth/logout')
if isinstance(exception, exceptions.Http302):
return shortcuts.redirect(exception.location)

9
horizon/horizon/tables/actions.py
View File

@ -23,6 +23,8 @@ from django.contrib import messages
from django.core import urlresolvers
from django.utils.translation import string_concat
from horizon import exceptions
LOG = logging.getLogger(__name__)
@ -382,10 +384,11 @@ class BatchAction(Action):
action_success.append(datum_display)
LOG.info('%s: "%s"' %
(self._conjugate(past=True), datum_display))
except Exception, e:
except:
action_str = self._conjugate().lower()
exceptions.handle(request,
_("Unable to %s.") % action_str)
action_failure.append(datum_display)
LOG.exception("Unable to %s: %s" %
(self._conjugate().lower(), e))
#Begin with success message class, downgrade to info if problems
success_message_level = messages.success

33
horizon/horizon/tests/auth_tests.py
View File

@ -21,7 +21,7 @@
from django import http
from django.contrib import messages
from django.core.urlresolvers import reverse
from openstackx.api import exceptions as api_exceptions
from keystoneclient import exceptions as keystone_exceptions
from mox import IsA
from horizon import api
@ -131,34 +131,35 @@ class AuthViewTests(test.BaseViewTests):
self.assertRedirectsNoFollow(res, DASH_INDEX_URL)
def test_login_invalid_credentials(self):
form_data = {'method': 'Login',
'password': self.PASSWORD,
'username': self.TEST_USER}
self.mox.StubOutWithMock(api, 'token_create')
unauthorized = api_exceptions.Unauthorized('unauth', message='unauth')
unauthorized = keystone_exceptions.Unauthorized("Invalid")
api.token_create(IsA(http.HttpRequest), "", self.TEST_USER,
self.PASSWORD).AndRaise(unauthorized)
self.mox.ReplayAll()
res = self.client.post(reverse('horizon:auth_login'), form_data)
form_data = {'method': 'Login',
'password': self.PASSWORD,
'username': self.TEST_USER}
res = self.client.post(reverse('horizon:auth_login'),
form_data,
follow=True)
self.assertTemplateUsed(res, 'splash.html')
def test_login_exception(self):
form_data = {'method': 'Login',
'password': self.PASSWORD,
'username': self.TEST_USER}
self.mox.StubOutWithMock(api, 'token_create')
api_exception = api_exceptions.ApiException('apiException',
message='apiException')
api.token_create(IsA(http.HttpRequest), "", self.TEST_USER,
self.PASSWORD).AndRaise(api_exception)
ex = keystone_exceptions.BadRequest('Cannot talk to keystone')
api.token_create(IsA(http.HttpRequest),
"",
self.TEST_USER,
self.PASSWORD).AndRaise(ex)
self.mox.ReplayAll()
form_data = {'method': 'Login',
'password': self.PASSWORD,
'username': self.TEST_USER}
res = self.client.post(reverse('horizon:auth_login'), form_data)
self.assertTemplateUsed(res, 'splash.html')
@ -167,7 +168,7 @@ class AuthViewTests(test.BaseViewTests):
res = self.client.get(reverse('horizon:auth_switch',
args=[self.TEST_TENANT]))
self.assertTemplateUsed(res, 'switch_tenants.html')
self.assertRedirects(res, reverse("horizon:auth_login"))
def test_switch_tenants(self):
NEW_TENANT_ID = '6'

13
horizon/horizon/views/auth.py
View File

@ -53,7 +53,7 @@ def login(request):
if handled:
return handled
# FIXME(gabriel): we don't ship a view named splash
# FIXME(gabriel): we don't ship a template named splash.html
return shortcuts.render(request, 'splash.html', {'form': form})
@ -77,14 +77,11 @@ def switch_tenants(request, tenant_id):
_set_session_data(request, token)
user = users.User(users.get_user_from_request(request))
return shortcuts.redirect(Horizon.get_user_home(user))
except exceptions.Unauthorized as e:
messages.error(_("You are not authorized for that tenant."))
except Exception, e:
exceptions.handle(request,
_("You are not authorized for that tenant."))
# FIXME(gabriel): we don't ship switch_tenants.html
return shortcuts.render(request,
'switch_tenants.html', {
'to_tenant': tenant_id,
'form': form})
return shortcuts.redirect("horizon:auth_login")
def logout(request):

136
horizon/horizon/views/auth_forms.py
View File

@ -27,7 +27,6 @@ import logging
from django import shortcuts
from django.contrib import messages
from django.utils.translation import ugettext as _
from openstackx.api import exceptions as api_exceptions
from keystoneclient import exceptions as keystone_exceptions
from horizon import api
@ -64,81 +63,82 @@ class Login(forms.SelfHandlingForm):
widget=forms.PasswordInput(render_value=False))
def handle(self, request, data):
try:
if data.get('tenant', None):
if data.get('tenant', None):
try:
token = api.token_create(request,
data.get('tenant'),
data['username'],
data['password'])
tenants = api.tenant_list_for_token(request, token.id)
tenant = None
for t in tenants:
if t.id == data.get('tenant'):
tenant = t
_set_session_data(request, token)
user = users.get_user_from_request(request)
return shortcuts.redirect(base.Horizon.get_user_home(user))
except Exception, e:
exceptions.handle(request,
message=_('Error authenticating: %s') % e,
escalate=True)
tenant = None
for t in tenants:
if t.id == data.get('tenant'):
tenant = t
_set_session_data(request, token)
user = users.get_user_from_request(request)
return shortcuts.redirect(base.Horizon.get_user_home(user))
elif data.get('username', None):
elif data.get('username', None):
try:
token = api.token_create(request,
'',
data['username'],
data['password'])
except keystone_exceptions.Unauthorized:
exceptions.handle(request,
_('Invalid user name or password.'))
except:
exceptions.handle(request, escalate=True)
# Unscoped token
request.session['unscoped_token'] = token.id
request.user.username = data['username']
# Get the tenant list, and log in using first tenant
# FIXME (anthony): add tenant chooser here?
try:
tenants = api.tenant_list_for_token(request, token.id)
except:
exceptions.handle(request)
tenants = []
# Abort if there are no valid tenants for this user
if not tenants:
messages.error(request,
_('No tenants present for user: %(user)s') %
{"user": data['username']},
extra_tags="login")
return
# Create a token.
# NOTE(gabriel): Keystone can return tenants that you're
# authorized to administer but not to log into as a user, so in
# the case of an Unauthorized error we should iterate through
# the tenants until one succeeds or we've failed them all.
while tenants:
tenant = tenants.pop()
try:
token = api.token_create(request,
'',
data['username'],
data['password'])
except keystone_exceptions.Unauthorized:
LOG.exception("Failed login attempt for %s."
% data['username'])
messages.error(request, _('Bad user name or password.'),
extra_tags="login")
return
token = api.token_create_scoped(request,
tenant.id,
token.id)
break
except:
# This will continue for recognized "unauthorized"
# exceptions from keystoneclient.
exceptions.handle(request, ignore=True)
token = None
if token is None:
raise exceptions.NotAuthorized(
_("You are not authorized for any available tenants."))
# Unscoped token
request.session['unscoped_token'] = token.id
request.user.username = data['username']
# Get the tenant list, and log in using first tenant
# FIXME (anthony): add tenant chooser here?
tenants = api.tenant_list_for_token(request, token.id)
# Abort if there are no valid tenants for this user
if not tenants:
messages.error(request,
_('No tenants present for user: %(user)s') %
{"user": data['username']},
extra_tags="login")
return
# Create a token.
# NOTE(gabriel): Keystone can return tenants that you're
# authorized to administer but not to log into as a user, so in
# the case of an Unauthorized error we should iterate through
# the tenants until one succeeds or we've failed them all.
while tenants:
tenant = tenants.pop()
try:
token = api.token_create_scoped(request,
tenant.id,
token.id)
break
except api_exceptions.Unauthorized as e:
token = None
if token is None:
raise exceptions.NotAuthorized(
_("You are not authorized for any available tenants."))
_set_session_data(request, token)
user = users.get_user_from_request(request)
return shortcuts.redirect(base.Horizon.get_user_home(user))
except api_exceptions.Unauthorized as e:
msg = _('Error authenticating: %s') % e.message
LOG.exception(msg)
messages.error(request, msg, extra_tags="login")
except api_exceptions.ApiException as e:
messages.error(request,
_('Error authenticating with keystone: %s') %
e.message, extra_tags="login")
_set_session_data(request, token)
user = users.get_user_from_request(request)
return shortcuts.redirect(base.Horizon.get_user_home(user))
class LoginWithTenant(Login):

2
openstack-dashboard/dashboard/views.py
View File

@ -41,6 +41,8 @@ def user_home(user):
@vary.vary_on_cookie
def splash(request):
if request.user.is_authenticated():
return shortcuts.redirect(user_home(request.user))
form, handled = auth_views.Login.maybe_handle(request)
if handled:
return handled