From 3790c469e9525a98aa32a19ea8b2254b9d8f6f4e Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Wed, 10 Oct 2018 13:11:50 +0000
Subject: [PATCH] outgoing bots: Report JSON errors to users.

We should arguably report these to bot owners
as well, but this is at least an improvement
over having the server crash.
---
 zerver/lib/outgoing_webhook.py                |  7 +++-
 .../tests/test_outgoing_webhook_interfaces.py | 39 ++++++++++++++++++-
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py
index ce073a1691..08f34de7a3 100644
--- a/zerver/lib/outgoing_webhook.py
+++ b/zerver/lib/outgoing_webhook.py
@@ -254,7 +254,12 @@ def request_retry(event: Dict[str, Any],
 def process_success_response(event: Dict[str, Any],
                              service_handler: Any,
                              response: Response) -> None:
-    response_json = json.loads(response.text)
+    try:
+        response_json = json.loads(response.text)
+    except ValueError:
+        fail_with_message(event, "Invalid JSON in response")
+        return
+
     success_data = service_handler.process_success(response_json, event)
 
     if success_data is None:
diff --git a/zerver/tests/test_outgoing_webhook_interfaces.py b/zerver/tests/test_outgoing_webhook_interfaces.py
index 2c2e5f1a71..6ab19ee5b2 100644
--- a/zerver/tests/test_outgoing_webhook_interfaces.py
+++ b/zerver/tests/test_outgoing_webhook_interfaces.py
@@ -1,11 +1,12 @@
 # -*- coding: utf-8 -*-
-from typing import Any, Dict
+from typing import cast, Any, Dict
 
 import mock
 import json
+import requests
 
 from zerver.lib.outgoing_webhook import GenericOutgoingWebhookService, \
-    SlackOutgoingWebhookService
+    SlackOutgoingWebhookService, process_success_response
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.models import Service, get_realm, get_user
 
@@ -25,6 +26,40 @@ class TestGenericOutgoingWebhookService(ZulipTestCase):
                                                      token='abcdef',
                                                      user_profile=self.bot_user)
 
+    def test_process_success_response(self) -> None:
+        class Stub:
+            def __init__(self, text: str) -> None:
+                self.text = text  # type: ignore
+
+        def make_response(text: str) -> requests.Response:
+            return cast(requests.Response, Stub(text=text))
+
+        event = dict(
+            user_profile_id=99,
+            message=dict(type='private')
+        )
+        service_handler = self.handler
+
+        response = make_response(text=json.dumps(dict(content='whatever')))
+
+        with mock.patch('zerver.lib.outgoing_webhook.send_response_message') as m:
+            process_success_response(
+                event=event,
+                service_handler=service_handler,
+                response=response,
+            )
+        self.assertTrue(m.called)
+
+        response = make_response(text='unparsable text')
+
+        with mock.patch('zerver.lib.outgoing_webhook.fail_with_message') as m:
+            process_success_response(
+                event=event,
+                service_handler=service_handler,
+                response=response
+            )
+        self.assertTrue(m.called)
+
     def test_process_event(self) -> None:
         rest_operation, request_data = self.handler.process_event(self.event)
         request_data = json.loads(request_data)