logging_handlers: Remove STAGING_ERROR_NOTIFICATIONS setting.

Running notify_server_error directly from the logging handler can lead
to database queries running in a random context.  Among the many
potential problems that could cause, one actual problem is a
SynchronousOnlyOperation exception when running in an asyncio event
loop.

Signed-off-by: Anders Kaseorg <[email protected]>

Author: andersk

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)
 
 

zerver/logging_handlers.py

@@ -105,20 +105,6 @@ def __init__(self) -> None:
     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 @@ def emit(self, record: logging.LogRecord) -> None:
             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 @@ def emit(self, record: logging.LogRecord) -> None:
                     # 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 @@ def emit(self, record: logging.LogRecord) -> None:
             )
 
         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.

zerver/tests/test_logging_handlers.py

@@ -200,21 +200,20 @@ def get_host_error() -> None:
             "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

zproject/default_settings.py

@@ -403,8 +403,6 @@ class JwtAuthKey(TypedDict):
 # 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

zproject/dev_settings.py

@@ -79,7 +79,6 @@
 EMBEDDED_BOTS_ENABLED = True
 
 SAVE_FRONTEND_STACKTRACES = True
-STAGING_ERROR_NOTIFICATIONS = True
 
 SYSTEM_ONLY_REALMS: Set[str] = set()
 USING_PGROONGA = True