SAIC-iSmart-API / saic-python-mqtt-gateway

MIT License
67 stars 20 forks source link

Request change on saic ignore relogin delay #133

Open an0Nym0us63 opened 8 months ago

an0Nym0us63 commented 8 months ago

Hello the team

As of now there is a saic relogin_delay if someone logs into his mobile app (ismart). That is a good idea to avoid someone being on the app and being kicked out because the gateway checks new message each minute

But i think there are at least two cases where this relogin delay must be ignored

1) if an action is performed via the gateway , i think in this case the gateway should relogin to perform the action

2) If a force refreshed is asked, i think the same should happen, in this case the gateway should relogin to perform the forced refresh

nanomad commented 8 months ago

@sarakha63 you actually raise a good point. I think we always went the wrong way about this. The re-login should be handled in the gateway and not the API, so that we can decide when it makes sense to do so or not....

an0Nym0us63 commented 8 months ago

@nanomad @tosate although i still think this is a good feature as if a make an action or ask for a refresh via the gateway i expect it to do it (without thinking "ahhh maybe my wife logged in in ismart and i'm stuck for xxx seconds)

but i think for now there is also an issue, i'm pretty sure as i made some tests. As of now if an ismart loggin is detected there will never be a relogin except if we restart the container.

i setted the dely to 120 seconds for testing purposes and confirmation

and once someones logs in ismart

the gateway will trigger this each minutes

2024-01-29 09:54:50,619 [ INFO ] Job "Check for new messages (trigger: interval[0:01:00], next run at: 2024-01-29 09:55:50 CET)" executed successfully - apscheduler.executors.default 2024-01-29 09:55:50,547 [ INFO ] Running job "Check for new messages (trigger: interval[0:01:00], next run at: 2024-01-29 09:56:50 CET)" (scheduled at 2024-01-29 09:55:50.545740+01:00) - apscheduler.executors.default 2024-01-29 09:55:50,616 [ ERROR ] API call failed: {"code":401,"message":"Token expired"} - saic_ismart_client_ng.api.base 2024-01-29 09:55:50,616 [ ERROR ] MessageHandler poll loop failed - __main__ Traceback (most recent call last): File "/usr/src/app/./mqtt_gateway.py", line 449, in __polling unread_count = await self.saicapi.get_unread_messages_count() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/message/__init__.py", line 59, in get_unread_messages_count return await self.execute_api_call( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/base.py", line 90, in execute_api_call return await self.deserialize(req, response, out_type) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/base.py", line 131, in deserialize raise SaicApiException(f"API call failed with status code {response.status_code}: {response.text}") saic_ismart_client_ng.exceptions.SaicApiException: API call failed with status code 401: {"code":401,"message":"Token expired"} 2024-01-29 09:55:50,617 [ INFO ] Job "Check for new messages (trigger: interval[0:01:00], next run at: 2024-01-29 09:56:50 CET)" executed successfully - apscheduler.executors.default

or this one when it is supposed to relogin i guess

2024-01-29 10:02:50,603 [ ERROR ] API call failed: {"code":401,"message":"Token expired"} - saic_ismart_client_ng.api.base 2024-01-29 10:02:50,603 [ ERROR ] NOT Retrying since we got a generic exception - saic_ismart_client_ng.api.base 2024-01-29 10:02:50,603 [ ERROR ] handle_vehicle loop failed during SAIC API call. Waiting 30s before retrying - __main__ Traceback (most recent call last): File "/usr/src/app/./mqtt_gateway.py", line 91, in handle_vehicle vehicle_status = await self.update_vehicle_status() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/app/./mqtt_gateway.py", line 121, in update_vehicle_status vehicle_status_response = await self.saic_api.get_vehicle_status(self.vin_info.vin) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/vehicle/__init__.py", line 13, in get_vehicle_status return await self.execute_api_call_with_event_id( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/base.py", line 120, in execute_api_call_with_event_id return await execute_api_call_with_event_id_inner(event_id='0') ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/tenacity/_asyncio.py", line 88, in async_wrapped return await fn(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/tenacity/_asyncio.py", line 47, in __call__ do = self.iter(retry_state=retry_state) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/tenacity/__init__.py", line 314, in iter return fut.result() ^^^^^^^^^^^^ File "/usr/local/lib/python3.12/concurrent/futures/_base.py", line 449, in result return self.__get_result() ^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/concurrent/futures/_base.py", line 401, in __get_result raise self._exception File "/usr/local/lib/python3.12/site-packages/tenacity/_asyncio.py", line 50, in __call__ result = await fn(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/base.py", line 111, in execute_api_call_with_event_id_inner return await self.execute_api_call( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/base.py", line 90, in execute_api_call return await self.deserialize(req, response, out_type) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.12/site-packages/saic_ismart_client_ng/api/base.py", line 131, in deserialize raise SaicApiException(f"API call failed with status code {response.status_code}: {response.text}") saic_ismart_client_ng.exceptions.SaicApiException: API call failed with status code 401: {"code":401,"message":"Token expired"}

And even after half an hour it stills triggers this and never relogins. The only way is to restart the container.

I can create another issue besides this one because i think this issue is quite bothering and unexpected

an0Nym0us63 commented 8 months ago

in fact i looked at the code and made some debug and we never pass trough the dotted line as we have a raise Execption that is triggered


   if response.is_error:
            logger.error(f"API call failed: {response.text}")
            raise SaicApiException(f"API call failed with status code {response.status_code}: {response.text}")
.....................................................................................................................................................................................................................................
        try:
            request_event_id = request.headers.get('event-id')
            json_data = response.json()
            return_code = json_data.get('code', -1)
            error_message = json_data.get('message', 'Unknown error')
            logger.debug(f"Response code: {return_code} {response.text}")

            if return_code == 401:
                logger.error(f"API call return code is not acceptable: {return_code}: {response.text}")
                self.logout()
                if self.__configuration.relogin_delay:
                    logger.warning(f"Waiting since we got logged out: {return_code}: {response.text}")
                    await asyncio.sleep(self.__configuration.relogin_delay)
                logger.warning(f"Logging in since we got logged out")
                await self.login()
                raise SaicApiException(error_message, return_code=return_code)
nanomad commented 8 months ago

Nice catch :)

I'll fix that in the API code and prepare a new gateway release

nanomad commented 8 months ago

Fixed in 0.5.2. Feel free to try and report back.

The original issue still stands :)

an0Nym0us63 commented 8 months ago

@nanomad it seems doing the job as expected now. i still noticed some exceptions the first time it retried after the delay but the next iteration was ok. I will let time and report if needed

yes the original issue stands as i think this will be a great way of handling things in the gateway

thanks

nanomad commented 8 months ago

@sarakha63 the exception is there since we make the task fail on purpose. If we go forward with your approach, which i quite like, then it would be gracefully handled by the gateway