Repository: huggingface/huggingface_hub
Pull request #924 (2022-06-22 23:14:54)

Fix custom handling of refined HTTPError
As discussed [internally](https://huggingface.slack.com/archives/C02V5EA0A95/p1655838894807499), https://github.com/huggingface/huggingface_hub/pull/878 introduced a backwards compatibility issue as it does not contain all the information from the original error (such as the `status_code`), which was being used in downstream libraries such as [speechbrain](https://github.com/speechbrain/speechbrain/blob/d6bfe138a90dff3490f7196acbd9c62939289d46/speechbrain/pretrained/fetching.py#L115-L121).

This PR changes the error handling to re-use the original error but with additional custom message. To avoid this situation happening again, also introducing some tests.

HuggingFaceDocBuilderDev: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_924). All of your documentation changes will be reflected on that endpoint.

Patch:
diff --git a/setup.py b/setup.py
index 9a2eae17..01a512cf 100644
--- a/setup.py
+++ b/setup.py
@@ -43,7 +43,7 @@ extras["testing"] = [
 ]
 
 extras["quality"] = [
-    "black~=22.0",
+    "black==22.3",
     "isort>=5.5.4",
     "flake8>=3.8.3",
 ]
diff --git a/src/huggingface_hub/hf_api.py b/src/huggingface_hub/hf_api.py
index 389236e8..83a86633 100644
--- a/src/huggingface_hub/hf_api.py
+++ b/src/huggingface_hub/hf_api.py
@@ -44,7 +44,6 @@ from .constants import (
 from .utils import logging
 from .utils._deprecation import _deprecate_positional_args
 from .utils._errors import _raise_for_status, _raise_with_request_id
-from .utils._fixes import JSONDecodeError
 from .utils.endpoint_helpers import (
     AttributeDictionary,
     DatasetFilter,
@@ -1559,16 +1558,7 @@ class HfApi:
             headers={"authorization": f"Bearer {token}"},
             json=json,
         )
-        try:
-            _raise_for_status(r)
-        except requests.exceptions.RequestException as e:
-            try:
-                message = e.response.json()["error"]
-            except JSONDecodeError:
-                message = e.response.text
-            except AttributeError:
-                message = e.args[0]
-            raise type(e)(message) from e
+        _raise_for_status(r)
 
     def update_repo_visibility(
         self,
@@ -2057,7 +2047,7 @@ class HfApi:
         Example usage:
 
         ```python
-        >>> upload_file(
+        >>> upload_folder(
         ...     folder_path="local/checkpoints",
         ...     path_in_repo="remote/experiment/checkpoints",
         ...     repo_id="username/my-dataset",
@@ -2066,7 +2056,7 @@ class HfApi:
         ... )
         # "https://huggingface.co/datasets/username/my-dataset/tree/main/remote/experiment/checkpoints"
 
-        >>> upload_file(
+        >>> upload_folder(
         ...     folder_path="local/checkpoints",
         ...     path_in_repo="remote/experiment/checkpoints",
         ...     repo_id="username/my-dataset",
diff --git a/src/huggingface_hub/keras_mixin.py b/src/huggingface_hub/keras_mixin.py
index 3ec95eba..0a5e7ba8 100644
--- a/src/huggingface_hub/keras_mixin.py
+++ b/src/huggingface_hub/keras_mixin.py
@@ -1,3 +1,4 @@
+import collections.abc as collections
 import json
 import os
 import warnings
@@ -26,10 +27,41 @@ if is_tf_available():
     import tensorflow as tf
 
 
+def _flatten_dict(dictionary, parent_key=""):
+    """Flatten a nested dictionary.
+    Reference: https://stackoverflow.com/a/6027615/10319735
+
+    Args:
+        dictionary (`dict`):
+            The nested dictionary to be flattened.
+        parent_key (`str`):
+            The parent key to be prefixed to the childer keys.
+            Necessary for recursing over the nested dictionary.
+
+    Returns:
+        The flattened dictionary.
+    """
+    items = []
+    for key, value in dictionary.items():
+        new_key = f"{parent_key}.{key}" if parent_key else key
+        if isinstance(value, collections.MutableMapping):
+            items.extend(
+                _flatten_dict(
+                    value,
+                    new_key,
+                ).items()
+            )
+        else:
+            items.append((new_key, value))
+    return dict(items)
+
+
 def _create_hyperparameter_table(model):
     """Parse hyperparameter dictionary into a markdown table."""
     if model.optimizer is not None:
         optimizer_params = model.optimizer.get_config()
+        # flatten the configuration
+        optimizer_params = _flatten_dict(optimizer_params)
         optimizer_params[
             "training_precision"
         ] = tf.keras.mixed_precision.global_policy().name
diff --git a/src/huggingface_hub/repocard_types.py b/src/huggingface_hub/repocard_types.py
index 87537d0e..a87a37e3 100644
--- a/src/huggingface_hub/repocard_types.py
+++ b/src/huggingface_hub/repocard_types.py
@@ -41,7 +41,7 @@ class SingleMetric:
     The arguments passed during `Metric.compute()`. Example for `bleu`: max_order: 4
     """
 
-    verified: Optional[bool] = False
+    verified: Optional[bool] = None
     """
     If true, indicates that evaluation was generated by Hugging Face (vs. self-reported).
     """
@@ -52,8 +52,7 @@ class SingleResultTask:
     type: str
     """
     Example: automatic-speech-recognition Use task id from
-    https://github.com/huggingface/huggingface_hub/blob/main/js/src/lib/int
-    erfaces/Types.ts
+    hhttps://github.com/huggingface/hub-docs/blob/main/js/src/lib/interfaces/Types.ts
     """
 
     name: Optional[str] = None
@@ -64,11 +63,6 @@ class SingleResultTask:
 
 @dataclass
 class SingleResultDataset:
-    """
-    This will switch to required at some point. in any case, we need them to
-    link to PWC
-    """
-
     name: str
     """
     Example: Common Voice (French). A pretty name for the dataset.
@@ -96,7 +90,7 @@ class SingleResultDataset:
 
     args: Any = None
     """
-    Example: zh-CN
+    Optional. Additional arguments to `load_dataset()`. Example for wikipedia: { language: "en" }
     """
 
 
@@ -104,8 +98,8 @@ class SingleResultDataset:
 class SingleResult:
     task: "SingleResultTask"
     dataset: "Optional[SingleResultDataset]"
-    metrics: "List[SingleMetric]"
     """
     This will switch to required at some point. in any case, we need them to
     link to PWC
     """
+    metrics: "List[SingleMetric]"
diff --git a/src/huggingface_hub/utils/_errors.py b/src/huggingface_hub/utils/_errors.py
index 5966db50..2617acd8 100644
--- a/src/huggingface_hub/utils/_errors.py
+++ b/src/huggingface_hub/utils/_errors.py
@@ -15,6 +15,9 @@ class RepositoryNotFoundError(HTTPError):
     ```
     """
 
+    def __init__(self, message, response):
+        super().__init__(message, response=response)
+
 
 class RevisionNotFoundError(HTTPError):
     """
@@ -28,9 +31,11 @@ class RevisionNotFoundError(HTTPError):
     >>> hf_hub_download('bert-base-cased', 'config.json', revision='<non-existant-revision>')
     huggingface_hub.utils._errors.RevisionNotFoundError: 404 Client Error: Revision Not Found for url: <url>
     ```
-
     """
 
+    def __init__(self, message, response):
+        super().__init__(message, response=response)
+
 
 class EntryNotFoundError(HTTPError):
     """
@@ -46,42 +51,60 @@ class EntryNotFoundError(HTTPError):
     ```
     """
 
+    def __init__(self, message, response):
+        super().__init__(message, response=response)
+
+
+def _add_request_id_to_error_args(e, request_id):
+    if request_id is not None and len(e.args) > 0 and isinstance(e.args[0], str):
+        e.args = (e.args[0] + f" (Request ID: {request_id})",) + e.args[1:]
+
 
-def _raise_for_status(request):
+def _raise_for_status(response):
     """
-    Internal version of `request.raise_for_status()` that will refine a
+    Internal version of `response.raise_for_status()` that will refine a
     potential HTTPError.
     """
-    request_id = request.headers.get("X-Request-Id")
-
-    if "X-Error-Code" in request.headers:
-        error_code = request.headers["X-Error-Code"]
-        if error_code == "RepoNotFound":
-            raise RepositoryNotFoundError(
-                f"404 Client Error: Repository Not Found for url: {request.url}. If the"
-                " repo is private, make sure you are authenticated. (Request ID:"
-                f" {request_id})"
-            )
-        elif error_code == "RevisionNotFound":
-            raise RevisionNotFoundError(
-                f"404 Client Error: Revision Not Found for url: {request.url}. (Request"
-                f" ID: {request_id})"
-            )
-        elif error_code == "EntryNotFound":
-            raise EntryNotFoundError(
-                f"404 Client Error: Entry Not Found for url: {request.url}. (Request"
-                f" ID: {request_id})"
+    request_id = response.headers.get("X-Request-Id")
+    try:
+        response.raise_for_status()
+    except HTTPError as e:
+        if "X-Error-Code" in response.headers:
+            error_code = response.headers["X-Error-Code"]
+            if error_code == "RepoNotFound":
+                message = (
+                    f"{response.status_code} Client Error: Repository Not Found for"
+                    f" url: {response.url}. If the repo is private, make sure you are"
+                    " authenticated."
+                )
+                e = RepositoryNotFoundError(message, response)
+            elif error_code == "RevisionNotFound":
+                message = (
+                    f"{response.status_code} Client Error: Revision Not Found for url:"
+                    f" {response.url}."
+                )
+                e = RevisionNotFoundError(message, response)
+            if error_code == "EntryNotFound":
+                message = (
+                    f"{response.status_code} Client Error: Entry Not Found for url:"
+                    f" {response.url}."
+                )
+                e = EntryNotFoundError(message, response)
+            _add_request_id_to_error_args(e, request_id)
+            raise e
+
+        if response.status_code == 401:
+            # The repo was not found and the user is not Authenticated
+            message = (
+                f"{response.status_code} Client Error: Repository Not Found for url:"
+                f" {response.url}. If the repo is private, make sure you are"
+                " authenticated."
             )
+            e = RepositoryNotFoundError(message, response)
 
-    if request.status_code == 401:
-        # The repo was not found and the user is not Authenticated
-        raise RepositoryNotFoundError(
-            f"401 Client Error: Repository Not Found for url: {request.url}. If the"
-            " repo is private, make sure you are authenticated. (Request ID:"
-            f" {request_id})"
-        )
+        _add_request_id_to_error_args(e, request_id)
 
-    _raise_with_request_id(request)
+        raise e
 
 
 def _raise_with_request_id(request):
@@ -89,7 +112,6 @@ def _raise_with_request_id(request):
     try:
         request.raise_for_status()
     except Exception as e:
-        if request_id is not None and len(e.args) > 0 and isinstance(e.args[0], str):
-            e.args = (e.args[0] + f" (Request ID: {request_id})",) + e.args[1:]
+        _add_request_id_to_error_args(e, request_id)
 
         raise e


Test patch:
diff --git a/tests/test_utils_errors.py b/tests/test_utils_errors.py
new file mode 100644
index 00000000..f0b136b3
--- /dev/null
+++ b/tests/test_utils_errors.py
@@ -0,0 +1,85 @@
+import unittest
+
+from huggingface_hub.utils._errors import (
+    EntryNotFoundError,
+    RepositoryNotFoundError,
+    RevisionNotFoundError,
+    _raise_for_status,
+    _raise_with_request_id,
+)
+from requests.exceptions import HTTPError
+from requests.models import Response
+
+
+class TestErrorUtils(unittest.TestCase):
+    def test__raise_for_status_repo_not_found(self):
+        response = Response()
+        response.headers = {"X-Error-Code": "RepoNotFound", "X-Request-Id": 123}
+        response.status_code = 404
+        with self.assertRaisesRegex(
+            RepositoryNotFoundError, "Repository Not Found"
+        ) as context:
+            _raise_for_status(response)
+
+        self.assertEqual(context.exception.response.status_code, 404)
+        self.assertIn("Request ID: 123", str(context.exception))
+
+    def test__raise_for_status_repo_not_found_without_error_code(self):
+        response = Response()
+        response.headers = {"X-Request-Id": 123}
+        response.status_code = 401
+        with self.assertRaisesRegex(
+            RepositoryNotFoundError, "Repository Not Found"
+        ) as context:
+            _raise_for_status(response)
+
+        self.assertEqual(context.exception.response.status_code, 401)
+        self.assertIn("Request ID: 123", str(context.exception))
+
+    def test_raise_for_status_revision_not_found(self):
+        response = Response()
+        response.headers = {"X-Error-Code": "RevisionNotFound", "X-Request-Id": 123}
+        response.status_code = 404
+        with self.assertRaisesRegex(
+            RevisionNotFoundError, "Revision Not Found"
+        ) as context:
+            _raise_for_status(response)
+
+        self.assertEqual(context.exception.response.status_code, 404)
+        self.assertIn("Request ID: 123", str(context.exception))
+
+    def test_raise_for_status_entry_not_found(self):
+        response = Response()
+        response.headers = {"X-Error-Code": "EntryNotFound", "X-Request-Id": 123}
+        response.status_code = 404
+        with self.assertRaisesRegex(EntryNotFoundError, "Entry Not Found") as context:
+            _raise_for_status(response)
+
+        self.assertEqual(context.exception.response.status_code, 404)
+        self.assertIn("Request ID: 123", str(context.exception))
+
+    def test_raise_for_status_fallback(self):
+        response = Response()
+        response.status_code = 404
+        response.headers = {
+            "X-Request-Id": "test-id",
+        }
+        response.url = "test_URL"
+        with self.assertRaisesRegex(HTTPError, "Request ID: test-id") as context:
+            _raise_for_status(response)
+
+        self.assertEqual(context.exception.response.status_code, 404)
+        self.assertEqual(context.exception.response.url, "test_URL")
+
+    def test_raise_with_request_id(self):
+        response = Response()
+        response.status_code = 404
+        response.headers = {
+            "X-Request-Id": "test-id",
+        }
+        response.url = "test_URL"
+        with self.assertRaisesRegex(HTTPError, "Request ID: test-id") as context:
+            _raise_with_request_id(response)
+
+        self.assertEqual(context.exception.response.status_code, 404)
+        self.assertEqual(context.exception.response.url, "test_URL")