search

softonic / axios-retry

Axios plugin that intercepts failed requests and retries them whenever possible
Other
1.9k stars 167 forks source link

feat: abortable retry delay #268

Closed xpirt closed 5 months ago

xpirt commented 7 months ago

Context

If a web application implements axios-retry and also a network change detection system, he may want to re-call the API requests even if in the middle of the retries.

Problem

Currently, there is no way to clear the setTimeout and reject the Promise that handles the retries' attempts.

Solution

Implement the possibility to pass an AbortController's signal, so that a developer is able to stop pending retries.

mindhells commented 7 months ago

Hi @xpirt sorry for the delay. I have some questions/concerns about this PR:

xpirt commented 7 months ago

Hi @xpirt sorry for the delay. I have some questions/concerns about this PR:

  • changing the prettier rules should be part of a different PR, where you could explain the motivations for the change
  • it is important to cover review the tests, and add new ones if needed. Specially when adding new functionality
  • would't the axios instance cancellation mechanism work?
  1. I see, will revert that. Edit: reverted.
  2. Will try to come up with some tests
  3. Nope. The axios-retry retry mechanism uses a setTimeout which, despite the request being aborted, it is not cleared properly. This results into the timeout to expire and execute the request afterwards. Let me know if that's clear.
mindhells commented 7 months ago

Hi @xpirt sorry for the delay. I have some questions/concerns about this PR:

  • changing the prettier rules should be part of a different PR, where you could explain the motivations for the change
  • it is important to cover review the tests, and add new ones if needed. Specially when adding new functionality
  • would't the axios instance cancellation mechanism work?
  1. I see, will revert that. Edit: reverted.
  2. Will try to come up with some tests
  3. Nope. The axios-retry retry mechanism uses a setTimeout which, despite the request being aborted, it is not cleared properly. This results into the timeout to expire and execute the request afterwards. Let me know if that's clear.

Is there a way we can honor the original abort signal (if any) from the axios instance instead extending this interceptor interface?

michal-billtech commented 5 months ago

@mindhells @xpirt please take a look at https://github.com/softonic/axios-retry/pull/273. Is this what you had in mind?

mindhells commented 5 months ago

closing this one in favor of #273