refactor(browser): extract dns error predicate for testability [CL-4]

Extract the inline is_dns_error predicate from execute_command_sequence
into a module-level _is_dns_error() function so the test imports and
exercises the real code instead of a local copy. Remove unused
CommandExecutionError import from the test file.

Author: vringar

openwpm/browser_manager.py

@@ -43,6 +43,20 @@
     from .task_manager import TaskManager
 
 
+def _is_dns_error(command_status: str, error_text: Optional[str]) -> bool:
+    """Return True when the failure is a DNS resolution error (NXDOMAIN).
+
+    DNS resolution errors are expected when crawling large domain lists
+    (e.g. Tranco top-100k) and don't indicate a browser or instrumentation
+    failure.  # Only NXDOMAIN; DNS timeouts/SERVFAIL intentionally still count
+    """
+    return (
+        command_status == "neterror"
+        and error_text is not None
+        and error_text == "dnsNotFound"
+    )
+
+
 class BrowserManagerHandle:
     """The BrowserManagerHandle class is responsible for holding all the
     configuration and status information on BrowserManager process
@@ -156,7 +170,7 @@ def check_queue(launch_status: Dict[str, bool]) -> Any:
                 "BROWSER %i: Spawn attempt %i " % (self.browser_id, unsuccessful_spawns)
             )
             # Resets the command/status queues
-            (self.command_queue, self.status_queue) = (Queue(), Queue())
+            self.command_queue, self.status_queue = (Queue(), Queue())
 
             # builds and launches the browser_manager
 
@@ -462,8 +476,9 @@ def execute_command_sequence(
                 return
 
             if command_status != "ok":
-                with task_manager.threadlock:
-                    task_manager.failure_count += 1
+                if not _is_dns_error(command_status, error_text):
+                    with task_manager.threadlock:
+                        task_manager.failure_count += 1
                 if task_manager.failure_count > task_manager.failure_limit:
                     self.logger.critical(
                         "BROWSER %i: Command execution failure pushes failure "

test/test_webdriver_utils.py

@@ -1,3 +1,6 @@
+import pytest
+
+from openwpm.browser_manager import _is_dns_error
 from openwpm.commands.utils.webdriver_utils import parse_neterror
 from openwpm.utilities import db_utils
 
@@ -26,3 +29,75 @@ def test_parse_neterror_integration(default_params, task_manager_creator):
 
     assert get_command[0] == "neterror"
     assert get_command[1] == "dnsNotFound"
+
+
+@pytest.mark.slow
+def test_dns_error_does_not_count_against_failure_limit(
+    default_params, task_manager_creator
+):
+    """AC-4: 100+ DNS errors must not trigger CommandExecutionError even with
+    a low failure_limit.  Each navigation to a non-existent domain produces a
+    dnsNotFound neterror that should be excluded from the failure counter."""
+    manager_params, browser_params = default_params
+    manager_params.num_browsers = 1
+    manager_params.failure_limit = 5
+    manager, db = task_manager_creator((manager_params, browser_params[:1]))
+
+    num_domains = 110
+    for i in range(num_domains):
+        manager.get(f"http://domain-{i}.invalid")
+
+    manager.close()
+
+    rows = db_utils.query_db(
+        db,
+        "SELECT command_status, error FROM crawl_history WHERE command='GetCommand'",
+        as_tuple=True,
+    )
+    dns_rows = [(s, e) for s, e in rows if s == "neterror" and e == "dnsNotFound"]
+    assert len(dns_rows) == num_domains
+
+
+def test_is_dns_error_predicate():
+    """AC-5: Verify that _is_dns_error is True only for dnsNotFound neterrors.
+
+    Non-DNS neterror types (connectionRefused, netOffline, etc.) and other
+    command statuses must NOT be treated as DNS errors, so they continue to
+    increment failure_count as before.
+    """
+    # Only this exact combination qualifies
+    assert _is_dns_error("neterror", "dnsNotFound") is True
+
+    # Other neterror types must NOT be exempt
+    assert _is_dns_error("neterror", "connectionRefused") is False
+    assert _is_dns_error("neterror", "netOffline") is False
+    assert _is_dns_error("neterror", "proxyConnectFailure") is False
+
+    # Missing error_text must NOT be exempt
+    assert _is_dns_error("neterror", None) is False
+
+    # Non-neterror statuses must NOT be exempt
+    assert _is_dns_error("ok", "dnsNotFound") is False
+    assert _is_dns_error("critical", "dnsNotFound") is False
+    assert _is_dns_error("error", "dnsNotFound") is False
+    assert _is_dns_error("timeout", "dnsNotFound") is False
+
+    # parse_neterror returns the right code for a DNS failure message
+    dns_msg = (
+        "selenium.common.exceptions.WebDriverException: "
+        "Message: Reached error page: "
+        "about:neterror?e=dnsNotFound&u=http%3A//missing.example/&c=UTF-8&"
+        "f=regular&d=We+can%27t+connect"
+    )
+    assert parse_neterror(dns_msg) == "dnsNotFound"
+    assert _is_dns_error("neterror", parse_neterror(dns_msg)) is True
+
+    # A different neterror code does NOT trigger the exemption
+    conn_msg = (
+        "selenium.common.exceptions.WebDriverException: "
+        "Message: Reached error page: "
+        "about:neterror?e=connectionRefused&u=http%3A//localhost%3A1/&c=UTF-8&"
+        "f=regular&d=refused."
+    )
+    assert parse_neterror(conn_msg) == "connectionRefused"
+    assert _is_dns_error("neterror", parse_neterror(conn_msg)) is False