handle 405s

Handle werkzeug http errors separately. Also, improve the generic
`Exception` handler to be more flexible when exceptions don't look
like http errors.

note: because they're 405s the route isn't matched, so the v2 error
handlers don't trip properly. this might be an issue because the
internal endpoints expect a different return format. Might have to
think about how to handle this.
This commit is contained in:
Leo Hemsted
2019-07-26 13:26:20 +01:00
parent 359016de6a
commit 2c2eb2abad
2 changed files with 37 additions and 10 deletions

View File

@@ -3,7 +3,7 @@ import random
import string import string
import uuid import uuid
from flask import _request_ctx_stack, request, g, jsonify from flask import _request_ctx_stack, request, g, jsonify, make_response
from flask_sqlalchemy import SQLAlchemy as _SQLAlchemy from flask_sqlalchemy import SQLAlchemy as _SQLAlchemy
from flask_marshmallow import Marshmallow from flask_marshmallow import Marshmallow
from flask_migrate import Migrate from flask_migrate import Migrate
@@ -12,6 +12,7 @@ from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient
from notifications_utils.clients.statsd.statsd_client import StatsdClient from notifications_utils.clients.statsd.statsd_client import StatsdClient
from notifications_utils.clients.redis.redis_client import RedisClient from notifications_utils.clients.redis.redis_client import RedisClient
from notifications_utils import logging, request_helper from notifications_utils import logging, request_helper
from werkzeug.exceptions import HTTPException as WerkzeugHTTPException
from werkzeug.local import LocalProxy from werkzeug.local import LocalProxy
from app.celery.celery import NotifyCelery from app.celery.celery import NotifyCelery
@@ -263,7 +264,17 @@ def init_app(app):
def exception(error): def exception(error):
app.logger.exception(error) app.logger.exception(error)
# error.code is set for our exception types. # error.code is set for our exception types.
return jsonify(result='error', message=error.message), error.code or 500 msg = getattr(error, 'message', str(error))
code = getattr(error, 'code', 500)
return jsonify(result='error', message=msg), code
@app.errorhandler(WerkzeugHTTPException)
def werkzeug_exception(e):
return make_response(
jsonify(result='error', message=e.description),
e.code,
e.get_headers()
)
@app.errorhandler(404) @app.errorhandler(404)
def page_not_found(e): def page_not_found(e):

View File

@@ -1,4 +1,3 @@
import json
import pytest import pytest
from flask import url_for from flask import url_for
from sqlalchemy.exc import DataError from sqlalchemy.exc import DataError
@@ -10,9 +9,13 @@ def app_for_test(mocker):
from flask import Blueprint from flask import Blueprint
from app.authentication.auth import AuthError from app.authentication.auth import AuthError
from app.v2.errors import BadRequestError, TooManyRequestsError, JobIncompleteError from app.v2.errors import BadRequestError, TooManyRequestsError, JobIncompleteError
from app import init_app
app = flask.Flask(__name__) app = flask.Flask(__name__)
app.config['TESTING'] = True app.config['TESTING'] = True
init_app(app)
from app import statsd_client
statsd_client.init_app(app)
from app.v2.errors import register_errors from app.v2.errors import register_errors
blue = Blueprint("v2_under_test", __name__, url_prefix='/v2/under_test') blue = Blueprint("v2_under_test", __name__, url_prefix='/v2/under_test')
@@ -58,7 +61,7 @@ def test_auth_error(app_for_test):
with app_for_test.test_client() as client: with app_for_test.test_client() as client:
response = client.get(url_for('v2_under_test.raising_auth_error')) response = client.get(url_for('v2_under_test.raising_auth_error'))
assert response.status_code == 403 assert response.status_code == 403
error = json.loads(response.get_data(as_text=True)) error = response.json
assert error == {"status_code": 403, assert error == {"status_code": 403,
"errors": [{"error": "AuthError", "errors": [{"error": "AuthError",
"message": "some message"}]} "message": "some message"}]}
@@ -69,7 +72,7 @@ def test_bad_request_error(app_for_test):
with app_for_test.test_client() as client: with app_for_test.test_client() as client:
response = client.get(url_for('v2_under_test.raising_bad_request')) response = client.get(url_for('v2_under_test.raising_bad_request'))
assert response.status_code == 400 assert response.status_code == 400
error = json.loads(response.get_data(as_text=True)) error = response.json
assert error == {"status_code": 400, assert error == {"status_code": 400,
"errors": [{"error": "BadRequestError", "errors": [{"error": "BadRequestError",
"message": "you forgot the thing"}]} "message": "you forgot the thing"}]}
@@ -80,7 +83,7 @@ def test_too_many_requests_error(app_for_test):
with app_for_test.test_client() as client: with app_for_test.test_client() as client:
response = client.get(url_for('v2_under_test.raising_too_many_requests')) response = client.get(url_for('v2_under_test.raising_too_many_requests'))
assert response.status_code == 429 assert response.status_code == 429
error = json.loads(response.get_data(as_text=True)) error = response.json
assert error == {"status_code": 429, assert error == {"status_code": 429,
"errors": [{"error": "TooManyRequestsError", "errors": [{"error": "TooManyRequestsError",
"message": "Exceeded send limits (452) for today"}]} "message": "Exceeded send limits (452) for today"}]}
@@ -91,7 +94,7 @@ def test_validation_error(app_for_test):
with app_for_test.test_client() as client: with app_for_test.test_client() as client:
response = client.get(url_for('v2_under_test.raising_validation_error')) response = client.get(url_for('v2_under_test.raising_validation_error'))
assert response.status_code == 400 assert response.status_code == 400
error = json.loads(response.get_data(as_text=True)) error = response.json
assert len(error.keys()) == 2 assert len(error.keys()) == 2
assert error['status_code'] == 400 assert error['status_code'] == 400
assert len(error['errors']) == 2 assert len(error['errors']) == 2
@@ -106,7 +109,7 @@ def test_data_errors(app_for_test):
with app_for_test.test_client() as client: with app_for_test.test_client() as client:
response = client.get(url_for('v2_under_test.raising_data_error')) response = client.get(url_for('v2_under_test.raising_data_error'))
assert response.status_code == 404 assert response.status_code == 404
error = json.loads(response.get_data(as_text=True)) error = response.json
assert error == {"status_code": 404, assert error == {"status_code": 404,
"errors": [{"error": "DataError", "message": "No result found"}]} "errors": [{"error": "DataError", "message": "No result found"}]}
@@ -116,7 +119,7 @@ def test_job_incomplete_errors(app_for_test):
with app_for_test.test_client() as client: with app_for_test.test_client() as client:
response = client.get(url_for('v2_under_test.raising_job_incomplete_error')) response = client.get(url_for('v2_under_test.raising_job_incomplete_error'))
assert response.status_code == 500 assert response.status_code == 500
error = json.loads(response.get_data(as_text=True)) error = response.json
assert error == {"status_code": 500, assert error == {"status_code": 500,
"errors": [{"error": "JobIncompleteError", "message": "Raising job incomplete error"}]} "errors": [{"error": "JobIncompleteError", "message": "Raising job incomplete error"}]}
@@ -126,6 +129,19 @@ def test_internal_server_error_handler(app_for_test):
with app_for_test.test_client() as client: with app_for_test.test_client() as client:
response = client.get(url_for("v2_under_test.raising_exception")) response = client.get(url_for("v2_under_test.raising_exception"))
assert response.status_code == 500 assert response.status_code == 500
error = json.loads(response.get_data(as_text=True)) error = response.json
assert error == {"status_code": 500, assert error == {"status_code": 500,
"errors": [{"error": "AssertionError", "message": "Internal server error"}]} "errors": [{"error": "AssertionError", "message": "Internal server error"}]}
def test_bad_method(app_for_test):
with app_for_test.test_request_context():
with app_for_test.test_client() as client:
response = client.post(url_for("v2_under_test.raising_exception"))
assert response.status_code == 405
assert response.json == {
"result": "error",
"message": "The method is not allowed for the requested URL."
}