Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clearer error message on unprocessable entity. #1755

Merged
merged 3 commits into from
Oct 17, 2023

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Oct 17, 2023

Fix #1753 by adding a better error message to the 422 unprocessable entity. I chose not to re-raise a different error but appending the message instead (easier in term of backward compatibility and still valid).

>>> from huggingface_hub import InferenceClient
>>> client = InferenceClient(model="HuggingFaceH4/zephyr-7b-alpha")
>>> output = client.conversational("Hi how are you?")
huggingface_hub.utils._errors.HfHubHTTPError: 422 Client Error: Unprocessable Entity for url: https://api-inference.huggingface.co/models/HuggingFaceH4/zephyr-7b-alpha (Request ID: 8gb40cYT0epCW3jTQ5HUd)
Make sure 'conversational' task is supported by the model.
# python -m asyncio
>>> from huggingface_hub import AsyncInferenceClient
>>> client = AsyncInferenceClient(model="HuggingFaceH4/zephyr-7b-alpha")
>>> output = await client.conversational("Hi how are you?")
aiohttp.client_exceptions.ClientResponseError: 422, message="Unprocessable Entity. Make sure 'conversational' task is supported by the model.", url=URL('https://api-inference.huggingface.co/models/HuggingFaceH4/zephyr-7b-alpha')

Also added some unit tests about it.

cc @jamesbraza

@Wauplin Wauplin requested a review from LysandreJik October 17, 2023 14:53
Copy link
Contributor

@jamesbraza jamesbraza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Thanks

@@ -240,6 +240,10 @@ def post(
hf_raise_for_status(response)
return response.iter_lines() if stream else response.content
except HTTPError as error:
if error.response.status_code == 422 and task is not None:
error.args = (
error.args[0] + f"\nMake sure '{task}' task is supported by the model.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
error.args[0] + f"\nMake sure '{task}' task is supported by the model.",
f"{error.args[0]}\nMake sure '{task}' task is supported by the model.",

I think f-str is a bit easier than concatenation. Also, why does sync client delimit with "\n" and async client delimit with ". "?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! f-string makes sense yes 👍

Also, why does sync client delimit with "\n" and async client delimit with ". "?

For stylish reasons. The sync client raises a requests.HTTPError exception which print the message when serialized. I prefer the \n so that the warning is on its own line. For the async client we have a aiohttp.ClientResponseError which doesn't serialize the same way (see snippet). So I preferred to simple put ". ".
But no strong opinion on it ^^

@Wauplin Wauplin merged commit 85e3edd into main Oct 17, 2023
@Wauplin Wauplin deleted the 1753-better-error-message-on-unprocessable-entity branch October 17, 2023 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vague error of 422 Client Error: Unprocessable Entity
2 participants