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.
This commit is contained in:
Steve Howell 2020-09-02 08:03:58 +00:00 committed by Tim Abbott
parent 3b257d10df
commit e91e21c9e7
2 changed files with 14 additions and 1 deletions

View File

@ -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),
)

View File

@ -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',