diff --git a/zerver/lib/error_notify.py b/zerver/lib/error_notify.py index 1af7a2f81e..0f10086306 100644 --- a/zerver/lib/error_notify.py +++ b/zerver/lib/error_notify.py @@ -104,10 +104,10 @@ def zulip_browser_error(report: Dict[str, Any]) -> None: ) -def notify_server_error(report: Dict[str, Any], skip_error_zulip: bool = False) -> None: +def notify_server_error(report: Dict[str, Any]) -> None: report = defaultdict(lambda: None, report) email_server_error(report) - if settings.ERROR_BOT and not skip_error_zulip: + if settings.ERROR_BOT: zulip_server_error(report) diff --git a/zerver/logging_handlers.py b/zerver/logging_handlers.py index 47a47e19a9..5b5e3e8684 100644 --- a/zerver/logging_handlers.py +++ b/zerver/logging_handlers.py @@ -105,20 +105,6 @@ class AdminNotifyHandler(logging.Handler): def emit(self, record: logging.LogRecord) -> None: report: Dict[str, Any] = {} - # This parameter determines whether Zulip should attempt to - # send Zulip messages containing the error report. If there's - # syntax that makes the Markdown processor throw an exception, - # we really don't want to send that syntax into a new Zulip - # message in exception handler (that's the stuff of which - # recursive exception loops are made). - # - # We initialize is_markdown_rendering_exception to `True` to - # prevent the infinite loop of Zulip messages by ERROR_BOT if - # the outer try block here throws an exception before we have - # a chance to check the exception for whether it comes from - # the Markdown processor. - is_markdown_rendering_exception = True - try: report["node"] = platform.node() report["host"] = platform.node() @@ -131,9 +117,6 @@ class AdminNotifyHandler(logging.Handler): if record.exc_info: stack_trace = "".join(traceback.format_exception(*record.exc_info)) message = str(record.exc_info[1]) - is_markdown_rendering_exception = record.msg.startswith( - "Exception in Markdown parser" - ) else: stack_trace = "No stack trace available" message = record.getMessage() @@ -142,7 +125,6 @@ class AdminNotifyHandler(logging.Handler): # seem to result in super-long messages stack_trace = message message = message.split("\n")[0] - is_markdown_rendering_exception = False report["stack_trace"] = stack_trace report["message"] = message @@ -171,20 +153,13 @@ class AdminNotifyHandler(logging.Handler): ) try: - if settings.STAGING_ERROR_NOTIFICATIONS: - # On staging, process the report directly so it can happen inside this - # try/except to prevent looping - from zerver.lib.error_notify import notify_server_error - - notify_server_error(report, is_markdown_rendering_exception) - else: - queue_json_publish( - "error_reports", - dict( - type="server", - report=report, - ), - ) + queue_json_publish( + "error_reports", + dict( + type="server", + report=report, + ), + ) except Exception: # If this breaks, complain loudly but don't pass the traceback up the stream # However, we *don't* want to use logging.exception since that could trigger a loop. diff --git a/zerver/tests/test_logging_handlers.py b/zerver/tests/test_logging_handlers.py index 0cd7d699a3..44945b6def 100644 --- a/zerver/tests/test_logging_handlers.py +++ b/zerver/tests/test_logging_handlers.py @@ -200,21 +200,20 @@ class AdminNotifyHandlerTest(ZulipTestCase): "zerver.lib.error_notify.notify_server_error", side_effect=Exception("queue error") ): self.handler.emit(record) - with self.settings(STAGING_ERROR_NOTIFICATIONS=False): - with mock_queue_publish( - "zerver.logging_handlers.queue_json_publish", side_effect=Exception("queue error") - ) as m: - with patch("logging.warning") as log_mock: - self.handler.emit(record) - m.assert_called_once() - log_mock.assert_called_once_with( - "Reporting an exception triggered an exception!", exc_info=True - ) - with mock_queue_publish("zerver.logging_handlers.queue_json_publish") as m: - with patch("logging.warning") as log_mock: - self.handler.emit(record) - m.assert_called_once() - log_mock.assert_not_called() + with mock_queue_publish( + "zerver.logging_handlers.queue_json_publish", side_effect=Exception("queue error") + ) as m: + with patch("logging.warning") as log_mock: + self.handler.emit(record) + m.assert_called_once() + log_mock.assert_called_once_with( + "Reporting an exception triggered an exception!", exc_info=True + ) + with mock_queue_publish("zerver.logging_handlers.queue_json_publish") as m: + with patch("logging.warning") as log_mock: + self.handler.emit(record) + m.assert_called_once() + log_mock.assert_not_called() # Test no exc_info record.exc_info = None diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 5ab39c9691..1f5a7217f7 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -403,8 +403,6 @@ LOG_API_EVENT_TYPES = False # the server. # TODO: Replace this with a smarter "run on only one server" system. STAGING = False -# Configuration option for our email/Zulip error reporting. -STAGING_ERROR_NOTIFICATIONS = False # How long to wait before presence should treat a user as offline. # TODO: Figure out why this is different from the corresponding diff --git a/zproject/dev_settings.py b/zproject/dev_settings.py index 13708f7e29..45fd0a2dfe 100644 --- a/zproject/dev_settings.py +++ b/zproject/dev_settings.py @@ -79,7 +79,6 @@ INVITES_MIN_USER_AGE_DAYS = 0 EMBEDDED_BOTS_ENABLED = True SAVE_FRONTEND_STACKTRACES = True -STAGING_ERROR_NOTIFICATIONS = True SYSTEM_ONLY_REALMS: Set[str] = set() USING_PGROONGA = True