outgoing webhook: Log all non-200 responses.

This commit is contained in:
Robert Hönig
2017-08-28 18:51:13 +02:00
committed by Tim Abbott
parent 41e6704b99
commit 15a1bf2b58
2 changed files with 29 additions and 10 deletions

View File

@@ -13,6 +13,7 @@ from requests import Response
from django.utils.translation import ugettext as _
from zerver.context_processors import zulip_default_context
from zerver.models import Realm, UserProfile, get_user_profile_by_id, get_client, \
GENERIC_INTERFACE, Service, SLACK_INTERFACE, email_to_domain, get_service_profile
from zerver.lib.actions import check_send_message
@@ -196,9 +197,21 @@ def do_rest_call(rest_operation, request_data, event, service_handler, timeout=N
response_message = service_handler.process_success(response, event)
if response_message is not None:
succeed_with_message(event, response_message)
else:
context = zulip_default_context(request_data)
message_url = ("%(server)s/#narrow/stream/%(stream)s/subject/%(subject)s/near/%(id)s"
% {'server': context['realm_uri'],
'stream': event['message']['display_recipient'],
'subject': event['message']['subject'],
'id': str(event['message']['id'])})
logging.warning("Message %(message_url)s triggered an outgoing webhook, returning status "
"code %(status_code)s.\n Content of response (in quotes): \""
"%(response)s\""
% {'message_url': message_url,
'status_code': response.status_code,
'response': response.content})
# On 50x errors, try retry
elif str(response.status_code).startswith('5'):
if str(response.status_code).startswith('5'):
request_retry(event, "Internal Server error at third party.")
else:
failure_message = "Third party responded with %d" % (response.status_code)

View File

@@ -11,6 +11,12 @@ from zerver.lib.outgoing_webhook import do_rest_call, OutgoingWebhookServiceInte
from zerver.lib.test_classes import ZulipTestCase
from builtins import object
mock_event = {'message': {'display_recipient': '',
'subject': '',
'id': ''},
'command': '',
'service_name': ''}
rest_operation = {'method': "POST",
'relative_url_path': "",
'request_kwargs': {},
@@ -52,7 +58,7 @@ class DoRestCallTests(ZulipTestCase):
# type: (mock.Mock) -> None
response = ResponseMock(500, {"message": "testing"}, '')
with mock.patch('requests.request', return_value=response):
do_rest_call(rest_operation, None, None, service_handler, None)
do_rest_call(rest_operation, None, mock_event, service_handler, None)
self.assertTrue(mock_request_retry.called)
@mock.patch('zerver.lib.outgoing_webhook.fail_with_message')
@@ -60,7 +66,7 @@ class DoRestCallTests(ZulipTestCase):
# type: (mock.Mock) -> None
response = ResponseMock(400, {"message": "testing"}, '')
with mock.patch('requests.request', return_value=response):
do_rest_call(rest_operation, None, None, service_handler, None)
do_rest_call(rest_operation, None, mock_event, service_handler, None)
self.assertTrue(mock_fail_with_message.called)
@mock.patch('logging.info')
@@ -68,7 +74,7 @@ class DoRestCallTests(ZulipTestCase):
@mock.patch('zerver.lib.outgoing_webhook.request_retry')
def test_timeout_request(self, mock_request_retry, mock_requests_request, mock_logger):
# type: (mock.Mock, mock.Mock, mock.Mock) -> None
do_rest_call(rest_operation, None, {"command": "", "service_name": ""}, service_handler, None)
do_rest_call(rest_operation, None, mock_event, service_handler, None)
self.assertTrue(mock_request_retry.called)
@mock.patch('logging.exception')
@@ -76,5 +82,5 @@ class DoRestCallTests(ZulipTestCase):
@mock.patch('zerver.lib.outgoing_webhook.fail_with_message')
def test_request_exception(self, mock_fail_with_message, mock_requests_request, mock_logger):
# type: (mock.Mock, mock.Mock, mock.Mock) -> None
do_rest_call(rest_operation, None, {"command": ""}, service_handler, None)
do_rest_call(rest_operation, None, mock_event, service_handler, None)
self.assertTrue(mock_fail_with_message.called)