americanexpress / fetchye

✨ If you know how to use Fetch, you know how to use Fetchye [fetch-yae]. Simple React Hooks, Centralized Cache, Infinitely Extensible.
Apache License 2.0
42 stars 21 forks source link

WIP/Draft: Support Retry Error Callback + Retry Count #44

Closed TatisLois closed 8 months ago

TatisLois commented 3 years ago

Expose a way for the useFetchye hook to specify a max retry count and callback to be invoked on retries.

Description

I wanted to open a DRAFT/WIP with a proof of concept to start a conversation on the best way to and if useFetchye should support on error scenarios. The current implementation does not support this functionality. If a user wants to support retries they either have to create a custom fetch wrapper or leverage the returned run function.

I am proposing that taking an action based on an error is a common enough scenario that we should support functionality to trigger an action or reaction when it occurs.

The approach in this proposal is to add a property to the options object named errors that itself has support for two properties called maxOnErrorRetry (number) and onError (method). If a request succeeds it prevents further retries to take place. Using the run function bails out of the on error functionality.

implementation from the perspective of a consumer of useFetchye

implementation

outstanding questions

example object a consumer would define carbon (36)

Motivation and Context

While refactoring to use fetchye I encountered the scenario of wanting to retry the request after an initial error. In my specific use case we wanted a workflow like:

How Has This Been Tested?

Types of Changes

Checklist:

What is the Impact to Developers Using Fetchye?

Developers using Fetchye would have greater control over what to do in the event of a failed request without having to bail completely out of the regular fetchye control flow.

CLAassistant commented 3 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

:white_check_mark: nellyk
:white_check_mark: 10xLaCroixDrinker
:x: TatisLois
You have signed the CLA already but the status is still pending? Let us recheck it.

codesandbox-ci[bot] commented 3 years ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5cb7c7dc8f51ea1fce9287a5830a4d8e005c7735:

Sandbox Source
quick-install Configuration
fetchye-provider-install Configuration
fetchye-redux-provider-install Configuration
nextjs-fetchye-ssr Configuration
oneamexbot commented 3 years ago

This pull request is stale because it has been open 30 days with no activity.

code-forger commented 3 years ago

Hi @TatisLois, Thanks for your suggestion. I wonder if this would not be better as an entirly new hook useRetry:

const { isLoading, data, errors, run } = useFetchye(...);
const { isSettled } = useRetry({isLoading, isError: !!errors, retry: run, maxRetries: 3});

if (isLoading) return <p>Loading...</p>;
if (!isSettled) return <p>Retrying...</p>;
if (errors) return <p>Error...</p>;

return <p>Success!</p>;

This approach has two benefits:

Finally, the code would remain mostly the same. Just wrapped up in its own library instead of being embedded in fetchye.

Let me know your thoughts, and we can work with you to bring this idea to life.

TatisLois commented 3 years ago

At first the idea of decoupling the retry logic feature from the fetchye felt like it could become a point of confusion but after some thought I think this is a solid idea and a good direction to go with. The benefit of being able to use the hook outside of fetchye for any async operation that follows the retry hooks signature is dope!

Do you think it should live within this code base but as a separate importable hook, I get the impression it should be it's own utility library that can be installed and used alongside fetchye or any other lib. I can whip up a proof of concept on the hook.

What do you think the next steps should be?

code-forger commented 8 months ago

Closed due to inactivity