Merge pull request #732 from alphagov/change-v2-error-resp

Change v2 error resp

app/authentication/auth.py

@@ -16,10 +16,15 @@ class AuthError(Exception):
         self.code = code
 
     def to_dict_v2(self):
-        return {'code': self.code,
+        return {
-                'message': self.short_message,
+            'status_code': self.code,
-                'fields': self.message,
+            "errors": [
-                'link': 'link to docs'}
+                {
+                    "error": "AuthError",
+                    "message": self.short_message
+                }
+            ]
+        }
 
 
 def get_auth_token(req):

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):
@@ -26,10 +25,13 @@ class InvalidRequest(Exception):
         Version 2 of the public api error response.
         '''
         return {
-            "code": self.code,
+            "status_code": self.status_code,
-            "message": self.message,
+            "errors": [
-            "link": self.link,
+                {
-            "fields": self.fields
+                    "error": self.__class__.__name__,
+                    "message": self.message
+                }
+            ]
         }
 
     def __str__(self):

app/schema_validation/__init__.py

@@ -1,4 +1,5 @@
 import json
+
 from jsonschema import Draft4Validator, ValidationError
 
 
@@ -16,13 +17,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 = {"error": "ValidationError", "message": "{}{}".format(s[1], s[2])}
         fields.append(field)
     message = {
-        "code": "1001",
+        "status_code": 400,
-        "message": "Validation error occurred - {}".format(schema['title']),
+        "errors": fields
-        "link": "link to error documentation (not yet implemented)",
-        "fields": fields
     }
 
     return json.dumps(message)

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"
 }

app/v2/errors.py

@@ -1,19 +1,14 @@
 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
 
 
 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):
@@ -22,8 +17,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):
@@ -47,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):
@@ -56,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

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,10 +48,9 @@ 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
+    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.link == 'link to documentation'
     assert e.value.fields == [{'template': error_message}]
 
 
@@ -67,9 +65,7 @@ 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.link == "link to documentation"
     assert e.value.fields == [{'template': 'Template has been deleted'}]
 
 
@@ -122,9 +118,7 @@ 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.link == 'link to documentation'
     assert exec_info.value.fields == []
 
 
@@ -135,9 +129,7 @@ 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.link == 'link to documentation'
     assert e.value.fields == []
 
 
@@ -151,8 +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.link == 'link to documentation'
     assert e.value.fields == []

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,12 +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.keys()) == 2
-    assert len(error.get('fields')) == 2
+    assert error.get('status_code') == 400
-    assert {"phone_number": "is a required property"} in error['fields']
+    assert len(error.get('errors')) == 2
-    assert {"template_id": "not a valid UUID"} in error['fields']
+    assert {'error': 'ValidationError',
-    assert error.get('code') == '1001'
+            'message': "phone_number is a required property"} in error['errors']
-    assert error.get('link', None) is not None
+    assert {'error': 'ValidationError',
+            'message': "template_id is not a valid UUID"} in error['errors']
 
 
 def test_post_sms_schema_with_personalisation_that_is_not_a_dict():
@@ -46,11 +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('errors')) == 1
-    assert len(error.get('fields')) == 1
+    assert error['errors'] == [{'error': 'ValidationError',
-    assert error['fields'][0] == {"personalisation": "should contain key value pairs"}
+                                'message': "personalisation should contain key value pairs"}]
-    assert error.get('code') == '1001'
+    assert error.get('status_code') == 400
-    assert error.get('link', None) == 'link to error documentation (not yet implemented)'
+    assert len(error.keys()) == 2
 
 
 valid_response = {
@@ -77,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():
@@ -86,7 +87,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 'link to error documentation (not yet implemented)' == error['link']
+    assert error['errors'] == [{'error': 'ValidationError',
-    assert 'Validation error occurred - response v2/notifications/sms' == error['message']
+                               'message': "uri is a required property"}]
-    assert [{"uri": "is a required property"}] == error['fields']

tests/app/v2/notifications/test_post_notifications.py

@@ -55,10 +55,9 @@ 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['status_code'] == 400
-            assert error_json['message'] == 'Template not found'
+            assert error_json['errors'] == [{"error": "BadRequestError",
-            assert error_json['fields'] == [{'template': 'Template not found'}]
+                                             "message": '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):
@@ -77,10 +76,9 @@ 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['errors'] == [{'error': "AuthError",
-            assert error_resp['fields'] == {'token': ['Unauthorized, authentication token must be provided']}
+                                             'message': '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):
@@ -100,7 +98,7 @@ 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['errors'] == [{'error': 'ValidationError',
-            assert error_resp['link'] == "link to error documentation (not yet implemented)"
+                                             'message': "template_id is a required property"
-            assert error_resp['fields'] == [{"template_id": "is a required property"}]
+                                             }]

tests/app/v2/test_errors.py

@@ -0,0 +1,118 @@
+import json
+import pytest
+from flask import url_for
+from sqlalchemy.exc import DataError
+
+
+@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)
+
+    @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)
+
+    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))
+            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']
+            assert {'error': 'ValidationError',
+                    'message': "template_id is 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"}]}