From 5d4a352e2c91382b9b571bf611e900db238e6339 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20H=C3=B6nig?= <robhoenig@gmail.com>
Date: Thu, 4 Jan 2018 13:34:10 +0100
Subject: [PATCH] giphy bot: Fix check for invalid API key.

The previous check didn't notice invalid API keys.
This commit also makes giphy quit on any connectivity
issues during initialization.
---
 zulip_bots/zulip_bots/bots/giphy/giphy.py     | 17 +++++++++--------
 .../zulip_bots/bots/giphy/test_giphy.py       | 19 ++++++-------------
 2 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/zulip_bots/zulip_bots/bots/giphy/giphy.py b/zulip_bots/zulip_bots/bots/giphy/giphy.py
index f4208ad..ab0cad0 100644
--- a/zulip_bots/zulip_bots/bots/giphy/giphy.py
+++ b/zulip_bots/zulip_bots/bots/giphy/giphy.py
@@ -26,18 +26,19 @@ class GiphyHandler(object):
 
     def initialize(self, bot_handler: Any) -> None:
         self.config_info = bot_handler.get_config_info('giphy')
-        query = {'test': 'keyword',
+        query = {'s': 'Hello',
                  'api_key': self.config_info['key']}
         try:
             data = requests.get(GIPHY_TRANSLATE_API, params=query)
+            data.raise_for_status()
+        except ConnectionError as e:
+            bot_handler.quit(str(e))
         except HTTPError as e:
-            if (e.response.json()['error']['errors'][0]['reason'] == 'keyInvalid'):
-                bot_handler.quit('Invalid key.'
-                                 'Follow the instructions in doc.md for setting API key.')
-            else:
-                raise
-        except ConnectionError:
-            logging.warning('Bad connection')
+            error_message = str(e)
+            if (data.status_code == 403):
+                error_message += ('This is likely due to an invalid key.\n'
+                                  'Follow the instructions in doc.md for setting an API key.')
+            bot_handler.quit(error_message)
 
     def handle_message(self, message: Dict[str, str], bot_handler: Any) -> None:
         bot_response = get_bot_giphy_response(
diff --git a/zulip_bots/zulip_bots/bots/giphy/test_giphy.py b/zulip_bots/zulip_bots/bots/giphy/test_giphy.py
index 41dd55b..b3bfea9 100755
--- a/zulip_bots/zulip_bots/bots/giphy/test_giphy.py
+++ b/zulip_bots/zulip_bots/bots/giphy/test_giphy.py
@@ -1,4 +1,4 @@
-from unittest.mock import patch
+from unittest.mock import patch, MagicMock
 from requests.exceptions import HTTPError, ConnectionError
 
 from typing import Any, Union
@@ -33,21 +33,14 @@ class TestGiphyBot(BotTestCase):
         bot = get_bot_message_handler(self.bot_name)
         bot_handler = StubBotHandler()
 
-        with self.mock_config_info({'key': '12345678'}):
+        with self.mock_config_info({'key': '12345678'}), \
+                self.mock_http_conversation('test_403'),  \
+                self.assertRaises(bot_handler.BotQuitException):
             bot.initialize(bot_handler)
 
-        mock_message = {'content': 'Hello'}
-
-        with self.mock_http_conversation('test_403'):
-            with self.assertRaises(HTTPError):
-                # Call the native  handle_message here,
-                # since we don't want to assert a response,
-                # but an exception.
-                bot.handle_message(mock_message, bot_handler)
-
-    def test_connection_error(self) -> None:
+    def test_connection_error_while_running(self) -> None:
         with self.mock_config_info({'key': '12345678'}), \
-                patch('requests.get', side_effect=ConnectionError()), \
+                patch('requests.get', side_effect=[MagicMock(), ConnectionError()]), \
                 patch('logging.exception'):
             self.verify_reply(
                 'world without chocolate',