smartcar / retryify

Wrap a function to retry on specific errors.
MIT License
11 stars 1 forks source link

adding two new params to Options. retryOnSuccess, retryOnError #57

Closed rsimari closed 4 years ago

rsimari commented 4 years ago

RFC -

I added two params for retryify. Each is a function that control the retry logic. One function {retryOnSuccess} will run if the wrapped function runs without exception and given the response, if the function returns true, then we will retry, else return the success response. The {retryOnError} works similarly except it runs when the wrapped function throws an exception. {retryOnError} is similar to the current retryify where you specify specific error types to retry on. This function provides more flexibility without breaking the existing functionality. First we check the error type of match with the list of errors (if it matches we retry, nothing to see here). If there is no match then we run the {retryOnError} function where if it evaluates to true, we retry, else we throw the original error that was thrown by the wrapped function.

Considerations:

Note: With whatever we decide, I will make any necessary changes to the README.md

codecov[bot] commented 4 years ago

Codecov Report

Merging #57 into master will not change coverage by %. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #57   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           48        54    +6     
=========================================
+ Hits            48        54    +6     
Impacted Files Coverage Δ
index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8417ab1...1418449. Read the comment docs.

gurpreetatwal commented 4 years ago

Somewhat accomplished by #60 (basically adding the retryOnError function and dropping support for the errors param)

We decided at the time that it's best not to support the retryOnSuccess functionality in this library as that's more of a concern for polling / the function being wrapped (the function could throw any "successes" as errors). Limiting this library to only retry on errors keeps the logic and abstractions involved simple.