bugdown: Improve exception handler for BugdownRenderingException.

This has the benefit that we now get the usual data about the
user/request/etc. in error emails related to bugdown exceptions;
previously we were just getting the traceback in the emails (since our
`mail_admins` template was very simple) and no other debugging
details.

Comments tweaked by tabbott to help make clear exactly what's going on
here, since it's a little subtle and a little hacky.

Fixes #8843.

zerver/lib/bugdown/__init__.py

@@ -1967,14 +1967,12 @@ def do_convert(content: str,
         return rendered_content
     except Exception:
         cleaned = privacy_clean_markdown(content)
-
-        # Output error to log as well as sending a zulip and email
-        log_bugdown_error('Exception in Markdown parser: %sInput (sanitized) was: %s'
-                          % (traceback.format_exc(), cleaned))
-        subject = "Markdown parser failure on %s" % (platform.node(),)
-        mail.mail_admins(
-            subject, "Failed message: %s\n\n%s\n\n" % (cleaned, traceback.format_exc()),
-            fail_silently=False)
+        # NOTE: Don't change this message without also changing the
+        # logic in logging_handlers.py or we can create recursive
+        # exceptions.
+        exception_message = ('Exception in Markdown parser: %sInput (sanitized) was: %s'
+                             % (traceback.format_exc(), cleaned))
+        bugdown_logger.exception(exception_message)
 
         raise BugdownRenderingException()
     finally:

zerver/lib/error_notify.py

@@ -83,10 +83,10 @@ def zulip_browser_error(report: Dict[str, Any]) -> None:
     internal_send_message(realm, settings.ERROR_BOT,
                           "stream", "errors", format_subject(subject), body)
 
-def notify_server_error(report: Dict[str, Any]) -> None:
+def notify_server_error(report: Dict[str, Any], skip_error_zulip: Optional[bool]=False) -> None:
     report = defaultdict(lambda: None, report)
     email_server_error(report)
-    if settings.ERROR_BOT:
+    if settings.ERROR_BOT and not skip_error_zulip:
         zulip_server_error(report)
 
 def zulip_server_error(report: Dict[str, Any]) -> None:

zerver/logging_handlers.py

@@ -90,6 +90,20 @@ class AdminNotifyHandler(logging.Handler):
     def emit(self, record: logging.LogRecord) -> None:
         report = {}  # type: 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_bugdown_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
+        # bugdown.
+        is_bugdown_rendering_exception = True
+
         try:
             report['node'] = platform.node()
             report['host'] = platform.node()
@@ -99,6 +113,8 @@ class AdminNotifyHandler(logging.Handler):
             if record.exc_info:
                 stack_trace = ''.join(traceback.format_exception(*record.exc_info))
                 message = str(record.exc_info[1])
+                from zerver.lib.exceptions import BugdownRenderingException
+                is_bugdown_rendering_exception = record.msg.startswith('Exception in Markdown parser')
             else:
                 stack_trace = 'No stack trace available'
                 message = record.getMessage()
@@ -107,6 +123,7 @@ class AdminNotifyHandler(logging.Handler):
                     # seem to result in super-long messages
                     stack_trace = message
                     message = message.split('\n')[0]
+                is_bugdown_rendering_exception = False
             report['stack_trace'] = stack_trace
             report['message'] = message
 
@@ -133,7 +150,7 @@ class AdminNotifyHandler(logging.Handler):
                 # 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)
+                notify_server_error(report, is_bugdown_rendering_exception)
             else:
                 queue_json_publish('error_reports', dict(
                     type = "server",