From 9546c1025057bf7d94ef3469a6bd26907fc7ca10 Mon Sep 17 00:00:00 2001 From: Yuriy Zveryanskyy Date: Wed, 9 Oct 2013 11:26:29 +0300 Subject: [PATCH] Add custom error code to ClientSideError Added custom error status code for ClientSideError exception instead of hardcoded value 400. Fixed case when user exception with client error code formatted as server error. Pecan extension fixed. Change-Id: I2663db0aa88538b722eb2783d130585b0fc2335b --- tests/pecantest/test/controllers/ws.py | 3 +++ tests/pecantest/test/tests/test_ws.py | 24 +++++++++++++++--- tests/test_cornice.py | 2 +- tests/test_flask.py | 4 +-- tests/test_tg15.py | 4 +-- wsme/api.py | 9 +++++-- wsme/exc.py | 3 ++- wsme/tests/test_utils.py | 35 ++++++++++++++++++++------ wsme/utils.py | 14 ++++++++++- wsmeext/flask.py | 2 +- wsmeext/pecan.py | 6 ++--- wsmeext/tests/test_utils.py | 22 ---------------- wsmeext/tg1.py | 2 +- wsmeext/utils.py | 11 -------- 14 files changed, 83 insertions(+), 58 deletions(-) delete mode 100644 wsmeext/tests/test_utils.py delete mode 100644 wsmeext/utils.py diff --git a/tests/pecantest/test/controllers/ws.py b/tests/pecantest/test/controllers/ws.py index 1dcb0cb..acb4402 100644 --- a/tests/pecantest/test/controllers/ws.py +++ b/tests/pecantest/test/controllers/ws.py @@ -94,6 +94,9 @@ class AuthorsController(RestController): if id == 997: raise NonHttpException(id) + if id == 996: + raise wsme.exc.ClientSideError('Disabled ID', status_code=403) + if id == 911: return wsme.api.Response(Author(), status_code=401) diff --git a/tests/pecantest/test/tests/test_ws.py b/tests/pecantest/test/tests/test_ws.py index 4eb9607..ab274cd 100644 --- a/tests/pecantest/test/tests/test_ws.py +++ b/tests/pecantest/test/tests/test_ws.py @@ -5,7 +5,7 @@ import pecan import six -used_status_codes = [400, 401, 404, 500] +used_status_codes = [400, 401, 403, 404, 500] http_response_messages = {} for code in used_status_codes: http_response_messages[code] = '%s %s' % (code, http_client.responses[code]) @@ -96,14 +96,14 @@ class TestWS(FunctionalTest): ) self.assertEqual(res.status, expected_status) a = json.loads(res.body.decode('utf-8')) - assert a['faultcode'] == 'Server' + assert a['faultcode'] == 'Client' res = self.app.get( '/authors/998.xml', expect_errors=True ) self.assertEqual(res.status, expected_status) - assert 'Server' in res.body.decode('utf-8') + assert 'Client' in res.body.decode('utf-8') def test_custom_non_http_clientside_error(self): expected_status_code = 500 @@ -123,6 +123,24 @@ class TestWS(FunctionalTest): self.assertEqual(res.status, expected_status) assert 'Server' in res.body.decode('utf-8') + def test_clientsideerror_status_code(self): + expected_status_code = 403 + expected_status = http_response_messages[expected_status_code] + res = self.app.get( + '/authors/996.json', + expect_errors=True + ) + self.assertEqual(res.status, expected_status) + a = json.loads(res.body.decode('utf-8')) + assert a['faultcode'] == 'Client' + + res = self.app.get( + '/authors/996.xml', + expect_errors=True + ) + self.assertEqual(res.status, expected_status) + assert 'Client' in res.body.decode('utf-8') + def test_non_default_response(self): expected_status_code = 401 expected_status = http_response_messages[expected_status_code] diff --git a/tests/test_cornice.py b/tests/test_cornice.py index b950a39..c72fd90 100644 --- a/tests/test_cornice.py +++ b/tests/test_cornice.py @@ -181,5 +181,5 @@ class WSMECorniceTestCase(unittest.TestCase): expect_errors=True ) print resp.body - self.assertEquals(resp.json['faultcode'], 'Server') + self.assertEquals(resp.json['faultcode'], 'Client') self.assertEquals(resp.status_code, 401) diff --git a/tests/test_flask.py b/tests/test_flask.py index 33b7cca..2a34991 100644 --- a/tests/test_flask.py +++ b/tests/test_flask.py @@ -138,7 +138,7 @@ class FlaskrTestCase(unittest.TestCase): headers={'Accept': 'application/xml'} ) assert r.status_code == 403, r.status_code - assert r.data == ('Server' + assert r.data == ('Client' '403: Forbidden' '') @@ -155,7 +155,7 @@ class FlaskrTestCase(unittest.TestCase): headers={'Accept': 'application/xml'} ) assert r.status_code == 412, r.status_code - assert r.data == ('Server' + assert r.data == ('Client' 'FOO!' '') diff --git a/tests/test_tg15.py b/tests/test_tg15.py index 36c6860..b07db05 100644 --- a/tests/test_tg15.py +++ b/tests/test_tg15.py @@ -109,7 +109,7 @@ class TestController(testutil.TGTest): assert response.status_int == 400 assert simplejson.loads(response.body) == { "debuginfo": None, - "faultcode": "Server", + "faultcode": "Client", "faultstring": "(400, 'Cannot divide by zero!')" } @@ -120,7 +120,7 @@ class TestController(testutil.TGTest): expect_errors=True ) assert response.status_int == 400 - assert response.body == ("Server" + assert response.body == ("Client" "(400, 'Cannot divide by zero!')" "") diff --git a/wsme/api.py b/wsme/api.py index b21eff1..c0ec72e 100644 --- a/wsme/api.py +++ b/wsme/api.py @@ -6,6 +6,8 @@ import logging import wsme.exc import wsme.types +from wsme import utils + log = logging.getLogger(__name__) @@ -200,9 +202,12 @@ class Response(object): def format_exception(excinfo, debug=False): """Extract informations that can be sent to the client.""" error = excinfo[1] - if isinstance(error, wsme.exc.ClientSideError): + code = getattr(error, 'code', None) + if code and utils.is_valid_code(code) and utils.is_client_error(code): + faultstring = error.faultstring if hasattr(error, 'faultstring') \ + else str(error) r = dict(faultcode="Client", - faultstring=error.faultstring) + faultstring=faultstring) log.warning("Client-side error: %s" % r['faultstring']) r['debuginfo'] = None return r diff --git a/wsme/exc.py b/wsme/exc.py index 4b62476..0dfc575 100644 --- a/wsme/exc.py +++ b/wsme/exc.py @@ -4,8 +4,9 @@ from wsme.utils import _ class ClientSideError(RuntimeError): - def __init__(self, msg=None): + def __init__(self, msg=None, status_code=400): self.msg = msg + self.code = status_code super(ClientSideError, self).__init__(self.faultstring) @property diff --git a/wsme/tests/test_utils.py b/wsme/tests/test_utils.py index a9c5030..a1da20a 100644 --- a/wsme/tests/test_utils.py +++ b/wsme/tests/test_utils.py @@ -1,7 +1,7 @@ import datetime import unittest -from wsme.utils import parse_isodate, parse_isotime, parse_isodatetime +from wsme import utils class TestUtils(unittest.TestCase): @@ -18,9 +18,9 @@ class TestUtils(unittest.TestCase): '2012-02-30', ] for s, d in good_dates: - assert parse_isodate(s) == d + assert utils.parse_isodate(s) == d for s in ill_formatted_dates + out_of_range_dates: - self.assertRaises(ValueError, parse_isodate, s) + self.assertRaises(ValueError, utils.parse_isodate, s) def test_parse_isotime(self): good_times = [ @@ -35,9 +35,9 @@ class TestUtils(unittest.TestCase): '00:54:60', ] for s, t in good_times: - assert parse_isotime(s) == t + assert utils.parse_isotime(s) == t for s in ill_formatted_times + out_of_range_times: - self.assertRaises(ValueError, parse_isotime, s) + self.assertRaises(ValueError, utils.parse_isotime, s) def test_parse_isodatetime(self): good_datetimes = [ @@ -54,6 +54,27 @@ class TestUtils(unittest.TestCase): '2012-13-12T00:54:60', ] for s, t in good_datetimes: - assert parse_isodatetime(s) == t + assert utils.parse_isodatetime(s) == t for s in ill_formatted_datetimes + out_of_range_datetimes: - self.assertRaises(ValueError, parse_isodatetime, s) + self.assertRaises(ValueError, utils.parse_isodatetime, s) + + def test_validator_with_valid_code(self): + valid_code = 404 + assert ( + utils.is_valid_code(valid_code), + "Valid status code not detected" + ) + + def test_validator_with_invalid_int_code(self): + invalid_int_code = 648 + assert ( + not utils.is_valid_code(invalid_int_code), + "Invalid status code not detected" + ) + + def test_validator_with_invalid_str_code(self): + invalid_str_code = '404' + assert ( + not utils.is_valid_code(invalid_str_code), + "Invalid status code not detected" + ) diff --git a/wsme/utils.py b/wsme/utils.py index 3d7837a..d7f3486 100644 --- a/wsme/utils.py +++ b/wsme/utils.py @@ -1,7 +1,7 @@ import decimal import datetime import re -from six.moves import builtins +from six.moves import builtins, http_client try: import dateutil.parser @@ -84,6 +84,18 @@ def parse_isodatetime(value): raise ValueError("'%s' is a out-of-range datetime" % (value)) +def is_valid_code(code_value): + """ + This function checks if incoming value in http response codes range. + """ + return code_value in http_client.responses + + +def is_client_error(code): + """ Checks client error code (RFC 2616).""" + return 400 <= code < 500 + + try: from collections import OrderedDict except ImportError: diff --git a/wsmeext/flask.py b/wsmeext/flask.py index 4abd3dd..b497698 100644 --- a/wsmeext/flask.py +++ b/wsmeext/flask.py @@ -9,7 +9,7 @@ import wsme.api import wsme.rest.json import wsme.rest.xml import wsme.rest.args -from wsmeext.utils import is_valid_code +from wsme.utils import is_valid_code import flask diff --git a/wsmeext/pecan.py b/wsmeext/pecan.py index e116bd7..66b9e7a 100644 --- a/wsmeext/pecan.py +++ b/wsmeext/pecan.py @@ -11,7 +11,7 @@ import wsme.rest.xml import pecan -from wsmeext.utils import is_valid_code +from wsme.utils import is_valid_code class JSonRenderer(object): @@ -94,9 +94,7 @@ def wsexpose(*args, **kwargs): finally: del exception_info - if data['faultcode'] == 'Client': - pecan.response.status = 400 - elif orig_code and is_valid_code(orig_code): + if orig_code and is_valid_code(orig_code): pecan.response.status = orig_code else: pecan.response.status = 500 diff --git a/wsmeext/tests/test_utils.py b/wsmeext/tests/test_utils.py deleted file mode 100644 index 0ed798e..0000000 --- a/wsmeext/tests/test_utils.py +++ /dev/null @@ -1,22 +0,0 @@ -from wsmeext.utils import is_valid_code - - -class TestUtils(): - - def test_validator_with_valid_code(self): - valid_code = 404 - assert is_valid_code(valid_code), "Valid status code not detected" - - def test_validator_with_invalid_int_code(self): - invalid_int_code = 648 - assert ( - not is_valid_code(invalid_int_code), - "Invalid status code not detected" - ) - - def test_validator_with_invalid_str_code(self): - invalid_str_code = '404' - assert ( - not is_valid_code(invalid_str_code), - "Invalid status code not detected" - ) diff --git a/wsmeext/tg1.py b/wsmeext/tg1.py index 16a7bdf..87399f9 100644 --- a/wsmeext/tg1.py +++ b/wsmeext/tg1.py @@ -14,7 +14,7 @@ from wsme.rest import validate as wsvalidate import wsme.api import wsme.rest.args import wsme.rest.json -from wsmeext.utils import is_valid_code +from wsme.utils import is_valid_code import inspect diff --git a/wsmeext/utils.py b/wsmeext/utils.py deleted file mode 100644 index 58b4013..0000000 --- a/wsmeext/utils.py +++ /dev/null @@ -1,11 +0,0 @@ -""" -This File consists of utils functions used in wsmeext module. -""" -from six.moves import http_client - - -def is_valid_code(code_value): - """ - This function checks if incoming value in http response codes range. - """ - return code_value in http_client.responses