fix: Frequent issues with expired token

[lumiru]

We get sometimes issues after host suspend/resume with expiration date, so we get token expiration error from server.

[smirgol]

Hm I thought I had fixed that already. There is already some logic for that in api.py, starts here start():

• Check if we have cached session data.
• if we don't have any, go to create_session(refresh=False) and start a session from scratch
• if we have it, check the expiration date of the token.
• if it is expired, trigger create_session(refresh=True) and fetch a new token using the refresh_token. otherwise just return, because we still should have all we need for further requests.

create_session():

• either create session from scratch using login data (if refresh is false)
• or get a new token using the refresh token
• it is then checked if we got a 400, which should indicate an expired refresh token. if we get that, delete session cache and call create_session() again to create a session from scratch - up to two times, just to be sure.
• if either - login from scratch or fetching new token - succeeds, set up the session data and cache to file.

That's the idea. I haven't encountered a session issue since I added that (I suspend my Kodi after watching). I won't say that there might not be an error, but if there is one, I think we need to fix it here in these parts?

Maybe adding a check for 401 to this https://github.com/smirgol/plugin.video.crunchyroll/blob/841c51089c18b16c491bf2911c2afbc9db56c571/resources/lib/api.py#L131 as well might fix it?

[lumiru]

Hm I thought I had fixed that already. There is already some logic for that in api.py, starts here start():

* Check if we have cached session data.

* if we don't have any, go to `create_session(refresh=False)` and start a session from scratch

* if we have it, check the expiration date of the token.

* if it is expired, trigger `create_session(refresh=True)` and fetch a new token using the refresh_token. otherwise just return, because we still should have all we need for further requests.

This is a standard issue with token expiration, we cannot trust the host clock. The described logic is the best for token expiration, but is not enough for some cases.

Indeed, the main issue is the host clock could expose a misconfigured date at startup or after suspend, until it is synchronized with NTP server. If your motherboard battery is dead, or the clock is not so efficient when the host is suspended, or if you switch between multiple Operation System on the same device, the time will be wrong at restart and will be updated from NTP some seconds after startup or resume, which can create a large leap in the past or future.

[smirgol]

And that's why we can't have nice things sighs :)

Never thought about it that way, thanks for explaining. Actually a few years ago when I had issues with my host clock due to daylight saving change, I probably would have encountered that as well.

It's just I'm not super happy with splitting the requests method. While it might fix the underlying issue, it doesn't feel like a "nice" solution. I have yet to come up with a better solution, will have to think about it some more.

[smirgol]

How about this?

    def make_request(
            self,
            method: str,
            url: str,
            headers=None,
            params=None,
            data=None,
            json=None,
            is_retry=False
    ) -> Optional[Dict]:
        if params is None:
            params = dict()
        if headers is None:
            headers = dict()
        if self.account_data:
            if expiration := self.account_data.expires:
                current_time = utils.get_date()
                if current_time > utils.str_to_date(expiration):
                    self.create_session(refresh=True)
            params.update({
                "Policy": self.account_data.cms.policy,
                "Signature": self.account_data.cms.signature,
                "Key-Pair-Id": self.account_data.cms.key_pair_id
            })
        headers.update(self.api_headers)

        r = self.http.request(
            method,
            url,
            headers=headers,
            params=params,
            data=data,
            json=json
        )

        # something went wrong with authentication, possibly an expired token that wasn't caught above due to host
        # clock issues. set expiration date to 0 and re-call, triggering a full session refresh.
        if r.status_code == 401:
            if is_retry:
                raise LoginError('Request to API failed twice due to authentication issues.')

            utils.crunchy_log(self.args, "make_request: request failed due to auth error", xbmc.LOGERROR)
            self.account_data.expires = 0
            return self.make_request(method, url, headers, params, data, json, True)

        return utils.get_json_from_response(r)

Would require to change the None return to throwing an Exception (which it should do anyway at this point) here

[lumiru]

Oh yes, it would be better.