From 50ace55f2b6309b59792aeda332e12ed79472a16 Mon Sep 17 00:00:00 2001 From: dgw Date: Sun, 20 Sep 2020 08:00:15 -0500 Subject: [PATCH 1/3] isup: give specific reason for "down" verdict Specified timeout values, both to improve response time of the command and to make the connect/read timeout errors relevant. (By default, all we'd get is a generic connection error.) The general `RequestException` is no longer considered as a hard "down". Catching all of the relevant sub-types means something truly strange has to happen in order for none of the `except` clauses to match. Sopel's general exception handling is fine for that, since any occurrences of that should be reported as possible bugs. --- sopel/modules/isup.py | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/sopel/modules/isup.py b/sopel/modules/isup.py index afb6049b17..728c3213bc 100644 --- a/sopel/modules/isup.py +++ b/sopel/modules/isup.py @@ -60,22 +60,36 @@ def handle_isup(bot, trigger, secure=True): """ try: site = get_site_url(trigger.group(2)) - response = requests.head(site, verify=secure) + response = requests.head(site, verify=secure, timeout=(10.0, 5.0)) response.raise_for_status() response = response.headers except ValueError as error: bot.reply(str(error)) except requests.exceptions.SSLError: bot.say( - '%s looks down from here. Try using %sisupinsecure' - % (site, bot.config.core.help_prefix)) - except requests.RequestException: - bot.say('%s looks down from here.' % site) + '{} looks down to me (SSL error). Try using `{}isupinsecure`.' + .format(site, bot.config.core.help_prefix)) + except requests.HTTPError: + bot.say( + '{} looks down to me (HTTP {} "{}").' + .format(site, response.status_code, response.reason)) + except requests.ConnectTimeout: + bot.say( + '{} looks down to me (timed out while connecting).' + .format(site)) + except requests.ReadTimeout: + bot.say( + '{} looks down to me (timed out waiting for reply).' + .format(site)) + except requests.ConnectionError: + bot.say( + '{} looks down to me (connection error).' + .format(site)) else: if response: bot.say(site + ' looks fine to me.') - else: - bot.say(site + ' is down from here.') + else: # TODO: Is it even possible to get here any more? + bot.say(site + ' looks down to me.') @plugin.command('isupinsecure') From 5af0eea249238485d70c8e14743358a50115b0f4 Mon Sep 17 00:00:00 2001 From: dgw Date: Fri, 2 Oct 2020 11:39:02 -0500 Subject: [PATCH 2/3] isup: refactor obsolete if/else Simplifies handler function (flattens the logic a bit). --- sopel/modules/isup.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sopel/modules/isup.py b/sopel/modules/isup.py index 728c3213bc..f2f1bfe8a5 100644 --- a/sopel/modules/isup.py +++ b/sopel/modules/isup.py @@ -62,7 +62,6 @@ def handle_isup(bot, trigger, secure=True): site = get_site_url(trigger.group(2)) response = requests.head(site, verify=secure, timeout=(10.0, 5.0)) response.raise_for_status() - response = response.headers except ValueError as error: bot.reply(str(error)) except requests.exceptions.SSLError: @@ -85,11 +84,9 @@ def handle_isup(bot, trigger, secure=True): bot.say( '{} looks down to me (connection error).' .format(site)) - else: - if response: - bot.say(site + ' looks fine to me.') - else: # TODO: Is it even possible to get here any more? - bot.say(site + ' looks down to me.') + + # If no exception happened, the request succeeded. + bot.say(site + ' looks fine to me.') @plugin.command('isupinsecure') From 0059659f32ba106e21817ff3d7d2350b0d62dcc8 Mon Sep 17 00:00:00 2001 From: dgw Date: Fri, 2 Oct 2020 11:46:36 -0500 Subject: [PATCH 3/3] test, isup: verify behavior of `None` in `get_site_url()` --- test/modules/test_modules_isup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/modules/test_modules_isup.py b/test/modules/test_modules_isup.py index 2241c8ea80..5ec854d641 100644 --- a/test/modules/test_modules_isup.py +++ b/test/modules/test_modules_isup.py @@ -31,6 +31,7 @@ def test_get_site_url(site, expected): INVALID_SITE_URLS = ( + None, # missing '', # empty ' ', # empty once stripped 'steam://browsemedia', # invalid protocol