chore: refactor cache client auth flow #589
Conversation
There was a problem hiding this comment.
Seems like a reasonable start 👍
One question I have is whether or not it is important to preserve the order of the execution of the failed requests?
Previously, we were doing this.failedRequests = this.failedRequests.filter((callback) => callback());. I think this likely would have run the requests in the order of the failedRequests array. In the new promiseRetry approach, we have several requests awaiting the same promise, but it's not clear to me which will be selected when the promise resolved. So if there were multiple database updates, they could be out of order, for instance.
Does this analysis seem correct?
|
It does not matter in my opinion. The execution of the failed request is manged by the code itself. We are operating in the same context of the promise. So if we have something like this and update 1 will fail, it will try to run it 3 times and eventually it will fail, but update 2 will not execute, bc first update throw an error. So from the code perspective its the same promise that needs to be fulfilled. |
Harasz
force-pushed
the
task/ICL-299-auth-promise-retry
branch
from
4572194 to
6b92779
Compare
|
|
||
| private async makeRetryRequest<T>(request: () => Promise<T>): Promise<T> { | ||
| return promiseRetry( | ||
| async (retry) => { |
There was a problem hiding this comment.
I don't think so, thanks for catch it
|
@JGiter hymm, it would be great, but i dont think it's possible :/ |
|
Have you considered using Axios interceptor to implement retrying failed requests? |
|
@artursudnik request interceptor is only accepting axios config, so i can't override promise there. I can use response interceptor and catch errors, but it's not an ideal approach in my opinion. |
Harasz
force-pushed
the
task/ICL-299-auth-promise-retry
branch
from
6b92779 to
4e44445
Compare
* chore(cache-client): refactor request error handler * test(cache-client): add authorization tests * chore: use correct logger methods
|
🎉 This PR is included in version 6.0.0-alpha.29 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 6.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Contributor checklist