watchforstock / evohome-client

Python client to access the Evohome web service
Apache License 2.0
88 stars 52 forks source link

Auto renew - add r.raise_for_status() after r = requests... #47

Closed zxdavb closed 6 years ago

zxdavb commented 6 years ago

I have added the following after every instance of r = requests.xxx():

if r.status_code != requests.codes.ok:
    r.raise_for_status()

I feel this is the bare minimum: it might be better to include r.text in the raise_for_status.

BTW, for consistency, I also changed all response = requests.xxx to r = requests.xxx.

zxdavb commented 6 years ago

Sorry, just realised I've got two commits here. Can you get rid of the 'More logging is needed' commit at your end? Or do I have to rebase or something?

zxdavb commented 6 years ago

If you look at the 'extraneous' self.locations = [], you can see that it inappropriately wipes the array the first time the access token expires, and headers() calls _basic_login(). In my case, I was unable to call client.locations[0].status() and got an IndexError (as len(client.locations) == 0) after 30 mins.

Similarly, I think anything like:

self.access_token
self.access_token_expires
self.account_info
self.locations

which are set after the results of a r = requests.xxx should be None (or similar) immediately before the request is made so that if the request does fail, they do not contain stale data. This is especially true for access_token and access_token_expires, but less so for account_info and locations (you would expect them to be more stable).

Nonetheless, Honeywell's api must, unfortunately, be polled, and I take the view that every time the access token times out is a good time to do a refresh (e.g. residential address, zone names, and/or schedules may have changed in the last 30-60 minutes). This is the way I do it presently, by invoking client._login().

I think there is only a marginal argument for evohome-client to handle this scenario. My code, for example, can rely upon your trick with self.headers() --> self._basic_login(), and simply continue to periodically call client._login, albeit less often.

zxdavb commented 6 years ago

And here is an example of it working with a 503:

2018-06-23 09:11:01 INFO (SyncWorker_7) [custom_components.evohome] Calling client v2 API [1 request(s)]: client.locations[0].status()...
2018-06-23 09:11:02 ERROR (MainThread) [homeassistant.helpers.entity] Update for climate._my_home fails
Traceback (most recent call last):
  File "/srv/hass/lib/python3.6/site-packages/homeassistant/helpers/entity.py", line 197, in async_update_ha_state
    yield from self.async_device_update()
  File "/srv/hass/lib/python3.6/site-packages/homeassistant/helpers/entity.py", line 320, in async_device_update
    yield from self.hass.async_add_job(self.update)
  File "/usr/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/root/.homeassistant/custom_components/evohome.py", line 669, in update
    _updateStateData(self.client, self.hass.data[DATA_EVOHOME])
  File "/root/.homeassistant/custom_components/evohome.py", line 202, in _updateStateData
    = _returnTempsAndModes(evo_client)
  File "/root/.homeassistant/custom_components/evohome.py", line 255, in _returnTempsAndModes
    ec2_status = client.locations[0].status()  # get latest modes/temps
  File "/srv/hass/lib/python3.6/site-packages/evohomeclient2/location.py", line 25, in status
    r.raise_for_status()
  File "/srv/hass/lib/python3.6/site-packages/requests/models.py", line 935, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 503 Server Error: Service Unavailable for url: https://tccna.honeywell.com/WebAPI/emea/api/v1/location/2738909/status?includeTemperatureControlSystems=True