cibernox / homeassistant-poolstation

HomeAssistant custom component for integrating the Poolstation platform.
MIT License
10 stars 4 forks source link

Reauthentication #10

Closed straybiker closed 2 months ago

straybiker commented 1 year ago

Once in a while I need to reauthenticate the integration, which causes my automation to stop. Could you make the integration as such that it tries to reauthenticate itself?

cibernox commented 1 year ago

I'd love to, it's also driving me mad. I tried for a while it's quite hard to debug and replicate. Also the time that the integration stays logged is somewhat inconsistent. I've seen it stay authenticated for well over a month and I've seen asking for reauthentication twice in a single day. I suspect it's hitting some kind of API quota.

straybiker commented 1 year ago

Yes indeed, it seems a random period. If HA would be able to alert on this, it would be fine because reauthentication is not that urgent often. Does the call token = await account.login() fails?

cibernox commented 1 year ago

TBH i last looked at this last summer, so by now I don't remember anything.

straybiker commented 1 year ago

I added some logging in my local py file. Will look at it when it occurs again.

straybiker commented 1 year ago

It looks like the reauthentication funtion is never called.

cibernox commented 1 year ago

Thanks for investigating. TBH i copied this feature from another integration, I might have to check for how other integrations do it to see if I can spot what's wrong this implementation.

RienduPre commented 9 months ago

Any news on this subject?

lobmarib commented 6 months ago

Would be nice to have a configuration file where it could be the authentication data and reauthenticate automatically...

MattrixST commented 6 months ago

I can buy you a beer if this helps adding this, any Paypal account? ;)

straybiker commented 6 months ago

The problem seems to be that the code to authenticate again is never called. The problem is also hard to reproduce since it happens randomly. So even when it is stored in a config file, the data will never be read. Also going with this approach the secret file would be a better option.

straybiker commented 6 months ago

I was looking again at the code and noticed that in async def _async_update_data(self) -> None: """Fetch data from poolstation.net.""" try: await self.pool.sync_info() except AuthenticationException as err: raise ConfigEntryAuthFailed from err

Only an authentication error is handled. While comments the same while indicate that the error responses are not correct returned. Shouldn't other errors also try to reauthenticate?

sergio-scl commented 5 months ago

Any news with the problem of losing authentication and having to re-enter credentials?

Thanks in advance!

straybiker commented 5 months ago

Now that summer is coming, the problem seems to be worse. I tried to reduce the read frequency to 1min to check if it is related to the local connection point, but it doesn't seem to help. So it looks like this is a problem on the server side, I assume something related to the load and performance.

RienduPre commented 5 months ago

It’s getting worse over here too. I have several autentication issues a day. Very inconvinient , because I have several automations related to entities of this integration. I will do all I can to help solving the issue.🙏

straybiker commented 5 months ago

I think I traced back the error to the pypoolstation library by enabling the debug logging and adding looking at the traces. All ClientErrors are thrown as ClientAuthentication errors.

` async def post(self, url, data=""): try: resp = await self._session.post( url, data=f"Authorization=Bearer {self._token}" + data, headers={"accept": "application/json", "content-type": "application/x-www-form-urlencoded"} ) resp.raise_for_status() except ClientResponseError as err:

Unfortunately, the API is aweful and making a request with an expired token seems to trigger a generic 500 error.

        # For all I could find, there's not way of
        raise AuthenticationException("Request failed. Maybe token has expired.")
    return await resp.json()`

This means that a serverside timeout results in an Authentication error. I worked around this by ignoring the error. This means that I miss an update after a timeout, but up to know it seems to be working fine. I don't know how to catch real expired token errors. Are these returned with error code 500? Timeouts are returned as errorcode 504, so if Authentication errors are returned with a 500, it may be possible to differentiate. See the comment of cibernox.

@cibernox , how can I make local changes for testing to pypoolstation since this component is imported?

cibernox commented 5 months ago

Hi everyone. I haven't been able to make any changes or improvements on this project because a flood damaged my ethernet cable that connected the chlorinator, so so far this year I've been operating as if it was a dumb chlorinator. I don't feel like buying 100m of ethernet table and running the wire again, so I ordered a powerline adapter. The first and cheap one didn't work, possibly because it didn't had enough range, so I ordered a more reputable brand now and hopefully I'll get it working this time.

Now, back to the topic, about the timeout getting falsely mistaken for a 500 error, that's a boomer. The API is undocumented and while I wrote some of the developers at fluidra, the main company that develops this system, based near Barcelona, I didn't get a formal spec.

My initial solution would be to have some sort of counter of failures, and maybe allow three 500 errors before considering it an auth problem. PyPoolstation can be found in https://github.com/cibernox/PyPoolstation, you can just fork it and make changes. If you want I can also publish new "beta" versions to be used in this integration to verify they improve the situation.

I suspect that the problem with having to re-render the credentials is a separate issue. I believe that issue is in this project, not in pypoolstation, and I think the re-authentication logic is not triggering at all. I'm not particularly savvy in python and this was my very first homeassistant integration, possibly some of the built-in hooks for handling errors is not correct.

straybiker commented 5 months ago

I forked both projects already, but still figuring out how this import mechanism of pypoolstation works. Do you have any guidance on this?

I didn't look further why the re-authentiaction is not working as intended, but I can image that if the re-auth process is mistakenly started because a time-out error and the time-out occurs again during the re-auth, the dialog is shown. For me, re-entering the credentials because of a failing re-auth is not that big of an issue if the only happens rarely. So i'll focus first on proper client response handling.

straybiker commented 4 months ago

Created pull requests for a fix in PyPoolStation and Homeassistant-poolstation. The fix ran for a few days here without any issues. Was able to reproduce the issue by change the pw of my setup

RienduPre commented 4 months ago

That's good news:-)

cibernox commented 4 months ago

It's all merged now. Please, give it a go. I'm still without network conectivity on my chlorinator so I can't test it myself.

RienduPre commented 4 months ago

I still have the reauthentication issues with the latest build

straybiker commented 4 months ago

With the version of yesterday, or the one before? Not all required changes were merged in the first version.

RienduPre commented 4 months ago

Mine is 3 days old, I dont see a newer version

straybiker commented 4 months ago

The latest is 2 days old, from July 10

RienduPre commented 4 months ago

I re-downloaded it from HACS again yesterday This morning I had the error again (see screenshots)

How do I check if I use the latest version?

IMG_4207 IMG_4208 IMG_4209

straybiker commented 4 months ago

I also had it again once last night. The fix doesn't prevent any authentication errors but handles not all server errors, such as timeout, as authentication error. If the server still responds with an authentication, it is still treated as such.

straybiker commented 4 months ago

Btw, looking at the timing of your screenshot, I had the same error at the same moment. So I assume it was a serverside problem.

straybiker commented 3 months ago

I am testing a retry mechanism that ignores the first 10 authentication errors. With an update frequency of 1 minutes, that would give the connection 10 minutes to restore. I have no clue how fast an error is restored

RienduPre commented 2 months ago

@straybiker What are the results of your test? and how do you test this?

straybiker commented 2 months ago

Testing is just letting it run since I cannot reproduce the problem. But the error appeared few times a week before and now it doesn't anymore and I am not losing functionely or missing data. So it seems to work

RienduPre commented 2 months ago

Testing is just letting it run since I cannot reproduce the problem. But the error appeared few times a week before and now it doesn't anymore and I am not losing functionely or missing data. So it seems to work

Letting what run ?

straybiker commented 2 months ago

The integration with my changes and see if the error occurs again.

RienduPre commented 2 months ago

But your changes are not commited yet, so how do I install it?

cibernox commented 2 months ago

@straybiker if you have it working locally with good results I'm happy to merge it. Can you open a PR?

straybiker commented 2 months ago

I am not yet 100% sure it is working fine because I saw some oddities that I cannot fully explain. Once it is working well enough to my liking, I will open a PR. @cibernox there is another PR waiting for sensors from UV.

cibernox commented 2 months ago

All merged. IDK what the UV sensors are for btw. I assume those are nothing more than weather information, right? What are they useful for?

straybiker commented 2 months ago

You can connect an UV light to the controller to kill bacteria and such. This sensor provide the lifetime of the light, the number of ignitions and whether the light is switched on or not

cibernox commented 2 months ago

Oh, I thought it was some kind of UV index of the weather report (which the API does provide). I don't have that addon. ¿Link for info? Sounds interesting.

straybiker commented 2 months ago

you can find some info here for example: https://www.riverpoolsandspas.com/blog/pool-uv-systems-cost-pros-cons#:~:text=The%20UV%20light%20rays%20target,hurt%20you%20or%20any%20swimmers. You can hook it up to the controller and enable it in the settings. This is not a separte addon you have to buy for the controller

RienduPre commented 2 months ago

Hi, I did install the new version now I get a configuration error. See screenshot

IMG_0005

straybiker commented 2 months ago

@cibernox did you also merge the required version number in pypool?

RienduPre commented 2 months ago

Did a reinstall, but got still the same error

Is there any way I can solve this myself?

IMG_0008

Thanks

cibernox commented 2 months ago

We missed a publish step. I’ll fix it in q couple hours when I’m back in front of a computer. Try again this afternoon

RienduPre commented 2 months ago

Please let me know, when I can download the new version please.

Thx, Rien

cibernox commented 2 months ago

@RienduPre I just published the version of PyPoostation that was missing. I believe installing this should work now.

RienduPre commented 2 months ago

Thanks, but now I get a new error

Logger: homeassistant.config_entries Bron: config_entries.py:604 Eerst voorgekomen: 22:58:16 (2 gebeurtenissen) Laatst gelogd: 23:00:46

Error setting up entry rien.dupre@me.com for poolstation Traceback (most recent call last): File "/usr/src/homeassistant/homeassistant/config_entries.py", line 604, in async_setup result = await component.async_setup_entry(hass, self) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/config/custom_components/poolstation/init.py", line 32, in async_setup_entry pools = await Pool.get_all_pools(session, account=account) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/pypoolstation/init.py", line 78, in get_all_pools return list(map(lambda x: Pool(session, token, x['id'], account.logger), data["items"])) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/pypoolstation/init.py", line 78, in return list(map(lambda x: Pool(session, token, x['id'], account.logger), data["items"])) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/pypoolstation/init.py", line 110, in init seif.total_uv_timer = None ^^^^ NameError: name 'seif' is not defined. Did you mean: 'self'?

cibernox commented 2 months ago

That was indeed a typo. Fixed and pushed, try once more.

RienduPre commented 2 months ago

Thanks Problem solved. now let’s see if the reauthentication will occur or not

cibernox commented 2 months ago

fingers crossed. I suspect it may still happen, but less often.

straybiker commented 2 months ago

@cibernox this latest change did not include the retries for authentication but for the UV light sensors. The retries seem to work for small outages but not ones that take more then 10 minutes. If this happens, about once a week, I need to reload the plug-in. The credentials pop-up doesn't appear anymore. Still no idea why. It is progress, though going from multiple times a day to less then once a week.