From 11d89ec0b9cbc65be5e179e3bb6f87ca8d08e86b Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Tue, 17 Jan 2023 11:08:15 -0700 Subject: [PATCH] geoip: do not raise on geoip failure This is non-fatal, and users find the Traceback confusing. Sample log statement: 2023-01-17 18:18:19,958 DEBUG subiquity.server.geoip:113 geoip lookup failed: ClientConnectorError(ConnectionKey(host='lmao', port=443, is_ssl=True, ssl=None, proxy=None, proxy_auth=None, proxy_headers_hash=None), gaierror(-5, 'No address associated with hostname')) --- subiquity/server/geoip.py | 21 +++++++-------------- subiquity/server/tests/test_geoip.py | 8 ++++++++ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/subiquity/server/geoip.py b/subiquity/server/geoip.py index 1d38fc68..32146491 100644 --- a/subiquity/server/geoip.py +++ b/subiquity/server/geoip.py @@ -35,10 +35,6 @@ class CheckState(enum.IntEnum): DONE = enum.auto() -class LookupError(Exception): - """ Error to raise when retrieving the GeoIP information. """ - - class GeoIPStrategy(ABC): """ Base class for strategies (e.g. HTTP or dry-run) to retrieve the GeoIP information. """ @@ -76,13 +72,10 @@ class HTTPGeoIPStrategy(GeoIPStrategy): geoip.ubuntu.com service. """ async def get_response(self) -> str: url = "https://geoip.ubuntu.com/lookup" - try: - async with aiohttp.ClientSession() as session: - async with session.get(url) as response: - response.raise_for_status() - return await response.text() - except aiohttp.ClientError as e: - raise LookupError from e + async with aiohttp.ClientSession() as session: + async with session.get(url) as response: + response.raise_for_status() + return await response.text() class GeoIP: @@ -115,13 +108,13 @@ class GeoIP: async def _lookup(self): try: self.response_text = await self.strategy.get_response() - except LookupError: - log.exception("geoip lookup failed") + except aiohttp.ClientError as le: + log.warn("geoip lookup failed: %r", le) return False try: self.element = ElementTree.fromstring(self.response_text) except ElementTree.ParseError: - log.exception("parsing %r failed", self.response_text) + log.debug("parsing response failed: %r", self.response_text) return False changed = False diff --git a/subiquity/server/tests/test_geoip.py b/subiquity/server/tests/test_geoip.py index 5807331b..ebfad3d8 100644 --- a/subiquity/server/tests/test_geoip.py +++ b/subiquity/server/tests/test_geoip.py @@ -13,6 +13,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import aiohttp from aioresponses import aioresponses from subiquitycore.tests import SubiTestCase @@ -96,3 +97,10 @@ class TestGeoIPBadData(SubiTestCase): mocked.get("https://geoip.ubuntu.com/lookup", body=empty_tz) self.assertFalse(await self.geoip.lookup()) self.assertIsNone(self.geoip.timezone) + + async def test_lookup_error(self): + with aioresponses() as mocked: + mocked.get("https://geoip.ubuntu.com/lookup", + exception=aiohttp.ClientError('lookup failure')) + self.assertFalse(await self.geoip.lookup()) + self.assertIsNone(self.geoip.timezone)