Skip to content

Commit

Permalink
Merge pull request #5373 from hypothesis/pass-correct-exception-to-se…
Browse files Browse the repository at this point in the history
…ntry

Always pass correct exception object to Sentry in exception view
  • Loading branch information
robertknight committed Oct 12, 2018
2 parents af5ef8c + 7e82cd0 commit 472da02
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 19 deletions.
11 changes: 8 additions & 3 deletions h/util/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@
from pyramid.view import view_config


def handle_exception(request):
"""Handle an uncaught exception for the passed request."""
def handle_exception(request, exception):
"""
Handle an uncaught exception for the passed request.
:param request: The Pyramid request which caused the exception.
:param exception: The exception passed as context to the exception-handling view.
"""
request.response.status_int = 500
request.sentry.captureException()
request.sentry.captureException(exception)
# In debug mode we should just reraise, so that the exception is caught by
# the debug toolbar.
if request.debug:
Expand Down
4 changes: 2 additions & 2 deletions h/views/api/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ def api_validation_error(context, request):


@json_view(context=Exception, path_info='/api/', decorator=cors_policy)
def json_error(request):
def json_error(context, request):
"""Handle an unexpected exception in an API view."""
handle_exception(request)
handle_exception(request, exception=context)
message = _("Hypothesis had a problem while handling this request. "
"Our team has been notified. Please contact support@hypothes.is"
" if the problem persists.")
Expand Down
8 changes: 4 additions & 4 deletions h/views/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ def notfound(request):
@view_config(context=Exception,
accept='text/html',
renderer='h:templates/5xx.html.jinja2')
def error(request):
def error(context, request):
"""Handle a request for which the handler threw an exception."""
handle_exception(request)
handle_exception(request, exception=context)
return {}


@json_view(context=Exception)
def json_error(request):
def json_error(context, request):
"""Handle an unexpected exception where the request asked for JSON."""
handle_exception(request)
handle_exception(request, exception=context)
message = _("Hypothesis had a problem while handling this request. "
"Our team has been notified. Please contact support@hypothes.is"
" if the problem persists.")
Expand Down
9 changes: 5 additions & 4 deletions tests/h/util/view_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@

class TestHandleException(object):
def test_sets_response_status_500(self, pyramid_request):
handle_exception(pyramid_request)
handle_exception(pyramid_request, Mock())

assert pyramid_request.response.status_int == 500

def test_triggers_sentry_capture(self, pyramid_request):
handle_exception(pyramid_request)
exception = Mock()
handle_exception(pyramid_request, exception)

pyramid_request.sentry.captureException.assert_called_once_with()
pyramid_request.sentry.captureException.assert_called_once_with(exception)

def test_reraises_in_debug_mode(self, pyramid_request):
pyramid_request.debug = True
Expand All @@ -27,7 +28,7 @@ def test_reraises_in_debug_mode(self, pyramid_request):
raise dummy_exc
except:
with pytest.raises(ValueError) as exc:
handle_exception(pyramid_request)
handle_exception(pyramid_request, Mock())
assert exc.value == dummy_exc

@pytest.fixture
Expand Down
7 changes: 5 additions & 2 deletions tests/h/views/api/exceptions_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from __future__ import unicode_literals

from mock import Mock

from h.exceptions import APIError
from h.schemas import ValidationError
from h.views.api import exceptions as views
Expand Down Expand Up @@ -38,8 +40,9 @@ def test_api_validation_error(pyramid_request):
def test_json_error_view(patch, pyramid_request):
handle_exception = patch('h.views.api.exceptions.handle_exception')

result = views.json_error(pyramid_request)
exception = Mock()
result = views.json_error(exception, pyramid_request)

handle_exception.assert_called_once_with(pyramid_request)
handle_exception.assert_called_once_with(pyramid_request, exception)
assert result['status'] == 'failure'
assert result['reason']
12 changes: 8 additions & 4 deletions tests/h/views/exceptions_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from __future__ import unicode_literals

from mock import Mock

from h.views.exceptions import notfound, error, json_error


Expand All @@ -14,18 +16,20 @@ def test_notfound_view(pyramid_request):

def test_error_view(patch, pyramid_request):
handle_exception = patch('h.views.exceptions.handle_exception')
exception = Mock()

result = error(pyramid_request)
result = error(exception, pyramid_request)

handle_exception.assert_called_once_with(pyramid_request)
handle_exception.assert_called_once_with(pyramid_request, exception)
assert result == {}


def test_json_error_view(patch, pyramid_request):
handle_exception = patch('h.views.exceptions.handle_exception')
exception = Mock()

result = json_error(pyramid_request)
result = json_error(exception, pyramid_request)

handle_exception.assert_called_once_with(pyramid_request)
handle_exception.assert_called_once_with(pyramid_request, exception)
assert result['status'] == 'failure'
assert result['reason']

0 comments on commit 472da02

Please sign in to comment.