From 87e922ac488a45260b068ebdbab4358d7d9effc3 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Fri, 15 Nov 2024 14:30:33 +0100 Subject: [PATCH] Catch ASN Parse error more gracefully --- apps/greencheck/domain_check.py | 7 +++++-- apps/greencheck/tests/test_domain_checker.py | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/apps/greencheck/domain_check.py b/apps/greencheck/domain_check.py index 8080a848..39933efc 100644 --- a/apps/greencheck/domain_check.py +++ b/apps/greencheck/domain_check.py @@ -23,7 +23,7 @@ import tld from django.utils import timezone from ipwhois.asn import IPASN -from ipwhois.exceptions import IPDefinedError +from ipwhois.exceptions import IPDefinedError, ASNParseError from ipwhois.net import Net from .choices import GreenlistChoice @@ -309,7 +309,10 @@ def check_for_matching_asn(self, ip_address): try: asn_result = self.asn_from_ip(ip_address) - except IPDefinedError: + except (ASNParseError, IPDefinedError) as ex: + logger.warning( + f"Unable to parse ASN for IP: {ip_address} - error type: {type(ex).__name__} {ex}" + ) return False except Exception as err: logger.exception(err) diff --git a/apps/greencheck/tests/test_domain_checker.py b/apps/greencheck/tests/test_domain_checker.py index 8657d720..838dea69 100644 --- a/apps/greencheck/tests/test_domain_checker.py +++ b/apps/greencheck/tests/test_domain_checker.py @@ -104,6 +104,20 @@ def test_with_green_domain_by_asn_double(self, green_asn, checker): assert isinstance(res, legacy_workers.SiteCheck) assert res.hosting_provider_id == green_asn.hostingprovider.id + def test_asn_from_ip_fails_gracefully_with_bad_asn_lookup(self, checker, caplog): + """ + Sometimes calling lookup() on an IP address raises a ASNParseError. + Do we catch this exception and log it? + """ + + checker.asn_from_ip = mock.MagicMock(side_effect=domain_check.ASNParseError) + with caplog.at_level(logging.WARNING): + res = checker.check_for_matching_asn("23.32.24.203") + + assert res is False + logged_error = caplog.text + assert "ASNParseError" in logged_error + def test_with_green_domain_by_non_resolving_asn(self, green_asn, checker): """ Sometimes the service we use for resolving ASNs returns @@ -210,7 +224,6 @@ class TestDomainCheckByCarbonTxt: def test_lookup_green_domain( self, green_domain_factory, green_ip_factory, mocker, checker ): - # mock our network lookup, so we get a consistent response when # looking up our domains green_ip = green_ip_factory.create()