Move task error handling to function
* Replaces large amounts of repeated code on the task view with a _handle_task_error() function * try and unify error handling to always return a list 'errors' or a dict when specific field name errors are known. Change-Id: I9d140db0af204524eadc4ba3a6d3eb7299b239b4
This commit is contained in:
parent
16be61428f
commit
dab3938ab2
|
@ -281,7 +281,7 @@ class UserRoles(tasks.TaskView):
|
|||
if errors:
|
||||
self.logger.info("(%s) - Validation errors with registration." %
|
||||
timezone.now())
|
||||
return Response(errors, status=status)
|
||||
return Response({'errors': errors}, status=status)
|
||||
|
||||
response_dict = {'notes': processed.get('notes')}
|
||||
|
||||
|
@ -493,7 +493,7 @@ class UpdateProjectQuotas(tasks.TaskView):
|
|||
if errors:
|
||||
self.logger.info("(%s) - Validation errors with task." %
|
||||
timezone.now())
|
||||
return Response(errors, status=status)
|
||||
return Response({'errors': errors}, status=status)
|
||||
|
||||
if processed.get('auto_approved', False):
|
||||
response_dict = {'notes': processed['notes']}
|
||||
|
|
|
@ -163,19 +163,12 @@ class TaskView(APIViewWithLogger):
|
|||
# Instantiate Task
|
||||
ip_address = request.META['REMOTE_ADDR']
|
||||
keystone_user = request.keystone_user
|
||||
try:
|
||||
task = Task.objects.create(
|
||||
ip_address=ip_address,
|
||||
keystone_user=keystone_user,
|
||||
project_id=keystone_user['project_id'],
|
||||
task_type=self.task_type,
|
||||
hash_key=hash_key)
|
||||
except KeyError:
|
||||
task = Task.objects.create(
|
||||
ip_address=ip_address,
|
||||
keystone_user=keystone_user,
|
||||
task_type=self.task_type,
|
||||
hash_key=hash_key)
|
||||
task = Task.objects.create(
|
||||
ip_address=ip_address,
|
||||
keystone_user=keystone_user,
|
||||
project_id=keystone_user.get('project_id'),
|
||||
task_type=self.task_type,
|
||||
hash_key=hash_key)
|
||||
task.save()
|
||||
|
||||
# Instantiate actions with serializers
|
||||
|
@ -195,24 +188,8 @@ class TaskView(APIViewWithLogger):
|
|||
try:
|
||||
action_instance.pre_approve()
|
||||
except Exception as e:
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical((
|
||||
"(%s) - Exception escaped! %s\nTrace: \n%s") % (
|
||||
timezone.now(), e, trace))
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while setting up task. " +
|
||||
"See task itself for details.") % e]
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the server. " +
|
||||
"It will be looked into shortly."]
|
||||
}
|
||||
return response_dict, 500
|
||||
return self._handle_task_error(
|
||||
e, task, error_text='while setting up task')
|
||||
|
||||
# send initial confirmation email:
|
||||
email_conf = class_conf.get('emails', {}).get('initial', None)
|
||||
|
@ -255,25 +232,8 @@ class TaskView(APIViewWithLogger):
|
|||
send_stage_email(task, email_conf, token)
|
||||
return {'notes': ['created token']}, 200
|
||||
except KeyError as e:
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical((
|
||||
"(%s) - Exception escaped! %s\nTrace: \n%s") % (
|
||||
timezone.now(), e, trace))
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while sending " +
|
||||
"token. See task " +
|
||||
"itself for details.") % e]
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the " +
|
||||
"server. It will be looked into shortly."]
|
||||
}
|
||||
return response_dict, 500
|
||||
return self._handle_task_error(
|
||||
e, task, error_text='while sending token')
|
||||
|
||||
def approve(self, request, task):
|
||||
"""
|
||||
|
@ -297,30 +257,15 @@ class TaskView(APIViewWithLogger):
|
|||
valid = all([act.valid for act in actions])
|
||||
if not valid:
|
||||
return {'errors': ['actions invalid']}, 400
|
||||
# TODO(amelia): get action invalidation reasons
|
||||
|
||||
# post_approve all actions
|
||||
for action in actions:
|
||||
try:
|
||||
action.post_approve()
|
||||
except Exception as e:
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical((
|
||||
"(%s) - Exception escaped! %s\nTrace: \n%s") % (
|
||||
timezone.now(), e, trace))
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while approving task. " +
|
||||
"See task itself for details.") % e]
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the server. " +
|
||||
"It will be looked into shortly."]
|
||||
}
|
||||
return response_dict, 500
|
||||
return self._handle_task_error(
|
||||
e, task, error_text='while approving task')
|
||||
|
||||
valid = all([act.valid for act in actions])
|
||||
if not valid:
|
||||
|
@ -335,25 +280,8 @@ class TaskView(APIViewWithLogger):
|
|||
try:
|
||||
action.submit({})
|
||||
except Exception as e:
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical((
|
||||
"(%s) - Exception escaped! %s\nTrace: \n%s") % (
|
||||
timezone.now(), e, trace))
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while submitting " +
|
||||
"task. See task " +
|
||||
"itself for details.") % e]
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the " +
|
||||
"server. It will be looked into shortly."]
|
||||
}
|
||||
return response_dict, 500
|
||||
self._handle_task_error(
|
||||
e, task, error_text='while submitting task')
|
||||
|
||||
task.completed = True
|
||||
task.completed_on = timezone.now()
|
||||
|
@ -403,7 +331,7 @@ class CreateProject(TaskView):
|
|||
if errors:
|
||||
self.logger.info("(%s) - Validation errors with task." %
|
||||
timezone.now())
|
||||
return Response(errors, status=status)
|
||||
return Response({'errors': errors}, status=status)
|
||||
|
||||
notes = {
|
||||
'notes':
|
||||
|
@ -452,8 +380,6 @@ class InviteUser(TaskView):
|
|||
self.logger.info("(%s) - Validation errors with task." %
|
||||
timezone.now())
|
||||
|
||||
if isinstance(errors, dict):
|
||||
return Response(errors, status=status)
|
||||
return Response({'errors': errors}, status=status)
|
||||
|
||||
response_dict = {'notes': processed['notes']}
|
||||
|
@ -499,7 +425,7 @@ class ResetPassword(TaskView):
|
|||
if errors:
|
||||
self.logger.info("(%s) - Validation errors with task." %
|
||||
timezone.now())
|
||||
return Response(errors, status=status)
|
||||
return Response({'errors': errors}, status=status)
|
||||
|
||||
task = processed['task']
|
||||
self.logger.info("(%s) - AutoApproving Resetuser request."
|
||||
|
@ -581,7 +507,7 @@ class EditUser(TaskView):
|
|||
if errors:
|
||||
self.logger.info("(%s) - Validation errors with task." %
|
||||
timezone.now())
|
||||
return Response(errors, status=status)
|
||||
return Response({'errors': errors}, status=status)
|
||||
|
||||
response_dict = {'notes': processed.get('notes')}
|
||||
add_task_id_for_roles(request, processed, response_dict, ['admin'])
|
||||
|
@ -609,7 +535,7 @@ class UpdateEmail(TaskView):
|
|||
if errors:
|
||||
self.logger.info("(%s) - Validation errors with task." %
|
||||
timezone.now())
|
||||
return Response(errors, status=status)
|
||||
return Response({'errors': errors}, status=status)
|
||||
|
||||
response_dict = {'notes': processed['notes']}
|
||||
return Response(response_dict, status=status)
|
||||
|
|
|
@ -1511,3 +1511,53 @@ class AdminAPITests(APITestCase):
|
|||
response.json()['notes'],
|
||||
['If user with email exists, reset token will be issued.'])
|
||||
self.assertEqual(0, Token.objects.count())
|
||||
|
||||
@mock.patch('adjutant.common.tests.fake_clients.FakeManager.find_project')
|
||||
def test_apiview_error_handler(self, mocked_find):
|
||||
"""
|
||||
Ensure the _handle_task_error function works as expected for APIViews.
|
||||
"""
|
||||
|
||||
setup_identity_cache()
|
||||
|
||||
url = "/v1/actions/CreateProject"
|
||||
data = {'project_name': "test_project", 'email': "test@example.com"}
|
||||
response = self.client.post(url, data, format='json')
|
||||
self.assertEqual(
|
||||
response.status_code, status.HTTP_200_OK)
|
||||
|
||||
mocked_find.side_effect = KeyError("Forced key error.")
|
||||
|
||||
new_task = Task.objects.all()[0]
|
||||
url = "/v1/tasks/" + new_task.uuid
|
||||
data = {
|
||||
'project_name': "test_project", 'email': "test@example.com",
|
||||
'region': 'test'
|
||||
}
|
||||
headers = {
|
||||
'project_name': "test_project",
|
||||
'project_id': "test_project_id",
|
||||
'roles': "admin,_member_",
|
||||
'username': "test@example.com",
|
||||
'user_id': "test_user_id",
|
||||
'authenticated': True
|
||||
}
|
||||
response = self.client.put(url, data, format='json', headers=headers)
|
||||
self.assertEqual(
|
||||
response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
|
||||
self.assertEqual(
|
||||
response.json()['errors'],
|
||||
["Error: Something went wrong on the server. " +
|
||||
"It will be looked into shortly."])
|
||||
|
||||
new_task = Task.objects.all()[0]
|
||||
new_notification = Notification.objects.all()[1]
|
||||
|
||||
self.assertTrue(new_notification.error)
|
||||
self.assertEqual(
|
||||
new_notification.notes,
|
||||
{'errors': [
|
||||
"Error: KeyError('Forced key error.') while updating task. " +
|
||||
"See task itself for details."]})
|
||||
self.assertEqual(new_notification.task, new_task)
|
||||
|
|
|
@ -20,7 +20,7 @@ from django.core import mail
|
|||
|
||||
from rest_framework import status
|
||||
|
||||
from adjutant.api.models import Task, Token
|
||||
from adjutant.api.models import Task, Token, Notification
|
||||
from adjutant.api.v1.tasks import CreateProject
|
||||
from adjutant.common.tests.fake_clients import (
|
||||
FakeManager, setup_identity_cache)
|
||||
|
@ -61,17 +61,19 @@ class TaskViewTests(AdjutantAPITestCase):
|
|||
'project_id': project.id}
|
||||
response = self.client.post(url, data, format='json', headers=headers)
|
||||
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(response.json(),
|
||||
{'email': ['This field is required.']})
|
||||
self.assertEqual(
|
||||
response.json(),
|
||||
{'errors': {'email': ['This field is required.']}})
|
||||
|
||||
data = {'email': "not_a_valid_email", 'roles': ["not_a_valid_role"],
|
||||
'project_id': project.id}
|
||||
response = self.client.post(url, data, format='json', headers=headers)
|
||||
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(
|
||||
response.json(), {
|
||||
response.json(),
|
||||
{'errors': {
|
||||
'email': ['Enter a valid email address.'],
|
||||
'roles': ['"not_a_valid_role" is not a valid choice.']})
|
||||
'roles': ['"not_a_valid_role" is not a valid choice.']}})
|
||||
|
||||
def test_new_user(self):
|
||||
"""
|
||||
|
@ -837,8 +839,9 @@ class TaskViewTests(AdjutantAPITestCase):
|
|||
response = self.client.post(url, data, format='json', headers=headers)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(response.json(),
|
||||
{'new_email': [u'Enter a valid email address.']})
|
||||
self.assertEqual(
|
||||
response.json(),
|
||||
{'errors': {'new_email': [u'Enter a valid email address.']}})
|
||||
|
||||
@override_settings(USERNAME_IS_EMAIL=True)
|
||||
def test_update_email_pre_existing_user_with_email(self):
|
||||
|
@ -867,7 +870,7 @@ class TaskViewTests(AdjutantAPITestCase):
|
|||
response = self.client.post(url, data, format='json', headers=headers)
|
||||
|
||||
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(response.json(), ['actions invalid'])
|
||||
self.assertEqual(response.json(), {'errors': ['actions invalid']})
|
||||
self.assertEqual(len(Token.objects.all()), 0)
|
||||
|
||||
self.assertEqual(len(mail.outbox), 0)
|
||||
|
@ -1398,3 +1401,35 @@ class TaskViewTests(AdjutantAPITestCase):
|
|||
actions = new_task.actions
|
||||
observed_action_names = [a.action_name for a in actions]
|
||||
self.assertEqual(observed_action_names, expected_action_names)
|
||||
|
||||
@mock.patch('adjutant.common.tests.fake_clients.FakeManager.find_project')
|
||||
def test_task_error_handler(self, mocked_find):
|
||||
"""
|
||||
Ensure the _handle_task_error function works as expected.
|
||||
"""
|
||||
|
||||
setup_identity_cache()
|
||||
|
||||
mocked_find.side_effect = KeyError("Forced key error.")
|
||||
|
||||
url = "/v1/actions/CreateProject"
|
||||
data = {'project_name': "test_project", 'email': "test@example.com"}
|
||||
response = self.client.post(url, data, format='json')
|
||||
self.assertEqual(
|
||||
response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
|
||||
self.assertEqual(
|
||||
response.json(),
|
||||
{'errors': ["Error: Something went wrong on the server. " +
|
||||
"It will be looked into shortly."]})
|
||||
|
||||
new_task = Task.objects.all()[0]
|
||||
new_notification = Notification.objects.all()[0]
|
||||
|
||||
self.assertTrue(new_notification.error)
|
||||
self.assertEqual(
|
||||
new_notification.notes,
|
||||
{'errors': [
|
||||
"Error: KeyError('Forced key error.') while setting up " +
|
||||
"task. See task itself for details."]})
|
||||
self.assertEqual(new_notification.task, new_task)
|
||||
|
|
|
@ -41,6 +41,29 @@ class APIViewWithLogger(APIView):
|
|||
super(APIViewWithLogger, self).__init__(*args, **kwargs)
|
||||
self.logger = getLogger('adjutant')
|
||||
|
||||
def _handle_task_error(self, e, task, error_text="while running task",
|
||||
return_response=False):
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical((
|
||||
"(%s) - Exception escaped! %s\nTrace: \n%s") % (
|
||||
timezone.now(), e, trace))
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: %s(%s) %s. See task " +
|
||||
"itself for details.") % (type(e).__name__, e, error_text)]
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the " +
|
||||
"server. It will be looked into shortly."]
|
||||
}
|
||||
if return_response:
|
||||
return Response(response_dict, status=500)
|
||||
return response_dict, 500
|
||||
|
||||
|
||||
class StatusView(APIViewWithLogger):
|
||||
|
||||
|
@ -101,9 +124,9 @@ class NotificationList(APIViewWithLogger):
|
|||
try:
|
||||
notifications = paginator.page(page)
|
||||
except EmptyPage:
|
||||
return Response({'error': 'Empty page'}, status=400)
|
||||
return Response({'errors': ['Empty page']}, status=400)
|
||||
except PageNotAnInteger:
|
||||
return Response({'error': 'Page not an integer'},
|
||||
return Response({'errors': ['Page not an integer']},
|
||||
status=400)
|
||||
|
||||
note_list = []
|
||||
|
@ -211,9 +234,9 @@ class TaskList(APIViewWithLogger):
|
|||
try:
|
||||
tasks = paginator.page(page)
|
||||
except EmptyPage:
|
||||
return Response({'error': 'Empty page'}, status=400)
|
||||
return Response({'errors': ['Empty page']}, status=400)
|
||||
except PageNotAnInteger:
|
||||
return Response({'error': 'Page not an integer'},
|
||||
return Response({'errors': ['Page not an integer']},
|
||||
status=400)
|
||||
task_list = []
|
||||
for task in tasks:
|
||||
|
@ -315,26 +338,8 @@ class TaskDetail(APIViewWithLogger):
|
|||
try:
|
||||
act['action'].get_action().pre_approve()
|
||||
except Exception as e:
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while updating task. " +
|
||||
"See task itself for details.") % e],
|
||||
'task': task.uuid
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical(("(%s) - Exception escaped! %s\n" +
|
||||
"Trace: \n%s") %
|
||||
(timezone.now(), e, trace))
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the server. " +
|
||||
"It will be looked into shortly."]
|
||||
}
|
||||
return Response(response_dict, status=500)
|
||||
return self._handle_task_error(
|
||||
e, task, "while updating task", return_response=True)
|
||||
|
||||
return Response(
|
||||
{'notes': ["Task successfully updated."]},
|
||||
|
@ -419,21 +424,8 @@ class TaskDetail(APIViewWithLogger):
|
|||
try:
|
||||
act_model.post_approve()
|
||||
except Exception as e:
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while approving task. " +
|
||||
"See task itself for details.") % e],
|
||||
'task': task.uuid
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical(("(%s) - Exception escaped! %s\n" +
|
||||
"Trace: \n%s") %
|
||||
(timezone.now(), e, trace))
|
||||
|
||||
return Response(notes, status=500)
|
||||
return self._handle_task_error(
|
||||
e, task, "while approving task", return_response=True)
|
||||
|
||||
if not action.valid:
|
||||
valid = False
|
||||
|
@ -454,48 +446,16 @@ class TaskDetail(APIViewWithLogger):
|
|||
return Response({'notes': ['created token']},
|
||||
status=200)
|
||||
except KeyError as e:
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while sending " +
|
||||
"token. See task " +
|
||||
"itself for details.") % e],
|
||||
'task': task.uuid
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical(("(%s) - Exception escaped!" +
|
||||
" %s\n Trace: \n%s") %
|
||||
(timezone.now(), e, trace))
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the " +
|
||||
"server. It will be looked into shortly."]
|
||||
}
|
||||
return Response(response_dict, status=500)
|
||||
return self._handle_task_error(
|
||||
e, task, "while sending token", return_response=True)
|
||||
else:
|
||||
for action in actions:
|
||||
try:
|
||||
action.submit({})
|
||||
except Exception as e:
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while submitting " +
|
||||
"task. See task " +
|
||||
"itself for details.") % e],
|
||||
'task': task.uuid
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical(("(%s) - Exception escaped!" +
|
||||
" %s\n Trace: \n%s") %
|
||||
(timezone.now(), e, trace))
|
||||
|
||||
return Response(notes, status=500)
|
||||
return self._handle_task_error(
|
||||
e, task, "while submitting task",
|
||||
return_response=True)
|
||||
|
||||
task.completed = True
|
||||
task.completed_on = timezone.now()
|
||||
|
@ -582,7 +542,7 @@ class TokenList(APIViewWithLogger):
|
|||
uuid = request.data.get('task', None)
|
||||
if uuid is None:
|
||||
return Response(
|
||||
{'task': ["This field is required.", ]},
|
||||
{'errors': {'task': ["This field is required.", ]}},
|
||||
status=400)
|
||||
try:
|
||||
if 'admin' in request.keystone_user['roles']:
|
||||
|
@ -625,27 +585,8 @@ class TokenList(APIViewWithLogger):
|
|||
email_conf = class_conf['emails']['token']
|
||||
send_stage_email(task, email_conf, token)
|
||||
except KeyError as e:
|
||||
notes = {
|
||||
'errors': [
|
||||
("Error: '%(error)s' while sending token. " +
|
||||
"See registration itself for details.") % {'error': e}
|
||||
],
|
||||
'task': task.uuid
|
||||
}
|
||||
create_notification(task, notes, error=True)
|
||||
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical(("(%s) - Exception escaped!" +
|
||||
" %s\n Trace: \n%s") %
|
||||
(timezone.now(), e, trace))
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the " +
|
||||
"server. It will be looked into shortly."]
|
||||
}
|
||||
return Response(response_dict, status=500)
|
||||
return self._handle_task_error(
|
||||
e, task, "while sending token", return_response=True)
|
||||
return Response(
|
||||
{'notes': ['Token reissued.']}, status=200)
|
||||
|
||||
|
@ -764,26 +705,9 @@ class TokenDetail(APIViewWithLogger):
|
|||
valid = False
|
||||
|
||||
except Exception as e:
|
||||
notes = {
|
||||
'errors':
|
||||
[("Error: '%s' while submitting task. " +
|
||||
"See task itself for details.") % e],
|
||||
'task': token.task.uuid
|
||||
}
|
||||
create_notification(token.task, notes, error=True)
|
||||
|
||||
import traceback
|
||||
trace = traceback.format_exc()
|
||||
self.logger.critical(("(%s) - Exception escaped! %s\n" +
|
||||
"Trace: \n%s") %
|
||||
(timezone.now(), e, trace))
|
||||
|
||||
response_dict = {
|
||||
'errors':
|
||||
["Error: Something went wrong on the server. " +
|
||||
"It will be looked into shortly."]
|
||||
}
|
||||
return Response(response_dict, status=500)
|
||||
return self._handle_task_error(
|
||||
e, token.task, "while submiting task",
|
||||
return_response=True)
|
||||
|
||||
if not valid:
|
||||
return Response({"errors": ["Actions invalid"]}, status=400)
|
||||
|
|
Loading…
Reference in New Issue