ngclient: Make DownloadErrors consistent

Fetcher interface should only raise DownloadErrors,
regardless of the implementation.
 * Make sure fetch() wraps non-DownloadError errors in a DownloadError
 * Make the abstract function private _fetch()
 * Try to be more consistent in doscstrings

This now makes the example client more sensible (when server does not
respond):
    $ ./client_example.py download qwerty
    ...
    Failed to download target qwerty: Failed to download url http://127.0.0.1:8000/metadata/2.root.json

(here the latter part of the error string comes from DownloadError
raised by FetcherInterface.fetch())

Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>

examples/client_example/client_example.py

@@ -10,7 +10,7 @@
 import shutil
 from pathlib import Path
 
-from tuf.api.exceptions import RepositoryError
+from tuf.api.exceptions import DownloadError, RepositoryError
 from tuf.ngclient import Updater
 
 # constants
@@ -73,8 +73,8 @@ def download(target: str) -> bool:
         path = updater.download_target(info)
         print(f"Target downloaded and available in {path}")
 
-    except (OSError, RepositoryError) as e:
-        print(str(e))
+    except (OSError, RepositoryError, DownloadError) as e:
+        print(f"Failed to download target {target}: {e}")
         return False
 
     return True

tests/repository_simulator.py

@@ -205,7 +205,7 @@ def publish_root(self) -> None:
         self.signed_roots.append(self.md_root.to_bytes(JSONSerializer()))
         logger.debug("Published root v%d", self.root.version)
 
-    def fetch(self, url: str) -> Iterator[bytes]:
+    def _fetch(self, url: str) -> Iterator[bytes]:
         """Fetches data from the given url and returns an Iterator (or yields
         bytes).
         """

tuf/ngclient/_internal/requests_fetcher.py

@@ -51,7 +51,7 @@ def __init__(self) -> None:
         self.socket_timeout: int = 4  # seconds
         self.chunk_size: int = 400000  # bytes
 
-    def fetch(self, url: str) -> Iterator[bytes]:
+    def _fetch(self, url: str) -> Iterator[bytes]:
         """Fetches the contents of HTTP/HTTPS url from a remote server
 
         Arguments:
@@ -61,7 +61,6 @@ def fetch(self, url: str) -> Iterator[bytes]:
             exceptions.SlowRetrievalError: A timeout occurs while receiving
                 data.
             exceptions.FetcherHTTPError: An HTTP error code is received.
-            exceptions.DownloadError: When there is a problem parsing the url.
 
         Returns:
             A bytes iterator

tuf/ngclient/fetcher.py

@@ -28,6 +28,26 @@ class FetcherInterface:
     __metaclass__ = abc.ABCMeta
 
     @abc.abstractmethod
+    def _fetch(self, url: str) -> Iterator[bytes]:
+        """Fetches the contents of HTTP/HTTPS url from a remote server.
+
+        Implementations must raise FetcherHTTPError if they receive an
+        HTTP error code.
+
+        Implementations may raise any errors but the ones that are not
+        DownloadErrors will be wrapped in a DownloadError by fetch().
+
+        Arguments:
+            url: A URL string that represents a file location.
+
+        Raises:
+            exceptions.FetcherHTTPError: An HTTP error code was received.
+
+        Returns:
+            A bytes iterator
+        """
+        raise NotImplementedError  # pragma: no cover
+
     def fetch(self, url: str) -> Iterator[bytes]:
         """Fetches the contents of HTTP/HTTPS url from a remote server.
 
@@ -35,14 +55,20 @@ def fetch(self, url: str) -> Iterator[bytes]:
             url: A URL string that represents a file location.
 
         Raises:
-            exceptions.SlowRetrievalError: A timeout occurs while receiving
-                data.
-            exceptions.FetcherHTTPError: An HTTP error code is received.
+            exceptions.DownloadError: An error occurred during download.
+            exceptions.FetcherHTTPError: An HTTP error code was received.
 
         Returns:
             A bytes iterator
         """
-        raise NotImplementedError  # pragma: no cover
+        # Ensure that fetch() only raises DownloadErrors, regardless of the
+        # fetcher implementation
+        try:
+            return self._fetch(url)
+        except Exception as e:
+            if isinstance(e, exceptions.DownloadError):
+                raise e
+            raise exceptions.DownloadError(f"Failed to download {url}") from e
 
     @contextmanager
     def download_file(self, url: str, max_length: int) -> Iterator[IO]:
@@ -50,16 +76,18 @@ def download_file(self, url: str, max_length: int) -> Iterator[IO]:
         up to 'max_length'.
 
         Args:
-          url: a URL string that represents the location of the file.
-          max_length: an integer value representing the length of
-              the file or an upper bound.
+            url: a URL string that represents the location of the file.
+            max_length: an integer value representing the length of
+                the file or an upper bound.
 
         Raises:
-          exceptions.DownloadLengthMismatchError: downloaded bytes exceed
-            'max_length'.
+            exceptions.DownloadError: An error occurred during download.
+            exceptions.DownloadLengthMismatchError: downloaded bytes exceed
+                'max_length'.
+            exceptions.FetcherHTTPError: An HTTP error code was received.
 
         Yields:
-          A TemporaryFile object that points to the contents of 'url'.
+            A TemporaryFile object that points to the contents of 'url'.
         """
         logger.debug("Downloading: %s", url)
 
@@ -96,8 +124,10 @@ def download_bytes(self, url: str, max_length: int) -> bytes:
             max_length: upper bound of data size in bytes.
 
         Raises:
+            exceptions.DownloadError: An error occurred during download.
             exceptions.DownloadLengthMismatchError: downloaded bytes exceed
                 'max_length'.
+            exceptions.FetcherHTTPError: An HTTP error code was received.
 
         Returns:
             The content of the file in bytes.