Occasional 401 Response when token expires

[mchieco]

I am currently running this on a long standing server that caches the client instance for the lifetime of the application.

Every once and a while, there is a flaky 401 that comes back from the client, subsequent runs work perfectly fine.

After reading through the client code, it looks like it may be a race condition with the expiry of the github token cache and then making the api call?

https://github.com/yanyongyu/githubkit/blob/master/githubkit/auth/app.py#L208 will check to see if the key is in the cache, so if its expired, it will fetch a new one perfectly fine. But what happens if the cache is there, but then the token expires within milliseconds of it attaching it to the api call. I believe that's what is leading to the 401 in these cases.

If you do believe that is the issue, I can certainly open up a PR to add some sort of buffer to the cache expiry to refresh the token x seconds (whatever we think is best) before it actually expires.

Happy to provide any other details/findings.

The exception is

RequestFailed
Response(401 Unauthorized, data_model=typing.Any)

[yanyongyu]

I forget to minus one minute to the expire time here

githubkit/githubkit/auth/app.py

Lines 215 to 217 in 5632918

	expire = datetime.strptime(
	response.parsed_data.expires_at, "%Y-%m-%dT%H:%M:%SZ"
	).replace(tzinfo=timezone.utc) - datetime.now(timezone.utc)

and here

githubkit/githubkit/auth/app.py

Lines 253 to 255 in 5632918

	expire = datetime.strptime(
	response.parsed_data.expires_at, "%Y-%m-%dT%H:%M:%SZ"
	).replace(tzinfo=timezone.utc) - datetime.now(timezone.utc)

The jwt token for github app has already done this here. Thanks for catching this bug.

[mchieco]

No problem, would you like me to contribute? Or will you open up a change for this?

Want to make sure im helping you out if you don't have time to get to it soon.

[yanyongyu]

pr welcome！

[mchieco]

Just opened the PR, didn't see any unit tests associated to this (nor would it be valueable due to this edgecase) so it should be ready to go!