diff --git a/atst/routes/errors.py b/atst/routes/errors.py index 7d03d29d..cc3f5f21 100644 --- a/atst/routes/errors.py +++ b/atst/routes/errors.py @@ -12,15 +12,22 @@ from atst.domain.authnid.crl import CRLInvalidException from atst.domain.portfolios import PortfolioError from atst.utils.flash import formatted_flash as flash +NO_NOTIFY_STATUS_CODES = set([404, 401]) + def log_error(e): error_message = e.message if hasattr(e, "message") else str(e) current_app.logger.exception(error_message) +def notify(e, message, code): + if code not in NO_NOTIFY_STATUS_CODES: + current_app.notification_sender.send(message) + + def handle_error(e, message="Not Found", code=404): log_error(e) - current_app.notification_sender.send(message) + notify(e, message, code) return render_template("error.html", message=message), code @@ -57,13 +64,9 @@ def make_error_pages(app): @app.errorhandler(Exception) # pylint: disable=unused-variable def exception(e): - log_error(e) if current_app.debug: raise e - return ( - render_template("error.html", message="An Unexpected Error Occurred"), - 500, - ) + return handle_error(e, message="An Unexpected Error Occurred", code=500) @app.errorhandler(InvitationError) @app.errorhandler(InvitationWrongUserError) diff --git a/tests/conftest.py b/tests/conftest.py index 0b6d4f52..cede1f3c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -309,7 +309,7 @@ def mock_logger(app): app.logger = real_logger -@pytest.fixture(scope="session", autouse=True) +@pytest.fixture(scope="function", autouse=True) def notification_sender(app): real_notification_sender = app.notification_sender app.notification_sender = FakeNotificationSender() diff --git a/tests/routes/test_errors.py b/tests/routes/test_errors.py index 0ffff430..f5b0289e 100644 --- a/tests/routes/test_errors.py +++ b/tests/routes/test_errors.py @@ -1,5 +1,7 @@ import pytest from flask import url_for +from copy import copy +from tests.factories import UserFactory @pytest.fixture @@ -20,3 +22,16 @@ def test_csrf_error(csrf_enabled_app, client): body = response.data.decode() assert "Session Expired" in body assert "Log in required" in body + + +def test_errors_generate_notifications(app, client, user_session, notification_sender): + user_session(UserFactory.create()) + new_app = copy(app) + + @new_app.route("/throw") + def throw(): + raise ValueError() + + new_app.test_client().get("/throw") + + notification_sender.send.assert_called_once() diff --git a/tests/utils.py b/tests/utils.py index cd7c1bba..913a80e7 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -42,4 +42,4 @@ class FakeLogger: self.extras.append(kwargs["extra"]) -FakeNotificationSender = Mock(spec=NotificationSender) +FakeNotificationSender = lambda: Mock(spec=NotificationSender)