From 4692941efa493497d93f47b45eb5863cbbce8ae3 Mon Sep 17 00:00:00 2001 From: Chris Caron Date: Sun, 24 Nov 2019 16:26:11 -0500 Subject: [PATCH] Improved exception handling of JSON responses --- README.md | 4 ++-- apprise/plugins/NotifyD7Networks.py | 20 +++++++++++------- apprise/plugins/NotifyEmby.py | 19 ++++++++++------- apprise/plugins/NotifyGitter.py | 3 ++- apprise/plugins/NotifyMatrix.py | 5 ++++- apprise/plugins/NotifyPushBullet.py | 9 +++++--- apprise/plugins/NotifyRocketChat.py | 12 ++++++++++- apprise/plugins/NotifySlack.py | 6 ++++-- apprise/plugins/NotifyTelegram.py | 32 +++++++++++++++++++++++------ apprise/plugins/NotifyTwilio.py | 10 +++++---- apprise/plugins/NotifyTwitter.py | 3 ++- test/test_rest_plugins.py | 7 +++++++ test/test_telegram.py | 16 +++++++++++++++ 13 files changed, 111 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 6ca73cd7..ff9c58ff 100644 --- a/README.md +++ b/README.md @@ -125,8 +125,8 @@ uptime | apprise \ ### Configuration Files No one wants to put there credentials out for everyone to see on the command line. No problem *apprise* also supports configuration files. It can handle both a specific [YAML format](https://github.com/caronc/apprise/wiki/config_yaml) or a very simple [TEXT format](https://github.com/caronc/apprise/wiki/config_text). You can also pull these configuration files via an HTTP query too! You can read more about the expected structure of the configuration files [here](https://github.com/caronc/apprise/wiki/config). ```bash -# By default now if no url or configuration is specified aprise will -# peek for this data in: +# By default if no url or configuration is specified aprise will attempt to load +# configuration files (if present): # ~/.apprise # ~/.apprise.yml # ~/.config/apprise diff --git a/apprise/plugins/NotifyD7Networks.py b/apprise/plugins/NotifyD7Networks.py index 4de5a285..d784f1cd 100644 --- a/apprise/plugins/NotifyD7Networks.py +++ b/apprise/plugins/NotifyD7Networks.py @@ -318,11 +318,13 @@ class NotifyD7Networks(NotifyBase): json_response = loads(r.content) status_str = json_response.get('message', status_str) - except (AttributeError, ValueError): - # could not parse JSON response... just use the status - # we already have. + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None - # AttributeError means r.content was None + # We could not parse JSON response. + # We will just use the status we already have. pass self.logger.warning( @@ -350,9 +352,13 @@ class NotifyD7Networks(NotifyBase): count = int(json_response.get( 'data', {}).get('messageCount', -1)) - except (AttributeError, ValueError, TypeError): - # could not parse JSON response... just assume - # that our delivery is okay for now + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None + + # We could not parse JSON response. Assume that + # our delivery is okay for now. pass if count != len(self.targets): diff --git a/apprise/plugins/NotifyEmby.py b/apprise/plugins/NotifyEmby.py index 19afc5ad..c792b49b 100644 --- a/apprise/plugins/NotifyEmby.py +++ b/apprise/plugins/NotifyEmby.py @@ -240,9 +240,12 @@ class NotifyEmby(NotifyBase): try: results = loads(r.content) - except ValueError: - # A string like '' would cause this; basicallly the content - # that was provided was not a JSON string. We can stop here + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None + + # This is a problem; abort return False # Acquire our Access Token @@ -400,10 +403,12 @@ class NotifyEmby(NotifyBase): try: results = loads(r.content) - except ValueError: - # A string like '' would cause this; basicallly the content - # that was provided was not a JSON string. There is nothing - # more we can do at this point + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None + + # We need to abort at this point return sessions for entry in results: diff --git a/apprise/plugins/NotifyGitter.py b/apprise/plugins/NotifyGitter.py index d3632b2f..84a2322c 100644 --- a/apprise/plugins/NotifyGitter.py +++ b/apprise/plugins/NotifyGitter.py @@ -333,9 +333,10 @@ class NotifyGitter(NotifyBase): try: content = loads(r.content) - except (TypeError, ValueError): + except (AttributeError, TypeError, ValueError): # ValueError = r.content is Unparsable # TypeError = r.content is None + # AttributeError = r is None content = {} try: diff --git a/apprise/plugins/NotifyMatrix.py b/apprise/plugins/NotifyMatrix.py index 47c848c5..97ab127c 100644 --- a/apprise/plugins/NotifyMatrix.py +++ b/apprise/plugins/NotifyMatrix.py @@ -921,8 +921,11 @@ class NotifyMatrix(NotifyBase): # Return; we're done return (False, response) - except ValueError: + except (AttributeError, TypeError, ValueError): # This gets thrown if we can't parse our JSON Response + # - ValueError = r.content is Unparsable + # - TypeError = r.content is None + # - AttributeError = r is None self.logger.warning('Invalid response from Matrix server.') self.logger.debug( 'Response Details:\r\n{}'.format(r.content)) diff --git a/apprise/plugins/NotifyPushBullet.py b/apprise/plugins/NotifyPushBullet.py index 42b98174..af239c40 100644 --- a/apprise/plugins/NotifyPushBullet.py +++ b/apprise/plugins/NotifyPushBullet.py @@ -307,10 +307,13 @@ class NotifyPushBullet(NotifyBase): try: response = loads(r.content) - except (TypeError, AttributeError, ValueError): - # AttributeError means r.content was None + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None + + # Fall back to the existing unparsed value response = r.content - pass if r.status_code not in ( requests.codes.ok, requests.codes.no_content): diff --git a/apprise/plugins/NotifyRocketChat.py b/apprise/plugins/NotifyRocketChat.py index feab2f74..cca947ed 100644 --- a/apprise/plugins/NotifyRocketChat.py +++ b/apprise/plugins/NotifyRocketChat.py @@ -564,9 +564,19 @@ class NotifyRocketChat(NotifyBase): self.headers['X-User-Id'] = response.get( 'data', {'userId': None}).get('userId') + except (AttributeError, TypeError, ValueError): + # Our response was not the JSON type we had expected it to be + # - ValueError = r.content is Unparsable + # - TypeError = r.content is None + # - AttributeError = r is None + self.logger.warning( + 'A commuication error occured authenticating {} on ' + 'Rocket.Chat.'.format(self.user)) + return False + except requests.RequestException as e: self.logger.warning( - 'A Connection error occured authenticating {} on ' + 'A connection error occured authenticating {} on ' 'Rocket.Chat.'.format(self.user)) self.logger.debug('Socket Exception: %s' % str(e)) return False diff --git a/apprise/plugins/NotifySlack.py b/apprise/plugins/NotifySlack.py index 62b3bcb7..e16885e6 100644 --- a/apprise/plugins/NotifySlack.py +++ b/apprise/plugins/NotifySlack.py @@ -518,8 +518,10 @@ class NotifySlack(NotifyBase): try: response = loads(r.content) - except (AttributeError, ValueError): - # AttributeError means r.content was None + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None pass if not (response and response.get('ok', True)): diff --git a/apprise/plugins/NotifyTelegram.py b/apprise/plugins/NotifyTelegram.py index 62ab7675..11bfe3e7 100644 --- a/apprise/plugins/NotifyTelegram.py +++ b/apprise/plugins/NotifyTelegram.py @@ -378,6 +378,9 @@ class NotifyTelegram(NotifyBase): 'Telegram User Detection POST URL: %s (cert_verify=%r)' % ( url, self.verify_certificate)) + # Track our response object + response = None + try: r = requests.post( url, @@ -392,9 +395,12 @@ class NotifyTelegram(NotifyBase): try: # Try to get the error message if we can: - error_msg = loads(r.content)['description'] + error_msg = loads(r.content).get('description', 'unknown') - except Exception: + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None error_msg = None if error_msg: @@ -414,6 +420,18 @@ class NotifyTelegram(NotifyBase): return 0 + # Load our response and attempt to fetch our userid + response = loads(r.content) + + except (AttributeError, TypeError, ValueError): + # Our response was not the JSON type we had expected it to be + # - ValueError = r.content is Unparsable + # - TypeError = r.content is None + # - AttributeError = r is None + self.logger.warning( + 'A communication error occured detecting the Telegram User.') + return 0 + except requests.RequestException as e: self.logger.warning( 'A connection error occured detecting the Telegram User.') @@ -442,8 +460,6 @@ class NotifyTelegram(NotifyBase): # "text":"/start", # "entities":[{"offset":0,"length":6,"type":"bot_command"}]}}] - # Load our response and attempt to fetch our userid - response = loads(r.content) if 'ok' in response and response['ok'] is True \ and 'result' in response and len(response['result']): entry = response['result'][0] @@ -584,9 +600,13 @@ class NotifyTelegram(NotifyBase): try: # Try to get the error message if we can: - error_msg = loads(r.content)['description'] + error_msg = loads(r.content).get( + 'description', 'unknown') - except Exception: + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None error_msg = None self.logger.warning( diff --git a/apprise/plugins/NotifyTwilio.py b/apprise/plugins/NotifyTwilio.py index 0a5fd503..ec78e46e 100644 --- a/apprise/plugins/NotifyTwilio.py +++ b/apprise/plugins/NotifyTwilio.py @@ -321,11 +321,13 @@ class NotifyTwilio(NotifyBase): status_code = json_response.get('code', status_code) status_str = json_response.get('message', status_str) - except (AttributeError, ValueError): - # could not parse JSON response... just use the status - # we already have. + except (AttributeError, TypeError, ValueError): + # ValueError = r.content is Unparsable + # TypeError = r.content is None + # AttributeError = r is None - # AttributeError means r.content was None + # We could not parse JSON response. + # We will just use the status we already have. pass self.logger.warning( diff --git a/apprise/plugins/NotifyTwitter.py b/apprise/plugins/NotifyTwitter.py index 829d2197..f6e57624 100644 --- a/apprise/plugins/NotifyTwitter.py +++ b/apprise/plugins/NotifyTwitter.py @@ -534,9 +534,10 @@ class NotifyTwitter(NotifyBase): try: content = loads(r.content) - except (TypeError, ValueError): + except (AttributeError, TypeError, ValueError): # ValueError = r.content is Unparsable # TypeError = r.content is None + # AttributeError = r is None content = {} try: diff --git a/test/test_rest_plugins.py b/test/test_rest_plugins.py index 65971678..dad2f86c 100644 --- a/test/test_rest_plugins.py +++ b/test/test_rest_plugins.py @@ -5061,7 +5061,14 @@ def test_notify_rocketchat_plugin(mock_post, mock_get): # assert obj.logout() is True + # Invalid JSON during Login + mock_post.return_value.content = '{' + mock_get.return_value.content = '}' + assert obj.login() is False + # Prepare Mock to fail + mock_post.return_value.content = '' + mock_get.return_value.content = '' mock_post.return_value.status_code = requests.codes.internal_server_error mock_get.return_value.status_code = requests.codes.internal_server_error diff --git a/test/test_telegram.py b/test/test_telegram.py index d5a25783..af57b5ca 100644 --- a/test/test_telegram.py +++ b/test/test_telegram.py @@ -70,6 +70,22 @@ def test_notify_telegram_plugin(mock_post, mock_get, tmpdir): with pytest.raises(TypeError): plugins.NotifyTelegram(bot_token=None, targets=chat_ids) + # Invalid JSON while trying to detect bot owner + mock_get.return_value.content = '{' + mock_post.return_value.content = '}' + with pytest.raises(TypeError): + plugins.NotifyTelegram(bot_token=bot_token, targets=None) + + # Invalid JSON while trying to detect bot owner + 400 error + mock_get.return_value.status_code = requests.codes.internal_server_error + mock_post.return_value.status_code = requests.codes.internal_server_error + with pytest.raises(TypeError): + plugins.NotifyTelegram(bot_token=bot_token, targets=None) + + # Return status back to how they were + mock_post.return_value.status_code = requests.codes.ok + mock_get.return_value.status_code = requests.codes.ok + # Exception should be thrown about the fact an invalid bot token was # specifed with pytest.raises(TypeError):