Skip to content

Commit

Permalink
Remove raw HTML from error message content and improve request ID cap…
Browse files Browse the repository at this point in the history
…ture (#2584)

* Fix error formatting

* Add tests
  • Loading branch information
hanouticelina authored Oct 2, 2024
1 parent 6fcc7b3 commit ce38f46
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 7 deletions.
8 changes: 6 additions & 2 deletions src/huggingface_hub/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class HfHubHTTPError(HTTPError):
sent back by the server, it will be added to the error message.
Added details:
- Request id from "X-Request-Id" header if exists.
- Request id from "X-Request-Id" header if exists. If not, fallback to "X-Amzn-Trace-Id" header if exists.
- Server error message from the header "X-Error-Message".
- Server error message if we can found one in the response body.
Expand All @@ -68,7 +68,11 @@ class HfHubHTTPError(HTTPError):
"""

def __init__(self, message: str, response: Optional[Response] = None, *, server_message: Optional[str] = None):
self.request_id = response.headers.get("x-request-id") if response is not None else None
self.request_id = (
response.headers.get("x-request-id") or response.headers.get("X-Amzn-Trace-Id")
if response is not None
else None
)
self.server_message = server_message

super().__init__(
Expand Down
14 changes: 10 additions & 4 deletions src/huggingface_hub/utils/_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,9 @@ def _format(error_type: Type[HfHubHTTPError], custom_message: str, response: Res
server_errors.append(error["message"])

except JSONDecodeError:
# Case error is directly returned as text
if response.text:
# If content is not JSON and not HTML, append the text
content_type = response.headers.get("Content-Type", "")
if response.text and "html" not in content_type.lower():
server_errors.append(response.text)

# Strip all server messages
Expand All @@ -528,11 +529,16 @@ def _format(error_type: Type[HfHubHTTPError], custom_message: str, response: Res
final_error_message += "\n" + server_message
else:
final_error_message += "\n\n" + server_message

# Add Request ID
request_id = str(response.headers.get(X_REQUEST_ID, ""))
if len(request_id) > 0 and request_id.lower() not in final_error_message.lower():
if request_id:
request_id_message = f" (Request ID: {request_id})"
else:
# Fallback to X-Amzn-Trace-Id
request_id = str(response.headers.get(X_AMZN_TRACE_ID, ""))
if request_id:
request_id_message = f" (Amzn Trace ID: {request_id})"
if request_id and request_id.lower() not in final_error_message.lower():
if "\n" in final_error_message:
newline_index = final_error_message.index("\n")
final_error_message = (
Expand Down
16 changes: 15 additions & 1 deletion tests/test_utils_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
RepositoryNotFoundError,
RevisionNotFoundError,
)
from huggingface_hub.utils._http import REPO_API_REGEX, X_REQUEST_ID, _format, hf_raise_for_status
from huggingface_hub.utils._http import REPO_API_REGEX, X_AMZN_TRACE_ID, X_REQUEST_ID, _format, hf_raise_for_status


class TestErrorUtils(unittest.TestCase):
Expand Down Expand Up @@ -257,6 +257,20 @@ def test_hf_hub_http_error_init_with_error_message_duplicated_in_header_and_body
assert str(error) == "this is a message\n\nError message duplicated in headers and body."
assert error.server_message == "Error message duplicated in headers and body."

def test_hf_hub_http_error_without_request_id_with_amzn_trace_id(self) -> None:
"""Test request id is not duplicated in error message (case insensitive)"""
self.response.headers = {X_AMZN_TRACE_ID: "test-trace-id"}
error = _format(HfHubHTTPError, "this is a message", response=self.response)
assert str(error) == "this is a message (Amzn Trace ID: test-trace-id)"
assert error.request_id == "test-trace-id"

def test_hf_hub_http_error_with_request_id_and_amzn_trace_id(self) -> None:
"""Test request id is not duplicated in error message (case insensitive)"""
self.response.headers = {X_AMZN_TRACE_ID: "test-trace-id", X_REQUEST_ID: "test-id"}
error = _format(HfHubHTTPError, "this is a message", response=self.response)
assert str(error) == "this is a message (Request ID: test-id)"
assert error.request_id == "test-id"


@pytest.mark.parametrize(
("url", "should_match"),
Expand Down

0 comments on commit ce38f46

Please sign in to comment.