Closed mm-matthias closed 3 years ago
Hello, indeed the same issue was reported months ago on https://github.com/Colin-b/requests_auth/issues/60 as well.
Even with a slow network, 5s should be way enough (unless your system clock is not properly synchronized but that's something else). So I will put a default to 5s, and give the ability to change it to a higher (or lower) value.
Would that be ok with you ?
The retry on 401 is something else really as I am not sure we can get the actual response of the server for the end user query. But in any case I will also create a separate issue to check this feature
In other parts of our system we have implemented the 401 retry solution which works really great and is very robust.
I would agree that 5s should be sufficient for many cases, but it is certainly not always sufficient. Your request body can be many megabytes in size (e.g. when uploading images) and even with a 10 MBit/s connection this can take many seconds to send. Many servers process the payload and authentication only after its been fully received. In addition to this, the server might itself contact the oauth server to make sure the token is valid which can also add to the overhead. If you add different networks, proxies, loads and mobile devices to the mix it's not impossible to exceed 30 seconds in worst cases.
So I will put a default to 5s, and give the ability to change it to a higher (or lower) value. Would that be ok with you ?
It's your project, I am ok with anything :) If I had time to actually make a PR, I'd go for the 401 route as it will always work instead of the 90% solution with somewhat arbitrary timeouts.
I started working on it, I will finish the feature/bugfix tomorrow, so you can expect a release for monday at the latest. The default value will be set to 30s as I assume tokens have usually an expiry of more than a few minutes.
As for the retry feature, it will not be part of this release yet as it is less critical (once you will have a way to fine tune token expiry)
Release 0.8.0 is now providing early_expiry parameter. You do not have to use a custom cache anymore. The default value is 30s so it should suits your needs as well.
Thanks for reporting the issue. I have a lot on my plate so I don't know when I will be able to look at #24 but feel free to submit a PR (and it is in the open issues so definitively not skipped 👍 ).
Best Regards
Hey @Colin-b,
thank you very much for adding this feature and cutting a new release! We are on 0.8.0 now and it works great :)
The current code seems to use tokens exactly up to their expiry date (see https://github.com/Colin-b/httpx_auth/blob/99ba755d8728329b6d4a9045685ed41e3f51bdb5/httpx_auth/oauth2_tokens.py#L26 ). There can be two problems:
There could be two possible solutions:
expiry
is reached, but expire it a little earlier. E.g. If the token says to expire in 60 seconds, let it expire at 40 seconds and get a new one already. This leaves 20 seconds of leeway. The downside with this approach is that 20 seconds is just an arbitrary value and could still not be sufficient to prevent the problem. It will also not solve issue 2).Whenever a 401 is returned, acquire a new token and retry the request. This is also hinted at in the official httpx docs (c.f. https://www.python-httpx.org/advanced/#customizing-authentication ):
Of course there should be some kind of limit to the number of retries, so you don't get stuck indefinitely.
Unfortunately I don't have the time to prepare a proper PR, so I am using this as a workaround now (implementation of solution 1):