fix(proxies): skip a proxy provider that fails to load

A bare --proxy query moves past it; naming it (--proxy nordvpn:us) fails with the load error. dl, search and the REST path share one loader.

docs/guide/proxies-and-vpn.md

@@ -128,6 +128,11 @@ So if you have both Basic and NordVPN configured and use `--proxy us`, a `us` en
 your Basic config wins, because unshackle tries Basic first. To skip ahead to a specific
 proxy provider, prefix the query (`--proxy nordvpn:us`).
 
+If a proxy provider cannot load, for example because its server list cannot be fetched
+or its config is not valid, unshackle logs a warning and continues with the other proxy
+providers. A bare query skips the proxy provider that did not load. A query that names it,
+such as `--proxy nordvpn:us`, fails and shows the reason it did not load.
+
 !!! note "A bare region reaches Control D last"
     unshackle asks Control D for a bare region such as `--proxy us` only when no other
     loaded proxy provider has a proxy for it. Hola loads automatically when the

tests/core/test_proxy_loading.py

@@ -0,0 +1,63 @@
+"""Unit tests for the shared proxy provider loader: a proxy provider that fails to load is skipped
+by a bare query and reported by a query that names it."""
+
+from __future__ import annotations
+
+import logging
+
+import pytest
+import requests
+
+from unshackle.core.config import config
+from unshackle.core.proxies import NordVPN, SurfsharkVPN
+from unshackle.core.proxies.resolve import Unavailable, find_provider, load_proxy_providers, resolve_proxy
+
+pytestmark = pytest.mark.unit
+
+CREDENTIALS = {"username": "a" * 24, "password": "b" * 24}
+OTHERS = ("basic", "expressvpn", "protonvpn", "windscribevpn", "gluetun", "hola", "controld")
+
+
+@pytest.fixture(autouse=True)
+def nordvpn_down_surfshark_up(monkeypatch: pytest.MonkeyPatch) -> None:
+    def refuse() -> list[dict]:
+        raise requests.ConnectionError("no route to api.nordvpn.com")
+
+    clusters = [{"countryCode": "US", "connectionName": "us-dal.prod.surfshark.com", "location": "Dallas"}]
+    monkeypatch.setattr(config, "proxy_providers", {"nordvpn": CREDENTIALS, "surfsharkvpn": CREDENTIALS})
+    monkeypatch.setattr(NordVPN, "get_countries", staticmethod(refuse))
+    monkeypatch.setattr(SurfsharkVPN, "get_countries", staticmethod(lambda: clusters))
+
+
+def test_failed_provider_is_kept_as_unavailable(caplog: pytest.LogCaptureFixture) -> None:
+    with caplog.at_level(logging.WARNING):
+        nordvpn, surfshark = load_proxy_providers(OTHERS)
+    assert isinstance(nordvpn, Unavailable) and nordvpn.name == "NordVPN"
+    assert isinstance(surfshark, SurfsharkVPN)
+    assert "NordVPN proxy provider could not load" in caplog.text
+
+
+def test_bare_query_skips_the_failed_provider() -> None:
+    uri = resolve_proxy("us", load_proxy_providers(OTHERS))
+    assert uri.endswith("@us-dal.prod.surfshark.com:443")
+
+
+def test_query_naming_another_provider_still_works() -> None:
+    uri = resolve_proxy("surfsharkvpn:us", load_proxy_providers(OTHERS))
+    assert uri.endswith("@us-dal.prod.surfshark.com:443")
+
+
+@pytest.mark.parametrize("name", ["nordvpn", "NordVPN"])
+def test_query_naming_the_failed_provider_reports_why(name: str) -> None:
+    with pytest.raises(ValueError, match="NordVPN could not load: no route to api.nordvpn.com"):
+        resolve_proxy(f"{name}:us", load_proxy_providers(OTHERS))
+
+
+def test_query_naming_an_unknown_provider_lists_the_loaded_ones() -> None:
+    with pytest.raises(ValueError, match=r"'mullvad' was not found. Available: \['SurfsharkVPN'\]"):
+        find_provider(load_proxy_providers(OTHERS), "mullvad")
+
+
+def test_raise_errors_raises_the_load_failure() -> None:
+    with pytest.raises(requests.ConnectionError):
+        load_proxy_providers(OTHERS, raise_errors=True)

unshackle/commands/dl.py

@@ -53,18 +53,8 @@ from unshackle.core.events import events
 from unshackle.core.export_name import ExportNamer, move_export, plural, season_episodes, title_label
 from unshackle.core.providers.anilist import parse_anilist_ref
 from unshackle.core.providers.tvdb import SEASON_TYPES, parse_int
-from unshackle.core.proxies import (
-    Basic,
-    ControlD,
-    ExpressVPN,
-    Gluetun,
-    Hola,
-    NordVPN,
-    ProtonVPN,
-    SurfsharkVPN,
-    WindscribeVPN,
-)
-from unshackle.core.proxies.resolve import REGION, is_loopback, resolve_proxy
+from unshackle.core.proxies import Basic
+from unshackle.core.proxies.resolve import REGION, find_provider, is_loopback, load_proxy_providers, resolve_proxy
 from unshackle.core.service import Service, grow_session_pool
 from unshackle.core.services import Services
 from unshackle.core.temp import with_task_temp
@@ -1363,29 +1353,7 @@ class dl:
                 self.proxy_providers = list(proxy_providers)
             else:
                 with console.status("Loading Proxy Providers...", spinner="dots"):
-                    if config.proxy_providers.get("basic"):
-                        self.proxy_providers.append(Basic(**config.proxy_providers["basic"]))
-                    # ExpressVPN/ProtonVPN auto-load when their default cookie file exists (no yaml needed)
-                    expressvpn = ExpressVPN(**(config.proxy_providers.get("expressvpn") or {}))
-                    if config.proxy_providers.get("expressvpn") or expressvpn.cache_path.is_file():
-                        self.proxy_providers.append(expressvpn)
-                    if config.proxy_providers.get("nordvpn"):
-                        self.proxy_providers.append(NordVPN(**config.proxy_providers["nordvpn"]))
-                    proton = ProtonVPN(**(config.proxy_providers.get("protonvpn") or {}))
-                    if config.proxy_providers.get("protonvpn") or proton.cookie_path.is_file():
-                        self.proxy_providers.append(proton)
-                    if config.proxy_providers.get("surfsharkvpn"):
-                        self.proxy_providers.append(SurfsharkVPN(**config.proxy_providers["surfsharkvpn"]))
-                    if config.proxy_providers.get("windscribevpn"):
-                        self.proxy_providers.append(WindscribeVPN(**config.proxy_providers["windscribevpn"]))
-                    if config.proxy_providers.get("gluetun"):
-                        self.proxy_providers.append(Gluetun(**config.proxy_providers["gluetun"]))
-                    if binaries.HolaProxy:
-                        self.proxy_providers.append(Hola())
-                    if config.proxy_providers.get("controld"):
-                        self.proxy_providers.append(ControlD(**config.proxy_providers["controld"]))
-                    for proxy_provider in self.proxy_providers:
-                        self.log.info(f"Loaded {proxy_provider.__class__.__name__}: {proxy_provider}")
+                    self.proxy_providers = load_proxy_providers()
 
             if proxy and proxy.startswith("controld://"):
                 proxy = ctx.params["proxy"] = resolve_proxy(proxy, [])
@@ -1413,16 +1381,10 @@ class dl:
                     )
                     with console.status(status_msg, spinner="dots"):
                         if requested_provider:
-                            proxy_provider = next(
-                                (
-                                    x
-                                    for x in self.proxy_providers
-                                    if x.__class__.__name__.lower() == requested_provider.lower()
-                                ),
-                                None,
-                            )
-                            if not proxy_provider:
-                                self.log.error(f"The proxy provider '{requested_provider}' was not recognised.")
+                            try:
+                                proxy_provider = find_provider(self.proxy_providers, requested_provider)
+                            except ValueError as e:
+                                self.log.error(str(e))
                                 sys.exit(1)
                             proxy_uri = proxy_provider.get_proxy(proxy)
                             if not proxy_uri:

unshackle/commands/search.py

@@ -12,23 +12,12 @@ from rich.rule import Rule
 from rich.tree import Tree
 
 from unshackle.commands.dl import dl
-from unshackle.core import binaries
 from unshackle.core.config import config
 from unshackle.core.console import console
 from unshackle.core.constants import context_settings
-from unshackle.core.proxies import (
-    Basic,
-    ControlD,
-    ExpressVPN,
-    Gluetun,
-    Hola,
-    NordVPN,
-    ProtonVPN,
-    SurfsharkVPN,
-    WindscribeVPN,
-)
+from unshackle.core.proxies import Basic
 from unshackle.core.proxies.proxy import Proxy
-from unshackle.core.proxies.resolve import REGION
+from unshackle.core.proxies.resolve import REGION, find_provider, load_proxy_providers
 from unshackle.core.service import Service
 from unshackle.core.services import Services
 from unshackle.core.utils.click_types import ContextData
@@ -78,28 +67,7 @@ def search(ctx: click.Context, no_proxy: bool, profile: Optional[str] = None, pr
         ctx.params["proxy"] = None
     else:
         with console.status("Loading Proxy Providers...", spinner="dots"):
-            if config.proxy_providers.get("basic"):
-                proxy_providers.append(Basic(**config.proxy_providers["basic"]))
-            expressvpn = ExpressVPN(**(config.proxy_providers.get("expressvpn") or {}))
-            if config.proxy_providers.get("expressvpn") or expressvpn.cache_path.is_file():
-                proxy_providers.append(expressvpn)
-            if config.proxy_providers.get("nordvpn"):
-                proxy_providers.append(NordVPN(**config.proxy_providers["nordvpn"]))
-            proton = ProtonVPN(**(config.proxy_providers.get("protonvpn") or {}))
-            if config.proxy_providers.get("protonvpn") or proton.cookie_path.is_file():
-                proxy_providers.append(proton)
-            if config.proxy_providers.get("surfsharkvpn"):
-                proxy_providers.append(SurfsharkVPN(**config.proxy_providers["surfsharkvpn"]))
-            if config.proxy_providers.get("windscribevpn"):
-                proxy_providers.append(WindscribeVPN(**config.proxy_providers["windscribevpn"]))
-            if config.proxy_providers.get("gluetun"):
-                proxy_providers.append(Gluetun(**config.proxy_providers["gluetun"]))
-            if binaries.HolaProxy:
-                proxy_providers.append(Hola())
-            if config.proxy_providers.get("controld"):
-                proxy_providers.append(ControlD(**config.proxy_providers["controld"]))
-            for proxy_provider in proxy_providers:
-                log.info(f"Loaded {proxy_provider.__class__.__name__}: {proxy_provider}")
+            proxy_providers = load_proxy_providers()
 
         if proxy:
             requested_provider = None
@@ -109,11 +77,10 @@ def search(ctx: click.Context, no_proxy: bool, profile: Optional[str] = None, pr
                 proxy = proxy.lower()
                 with console.status(f"Getting a Proxy to {proxy}...", spinner="dots"):
                     if requested_provider:
-                        proxy_provider = next(
-                            (x for x in proxy_providers if x.__class__.__name__.lower() == requested_provider), None
-                        )
-                        if not proxy_provider:
-                            log.error(f"The proxy provider '{requested_provider}' was not recognised.")
+                        try:
+                            proxy_provider = find_provider(proxy_providers, requested_provider)
+                        except ValueError as e:
+                            log.error(str(e))
                             sys.exit(1)
                         proxy_uri = proxy_provider.get_proxy(proxy)
                         if not proxy_uri:

unshackle/core/proxies/nordvpn.py

@@ -163,7 +163,9 @@ class NordVPN(Proxy):
         Note: There may not always be more than one recommended server.
         """
         res = requests.get(
-            url="https://api.nordvpn.com/v1/servers/recommendations", params={"filters[country_id]": country_id}
+            url="https://api.nordvpn.com/v1/servers/recommendations",
+            params={"filters[country_id]": country_id},
+            timeout=10,
         )
         if not res.ok:
             raise ValueError(f"Failed to get a list of NordVPN countries [{res.status_code}]")
@@ -176,9 +178,7 @@ class NordVPN(Proxy):
     @staticmethod
     def get_countries() -> list[dict]:
         """Get a list of available Countries and their metadata."""
-        res = requests.get(
-            url="https://api.nordvpn.com/v1/servers/countries",
-        )
+        res = requests.get(url="https://api.nordvpn.com/v1/servers/countries", timeout=10)
         if not res.ok:
             raise ValueError(f"Failed to get a list of NordVPN countries [{res.status_code}]")
 

unshackle/core/proxies/resolve.py

@@ -1,7 +1,4 @@
-"""Shared proxy provider initialization and resolution.
-
-Used by both the REST API handlers and the remote service client.
-"""
+"""Shared proxy provider loading, lookup and resolution for dl, search, the REST API and the remote client."""
 
 from __future__ import annotations
 
@@ -11,6 +8,7 @@ import re
 from typing import Any, List, Optional
 from urllib.parse import urlparse
 
+from unshackle.core.proxies.proxy import Proxy
 from unshackle.core.utils.redact import mask_proxy
 
 log = logging.getLogger("proxies")
@@ -18,58 +16,106 @@ log = logging.getLogger("proxies")
 REGION = r"[a-z]{2,4}(?:_[a-z0-9]{2,4})?(?:-[a-z0-9]+|:[a-z][a-z0-9]*)*\d*"
 
 
-def initialize_proxy_providers(raise_errors: bool = False, quiet: bool = False) -> List[Any]:
-    """Initialise and return available proxy providers from config.
+class Unavailable(Proxy):
+    """Stands in for a proxy provider that failed to load: a bare query skips it, a query that names it fails."""
 
-    A proxy provider that fails to build is logged and skipped, unless *raise_errors* is set.
-    *quiet* drops the per-provider summary lines: rendering a proxy provider asks some of them
-    for their server catalogue over the network, which a repeated caller must not pay for.
+    def __init__(self, name: str, error: Exception) -> None:
+        self.name = name
+        # The message only: the exception's traceback would keep the failed build's frames alive.
+        self.error = str(error)
+
+    def __repr__(self) -> str:
+        return f"failed to load ({self.error})"
+
+    def get_proxy(self, query: str) -> Optional[str]:
+        return None
+
+
+def find_provider(proxy_providers: List[Any], name: str) -> Any:
     """
+    Find a proxy provider by its class name, case-insensitive.
+
+    Raises ValueError when no proxy provider has that name, or when it failed to load.
+    """
+    for provider in proxy_providers:
+        if isinstance(provider, Unavailable):
+            if provider.name.lower() == name.lower():
+                raise ValueError(f"The proxy provider {provider.name} could not load: {provider.error}")
+        elif provider.__class__.__name__.lower() == name.lower():
+            return provider
+    available = [x.__class__.__name__ for x in proxy_providers if not isinstance(x, Unavailable)]
+    raise ValueError(f"The proxy provider '{name}' was not found. Available: {available}")
+
+
+def load_proxy_providers(exclude: tuple[str, ...] = (), raise_errors: bool = False, quiet: bool = False) -> List[Any]:
+    """
+    Build the proxy providers set in config, in the order a bare query tries them.
+
+    Each proxy provider loads on its own. One that fails is logged and kept as Unavailable, so a bare
+    query skips it and a query that names it reports why. *raise_errors* raises the failure instead.
+    *exclude* holds lowercase class names to leave out.
+    """
+    from unshackle.core import binaries
+    from unshackle.core.config import config
+    from unshackle.core.proxies import (
+        Basic,
+        ControlD,
+        ExpressVPN,
+        Gluetun,
+        Hola,
+        NordVPN,
+        ProtonVPN,
+        SurfsharkVPN,
+        WindscribeVPN,
+    )
+
+    settings = config.proxy_providers
+    # Control D is last: a bare region reaches it only when nothing else answers, since it changes the account.
+    classes = (Basic, ExpressVPN, NordVPN, ProtonVPN, SurfsharkVPN, WindscribeVPN, Gluetun, Hola, ControlD)
+
+    def build(cls: type) -> Any:
+        key = cls.__name__.lower()
+        if cls is Hola:
+            return Hola() if binaries.HolaProxy else None
+        if cls is ExpressVPN or cls is ProtonVPN:
+            # These also load with no config when their default cookie file exists.
+            provider = cls(**(settings.get(key) or {}))
+            cookie = provider.cache_path if cls is ExpressVPN else provider.cookie_path
+            return provider if settings.get(key) or cookie.is_file() else None
+        return cls(**settings[key]) if settings.get(key) else None
+
     proxy_providers: list = []
-    try:
-        from unshackle.core import binaries
-        from unshackle.core.config import config as main_config
-        from unshackle.core.proxies.basic import Basic
-        from unshackle.core.proxies.controld import ControlD
-        from unshackle.core.proxies.expressvpn import ExpressVPN
-        from unshackle.core.proxies.hola import Hola
-        from unshackle.core.proxies.nordvpn import NordVPN
-        from unshackle.core.proxies.proton import ProtonVPN
-        from unshackle.core.proxies.surfsharkvpn import SurfsharkVPN
+    for cls in classes:
+        name = cls.__name__
+        if name.lower() in exclude:
+            continue
+        try:
+            provider = build(cls)
+            if provider is None:
+                continue
+            if not quiet:
+                log.info(f"Loaded {name}: {provider}")
+        except Exception as e:
+            if raise_errors:
+                raise
+            log.warning(f"The {name} proxy provider could not load, so a bare --proxy query skips it: {e}")
+            provider = Unavailable(name, e)
+        proxy_providers.append(provider)
 
-        proxy_config = getattr(main_config, "proxy_providers", {})
+    return proxy_providers
 
-        if proxy_config.get("basic"):
-            proxy_providers.append(Basic(**proxy_config["basic"]))
-        # ExpressVPN/ProtonVPN auto-load when their default cookie file exists (no yaml needed)
-        expressvpn = ExpressVPN(**(proxy_config.get("expressvpn") or {}))
-        if proxy_config.get("expressvpn") or expressvpn.cache_path.is_file():
-            proxy_providers.append(expressvpn)
-        if proxy_config.get("nordvpn"):
-            proxy_providers.append(NordVPN(**proxy_config["nordvpn"]))
-        proton = ProtonVPN(**(proxy_config.get("protonvpn") or {}))
-        if proxy_config.get("protonvpn") or proton.cookie_path.is_file():
-            proxy_providers.append(proton)
-        if proxy_config.get("surfsharkvpn"):
-            proxy_providers.append(SurfsharkVPN(**proxy_config["surfsharkvpn"]))
-        if hasattr(binaries, "HolaProxy") and binaries.HolaProxy:
-            proxy_providers.append(Hola())
-        # last: a bare region reaches it only when nothing else answers, since it changes the account
-        if proxy_config.get("controld"):
-            proxy_providers.append(ControlD(**proxy_config["controld"]))
 
-        if not quiet:
-            for provider in proxy_providers:
-                log.info(f"Loaded {provider.__class__.__name__}: {provider}")
-
-            if not proxy_providers:
-                log.warning("No proxy providers were loaded. Check your proxy provider configuration in unshackle.yaml")
-
-    except Exception as e:
-        if raise_errors:
-            raise
-        log.warning(f"Failed to initialize some proxy providers: {e}")
+def initialize_proxy_providers(raise_errors: bool = False, quiet: bool = False) -> List[Any]:
+    """
+    Load the proxy providers for the REST API and the remote service client.
 
+    WindscribeVPN and Gluetun are not offered on this path. *quiet* drops the per-provider summary
+    lines: rendering a proxy provider asks some of them for their server catalogue over the network,
+    which a repeated caller must not pay for.
+    """
+    proxy_providers = load_proxy_providers(("windscribevpn", "gluetun"), raise_errors, quiet)
+    if not quiet and not proxy_providers:
+        log.warning("No proxy providers were loaded. Check your proxy provider configuration in unshackle.yaml")
     return proxy_providers
 
 
@@ -99,13 +145,7 @@ def resolve_proxy(proxy: str, proxy_providers: List[Any]) -> Optional[str]:
         requested_provider, query = proxy.split(":", maxsplit=1)
 
     if requested_provider:
-        provider = next(
-            (x for x in proxy_providers if x.__class__.__name__.lower() == requested_provider.lower()),
-            None,
-        )
-        if not provider:
-            available = [x.__class__.__name__ for x in proxy_providers]
-            raise ValueError(f"Proxy provider '{requested_provider}' not found. Available: {available}")
+        provider = find_provider(proxy_providers, requested_provider)
         proxy_uri = provider.get_proxy(query)
         if not proxy_uri:
             raise ValueError(f"Proxy provider {requested_provider} had no proxy for {query}")

unshackle/core/proxies/windscribevpn.py

@@ -160,6 +160,7 @@ class WindscribeVPN(Proxy):
                 "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/128.0.0.0 Safari/537.36",
                 "Content-Type": "application/json",
             },
+            timeout=10,
         )
         if not res.ok:
             raise ValueError(f"Failed to get a list of WindscribeVPN locations [{res.status_code}]")

unshackle/core/remote_service.py

@@ -729,15 +729,13 @@ def resolve_remote_proxy_arg(proxy_arg: str) -> Optional[str]:
     profile's resolver instead (`controld://`), and a bare region query skips Control D.
     """
     from unshackle.core.proxies.controld import ControlD
-    from unshackle.core.proxies.resolve import initialize_proxy_providers, resolve_proxy
+    from unshackle.core.proxies.resolve import find_provider, initialize_proxy_providers, resolve_proxy
 
     try:
         providers = initialize_proxy_providers()
         provider, _, query = proxy_arg.partition(":")
         if provider.lower() == "controld" and query:
-            controld = next((x for x in providers if isinstance(x, ControlD)), None)
-            if not controld:
-                raise ValueError("Proxy provider 'controld' is not configured")
+            controld = find_provider(providers, "controld")
             uri = controld.get_remote_proxy(query)
             if not uri:
                 raise ValueError(f"Control D has no location for {query}")

unshackle/core/service.py

@@ -23,6 +23,7 @@ from unshackle.core.constants import AnyTrack
 from unshackle.core.credential import Credential
 from unshackle.core.drm import DRM_T
 from unshackle.core.proxies.basic import Basic
+from unshackle.core.proxies.resolve import find_provider
 from unshackle.core.search_result import SearchResult
 from unshackle.core.session import (
     BACKOFF_FACTOR,
@@ -217,10 +218,10 @@ class Service(metaclass=ABCMeta):
                     )
                     if proxy_provider_name:
                         # Specific provider requested
-                        proxy_provider = next(
-                            (x for x in ctx.obj.proxy_providers if x.__class__.__name__.lower() == proxy_provider_name),
-                            None,
-                        )
+                        try:
+                            proxy_provider = find_provider(ctx.obj.proxy_providers, proxy_provider_name)
+                        except ValueError:
+                            proxy_provider = None
                         if proxy_provider:
                             mapped_proxy_uri = proxy_provider.get_proxy(mapped_value)
                             if mapped_proxy_uri: