From b15231dfc2e3e743606041ec062e0a4083ad875d Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 30 Nov 2017 14:06:21 -0800 Subject: [PATCH] logging: Rename AdminZulipHandler to AdminNotifyHandler. This name hasn't been right since f7f2ec0ac back in 2013; this handler sends the log record to a queue, whose consumer will not only maybe send a Zulip message but definitely send an email. I found this pretty confusing when I first worked on this logging code and was looking for how exception emails got sent; so now that I see exactly what's actually happening here, fix it. --- zerver/lib/bugdown/__init__.py | 2 +- zerver/logging_handlers.py | 6 +++--- zerver/tests/test_logging_handlers.py | 12 ++++++------ zproject/settings.py | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/zerver/lib/bugdown/__init__.py b/zerver/lib/bugdown/__init__.py index 4a39f9781d..ccb8866d85 100644 --- a/zerver/lib/bugdown/__init__.py +++ b/zerver/lib/bugdown/__init__.py @@ -1612,7 +1612,7 @@ db_data = None # type: Optional[Dict[Text, Any]] def log_bugdown_error(msg: str) -> None: """We use this unusual logging approach to log the bugdown error, in - order to prevent AdminZulipHandler from sending the santized + order to prevent AdminNotifyHandler from sending the santized original markdown formatting into another Zulip message, which could cause an infinite exception loop.""" logging.getLogger('').error(msg) diff --git a/zerver/logging_handlers.py b/zerver/logging_handlers.py index c107532cc1..4ebeb18de0 100644 --- a/zerver/logging_handlers.py +++ b/zerver/logging_handlers.py @@ -52,9 +52,9 @@ def add_request_metadata(report: Dict[str, Any], request: HttpRequest) -> None: # exception if the host is invalid report['host'] = platform.node() -class AdminZulipHandler(logging.Handler): - """An exception log handler that sends the exception to the queue to be - sent to the Zulip feedback server. +class AdminNotifyHandler(logging.Handler): + """An logging handler that sends the log/exception to the queue to be + turned into an email and/or a Zulip message for the server admins. """ # adapted in part from django/utils/log.py diff --git a/zerver/tests/test_logging_handlers.py b/zerver/tests/test_logging_handlers.py index 9a74c80688..4e3ecf370c 100644 --- a/zerver/tests/test_logging_handlers.py +++ b/zerver/tests/test_logging_handlers.py @@ -15,7 +15,7 @@ from typing import Any, Callable, Dict, Mapping, Optional, Text, Iterator from zerver.lib.request import JsonableError from zerver.lib.test_classes import ZulipTestCase -from zerver.logging_handlers import AdminZulipHandler +from zerver.logging_handlers import AdminNotifyHandler from zerver.middleware import JsonErrorHandler from zerver.views.compatibility import check_compatibility from zerver.worker.queue_processors import QueueProcessingWorker @@ -38,11 +38,11 @@ def capture_and_throw( return wrapped_view return wrapper -class AdminZulipHandlerTest(ZulipTestCase): +class AdminNotifyHandlerTest(ZulipTestCase): logger = logging.getLogger('django') def setUp(self) -> None: - self.handler = AdminZulipHandler() + self.handler = AdminNotifyHandler() # Prevent the exceptions we're going to raise from being printed # You may want to disable this when debugging tests settings.LOGGING_NOT_DISABLED = False @@ -55,14 +55,14 @@ class AdminZulipHandlerTest(ZulipTestCase): def tearDown(self) -> None: settings.LOGGING_NOT_DISABLED = True - def get_admin_zulip_handler(self) -> AdminZulipHandler: + def get_admin_zulip_handler(self) -> AdminNotifyHandler: return [ h for h in logging.getLogger('').handlers - if isinstance(h, AdminZulipHandler) + if isinstance(h, AdminNotifyHandler) ][0] def test_basic(self) -> None: - """A random exception passes happily through AdminZulipHandler""" + """A random exception passes happily through AdminNotifyHandler""" handler = self.get_admin_zulip_handler() try: raise Exception("Testing Error!") diff --git a/zproject/settings.py b/zproject/settings.py index 18b44938d2..65a59cbae7 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -1281,7 +1281,7 @@ LOGGING = { 'handlers': { 'zulip_admins': { 'level': 'ERROR', - 'class': 'zerver.logging_handlers.AdminZulipHandler', + 'class': 'zerver.logging_handlers.AdminNotifyHandler', # For testing the handler delete the next line 'filters': ['ZulipLimiter', 'require_debug_false', 'require_really_deployed'], 'formatter': 'default'