Fix bug in admin triggered by new domain lookup code

apps/greencheck/tests/test_domain_checker.py

@@ -1,5 +1,6 @@
 import ipaddress
 import logging
+import socket
 from unittest import mock
 
 import pytest
@@ -53,15 +54,16 @@ def test_domain_to_ip_supports_ipv4_and_ipv6(self, checker, domain):
 
     @pytest.mark.parametrize(
         "domain",
-        ["defintelynotavaliddomain", "defintelynotavaliddomain.com"],
+        [
+            "defintely-not-a-valid-domain",
+            "valid-domain-but-not-resolving.com",
+        ],
     )
     def test_domain_to_ip_does_not_fail_silently(self, checker, domain):
         """
-        When we check a domain that does resolve, are we notified so we
-        can catch exception appropriately?
+        When we check a domain that does not resolve successfully, are we
+        notified so we can catch exception appropriately?
         """
-        import ipaddress
-        import socket
 
         with pytest.raises((ipaddress.AddressValueError, socket.gaierror)):
             checker.convert_domain_to_ip(domain)

apps/theme/templatetags/admin_helpers.py

@@ -1,11 +1,13 @@
 import logging
+import socket
 import urllib
 
-from apps.greencheck import domain_check
 from django import template
 from django.contrib.auth.models import Group
 from django.utils.safestring import mark_safe
 
+from apps.greencheck import domain_check
+
 logger = logging.getLogger(__name__)
 register = template.Library()
 checker = domain_check.GreenDomainChecker()
@@ -30,15 +32,21 @@ def make_url(website_string: str) -> str:
 @register.simple_tag
 def link_to_ripe_stat(website_string: str) -> str:
     """
-    Add link to checker at stat.ripe.net for a domain
+    Add link to checker at stat.ripe.net for a domain.
+    Returns an empty string if the lookup does not work
     """
-    # check and return early we have nothing to check
-    if not website_string:
-        return
 
     url = make_url(website_string)
     domain = checker.validate_domain(url)
-    resolved_ip = checker.convert_domain_to_ip(domain)
+
+    try:
+        resolved_ip = checker.convert_domain_to_ip(domain)
+    except socket.gaierror as err:
+        logger.warning(f"Could not resolve domain {domain}: error was {err}")
+        resolved_ip = None
+    except Exception as err:
+        logger.exception(f"Unexpected error looking up {domain}: error was {err}")
+        resolved_ip = None
 
     if resolved_ip:
         return mark_safe(
@@ -49,6 +57,11 @@ def link_to_ripe_stat(website_string: str) -> str:
             f"</a>"
         )
 
+    # Return an empty string as a graceful fallback to avoid
+    # having `None` cast into a string saying "None"
+    # in our template
+    return ""
+
 
 @register.filter()
 def has_group(user, group_name) -> bool:

apps/theme/test_admin_helpers.py

@@ -34,9 +34,14 @@ def test_link_to_ripe_stat_returns_ip(self, mocker):
         assert mock_method.call_count == 1
 
     def test_link_to_ripe_stat_handles_empty(self, mocker):
-        mock_method = mocker.patch(
-            "apps.greencheck.domain_check.GreenDomainChecker.convert_domain_to_ip"
-        )
-        assert admin_helpers.link_to_ripe_stat(None) is None
-        # are we calling convert_domain_to_ip at all? We shouldn't be any more
-        assert mock_method.call_count == 0
+        result = admin_helpers.link_to_ripe_stat(None)
+        assert result == ""
+
+    def test_link_to_ripe_stat_handles_failing_lookup(self, mocker):
+        # Added to cover the case when there is a valid domain, but it doesn't
+        # resolve to an IP address. For more see the links below
+        # https://product-science.sentry.io/issues/5253649009/
+        # https://github.com/thegreenwebfoundation/admin-portal/pull/575
+
+        result = admin_helpers.link_to_ripe_stat("valid-but-not-resolving-to-ip.com")
+        assert result == ""