From db91a87fb238f7e35a88f356067f4b7d1b29a750 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 2 Nov 2016 14:48:15 +0000 Subject: [PATCH 1/7] Remove link from v2 error response --- app/errors.py | 2 -- app/schema_validation/__init__.py | 1 - app/v2/errors.py | 3 --- tests/app/notifications/test_validators.py | 5 ----- tests/app/v2/notifications/test_notification_schemas.py | 3 --- tests/app/v2/notifications/test_post_notifications.py | 3 --- 6 files changed, 17 deletions(-) diff --git a/app/errors.py b/app/errors.py index 8b027ba60..2588bd54a 100644 --- a/app/errors.py +++ b/app/errors.py @@ -10,7 +10,6 @@ from app.authentication.auth import AuthError class InvalidRequest(Exception): code = None - link = None fields = [] def __init__(self, message, status_code): @@ -28,7 +27,6 @@ class InvalidRequest(Exception): return { "code": self.code, "message": self.message, - "link": self.link, "fields": self.fields } diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 38837b467..e6a2c9748 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -21,7 +21,6 @@ def build_error_message(errors, schema): message = { "code": "1001", "message": "Validation error occurred - {}".format(schema['title']), - "link": "link to error documentation (not yet implemented)", "fields": fields } diff --git a/app/v2/errors.py b/app/v2/errors.py index b7bd5f586..b7f7d229c 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -11,9 +11,7 @@ from app.errors import InvalidRequest class TooManyRequestsError(InvalidRequest): status_code = 429 - # code and link will be in a static file code = "10429" - link = "link to docs" message_template = 'Exceeded send limits ({}) for today' def __init__(self, sending_limit): @@ -23,7 +21,6 @@ class TooManyRequestsError(InvalidRequest): class BadRequestError(InvalidRequest): status_code = 400 code = 10400 - link = "link to documentation" message = "An error occurred" def __init__(self, fields=[], message=None): diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index e41d5f81c..7855e95eb 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -52,7 +52,6 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n assert e.value.code == 10400 error_message = '{0} template is not suitable for {1} notification'.format(template_type, notification_type) assert e.value.message == error_message - assert e.value.link == 'link to documentation' assert e.value.fields == [{'template': error_message}] @@ -69,7 +68,6 @@ def test_check_template_is_active_fails(sample_template): assert e.value.status_code == 400 assert e.value.code == 10400 assert e.value.message == 'Template has been deleted' - assert e.value.link == "link to documentation" assert e.value.fields == [{'template': 'Template has been deleted'}] @@ -124,7 +122,6 @@ def test_service_can_send_to_recipient_fails_when_recipient_is_not_on_team(recip assert exec_info.value.status_code == 400 assert exec_info.value.code == 10400 assert exec_info.value.message == error_message - assert exec_info.value.link == 'link to documentation' assert exec_info.value.fields == [] @@ -137,7 +134,6 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(n assert e.value.status_code == 400 assert e.value.code == 10400 assert e.value.message == 'Can’t send to this recipient using a team-only API key' - assert e.value.link == 'link to documentation' assert e.value.fields == [] @@ -154,5 +150,4 @@ def test_check_sms_content_char_count_fails(char_count, notify_api): assert e.value.code == 10400 assert e.value.message == 'Content for template has a character count greater than the limit of {}'.format( notify_api.config['SMS_CHAR_COUNT_LIMIT']) - assert e.value.link == 'link to documentation' assert e.value.fields == [] diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 96f9e1284..13ccc1124 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -33,7 +33,6 @@ def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): assert {"phone_number": "is a required property"} in error['fields'] assert {"template_id": "not a valid UUID"} in error['fields'] assert error.get('code') == '1001' - assert error.get('link', None) is not None def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): @@ -50,7 +49,6 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): assert len(error.get('fields')) == 1 assert error['fields'][0] == {"personalisation": "should contain key value pairs"} assert error.get('code') == '1001' - assert error.get('link', None) == 'link to error documentation (not yet implemented)' valid_response = { @@ -87,6 +85,5 @@ def test_post_sms_response_schema_missing_uri(): validate(j, post_sms_response) error = json.loads(e.value.message) assert '1001' == error['code'] - assert 'link to error documentation (not yet implemented)' == error['link'] assert 'Validation error occurred - response v2/notifications/sms' == error['message'] assert [{"uri": "is a required property"}] == error['fields'] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index bbc07e309..e8e500410 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -58,7 +58,6 @@ def test_post_sms_notification_returns_404_and_missing_template(notify_api, samp assert error_json['code'] == 10400 assert error_json['message'] == 'Template not found' assert error_json['fields'] == [{'template': 'Template not found'}] - assert error_json['link'] == 'link to documentation' def test_post_sms_notification_returns_403_and_well_formed_auth_error(notify_api, sample_template): @@ -80,7 +79,6 @@ def test_post_sms_notification_returns_403_and_well_formed_auth_error(notify_api assert error_resp['code'] == 401 assert error_resp['message'] == 'Unauthorized, authentication token must be provided' assert error_resp['fields'] == {'token': ['Unauthorized, authentication token must be provided']} - assert error_resp['link'] == 'link to docs' def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, sample_template): @@ -102,5 +100,4 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s error_resp = json.loads(response.get_data(as_text=True)) assert error_resp['code'] == '1001' assert error_resp['message'] == 'Validation error occurred - POST v2/notifications/sms' - assert error_resp['link'] == "link to error documentation (not yet implemented)" assert error_resp['fields'] == [{"template_id": "is a required property"}] From 4cb38e2d12fd66d59af11976939158f851134054 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 2 Nov 2016 14:58:39 +0000 Subject: [PATCH 2/7] Use status_code in error response. Remove code. --- app/authentication/auth.py | 9 +++++---- app/errors.py | 2 +- app/schema_validation/__init__.py | 2 +- app/v2/errors.py | 2 -- tests/app/notifications/test_validators.py | 7 +------ tests/app/v2/notifications/test_notification_schemas.py | 6 +++--- tests/app/v2/notifications/test_post_notifications.py | 5 ++--- 7 files changed, 13 insertions(+), 20 deletions(-) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 0ca700a62..0f18f2322 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -16,10 +16,11 @@ class AuthError(Exception): self.code = code def to_dict_v2(self): - return {'code': self.code, - 'message': self.short_message, - 'fields': self.message, - 'link': 'link to docs'} + return { + 'status_code': self.code, + 'message': self.short_message, + 'fields': self.message + } def get_auth_token(req): diff --git a/app/errors.py b/app/errors.py index 2588bd54a..db5826cbd 100644 --- a/app/errors.py +++ b/app/errors.py @@ -25,7 +25,7 @@ class InvalidRequest(Exception): Version 2 of the public api error response. ''' return { - "code": self.code, + "status_code": self.code, "message": self.message, "fields": self.fields } diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index e6a2c9748..51d43df7b 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -19,7 +19,7 @@ def build_error_message(errors, schema): field = {s[1]: s[2].strip()} fields.append(field) message = { - "code": "1001", + "status_code": 400, "message": "Validation error occurred - {}".format(schema['title']), "fields": fields } diff --git a/app/v2/errors.py b/app/v2/errors.py index b7f7d229c..8c98e48f8 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -11,7 +11,6 @@ from app.errors import InvalidRequest class TooManyRequestsError(InvalidRequest): status_code = 429 - code = "10429" message_template = 'Exceeded send limits ({}) for today' def __init__(self, sending_limit): @@ -20,7 +19,6 @@ class TooManyRequestsError(InvalidRequest): class BadRequestError(InvalidRequest): status_code = 400 - code = 10400 message = "An error occurred" def __init__(self, fields=[], message=None): diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 7855e95eb..2810e75e5 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -28,7 +28,6 @@ def test_check_service_message_limit_over_message_limit_fails(key_type, notify_d with pytest.raises(TooManyRequestsError) as e: check_service_message_limit(key_type, service) assert e.value.status_code == 429 - assert e.value.code == '10429' assert e.value.message == 'Exceeded send limits (4) for today' assert e.value.fields == [] @@ -49,7 +48,7 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n with pytest.raises(BadRequestError) as e: check_template_is_for_notification_type(notification_type=notification_type, template_type=template_type) - assert e.value.code == 10400 + e.value.status_code == 400 error_message = '{0} template is not suitable for {1} notification'.format(template_type, notification_type) assert e.value.message == error_message assert e.value.fields == [{'template': error_message}] @@ -66,7 +65,6 @@ def test_check_template_is_active_fails(sample_template): with pytest.raises(BadRequestError) as e: check_template_is_active(sample_template) assert e.value.status_code == 400 - assert e.value.code == 10400 assert e.value.message == 'Template has been deleted' assert e.value.fields == [{'template': 'Template has been deleted'}] @@ -120,7 +118,6 @@ def test_service_can_send_to_recipient_fails_when_recipient_is_not_on_team(recip key_type, trial_mode_service) assert exec_info.value.status_code == 400 - assert exec_info.value.code == 10400 assert exec_info.value.message == error_message assert exec_info.value.fields == [] @@ -132,7 +129,6 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(n 'team', live_service) assert e.value.status_code == 400 - assert e.value.code == 10400 assert e.value.message == 'Can’t send to this recipient using a team-only API key' assert e.value.fields == [] @@ -147,7 +143,6 @@ def test_check_sms_content_char_count_fails(char_count, notify_api): with pytest.raises(BadRequestError) as e: check_sms_content_char_count(char_count) assert e.value.status_code == 400 - assert e.value.code == 10400 assert e.value.message == 'Content for template has a character count greater than the limit of {}'.format( notify_api.config['SMS_CHAR_COUNT_LIMIT']) assert e.value.fields == [] diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 13ccc1124..ac63ec6d7 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -32,7 +32,7 @@ def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): assert len(error.get('fields')) == 2 assert {"phone_number": "is a required property"} in error['fields'] assert {"template_id": "not a valid UUID"} in error['fields'] - assert error.get('code') == '1001' + assert error.get('status_code') == 400 def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): @@ -48,7 +48,7 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): assert "POST v2/notifications/sms" in error['message'] assert len(error.get('fields')) == 1 assert error['fields'][0] == {"personalisation": "should contain key value pairs"} - assert error.get('code') == '1001' + assert error.get('status_code') == 400 valid_response = { @@ -84,6 +84,6 @@ def test_post_sms_response_schema_missing_uri(): with pytest.raises(ValidationError) as e: validate(j, post_sms_response) error = json.loads(e.value.message) - assert '1001' == error['code'] + assert error['status_code'] == 400 assert 'Validation error occurred - response v2/notifications/sms' == error['message'] assert [{"uri": "is a required property"}] == error['fields'] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index e8e500410..4b93a7aa5 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -55,7 +55,6 @@ def test_post_sms_notification_returns_404_and_missing_template(notify_api, samp assert response.headers['Content-type'] == 'application/json' error_json = json.loads(response.get_data(as_text=True)) - assert error_json['code'] == 10400 assert error_json['message'] == 'Template not found' assert error_json['fields'] == [{'template': 'Template not found'}] @@ -76,7 +75,7 @@ def test_post_sms_notification_returns_403_and_well_formed_auth_error(notify_api assert response.status_code == 401 assert response.headers['Content-type'] == 'application/json' error_resp = json.loads(response.get_data(as_text=True)) - assert error_resp['code'] == 401 + assert error_resp['status_code'] == 401 assert error_resp['message'] == 'Unauthorized, authentication token must be provided' assert error_resp['fields'] == {'token': ['Unauthorized, authentication token must be provided']} @@ -98,6 +97,6 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s assert response.status_code == 400 assert response.headers['Content-type'] == 'application/json' error_resp = json.loads(response.get_data(as_text=True)) - assert error_resp['code'] == '1001' + assert error_resp['status_code'] == 400 assert error_resp['message'] == 'Validation error occurred - POST v2/notifications/sms' assert error_resp['fields'] == [{"template_id": "is a required property"}] From 346d90e3199f0def6a0d75dae23ad8babe13a0b5 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 9 Nov 2016 14:56:54 +0000 Subject: [PATCH 3/7] update V2 error response to {status_code: 403, errors: [error: AuthError, message: token has expired}] } --- app/authentication/auth.py | 8 +- app/errors.py | 10 ++- app/schema_validation/__init__.py | 7 +- .../test_notification_schemas.py | 25 +++--- .../notifications/test_post_notifications.py | 14 +-- tests/app/v2/test_errors.py | 89 +++++++++++++++++++ 6 files changed, 128 insertions(+), 25 deletions(-) create mode 100644 tests/app/v2/test_errors.py diff --git a/app/authentication/auth.py b/app/authentication/auth.py index f8ea32cd4..5b69c71c5 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -18,8 +18,12 @@ class AuthError(Exception): def to_dict_v2(self): return { 'status_code': self.code, - 'message': self.short_message, - 'fields': self.message + "errors": [ + { + "error": "AuthError", + "message": self.short_message + } + ] } diff --git a/app/errors.py b/app/errors.py index db5826cbd..546f75f0c 100644 --- a/app/errors.py +++ b/app/errors.py @@ -25,9 +25,13 @@ class InvalidRequest(Exception): Version 2 of the public api error response. ''' return { - "status_code": self.code, - "message": self.message, - "fields": self.fields + "status_code": self.status_code, + "errors": [ + { + "error": self.__class__.__name__, + "message": self.message + } + ] } def __str__(self): diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 51d43df7b..0f7099939 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -1,4 +1,6 @@ import json +from collections import OrderedDict + from jsonschema import Draft4Validator, ValidationError @@ -16,12 +18,11 @@ def build_error_message(errors, schema): field = "'{}' {}".format(e.path[0], e.schema.get('validationMessage')) if e.schema.get( 'validationMessage') else e.message s = field.split("'") - field = {s[1]: s[2].strip()} + field = OrderedDict({"error": "ValidationError", "message": {s[1]: s[2].strip()}}) fields.append(field) message = { "status_code": 400, - "message": "Validation error occurred - {}".format(schema['title']), - "fields": fields + "errors": fields } return json.dumps(message) diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index ac63ec6d7..d917a8cf7 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -20,7 +20,7 @@ valid_json_with_optionals = { @pytest.mark.parametrize("input", [valid_json, valid_json_with_optionals]) def test_post_sms_schema_is_valid(input): - validate(input, post_sms_request) + assert validate(input, post_sms_request) == input def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): @@ -28,11 +28,13 @@ def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): with pytest.raises(ValidationError) as e: validate(j, post_sms_request) error = json.loads(e.value.message) - assert "POST v2/notifications/sms" in error['message'] - assert len(error.get('fields')) == 2 - assert {"phone_number": "is a required property"} in error['fields'] - assert {"template_id": "not a valid UUID"} in error['fields'] + assert len(error.keys()) == 2 assert error.get('status_code') == 400 + assert len(error.get('errors')) == 2 + assert {'error': 'ValidationError', + 'message': {"phone_number": "is a required property"}} in error['errors'] + assert {'error': 'ValidationError', + 'message': {"template_id": "not a valid UUID"}} in error['errors'] def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): @@ -45,10 +47,11 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): with pytest.raises(ValidationError) as e: validate(j, post_sms_request) error = json.loads(e.value.message) - assert "POST v2/notifications/sms" in error['message'] - assert len(error.get('fields')) == 1 - assert error['fields'][0] == {"personalisation": "should contain key value pairs"} + assert len(error.get('errors')) == 1 + assert error['errors'] == [{'error': 'ValidationError', + 'message': {"personalisation": "should contain key value pairs"}}] assert error.get('status_code') == 400 + assert len(error.keys()) == 2 valid_response = { @@ -75,7 +78,7 @@ valid_response_with_optionals = { @pytest.mark.parametrize('input', [valid_response]) def test_post_sms_response_schema_is_valid(input): - validate(input, post_sms_response) + assert validate(input, post_sms_response) == input def test_post_sms_response_schema_missing_uri(): @@ -85,5 +88,5 @@ def test_post_sms_response_schema_missing_uri(): validate(j, post_sms_response) error = json.loads(e.value.message) assert error['status_code'] == 400 - assert 'Validation error occurred - response v2/notifications/sms' == error['message'] - assert [{"uri": "is a required property"}] == error['fields'] + assert error['errors'] == [{'error': 'ValidationError', + 'message': {"uri": "is a required property"}}] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 4b93a7aa5..6512eeff9 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -55,8 +55,8 @@ def test_post_sms_notification_returns_404_and_missing_template(notify_api, samp assert response.headers['Content-type'] == 'application/json' error_json = json.loads(response.get_data(as_text=True)) - assert error_json['message'] == 'Template not found' - assert error_json['fields'] == [{'template': 'Template not found'}] + assert error_json['errors'] == [{"error": "BadRequestError", + "message": 'Template not found'}] def test_post_sms_notification_returns_403_and_well_formed_auth_error(notify_api, sample_template): @@ -76,8 +76,8 @@ def test_post_sms_notification_returns_403_and_well_formed_auth_error(notify_api assert response.headers['Content-type'] == 'application/json' error_resp = json.loads(response.get_data(as_text=True)) assert error_resp['status_code'] == 401 - assert error_resp['message'] == 'Unauthorized, authentication token must be provided' - assert error_resp['fields'] == {'token': ['Unauthorized, authentication token must be provided']} + assert error_resp['errors'] == [{'error': "AuthError", + 'message': 'Unauthorized, authentication token must be provided'}] def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, sample_template): @@ -98,5 +98,7 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s assert response.headers['Content-type'] == 'application/json' error_resp = json.loads(response.get_data(as_text=True)) assert error_resp['status_code'] == 400 - assert error_resp['message'] == 'Validation error occurred - POST v2/notifications/sms' - assert error_resp['fields'] == [{"template_id": "is a required property"}] + print(error_resp['errors']) + assert error_resp['errors'] == [{'error': 'ValidationError', + 'message': {"template_id": "is a required property"} + }] diff --git a/tests/app/v2/test_errors.py b/tests/app/v2/test_errors.py new file mode 100644 index 000000000..15bd5af87 --- /dev/null +++ b/tests/app/v2/test_errors.py @@ -0,0 +1,89 @@ +import json + +import pytest +from flask import url_for + + +@pytest.fixture(scope='function') +def app_for_test(mocker): + import flask + from flask import Blueprint + from app.authentication.auth import AuthError + from app.v2.errors import BadRequestError, TooManyRequestsError + + app = flask.Flask(__name__) + app.config['TESTING'] = True + + from app.v2.errors import register_errors + blue = Blueprint("v2_under_test", __name__, url_prefix='/v2/under_test') + + @blue.route("/raise_auth_error", methods=["GET"]) + def raising_auth_error(): + raise AuthError("some message", 403) + + @blue.route("/raise_bad_request", methods=["GET"]) + def raising_bad_request(): + raise BadRequestError(message="you forgot the thing") + + @blue.route("/raise_too_many_requests", methods=["GET"]) + def raising_too_many_requests(): + raise TooManyRequestsError(sending_limit="452") + + @blue.route("/raise_validation_error", methods=["GET"]) + def raising_validation_error(): + from app.schema_validation import validate + from app.v2.notifications.notification_schemas import post_sms_request + validate({"template_id": "bad_uuid"}, post_sms_request) + + register_errors(blue) + app.register_blueprint(blue) + + return app + + +def test_auth_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_auth_error')) + assert response.status_code == 403 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 403, + "errors": [{"error": "AuthError", + "message": "some message"}]} + + +def test_bad_request_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_bad_request')) + assert response.status_code == 400 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 400, + "errors": [{"error": "BadRequestError", + "message": "you forgot the thing"}]} + + +def test_too_many_requests_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_too_many_requests')) + assert response.status_code == 429 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 429, + "errors": [{"error": "TooManyRequestsError", + "message": "Exceeded send limits (452) for today"}]} + + +def test_validation_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_validation_error')) + assert response.status_code == 400 + error = json.loads(response.get_data(as_text=True)) + assert len(error.keys()) == 2 + assert error['status_code'] == 400 + assert len(error['errors']) == 2 + assert {'error': 'ValidationError', + 'message': {'phone_number': 'is a required property'}} in error['errors'] + assert {'error': 'ValidationError', + 'message': {'template_id': 'not a valid UUID'}} in error['errors'] From 7d763260ce93b960a26a2fe1d9a34b62ab143b4d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 10 Nov 2016 14:53:39 +0000 Subject: [PATCH 4/7] Update format of the errors for DataError and exception --- app/v2/errors.py | 8 ++++---- tests/app/v2/test_errors.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/v2/errors.py b/app/v2/errors.py index 8c98e48f8..26cc3b36e 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -1,10 +1,8 @@ import json - from flask import jsonify, current_app from jsonschema import ValidationError from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound - from app.authentication.auth import AuthError from app.errors import InvalidRequest @@ -42,7 +40,8 @@ def register_errors(blueprint): @blueprint.errorhandler(DataError) def no_result_found(e): current_app.logger.exception(e) - return jsonify(message="No result found"), 404 + return jsonify(status_code=404, + errors=[{"error": e.__class__.__name__, "message": "No result found"}]), 404 @blueprint.errorhandler(AuthError) def auth_error(error): @@ -51,4 +50,5 @@ def register_errors(blueprint): @blueprint.errorhandler(Exception) def internal_server_error(error): current_app.logger.exception(error) - return jsonify(message='Internal server error'), 500 + return jsonify(status_code=500, + errors=[{"error": error.__class__.__name__, "message": 'Internal server error'}]), 500 diff --git a/tests/app/v2/test_errors.py b/tests/app/v2/test_errors.py index 15bd5af87..a95ddb680 100644 --- a/tests/app/v2/test_errors.py +++ b/tests/app/v2/test_errors.py @@ -1,7 +1,7 @@ import json - import pytest from flask import url_for +from sqlalchemy.exc import DataError @pytest.fixture(scope='function') @@ -35,6 +35,14 @@ def app_for_test(mocker): from app.v2.notifications.notification_schemas import post_sms_request validate({"template_id": "bad_uuid"}, post_sms_request) + @blue.route("raise_data_error", methods=["GET"]) + def raising_data_error(): + raise DataError("There was a db problem", "params", "orig") + + @blue.route("raise_exception", methods=["GET"]) + def raising_exception(): + raise AssertionError("Raising any old exception") + register_errors(blue) app.register_blueprint(blue) @@ -87,3 +95,23 @@ def test_validation_error(app_for_test): 'message': {'phone_number': 'is a required property'}} in error['errors'] assert {'error': 'ValidationError', 'message': {'template_id': 'not a valid UUID'}} in error['errors'] + + +def test_data_errors(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_data_error')) + assert response.status_code == 404 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 404, + "errors": [{"error": "DataError", "message": "No result found"}]} + + +def test_internal_server_error_handler(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for("v2_under_test.raising_exception")) + assert response.status_code == 500 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 500, + "errors": [{"error": "AssertionError", "message": "Internal server error"}]} From c758694b988a348ee093e7c7446489b04c68b757 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 10 Nov 2016 16:30:51 +0000 Subject: [PATCH 5/7] Change validation error message to a string from a dict. --- app/schema_validation/__init__.py | 2 +- app/schema_validation/definitions.py | 2 +- tests/app/v2/notifications/test_notification_schemas.py | 8 ++++---- tests/app/v2/notifications/test_post_notifications.py | 4 ++-- tests/app/v2/test_errors.py | 5 +++-- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 0f7099939..4cb27b12b 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -18,7 +18,7 @@ def build_error_message(errors, schema): field = "'{}' {}".format(e.path[0], e.schema.get('validationMessage')) if e.schema.get( 'validationMessage') else e.message s = field.split("'") - field = OrderedDict({"error": "ValidationError", "message": {s[1]: s[2].strip()}}) + field = OrderedDict({"error": "ValidationError", "message": "{}{}".format(s[1], s[2])}) fields.append(field) message = { "status_code": 400, diff --git a/app/schema_validation/definitions.py b/app/schema_validation/definitions.py index e65566d7b..3841b64a8 100644 --- a/app/schema_validation/definitions.py +++ b/app/schema_validation/definitions.py @@ -6,7 +6,7 @@ If the definition is specific to a version put it in a definition file in the ve uuid = { "type": "string", "pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$", - "validationMessage": "not a valid UUID", + "validationMessage": "is not a valid UUID", "code": "1001", # yet to be implemented "link": "link to our error documentation not yet implemented" } diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index d917a8cf7..74c734c2f 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -32,9 +32,9 @@ def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): assert error.get('status_code') == 400 assert len(error.get('errors')) == 2 assert {'error': 'ValidationError', - 'message': {"phone_number": "is a required property"}} in error['errors'] + 'message': "phone_number is a required property"} in error['errors'] assert {'error': 'ValidationError', - 'message': {"template_id": "not a valid UUID"}} in error['errors'] + 'message': "template_id is not a valid UUID"} in error['errors'] def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): @@ -49,7 +49,7 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): error = json.loads(e.value.message) assert len(error.get('errors')) == 1 assert error['errors'] == [{'error': 'ValidationError', - 'message': {"personalisation": "should contain key value pairs"}}] + 'message': "personalisation should contain key value pairs"}] assert error.get('status_code') == 400 assert len(error.keys()) == 2 @@ -89,4 +89,4 @@ def test_post_sms_response_schema_missing_uri(): error = json.loads(e.value.message) assert error['status_code'] == 400 assert error['errors'] == [{'error': 'ValidationError', - 'message': {"uri": "is a required property"}}] + 'message': "uri is a required property"}] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 6512eeff9..c02e2d8e9 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -55,6 +55,7 @@ def test_post_sms_notification_returns_404_and_missing_template(notify_api, samp assert response.headers['Content-type'] == 'application/json' error_json = json.loads(response.get_data(as_text=True)) + assert error_json['status_code'] == 400 assert error_json['errors'] == [{"error": "BadRequestError", "message": 'Template not found'}] @@ -98,7 +99,6 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s assert response.headers['Content-type'] == 'application/json' error_resp = json.loads(response.get_data(as_text=True)) assert error_resp['status_code'] == 400 - print(error_resp['errors']) assert error_resp['errors'] == [{'error': 'ValidationError', - 'message': {"template_id": "is a required property"} + 'message': "template_id is a required property" }] diff --git a/tests/app/v2/test_errors.py b/tests/app/v2/test_errors.py index a95ddb680..5f0ed0f81 100644 --- a/tests/app/v2/test_errors.py +++ b/tests/app/v2/test_errors.py @@ -88,13 +88,14 @@ def test_validation_error(app_for_test): response = client.get(url_for('v2_under_test.raising_validation_error')) assert response.status_code == 400 error = json.loads(response.get_data(as_text=True)) + print(error) assert len(error.keys()) == 2 assert error['status_code'] == 400 assert len(error['errors']) == 2 assert {'error': 'ValidationError', - 'message': {'phone_number': 'is a required property'}} in error['errors'] + 'message': "phone_number is a required property"} in error['errors'] assert {'error': 'ValidationError', - 'message': {'template_id': 'not a valid UUID'}} in error['errors'] + 'message': "template_id is not a valid UUID"} in error['errors'] def test_data_errors(app_for_test): From b0d88e0888cb5da0f6f928d8cb70fa6c41a02cef Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 11 Nov 2016 10:50:38 +0000 Subject: [PATCH 6/7] Removed OrderedDict Added missing assert in test --- app/schema_validation/__init__.py | 2 +- tests/app/notifications/test_validators.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 4cb27b12b..88f0e2421 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -18,7 +18,7 @@ def build_error_message(errors, schema): field = "'{}' {}".format(e.path[0], e.schema.get('validationMessage')) if e.schema.get( 'validationMessage') else e.message s = field.split("'") - field = OrderedDict({"error": "ValidationError", "message": "{}{}".format(s[1], s[2])}) + field = {"error": "ValidationError", "message": "{}{}".format(s[1], s[2])} fields.append(field) message = { "status_code": 400, diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 2810e75e5..03781bcb8 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -48,7 +48,7 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n with pytest.raises(BadRequestError) as e: check_template_is_for_notification_type(notification_type=notification_type, template_type=template_type) - e.value.status_code == 400 + assert e.value.status_code == 400 error_message = '{0} template is not suitable for {1} notification'.format(template_type, notification_type) assert e.value.message == error_message assert e.value.fields == [{'template': error_message}] From 488ba7a1ef35f5f6a3e28aa559bf16ee4ee90560 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 11 Nov 2016 10:56:21 +0000 Subject: [PATCH 7/7] Remove import --- app/schema_validation/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 88f0e2421..71dd37c18 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -1,5 +1,4 @@ import json -from collections import OrderedDict from jsonschema import Draft4Validator, ValidationError