RxSwiftCommunity / RxSwiftExt

A collection of Rx operators & tools not found in the core RxSwift distribution
MIT License
1.32k stars 213 forks source link

Retry with behavior produce one less retry than specified in the maxCount #260

Closed anton-plebanovich closed 3 years ago

anton-plebanovich commented 3 years ago

Name and description

Retry with behavior produce one less retry than specified in the maxCount parameter. I tried to retry sequence once using maxCount: 1 but it wasn't retried at all. The issue is that currentAttempt: 1 by default and there is a check also guard currentAttempt > 0 so to retry once I had to specify maxCount: 2 which is counterintuitive especially given behavior description Will be immediatelly repeated specified number of times.

I can fix that but it'll affect existing usages for all users so it probably should go through a deprecation of some sort.

Example:

        _ = Observable<Int>.error(NSError(domain: "", code: 0))
            .debug()
            .retry(.immediate(maxCount: 1))
            .catchErrorJustComplete()
            .subscribe()

Example output:

2021-01-28 05:54:37.520: HomeVC.swift:25 (viewDidLoad()) -> subscribed
2021-01-28 05:54:37.521: HomeVC.swift:25 (viewDidLoad()) -> Event error(Error Domain= Code=0 "(null)")
2021-01-28 05:54:37.521: HomeVC.swift:25 (viewDidLoad()) -> isDisposed

Valid output:

2021-01-28 05:55:13.624: HomeVC.swift:25 (viewDidLoad()) -> subscribed
2021-01-28 05:55:13.625: HomeVC.swift:25 (viewDidLoad()) -> Event error(Error Domain= Code=0 "(null)")
2021-01-28 05:55:13.625: HomeVC.swift:25 (viewDidLoad()) -> subscribed
2021-01-28 05:55:13.625: HomeVC.swift:25 (viewDidLoad()) -> Event error(Error Domain= Code=0 "(null)")
2021-01-28 05:55:13.625: HomeVC.swift:25 (viewDidLoad()) -> isDisposed
2021-01-28 05:55:13.625: HomeVC.swift:25 (viewDidLoad()) -> isDisposed
freak4pc commented 3 years ago

Hey Anton, i believe this follows the behavior of the original retry where retry(3) would perform the initial attempt + 2 tries.

Do you mind confirming that?

anton-plebanovich commented 3 years ago

@freak4pc you are right about that one! Should I create a pull request with documentation update then to be in accordance with the original retry?

- parameter maxAttemptCount: Maximum number of times to repeat the sequence.

The same question about the parameter name it's called maxAttemptCount for the original retry instead of maxCount.

freak4pc commented 3 years ago

Changing the parameter would be breaking so no, but happy to accept a doc fix :) Thanks!

anton-plebanovich commented 3 years ago

@freak4pc https://github.com/RxSwiftCommunity/RxSwiftExt/pull/262 I am not a native speaker though so corrections are welcome