From 30e174a086c1fbd2464e46051d7f119a533a6816 Mon Sep 17 00:00:00 2001 From: gnaponie Date: Nov 07 2017 20:07:05 +0000 Subject: [PATCH 1/4] Returned error messages in JSON --- diff --git a/waiverdb/app.py b/waiverdb/app.py index aa3e486..b681e28 100644 --- a/waiverdb/app.py +++ b/waiverdb/app.py @@ -10,7 +10,10 @@ from waiverdb.events import publish_new_waiver from waiverdb.logger import init_logging from waiverdb.api_v1 import api_v1 from waiverdb.models import db +from waiverdb.utils import json_error from flask_oidc import OpenIDConnect +from werkzeug.exceptions import default_exceptions +from requests import ConnectionError, Timeout def load_config(app): @@ -59,6 +62,14 @@ def create_app(config_obj=None): load_config(app) if app.config['PRODUCTION'] and app.secret_key == 'replace-me-with-something-random': raise Warning("You need to change the app.secret_key value for production") + + + # register error handlers + for code in default_exceptions.iterkeys(): + app.register_error_handler(code, json_error) + app.register_error_handler(ConnectionError, json_error) + app.register_error_handler(Timeout, json_error) + populate_db_config(app) if app.config['AUTH_METHOD'] == 'OIDC': app.oidc = OpenIDConnect(app) diff --git a/waiverdb/utils.py b/waiverdb/utils.py index 83fa825..6e529e2 100644 --- a/waiverdb/utils.py +++ b/waiverdb/utils.py @@ -6,7 +6,7 @@ import stomp from flask import request, url_for, jsonify, current_app from flask_restful import marshal from waiverdb.fields import waiver_fields -from werkzeug.exceptions import NotFound +from werkzeug.exceptions import NotFound, HTTPException from contextlib import contextmanager @@ -57,6 +57,27 @@ def json_collection(query, page=1, limit=10): return pages +def json_error(error): + """ + Return error responses in JSON. + + :param error: One of Exceptions. It could be HTTPException, ConnectionError, or + Timeout. + :return: JSON error response. + + """ + if isinstance(error, HTTPException): + response = jsonify(message=error.description) + response.status_code = error.code + else: + # Could be ConnectionError or Timeout + current_app.logger.exception('Returning 500 to user.') + response = jsonify(message=str(error.message)) + response.status_code = 500 + + return insert_headers(response) + + def jsonp(func): """Wraps Jsonified output for JSONP requests.""" @functools.wraps(func) @@ -96,3 +117,15 @@ def stomp_connection(): else: raise RuntimeError('stomp was configured to publish messages, ' 'but STOMP_CONFIGS is not configured') + + +def insert_headers(response): + """ Insert the CORS headers for the give reponse if there are any + configured for the application. + """ + if current_app.config.get('CORS_URL'): + response.headers['Access-Control-Allow-Origin'] = \ + current_app.config['CORS_URL'] + response.headers['Access-Control-Allow-Headers'] = 'Content-Type' + response.headers['Access-Control-Allow-Method'] = 'POST, OPTIONS' + return response From 24d551730ab6653a8889abe8cf36a602c582d7e6 Mon Sep 17 00:00:00 2001 From: gnaponie Date: Nov 07 2017 20:07:38 +0000 Subject: [PATCH 2/4] Fixed some errors in the doc. --- diff --git a/README.md b/README.md index 92bf605..e4d43aa 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ You can verify the server is running correctly by visiting Date: Nov 09 2017 00:25:43 +0000 Subject: [PATCH 3/4] Added test for 404 and 500 and json format Added test for: * a request with 404 status code * a request with 500 status code, mocking a ConnectionError even if it should be a response with status code 200 * check if the response of the error is in JSON format --- diff --git a/tests/test_api_v10.py b/tests/test_api_v10.py index 6fe55e2..82ca011 100644 --- a/tests/test_api_v10.py +++ b/tests/test_api_v10.py @@ -3,6 +3,7 @@ import json from .utils import create_waiver import datetime +from requests import ConnectionError from mock import patch from waiverdb import __version__ @@ -55,6 +56,19 @@ def test_get_waiver(client, session): def test_404_for_nonexistent_waiver(client, session): r = client.get('/api/v1.0/waivers/foo') assert r.status_code == 404 + res_data = json.loads(r.get_data(as_text=True)) + message = ( + 'The requested URL was not found on the server. If you entered the ' + 'URL manually please check your spelling and try again.') + assert res_data['message'] == message + + +@patch('waiverdb.api_v1.AboutResource.get', side_effect=ConnectionError) +def test_500(mocked, client, session): + r = client.get('/api/v1.0/about') + assert r.status_code == 500 + res_data = json.loads(r.get_data(as_text=True)) + assert res_data['message'] == '' def test_get_waivers(client, session): diff --git a/waiverdb/app.py b/waiverdb/app.py index b681e28..300641b 100644 --- a/waiverdb/app.py +++ b/waiverdb/app.py @@ -62,14 +62,14 @@ def create_app(config_obj=None): load_config(app) if app.config['PRODUCTION'] and app.secret_key == 'replace-me-with-something-random': raise Warning("You need to change the app.secret_key value for production") - - + + # register error handlers for code in default_exceptions.iterkeys(): app.register_error_handler(code, json_error) app.register_error_handler(ConnectionError, json_error) app.register_error_handler(Timeout, json_error) - + populate_db_config(app) if app.config['AUTH_METHOD'] == 'OIDC': app.oidc = OpenIDConnect(app) From ca8e233876b1f653c9babe3848efc166e6fd66dc Mon Sep 17 00:00:00 2001 From: gnaponie Date: Nov 09 2017 01:04:10 +0000 Subject: [PATCH 4/4] Added more information for setup environment Addes information about some required packages to install for setup environment --- diff --git a/docs/developer-guide.rst b/docs/developer-guide.rst index 38e8e0e..0299f60 100644 --- a/docs/developer-guide.rst +++ b/docs/developer-guide.rst @@ -5,6 +5,10 @@ Development Guide Quick development setup ======================= +Install packages required by pip to compile some python packages:: + + $ sudo dnf install swig systemd-devel openssl-devel cpp gcc + Set up a python virtualenv:: $ sudo dnf install python-virtualenv