Allow session reuse

[zxdavb]

Regarding this code:

import json
import requests

username = "billg@micro.com"
password = "bad_password"

hostname="https://tccna.honeywell.com"
url=hostname + "/WebAPI/api/Session"

postdata = {'Username': username, 'Password': password, 'ApplicationId': '91db1612-73fd-4500-91b2-e63b069b185c'}
headers = {'content-type': 'application/json'}

response = requests.post(url, data=json.dumps(postdata), headers=headers)
print(response.text)
response.raise_for_status()

It gives:

[{
    "code": "EmailOrPasswordIncorrect",
    "message": "The email or password provided is incorrect."
}]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/srv/hass/lib/python3.6/site-packages/requests/models.py", line 940, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url: https://tccna.honeywell.com/WebAPI/api/Session

This implies (and I strongly suspect its true that) you'll get a different "code" for an expired session.

You do not want to do a retry for EmailOrPasswordIncorrect, only for an expired session.

This existing code is:

if response.status_code == requests.codes.unauthorized and retry:
    # Attempt to refresh session id if request was unauthorised
    if 'Unauthorized' in response.text:
        do_retry()

but should be:

if response.status_code == requests.codes.unauthorized and retry:
    # Attempt to refresh session id if request was unauthorised
    if 'code' in response.text:  # don't use response.json() here!
        if response.json()['code'] = "XXXXX":
            do_retry()
        else:
            message = ("HTTP Status = " + str(response.status_code) + ", Response = " + response.text)
            raise requests.HTTPError(message)

[watchforstock]

Regarding this code:

import json
import requests

username = "billg@micro.com"
password = "bad_password"

hostname="https://tccna.honeywell.com"
url=hostname + "/WebAPI/api/Session"

postdata = {'Username': username, 'Password': password, 'ApplicationId': '91db1612-73fd-4500-91b2-e63b069b185c'}
headers = {'content-type': 'application/json'}

response = requests.post(url, data=json.dumps(postdata), headers=headers)
print(response.text)
response.raise_for_status()

It gives:

[{
    "code": "EmailOrPasswordIncorrect",
    "message": "The email or password provided is incorrect."
}]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/srv/hass/lib/python3.6/site-packages/requests/models.py", line 940, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url: https://tccna.honeywell.com/WebAPI/api/Session

OK great thanks - The check for Unauthorized that I've put in is probably correct then...

[zxdavb]

Sorry, I have a habit of posting, and then extensively editing...

OK great thanks - The check for Unauthorized that I've put in is probably correct then..

No, I'm saying the opposite!

The good news is that I've finished editing that post now.

You'll have to get the JSON response for an expired session, so you can know the code.

[watchforstock]

Sorry, I have a habit of posting, and then extensively editing...

OK great thanks - The check for Unauthorized that I've put in is probably correct then..

No, I'm saying the opposite!

The good news is that I've finished editing that post now.

Maybe I've misunderstood then - I sent a call with valid credentials but an invalid sessionId but got back:

[{
    "code": "Unauthorized",
    "message": "Unauthorized"
}]

So I take that as being that's when I should be retrying (because the sessionId has expired), but if we'd just supplied invalid credentials then we'd get a different message. I guess I could be more specific in checking the code field, rather than just looking for the string...

[zxdavb]

Oh, that's a co-incidence (I didn't know the JSON), but maybe like this:

Instead of:

if response.status_code == requests.codes.unauthorized and retry:
    # Attempt to refresh session id if request was unauthorised
    if 'Unauthorized' in response.text:
        do_retry()

I suggest:

if response.status_code == requests.codes.unauthorized and retry:
    # Attempt to refresh session id if request was unauthorised
    if 'code' in response.text:  # don't use response.json() here!
        if response.json()['code'] = "Unauthorized":
            do_retry()
        else:
            message = ("HTTP Status = " + str(response.status_code) + ", Response = " + response.text)
            raise requests.HTTPError(message)

Then via the raise they'll get a nice error message for other issues, like:

[{
    "code": "EmailOrPasswordIncorrect",
    "message": "The email or password provided is incorrect."
}]

As a rule requests.HTTPError(message) is much more informative that response.raise_for_status(), which never includes response.text.

[zxdavb]

And the final response.raise_for_status() needs to move left four spaces, so that an exception if always raised, if status != SUCCESS (2xx).

[watchforstock]

And the final response.raise_for_status() needs to move left four spaces, so that an exception if always raised, if status != SUCCESS (2xx).

Thanks. I've just pushed an update to address these comments...

[zxdavb]

OMG, something happened at my end & I can't submit a PR, so:

    def _do_request(self, method, url, data=None, retry=True):
        if method == 'get':
            func = requests.get
        elif method == 'put':
            func = requests.put
        elif method == 'post':
            func = requests.post

        response = func(url, data=data, headers=self.headers)

        # catch 401/unauthorized since we may retry
        if response.status_code == requests.codes.unauthorized and retry:        # pylint: disable=no-member
            # Attempt to refresh sessionId if it has expired
            if 'code' in response.text:  # don't use response.json() here!
                if response.json()['code'] == "Unauthorized":
                    # Get a new sessionId
                    self.user_data = None
                    self._populate_user_info()
                    # Set headers with new sessionId
                    session_id = self.user_data['sessionId']
                    self.headers['sessionId'] = session_id

                    response = func(url, data=data, headers=self.headers)

        # display error message if the vendor provided one
        if response.status_code != requests.codes.ok:                            # pylint: disable=no-member
            if 'code' in response.text:  # don't use response.json()!
                message = ("HTTP Status = " + str(response.status_code)
                           + ", Response = " + response.text)
                raise requests.HTTPError(message)

        response.raise_for_status()

        return response

Sorry, I'm asking to take something out, after having asked to put in, but this is a better piece of code (and it does the same job)

[watchforstock]

OMG, something happened at my end & I can't submit a PR, so:

    def _do_request(self, method, url, data=None, retry=True):
        if method == 'get':
            func = requests.get
        elif method == 'put':
            func = requests.put
        elif method == 'post':
            func = requests.post

        response = func(url, data=data, headers=self.headers)

        # catch 401/unauthorized since we may retry
        if response.status_code == requests.codes.unauthorized and retry:        # pylint: disable=no-member
            # Attempt to refresh sessionId if it has expired
            if 'code' in response.text:  # don't use response.json() here!
                if response.json()['code'] == "Unauthorized":
                    # Get a new sessionId
                    self.user_data = None
                    self._populate_user_info()
                    # Set headers with new sessionId
                    session_id = self.user_data['sessionId']
                    self.headers['sessionId'] = session_id

                    response = func(url, data=data, headers=self.headers)

        # display error message if the vendor provided one
        if response.status_code != requests.codes.ok:                            # pylint: disable=no-member
            if 'code' in response.text:  # don't use response.json()!
                message = ("HTTP Status = " + str(response.status_code)
                           + ", Response = " + response.text)
                raise requests.HTTPError(message)

        response.raise_for_status()

        return response

Sorry, I'm asking to take something out, after having asked to put in, but this is a better piece of code (and it does the same job)

Just pushed...

[DBMandrake]

So I take that as being that's when I should be retrying (because the sessionId has expired), but if we'd just supplied invalid credentials then we'd get a different message. I guess I could be more specific in checking the code field, rather than just looking for the string...

Sorry to butt in here as it looks like you guys have this figured out, but I don't think there should be any ambiguity in working out whether an HTTP/1.1 401 Unauthorized response means incorrect username/password vs invalid or timed out session-id, because the URL that is being request at the time will be different...

During username/password authentication the URL is POST /WebAPI/api/Session..... so a 401 response there can only mean wrong username or password.

If the session id has expired the 401 response will come from a query like GET /WebAPI/api/locations?userId=...... so the error is the same but in response to a different kind of request... Or did I miss something ? :)

I see there is a lot of reworking of the V1 automatic re-authentication here so I'm going to go through my test cases again to make sure all situations are still working.

I like the new error reporting btw. Instead of the more generic error I log in evohome-munin now when an API query fails, I'll be able to pass through the string that says it was rate limited, username/password was incorrect etc...

[watchforstock]

During username/password authentication the URL is POST /WebAPI/api/Session..... so a 401 response there can only mean wrong username or password.

@DBMandrake That's a very good point. Based on the testing so far, it looks like there is an explicit message for bad username/password, but if we find that's not sufficient we can be testing the URL.

I'm glad that the error reporting that @zxdavb has contributed is improving your plugin too. Unless anything serious comes up in the next couple of days I'll merge this into master and cut a new release on pypi

[zxdavb]

Unless anything serious comes up in the next couple of days

Just pushed a PR to tidy-up both __init__.pys.

I also intend to push a commit to use __LOGGER.info() instead of print() in evohomeclient2.

...no plans after that!

[DBMandrake]

I've continued to keep up to date with the more recent commits and all seems to be good here. (Although I see many of them look to be unit test updates that won't be affecting me anyway)

Apart from some 500 Server errors for a period yesterday (presumably due to actual server overloading) I haven't seen any request failures on either API so the session_id/access_token save and restore process seems solid.

On the V1 API polling every 5 minutes it managed to keep the same session id in use for more than a day, it only finally ended up having to get a new one after the 500 Server error outage where it wasn't able to get through for over 15 minutes, causing the session id to time out. And it seamlessly obtained a new session id once the 500 error period was over and maintained that session id until the next >15 minute 500 Server error outage.

The V2 API likewise has been working well, polling every 5 minutes and only having to authenticate once every 30 minutes, and when there is only one authentication per 30 minutes I'm not hitting any rate limiting.

The only thing I've noticed really is the inconsistent exception error reporting between the V1 and V2 API's, where the V1 API recently got new error reporting while the V2 API did not. For example here is rate limiting as reported by both API's:

Traceback (most recent call last):
  File "./evohome_V1", line 9, in <module>
    for device in client.temperatures():
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 117, in temperatures
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 78, in _populate_full_data
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 109, in _populate_user_info
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 197, in _do_request
requests.exceptions.HTTPError: HTTP Status = 429, Response = [
  {
    "code": "TooManyRequests",
    "message": "Request count limitation exceeded, please try again later."
  }
]

pi@pi1monitor:~ $ ./evohome_V2
Traceback (most recent call last):
  File "./evohome_V2", line 6, in <module>
    client = EvohomeClient('**************', '***********')
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 69, in __init__
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 72, in _login
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 193, in user_account
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 79, in _headers
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 116, in _basic_login
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 147, in _obtain_access_token
  File "/usr/local/lib/python2.7/dist-packages/requests/models.py", line 840, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 429 Client Error: Too Many Requests for url: https://tccna.honeywell.com/Auth/OAuth/Token

And here is invalid username/password as reported by both API's:

pi@pi1monitor:~ $ ./evohome_V1
Traceback (most recent call last):
  File "./evohome_V1", line 9, in <module>
    for device in client.temperatures():
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 117, in temperatures
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 78, in _populate_full_data
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 109, in _populate_user_info
  File "build/bdist.linux-armv6l/egg/evohomeclient/__init__.py", line 197, in _do_request
requests.exceptions.HTTPError: HTTP Status = 401, Response = [
  {
    "code": "EmailOrPasswordIncorrect",
    "message": "The email or password provided is incorrect."
  }
]

pi@pi1monitor:~ $ ./evohome_V2
Traceback (most recent call last):
  File "./evohome_V2", line 6, in <module>
    client = EvohomeClient('************', '*********')
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 69, in __init__
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 72, in _login
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 193, in user_account
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 79, in _headers
  File "build/bdist.linux-armv6l/egg/evohomeclient2/__init__.py", line 117, in _basic_login
ValueError: Bad Username/Password, unable to continue.

I wonder if it was an oversight for the error reporting on the V2 API not to be updated to the same style the V1 API now uses ?

I'm only using the V1 API in evohome-munin now and I'm just printing the error report pretty much as is from the exception without trying to interpret the fields - the indented json appearance is perfectly readable in a log file:

            try:
                client = EvohomeClient(self.username, self.password, user_data=user_data)
                for device in client.temperatures():
                    data.append(device)
            except Exception as error:
                sys.stderr.write('Connection failed: ' + str(error) + '\n')
                sys.exit(1)

It means I can leave debugging off but if there is an error a proper error report with the cause of the error (instead of my previous generic server connection error) will appear in the log which is nice and is what allowed me to see that the failures I got yesterday were 500 Server errors, not 429 rate limiting or 401 authentication errors.