FIX / Hub: Also catch for exceptions.ConnectionError

[younesbelkada]

What does this PR do?

Fixes: #30920

Please see the detailed issue description for more details, it seems in transformers we hit a corner case when using HF offline mode + loading a non-safetensors model offline

cc @Wauplin as I am not 100% sure about this and why this is not catched within hf_raise_for_status in huggingface_hub

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[Wauplin]

@younesbelkada I'm surprised that a requests.exceptions.ConnectionError can be raised by hf_raise_for_status. In theory, raise_for_status (from Python's requests package) is meant for when a HTTP response has been returned by the server and you want to raise if the server returned an error. A ConnectionError is different and happens before or while getting a response from the server. So to catch it you should do it here IMO.

While you are at it, you can also include read timeout and remove SSLError and ProxyError for which it's better to raise:

        except (requests.exceptions.SSLError, requests.exceptions.ProxyError):
            # Actually raise for those subclasses of ConnectionError
            raise
        except (
            requests.exceptions.ConnectionError,
            requests.exceptions.Timeout,
            OfflineModeIsEnabled,
        ):
            return has_file_in_cache

This is what we do in huggingface_hub download method.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

[Wauplin]

@younesbelkada @huggingface/transformers-core-maintainers I still think this PR is worth updating + merging (see previous comment #31469 (comment)).

[amyeroberts]

@Wauplin I updated following your comments, but not 100% I'm catching in the right way. LMK!

[Wauplin]

Thanks for getting back to it @amyeroberts . I had a fresh look at it and I think the changes should be as suggested below. First we do the call and check for offlinemode/connection error/timeout error. And then check for httperror independently

[Wauplin]

Thanks!

[amyeroberts]

@Wauplin Thanks for writing the correct code! As we both contributed to this - let's get a final pair of eyes to double check this is OK

[ArthurZucker]

thanks, this was causing some confusing indeed