Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature suggestion: retrying success flows #289

Open
JPPereira123 opened this issue Nov 13, 2024 · 1 comment
Open

Feature suggestion: retrying success flows #289

JPPereira123 opened this issue Nov 13, 2024 · 1 comment

Comments

@JPPereira123
Copy link

JPPereira123 commented Nov 13, 2024

Current Behavior:
Currently, the library only supports retry conditions for error flows (i.e., retrying when an error occurs). However, there are cases where we may want to retry even after a successful response, based on custom conditions.

Proposed Enhancement:
Allow retries based on success conditions, such as an API returning a 200 status code but a response body indicating the operation is still "in progress". This would enable retries based on specific business logic needs, beyond just error handling.

How this could work in practice?

  • axiosInstance.interceptors.request.use will invoke its onFulfilled hook so we can handle successful responses.
  • we keep a discrete count of successful retires vs rejected retries (could use some suggestions with naming conventions here - success / error || fulfilled / rejected || ...others ?)
  • max retries could still be a sum of success + failed retries (as defined by retries)
  • the success retry condition would look like this: retrySuccessCondition?: ((response: AxiosResponse) => boolean | Promise<boolean>) | null; - alternatively we could expand the existing retryCondition signature like this: retryCondition?: (error?: AxiosError, onFulfilledResponse?: AxiosResponse) => boolean | Promise<boolean>;
    `
  • we can then invoke an onRetrySuccess?: ( retryCount: number, response: AxiosResponse, requestConfig: AxiosRequestConfig ) => Promise<void> | void; like we're doing with onRetry.

What do you think?

Benefits:
Expands the library’s capabilities to handle business logic-specific retries that go beyond traditional error scenarios.
Makes the retry mechanism more flexible for real-world use cases where APIs may return intermediate statuses (e.g., polling for task completion).

Go/No-Go?

I'm happy to make a PR.

@JPPereira123
Copy link
Author

JPPereira123 commented Nov 13, 2024

I've just realised we can combine the existing validateResponse and retryCondition to achieve this... it requires us to treat it as an error (by rejecting the 200 response). I"m happy to close this if that's the intended approach, but splitting the concerns (polling vs retrying), while leveraging the axios onFulfilled hook, might be a better way forward?

ps: Many thanks to all the contributors for this very handy lib!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant