From e91e21c9e79e8518f91af0f967abef83f1b6cf32 Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulip.com>
Date: Wed, 2 Sep 2020 08:03:58 +0000
Subject: [PATCH] webhook logger: Add summary field.

Before this the only way we took advantage
of the summary from UnexpectedWebhookEventType
was by looking at exc_info().

Now we just explicitly add it to the log
message, which also sets us up to call
log_exception_to_webhook_logger directly
with some sort of "summary" info
when we don't actually want a real
exception (for example, we might want to
report anomalous webhook data but still
continue the transaction).

A minor change in passing is that I move
the payload parameter lexically.
---
 zerver/decorator.py             | 7 ++++++-
 zerver/tests/test_decorators.py | 8 ++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/zerver/decorator.py b/zerver/decorator.py
index 82e5b7c9dc..1bbc8d5f30 100644
--- a/zerver/decorator.py
+++ b/zerver/decorator.py
@@ -266,6 +266,7 @@ def access_user_by_api_key(request: HttpRequest, api_key: str, email: Optional[s
 def log_exception_to_webhook_logger(
     request: HttpRequest,
     user_profile: UserProfile,
+    summary: str,
     payload: str,
     unexpected_event: bool,
 ) -> None:
@@ -286,6 +287,7 @@ def log_exception_to_webhook_logger(
     header_message = header_text if header_text else None
 
     message = """
+summary: {summary}
 user: {email} ({realm})
 client: {client_name}
 URL: {path_info}
@@ -296,6 +298,7 @@ body:
 
 {body}
     """.format(
+        summary=summary,
         email=user_profile.delivery_email,
         realm=user_profile.realm.string_id,
         client_name=request.client.name,
@@ -345,9 +348,10 @@ def api_key_only_webhook_view(
                     notify_bot_owner_about_invalid_json(user_profile, webhook_client_name)
                 else:
                     log_exception_to_webhook_logger(
-                        payload=request.body,
                         request=request,
                         user_profile=user_profile,
+                        summary=str(err),
+                        payload=request.body,
                         unexpected_event=isinstance(err, UnexpectedWebhookEventType),
                     )
                 raise err
@@ -597,6 +601,7 @@ def authenticated_rest_api_view(
                         log_exception_to_webhook_logger(
                             request=request,
                             user_profile=profile,
+                            summary=str(err),
                             payload=request_body,
                             unexpected_event=isinstance(err, UnexpectedWebhookEventType),
                         )
diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py
index 9ea828a4cf..f41c008a09 100644
--- a/zerver/tests/test_decorators.py
+++ b/zerver/tests/test_decorators.py
@@ -334,6 +334,7 @@ class DecoratorTestCase(ZulipTestCase):
                 my_webhook_raises_exception(request)
 
             message = """
+summary: {summary}
 user: {email} ({realm})
 client: {client_name}
 URL: {path_info}
@@ -346,6 +347,7 @@ body:
                 """
             message = message.strip(' ')
             mock_exception.assert_called_with(message.format(
+                summary="raised by webhook function",
                 email=webhook_bot_email,
                 realm=webhook_bot_realm.string_id,
                 client_name=webhook_client_name,
@@ -365,6 +367,7 @@ body:
                 my_webhook_raises_exception_unexpected_event(request)
 
             message = """
+summary: {summary}
 user: {email} ({realm})
 client: {client_name}
 URL: {path_info}
@@ -377,6 +380,7 @@ body:
                 """
             message = message.strip(' ')
             mock_exception.assert_called_with(message.format(
+                summary=exception_msg,
                 email=webhook_bot_email,
                 realm=webhook_bot_realm.string_id,
                 client_name=webhook_client_name,
@@ -503,6 +507,7 @@ class DecoratorLoggingTestCase(ZulipTestCase):
                 my_webhook_raises_exception(request)
 
             message = """
+summary: {summary}
 user: {email} ({realm})
 client: {client_name}
 URL: {path_info}
@@ -515,6 +520,7 @@ body:
                 """
             message = message.strip(' ')
             mock_exception.assert_called_with(message.format(
+                summary="raised by webhook function",
                 email=webhook_bot_email,
                 realm=webhook_bot_realm.string_id,
                 client_name='ZulipClientNameWebhook',
@@ -547,6 +553,7 @@ body:
                 my_webhook_raises_exception(request)
 
             message = """
+summary: {summary}
 user: {email} ({realm})
 client: {client_name}
 URL: {path_info}
@@ -559,6 +566,7 @@ body:
                 """
             message = message.strip(' ')
             mock_exception.assert_called_with(message.format(
+                summary=exception_msg,
                 email=webhook_bot_email,
                 realm=webhook_bot_realm.string_id,
                 client_name='ZulipClientNameWebhook',