diff --git a/zulip_bots/zulip_bots/bots/giphy/giphy.py b/zulip_bots/zulip_bots/bots/giphy/giphy.py index e2fd19c..91f7d98 100644 --- a/zulip_bots/zulip_bots/bots/giphy/giphy.py +++ b/zulip_bots/zulip_bots/bots/giphy/giphy.py @@ -7,6 +7,8 @@ import os import re from requests.exceptions import HTTPError, ConnectionError +from zulip_bots.custom_exceptions import ConfigValidationError + GIPHY_TRANSLATE_API = 'http://api.giphy.com/v1/gifs/translate' class GiphyHandler(object): @@ -24,21 +26,24 @@ class GiphyHandler(object): The bot responds also to private messages. ''' - def initialize(self, bot_handler: Any) -> None: - self.config_info = bot_handler.get_config_info('giphy') + @staticmethod + def validate_config(config_info: Dict[str, str]) -> None: query = {'s': 'Hello', - 'api_key': self.config_info['key']} + 'api_key': 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)) + raise ConfigValidationError(str(e)) except HTTPError as e: 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) + raise ConfigValidationError(error_message) + + def initialize(self, bot_handler: Any) -> None: + self.config_info = bot_handler.get_config_info('giphy') 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 b3bfea9..09bcaba 100755 --- a/zulip_bots/zulip_bots/bots/giphy/test_giphy.py +++ b/zulip_bots/zulip_bots/bots/giphy/test_giphy.py @@ -29,18 +29,22 @@ class TestGiphyBot(BotTestCase): 'Sorry, I don\'t have a GIF for "world without zulip"! :astonished:', ) - def test_403(self) -> None: + def test_invalid_config(self) -> None: bot = get_bot_message_handler(self.bot_name) bot_handler = StubBotHandler() + with self.mock_http_conversation('test_403'): + self.validate_invalid_config({'key': '12345678'}, + "This is likely due to an invalid key.\n") - with self.mock_config_info({'key': '12345678'}), \ - self.mock_http_conversation('test_403'), \ - self.assertRaises(bot_handler.BotQuitException): - bot.initialize(bot_handler) + def test_valid_config(self) -> None: + bot = get_bot_message_handler(self.bot_name) + bot_handler = StubBotHandler() + with self.mock_http_conversation('test_normal'): + self.validate_valid_config({'key': '12345678'}) def test_connection_error_while_running(self) -> None: with self.mock_config_info({'key': '12345678'}), \ - patch('requests.get', side_effect=[MagicMock(), ConnectionError()]), \ + patch('requests.get', side_effect=[ConnectionError()]), \ patch('logging.exception'): self.verify_reply( 'world without chocolate', diff --git a/zulip_bots/zulip_bots/custom_exceptions.py b/zulip_bots/zulip_bots/custom_exceptions.py new file mode 100644 index 0000000..7b7f92b --- /dev/null +++ b/zulip_bots/zulip_bots/custom_exceptions.py @@ -0,0 +1,11 @@ +# This file implements some custom exceptions that can +# be used by all bots. +# We avoid adding these exceptions to lib.py, because the +# current architecture works by lib.py importing bots, not +# the other way around. + +class ConfigValidationError(Exception): + ''' + Raise if the config data passed to a bot's validate_config() + is invalid (e.g. wrong API key, invalid email, etc.). + ''' diff --git a/zulip_bots/zulip_bots/lib.py b/zulip_bots/zulip_bots/lib.py index 4bf4494..9c4928a 100644 --- a/zulip_bots/zulip_bots/lib.py +++ b/zulip_bots/zulip_bots/lib.py @@ -18,6 +18,7 @@ from typing import Any, Optional, List, Dict, IO, Text, Set from types import ModuleType from zulip import Client, ZulipError +from zulip_bots.custom_exceptions import ConfigValidationError class NoBotConfigException(Exception): pass @@ -272,6 +273,14 @@ def run_message_handler_for_bot(lib_module, quiet, config_file, bot_config_file, restricted_client = ExternalBotHandler(client, bot_dir, bot_details, bot_config_file) message_handler = lib_module.handler_class() + if hasattr(message_handler, 'validate_config'): + config_data = restricted_client.get_config_info(bot_name) + try: + lib_module.handler_class.validate_config(config_data) + except ConfigValidationError as e: + print("There was a problem validating your config file:\n\n{}".format(e)) + sys.exit(1) + if hasattr(message_handler, 'initialize'): message_handler.initialize(bot_handler=restricted_client) diff --git a/zulip_bots/zulip_bots/test_lib.py b/zulip_bots/zulip_bots/test_lib.py index bc44197..24a9f64 100755 --- a/zulip_bots/zulip_bots/test_lib.py +++ b/zulip_bots/zulip_bots/test_lib.py @@ -3,6 +3,10 @@ from unittest import TestCase from typing import List, Dict, Any, Tuple +from zulip_bots.custom_exceptions import ( + ConfigValidationError, +) + from zulip_bots.request_test_lib import ( mock_http_conversation, ) @@ -149,6 +153,15 @@ class BotTestCase(TestCase): response = bot_handler.unique_response() self.assertEqual(expected_response, response['content']) + def validate_invalid_config(self, config_data: Dict[str, str], error_regexp: str) -> None: + bot_class = type(get_bot_message_handler(self.bot_name)) + with self.assertRaisesRegexp(ConfigValidationError, error_regexp): + bot_class.validate_config(config_data) + + def validate_valid_config(self, config_data: Dict[str, str]) -> None: + bot_class = type(get_bot_message_handler(self.bot_name)) + bot_class.validate_config(config_data) + def test_bot_usage(self): # type: () -> None bot = get_bot_message_handler(self.bot_name)