Skip to content

Commit

Permalink
🐛 Fix issues with statsd counting exceptions
Browse files Browse the repository at this point in the history
* Refactors & bug fixes & rev
  • Loading branch information
bbelyeu committed Feb 17, 2018
1 parent 4eee8ff commit 911ad10
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 82 deletions.
16 changes: 12 additions & 4 deletions README.md
Expand Up @@ -36,10 +36,18 @@ to catch the base exception used (APIException).
response.status_code = error.status_code
return response

You may add a statsd ([pystatsd](https://pypi.python.org/pypi/pystatsd/) interface) counter via app
configuration with `EXCEPTION_COUNTER`. Pass the namespaced path to the instantiated statsd
StatsClient-like object.
`app.statsd` is the default location the extension will look for a statsd StatsClient-like object.
You may add a statsd ([pystatsd](https://pypi.python.org/pypi/pystatsd/) interface) counters to
exceptions by passing a statsd kwarg to the initialization. It can be any counter compliant with
the statsd(https://pypi.python.org/pypi/statsd) interface. You can use this extension with the
Flask-StatsDClient(https://pypi.python.org/pypi/Flask-StatsDClient/1.0.1) simply like:

from flask import Flask
from flask_exceptions import AddExceptions
from flask_statsdclient import StatsDClient

app = Flask(__name__)
statsd = StatsDClient(app)
exceptions = AddExceptions(app, statsd)

The default StatsD counter will be in the form `exceptions.xxx` with xxx being the status code.
This does not take into account any prefix you added when instantiating StatsClient itself.
Expand Down
10 changes: 4 additions & 6 deletions flask_exceptions/extension.py
Expand Up @@ -2,7 +2,6 @@
from functools import wraps

DEFAULT_PREFIX = 'exceptions'
DEFAULT_COUNTER = 'statsd'


def exception(message):
Expand Down Expand Up @@ -102,15 +101,15 @@ def __init__(self, **kwargs):
class AddExceptions(object):
"""Class to wrap Flask app and provide access to additional exceptions."""

def __init__(self, app=None, config=None):
def __init__(self, app=None, config=None, statsd=None):
self.config = config
if app is not None:
self.app = app
self.init_app(app)
self.init_app(app, statsd=statsd)
else:
self.app = None

def init_app(self, app, config=None):
def init_app(self, app, config=None, statsd=None):
"""Init Flask Extension."""
if config is not None:
self.config = config
Expand All @@ -119,8 +118,7 @@ def init_app(self, app, config=None):

self.messages = self.config.get('EXCEPTION_MESSAGE', True)
self.prefix = self.config.get('EXCEPTION_PREFIX', DEFAULT_PREFIX)
statsd = self.config.get('EXCEPTION_COUNTER', DEFAULT_COUNTER)
self.statsd = getattr(app, str(statsd), None)
self.statsd = statsd

# Exception class wrappers sorted by error code

Expand Down
97 changes: 27 additions & 70 deletions flask_exceptions/tests/test_extension.py
@@ -1,7 +1,6 @@
"""Test the statsd extension module."""
# pylint: disable=no-member
import unittest
from functools import wraps
from unittest.mock import MagicMock

from flask import Flask
Expand All @@ -17,19 +16,6 @@ def create_app():
return app


def mock_statsd(func):
"""Decorator to mock statsd object."""

@wraps(func)
def wrapper(self, *args, **kwargs):
"""Wrapper for mocking statsd."""
self.app.statsd = MagicMock()
self.app.statsd.incr = MagicMock()
return func(self, *args, **kwargs)

return wrapper


class TestExceptions(unittest.TestCase):
"""Test extension module."""

Expand All @@ -54,10 +40,8 @@ def test_custom_app_config(self):
"""Test custom configs set on app."""
self.app.config['EXCEPTION_MESSAGE'] = False
self.app.config['EXCEPTION_PREFIX'] = 'foo'
self.app.config['EXCEPTION_COUNTER'] = 'instrumentation'

self.app.instrumentation = MagicMock()
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())

self.assertEqual(False, exceptions.messages)
self.assertEqual('foo', exceptions.prefix)
Expand All @@ -68,11 +52,9 @@ def test_init_kwarg_config(self):
config = {
'EXCEPTION_MESSAGE': False,
'EXCEPTION_PREFIX': 'foo',
'EXCEPTION_COUNTER': 'instrumentation'
}

self.app.instrumentation = MagicMock()
exceptions = AddExceptions(self.app, config)
exceptions = AddExceptions(self.app, config, statsd=MagicMock())

self.assertEqual(False, exceptions.messages)
self.assertEqual('foo', exceptions.prefix)
Expand All @@ -83,150 +65,125 @@ def test_init_app_kwarg_config(self):
config = {
'EXCEPTION_MESSAGE': False,
'EXCEPTION_PREFIX': 'foo',
'EXCEPTION_COUNTER': 'instrumentation'
}

self.app.instrumentation = MagicMock()
exceptions = AddExceptions()
exceptions.init_app(self.app, config)

self.assertEqual(False, exceptions.messages)
self.assertEqual('foo', exceptions.prefix)
self.assertIsInstance(exceptions.statsd, MagicMock)
self.assertEqual(None, exceptions.statsd)

# Test ALL flows with 400/Bad Request once, then each exception shouldn't need to test
# each edge case usage

@mock_statsd
def test_bad_request(self):
"""Test BadRequest/400 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
bad_request = exceptions.bad_request()

self.assertIsInstance(bad_request, extension.BadRequest)
self.assertDictEqual(bad_request.to_dict(), {'message': 'Invalid request parameters'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')

@mock_statsd
def test_bad_request_custom_msg_arg(self):
"""Test BadRequest/400 exception with a custom error message passed as an arg."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
bad_request = exceptions.bad_request('Oh noes!')

self.assertIsInstance(bad_request, extension.BadRequest)
self.assertDictEqual(bad_request.to_dict(), {'message': 'Oh noes!'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')

@mock_statsd
def test_bad_request_custom_msg(self):
"""Test BadRequest/400 exception with a custom error message passed as a kwarg."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
bad_request = exceptions.bad_request(message='Oh noes!')

self.assertIsInstance(bad_request, extension.BadRequest)
self.assertDictEqual(bad_request.to_dict(), {'message': 'Oh noes!'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')

@mock_statsd
def test_bad_request_no_msg(self):
"""Test BadRequest/400 exception with no message."""
self.app.config['EXCEPTION_MESSAGE'] = False
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
bad_request = exceptions.bad_request()

self.assertIsInstance(bad_request, extension.BadRequest)
self.assertDictEqual(bad_request.to_dict(), {})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')

@mock_statsd
def test_bad_request_payload(self):
"""Test BadRequest/400 exception with custom payload."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
bad_request = exceptions.bad_request(payload={'code': '4-8-15-16-23-42'})

self.assertIsInstance(bad_request, extension.BadRequest)
self.assertDictEqual(bad_request.to_dict(), {
'code': '4-8-15-16-23-42', 'message': 'Invalid request parameters'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.400')

@mock_statsd
def test_bad_request_no_statsd(self):
"""Test BadRequest/400 exception with no statsd object in config."""
self.app.config['EXCEPTION_COUNTER'] = None
exceptions = AddExceptions(self.app)
bad_request = exceptions.bad_request()

self.assertIsInstance(bad_request, extension.BadRequest)
self.assertDictEqual(bad_request.to_dict(), {'message': 'Invalid request parameters'})
self.app.statsd.incr.assert_not_called()

@mock_statsd
def test_unauthorized(self):
"""Test Unauthorized/401 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
unauthorized = exceptions.unauthorized()

self.assertIsInstance(unauthorized, extension.Unauthorized)
self.assertDictEqual(unauthorized.to_dict(), {'message': 'Unauthorized'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.401')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.401')

@mock_statsd
def test_forbidden(self):
"""Test Forbidden/403 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
forbidden = exceptions.forbidden()

self.assertIsInstance(forbidden, extension.Forbidden)
self.assertDictEqual(forbidden.to_dict(), {'message': 'Forbidden'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.403')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.403')

@mock_statsd
def test_not_found(self):
"""Test NotFound/404 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
not_found = exceptions.not_found()

self.assertIsInstance(not_found, extension.NotFound)
self.assertDictEqual(not_found.to_dict(), {'message': 'Resource not found'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.404')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.404')

@mock_statsd
def test_conflict(self):
"""Test Conflict/409 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
conflict = exceptions.conflict()

self.assertIsInstance(conflict, extension.Conflict)
self.assertDictEqual(conflict.to_dict(), {'message': 'Conflict'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.409')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.409')

@mock_statsd
def test_gone(self):
"""Test Gone/410 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
gone = exceptions.gone()

self.assertIsInstance(gone, extension.Gone)
self.assertDictEqual(gone.to_dict(), {'message': 'Gone'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.410')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.410')

@mock_statsd
def test_unsupported_media(self):
"""Test UnsupportedMedia/415 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
unsupported_media = exceptions.unsupported_media()

self.assertIsInstance(unsupported_media, extension.UnsupportedMedia)
self.assertDictEqual(unsupported_media.to_dict(), {'message': 'Unsupported Media'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.415')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.415')

@mock_statsd
def test_unprocessable_entity(self):
"""Test UnprocessableEntity/422 exception."""
exceptions = AddExceptions(self.app)
exceptions = AddExceptions(self.app, statsd=MagicMock())
unprocessable_entity = exceptions.unprocessable_entity()

self.assertIsInstance(unprocessable_entity, extension.UnprocessableEntity)
self.assertDictEqual(unprocessable_entity.to_dict(), {'message':
'Unprocessable Entity'})
self.app.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.422')
exceptions.statsd.incr.assert_called_once_with(extension.DEFAULT_PREFIX + '.422')
4 changes: 2 additions & 2 deletions setup.py
Expand Up @@ -3,9 +3,9 @@

setup(
name='Flask-Exceptions',
version='1.1.1',
version='1.2.0',
url='https://github.com/bbelyeu/flask-exceptions',
download_url='https://github.com/bbelyeu/flask-exceptions/archive/1.1.1.zip',
download_url='https://github.com/bbelyeu/flask-exceptions/archive/1.2.0.zip',
license='MIT',
author='Brad Belyeu',
author_email='bradleylamar@gmail.com',
Expand Down

0 comments on commit 911ad10

Please sign in to comment.