From 4efd9496dc18dbfecfd8d3a322871028546b291e Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 31 Jan 2022 13:43:34 +0200 Subject: [PATCH] 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 --- examples/client_example/client_example.py | 6 +-- tests/repository_simulator.py | 2 +- tuf/ngclient/_internal/requests_fetcher.py | 3 +- tuf/ngclient/fetcher.py | 50 +++++++++++++++++----- 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/examples/client_example/client_example.py b/examples/client_example/client_example.py index bed32b9d..e747abef 100755 --- a/examples/client_example/client_example.py +++ b/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 diff --git a/tests/repository_simulator.py b/tests/repository_simulator.py index 5988101c..ed6e51f6 100644 --- a/tests/repository_simulator.py +++ b/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). """ diff --git a/tuf/ngclient/_internal/requests_fetcher.py b/tuf/ngclient/_internal/requests_fetcher.py index 5b95e2bf..53c25999 100644 --- a/tuf/ngclient/_internal/requests_fetcher.py +++ b/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 diff --git a/tuf/ngclient/fetcher.py b/tuf/ngclient/fetcher.py index a56c66dc..0de98833 100644 --- a/tuf/ngclient/fetcher.py +++ b/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.