search

scolby33 / pushover_complete

A Python package for interacting with *all* aspects of the Pushover API.
MIT License
6 stars 5 forks source link

Generate BadAPIRequestError when network is down rather than terminating and emitting urllib3.exceptions.MaxRetryError #15

Closed pletch closed 3 months ago

pletch commented 3 months ago

Home Assistant is leveraging this library in the Pushover integration. With the current implementation, if Home Assistant is started before the network is up, the integration will crash with requests associated urllib3.exceptions.MaxRetryError. This error is not specific to the pushover_complete library and causes the integration startup to fail (HA issue 110733).

Request that library is modified to handle connection failure by emitting an appropriately indicative BadAPIRequestError under conditions of network absence. This will allow error handling in programs leveraging this library to catch the error and gracefully handle restarting.

scolby33 commented 3 months ago

Hi there! As was noticed in that discussion, this project has languished for a while. I just spent a few hours "modernizing" the project in #16 and as soon as I've pushed that, I'll work on a fix for the issue that HA is encountering. I'd estimate that to happen by the end of next week.

pletch commented 3 months ago

Thank you for having a look at this!

In a local modification, I encapsulated the get / post requests in pushover_api.py in try / except blocks. This seemed to take care of the issue in Home Assistant for me.

        try:
            resp = post(
               urljoin(PUSHOVER_API_URL, endpoint.format(url_parameter)),
               data=payload,
               files=files
            )
            resp_body = resp.json()
            if resp_body.get('status', None) != 1:
               raise BadAPIRequestError('{}: {}'.format(resp.status_code, ': '.join(resp_body.get('errors'))))
            return resp_body
        except requests.ConnectionError:
            raise BadAPIRequestError('Failed to connect to Pushover API endpoint. Is network up?')
        except:
            raise BadAPIRequestError('Error performing POST request to Pushover API endpoint.')
scolby33 commented 3 months ago

I think that that would be the wrong solution, as the BadAPIRequestError is raised when the Pushover API responds with an error and is not related to underlying networking issues. Home Assistant just merged https://github.com/home-assistant/core/pull/115802, which fixes the problem on their end. This was exactly the solution I was going to suggest anyway!