black-forest-labs / flux

Official inference repo for FLUX.1 models
Apache License 2.0
15.34k stars 1.1k forks source link

Enhance error handling in `ImageRequest` class #136

Closed mandlinsarah closed 1 month ago

mandlinsarah commented 1 month ago

Improved the error handling in the ImageRequest class by using the raise_for_status() method to automatically raise exceptions for HTTP errors. This change makes the code cleaner and improves reliability.

jenuk commented 1 month ago

Hi, thanks for taking time to improve the codebase.

With your implementation the specifics of the errors are lost. E.g. if you have wrong api key set, you will get

# previously
Traceback (most recent call last):
[...]
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 99, in __init__
    self.request()
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 118, in request
    raise ApiException(status_code=response.status_code, detail=result.get("detail"))
__main__.ApiException: ApiException(self.status_code=403, message='Not authenticated', detail=Not authenticated)

# your implementation
Traceback (most recent call last):
[...]
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 99, in __init__
    self.request()
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 116, in request
    response.raise_for_status()  # Automatically raises an HTTPError for bad responses
  File "[...]/black-forest-labs/flux/.venv/lib/python3.10/site-packages/requests/models.py", line 1024, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://api.bfl.ml/v1/image

or when having an invalid request_id (as an example for any other kind of error):

# previously
Traceback (most recent call last):
  [...]
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 169, in url
    result = self.retrieve()
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 147, in retrieve
    raise ApiException(status_code=200, detail=f"API returned status '{result['status']}'")
__main__.ApiException: ApiException(self.status_code=200, message="API returned status 'Task not found'", detail=API returned status 'Task not found')

# your implementation
Traceback (most recent call last):
[...]
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 167, in url
    result = self.retrieve()
  File "[...]/black-forest-labs/flux/src/flux/api.py", line 138, in retrieve
    response.raise_for_status()  # Automatically raises an HTTPError for bad responses
  File "[...]/black-forest-labs/flux/.venv/lib/python3.10/site-packages/requests/models.py", line 1024, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://api.bfl.ml/v1/get_result?id=210

Having this specific error here makes it clear that something went wrong with the API call and the reason is human-readable.

This change [...] improves reliability.

Did you run into any cases where the code raised any exceptions other than the ApiException? I'm not sure how the reliability would be increased by these changes.