From 4f33b6f406611c73aab4367d753ff5717a04e3aa Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Wed, 17 Feb 2016 17:04:50 +0000 Subject: [PATCH] Wire up error handlers. Replace some 400s with more appropriate 500s. DAO methods that cause unexpected exceptions get caught and logged by errors.py 500 error handler. --- app/errors.py | 54 ++++++++++++++++------------------ app/job/rest.py | 33 ++++++++++----------- app/notifications/rest.py | 3 ++ app/service/rest.py | 16 +++++----- app/template/rest.py | 10 +++++-- app/user/rest.py | 3 ++ tests/app/service/test_rest.py | 2 +- 7 files changed, 66 insertions(+), 55 deletions(-) diff --git a/app/errors.py b/app/errors.py index 2c09fbc47..3deac5600 100644 --- a/app/errors.py +++ b/app/errors.py @@ -1,36 +1,34 @@ -from flask import (jsonify, current_app) - -from app.main import main +from flask import ( + jsonify, + current_app +) -@main.app_errorhandler(400) -def bad_request(e): - return jsonify(error=str(e.description)), 400 +def register_errors(blueprint): + @blueprint.app_errorhandler(400) + def bad_request(e): + return jsonify(error=str(e.description)), 400 -@main.app_errorhandler(401) -def unauthorized(e): - error_message = "Unauthorized, authentication token must be provided" - return jsonify(error=error_message), 401, [('WWW-Authenticate', 'Bearer')] + @blueprint.app_errorhandler(401) + def unauthorized(e): + error_message = "Unauthorized, authentication token must be provided" + return jsonify(error=error_message), 401, [('WWW-Authenticate', 'Bearer')] + @blueprint.app_errorhandler(403) + def forbidden(e): + error_message = "Forbidden, invalid authentication token provided" + return jsonify(error=error_message), 403 -@main.app_errorhandler(403) -def forbidden(e): - error_message = "Forbidden, invalid authentication token provided" - return jsonify(error=error_message), 403 + @blueprint.app_errorhandler(404) + def page_not_found(e): + return jsonify(error=e.description or "Not found"), 404 + @blueprint.app_errorhandler(429) + def limit_exceeded(e): + return jsonify(error=str(e.description)), 429 -@main.app_errorhandler(404) -def page_not_found(e): - return jsonify(error=e.description or "Not found"), 404 - - -@main.app_errorhandler(429) -def limit_exceeded(e): - return jsonify(error=str(e.description)), 429 - - -@main.app_errorhandler(500) -def internal_server_error(e): - current_app.logger.exception(e) - return jsonify(error="Internal error"), 500 + @blueprint.app_errorhandler(500) + def internal_server_error(e): + current_app.logger.exception(e) + return jsonify(error="Internal error"), 500 diff --git a/app/job/rest.py b/app/job/rest.py index 1928e5d66..c3e4cf772 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -31,6 +31,10 @@ from app.schemas import ( job = Blueprint('job', __name__, url_prefix='/service//job') +from app.errors import register_errors +register_errors(job) + + @job.route('/', methods=['GET']) @job.route('', methods=['GET']) def get_job_for_service(service_id, job_id=None): @@ -54,11 +58,10 @@ def create_job(service_id): job, errors = job_schema.load(request.get_json()) if errors: return jsonify(result="error", message=errors), 400 - try: - save_job(job) - _enqueue_job(job) - except Exception as e: - return jsonify(result="error", message=str(e)), 500 + + save_job(job) + _enqueue_job(job) + return jsonify(data=job_schema.dump(job).data), 201 @@ -69,10 +72,9 @@ def update_job(service_id, job_id): update_dict, errors = job_schema_load_json.load(request.get_json()) if errors: return jsonify(result="error", message=errors), 400 - try: - save_job(job, update_dict=update_dict) - except Exception as e: - return jsonify(result="error", message=str(e)), 400 + + save_job(job, update_dict=update_dict) + return jsonify(data=job_schema.dump(job).data), 200 @@ -84,10 +86,9 @@ def create_notification_for_job(service_id, job_id): notification, errors = notification_status_schema.load(request.get_json()) if errors: return jsonify(result="error", message=errors), 400 - try: - notifications_dao.save_notification(notification) - except Exception as e: - return jsonify(result="error", message=str(e)), 500 + + notifications_dao.save_notification(notification) + return jsonify(data=notification_status_schema.dump(notification).data), 201 @@ -117,10 +118,8 @@ def update_notification_for_job(service_id, job_id, notification_id): if errors: return jsonify(result="error", message=errors), 400 - try: - notifications_dao.save_notification(notification, update_dict=update_dict) - except Exception as e: - return jsonify(result="error", message=str(e)), 400 + + notifications_dao.save_notification(notification, update_dict=update_dict) return jsonify(data=job_schema.dump(notification).data), 200 diff --git a/app/notifications/rest.py b/app/notifications/rest.py index fdc3bc747..1d4a04770 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -19,6 +19,9 @@ from sqlalchemy.orm.exc import NoResultFound notifications = Blueprint('notifications', __name__) +from app.errors import register_errors +register_errors(notifications) + def create_notification_id(): return str(uuid.uuid4()) diff --git a/app/service/rest.py b/app/service/rest.py index dd12cada5..393e328b2 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1,10 +1,9 @@ from datetime import datetime -from flask import (jsonify, request) +from flask import (jsonify, request, abort) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app import db from app.dao import DAOException from app.dao.services_dao import ( save_model_service, get_model_services, delete_model_service) @@ -19,6 +18,9 @@ from app.schemas import ( from flask import Blueprint service = Blueprint('service', __name__) +from app.errors import register_errors +register_errors(service) + @service.route('', methods=['POST']) def create_service(): @@ -31,7 +33,7 @@ def create_service(): try: save_model_service(service) except DAOException as e: - return jsonify(result="error", message=str(e)), 400 + return jsonify(result="error", message=str(e)), 500 return jsonify(data=service_schema.dump(service).data), 201 @@ -54,7 +56,7 @@ def update_service(service_id): try: save_model_service(service, update_dict=update_dict) except DAOException as e: - return jsonify(result="error", message=str(e)), 400 + return jsonify(result="error", message=str(e)), 500 return jsonify(data=service_schema.dump(service).data), status_code @@ -88,7 +90,7 @@ def renew_api_key(service_id=None): key = ApiKey(service=service, name=secret_name) save_model_api_key(key) except DAOException as e: - return jsonify(result='error', message=str(e)), 400 + return jsonify(result='error', message=str(e)), 500 unsigned_api_key = get_unsigned_secret(key.id) return jsonify(data=unsigned_api_key), 201 @@ -122,7 +124,7 @@ def get_api_keys(service_id, key_id=None): else: api_keys = get_model_api_keys(service_id=service_id) except DAOException as e: - return jsonify(result='error', message=str(e)), 400 + return jsonify(result='error', message=str(e)), 500 except NoResultFound: return jsonify(result="error", message="API key not found"), 404 @@ -172,7 +174,7 @@ def update_template(service_id, template_id): try: save_model_template(template, update_dict=update_dict) except DAOException as e: - return jsonify(result="error", message=str(e)), 400 + return jsonify(result="error", message=str(e)), 500 return jsonify(data=template_schema.dump(template).data), status_code diff --git a/app/template/rest.py b/app/template/rest.py index 036c90245..89e728774 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -1,5 +1,8 @@ -from flask import Blueprint -from flask import (jsonify) +from flask import ( + Blueprint, + jsonify +) + from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound @@ -8,6 +11,9 @@ from app.schemas import (template_schema, templates_schema) template = Blueprint('template', __name__) +from app.errors import register_errors +register_errors(template) + # I am going to keep these for admin like operations # Permissions should restrict who can access this endpoint diff --git a/app/user/rest.py b/app/user/rest.py index 695270e41..6cf661c64 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -22,6 +22,9 @@ from app import api_user user = Blueprint('user', __name__) +from app.errors import register_errors +register_errors(user) + @user.route('', methods=['POST']) def create_user(): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 11a0a68a3..cd74568e5 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -146,7 +146,7 @@ def test_post_service_without_users_attribute(notify_api, notify_db, notify_db_s url_for('service.create_service'), data=json.dumps(data), headers=headers) - assert resp.status_code == 400 + assert resp.status_code == 500 assert Service.query.count() == 0 json_resp = json.loads(resp.get_data(as_text=True)) assert json_resp['message'] == '{"users": ["Missing data for required attribute"]}'