RetryPolicy controller blocked issue for 3.x

[KamaniAman]

I can see there is a fix #590 merged. Which is released under 4.x.x version of feign-reactive.

But I am using spring boot 2.7.10. Is feign-reactive 4.x.x supported with spring boot 2.7.10?

If yes, then while implementing dependency in build.gradle file. Do we need to exclude or overwrite any other dependencies?

@kptfh

[ChiragMoradiya]

@kptfh We area also experiencing #590, and are on Spring boot 2.7.x.

Is this possible to port the fix under version 3.x.x?

[ChiragMoradiya]

This is an important fix for us, without this we are not able to upgrade Spring Boot & Spring Cloud Versions. Shall we attempt to submit a PR for this fix, OR anyone is already working on this?

[KamaniAman]

@ChiragMoradiya, do you have any PR or solution? If so, can you please provide it?

[ChiragMoradiya]

No PR is ready yet. But, the fix applied for version 4.0 can be ported to 3.x

…

On Tue, 19 Dec 2023 at 4:13 PM, KamaniAman ***@***.***> wrote: @ChiragMoradiya <https://github.com/ChiragMoradiya>, do you have any PR or solution? If so, can you please provide it? — Reply to this email directly, view it on GitHub <#650 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASJ3BPKG64FUH3BN7OQ27LYKFVV3AVCNFSM6AAAAABAD6IEQCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRSGUZDIOJUGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[KamaniAman]

@ChiragMoradiya

I have created & tested custom ReactiveRetryPolicy (Combining Basic & Filtered) which solves the issue without any version change.

/**
 * @see <a href="https://github.com/PlaytikaOSS/feign-reactive/issues/590">ISSUE#590</a>
 * @see <a href="https://github.com/PlaytikaOSS/feign-reactive/issues/650">ISSUE#650</a>
 * @see <a href="https://github.com/PlaytikaOSS/feign-reactive/pull/607">Solution Referred</a>
 *
 * This class is combination of {@link BasicReactiveRetryPolicy} & {@link reactivefeign.retry.FilteredReactiveRetryPolicy}
 * {@link BasicReactiveRetryPolicy}: Supports to retry only 503 failures
 * {@link reactivefeign.retry.FilteredReactiveRetryPolicy}: Supports to retry custom failures/other than 503 statuses
 *
 * Unlike {@link reactivefeign.retry.FilteredReactiveRetryPolicy}, {@link BasicReactiveRetryPolicy} supports to UNWRAP the {@link reactivefeign.publisher.retry.OutOfRetriesException} Exception
 * So combining both the RetryPolicies
 */
public class ReactiveFeignExtendedRetryPolicy extends SimpleReactiveRetryPolicy {

    private final int maxRetries;
    private final long backOffInMillis;
    private final Scheduler scheduler;
    private final Clock clock = Clock.systemUTC();
    private final Predicate<Throwable> toRetryOn;

    public ReactiveFeignExtendedRetryPolicy(int maxRetries, long backOffInMillis, Scheduler scheduler, ExceptionPropagationPolicy exceptionPropagationPolicy, Predicate<Throwable> toRetryOn) {
        super(scheduler, exceptionPropagationPolicy);
        this.maxRetries = maxRetries;
        this.backOffInMillis = backOffInMillis;
        this.scheduler = scheduler;
        this.toRetryOn = toRetryOn;
    }

    @SafeVarargs
    @SuppressWarnings("varargs")
    public ReactiveFeignExtendedRetryPolicy(int maxRetries, long backOffInMillis, Scheduler scheduler, ExceptionPropagationPolicy exceptionPropagationPolicy, Class<? extends Throwable>... retryableExceptions) {
        this(maxRetries, backOffInMillis, scheduler, exceptionPropagationPolicy, throwable -> Stream.of(retryableExceptions).noneMatch(errorClass -> errorClass.isAssignableFrom(throwable.getClass())));
    }


    /**
     * Taken from BasicReactiveRetryPolicy {@link BasicReactiveRetryPolicy#retryDelay}
     * @param error
     * @param attemptNo
     * @return
     */
    @Override
    public long retryDelay(Throwable error, int attemptNo) {
        if (attemptNo <= maxRetries) {
            if(backOffInMillis > 0) {
                long delay;
                Date retryAfter;
                // "Retry-After" header set
                if (error instanceof RetryableException
                        && (retryAfter = ((RetryableException) error)
                        .retryAfter()) != null) {
                    delay = retryAfter.getTime() - clock.millis();
                    delay = Math.min(delay, backOffInMillis);
                    delay = Math.max(delay, 0);
                } else {
                    delay = backOffInMillis;
                }
                return delay;
            } else {
                return 0;
            }
        } else {
            return -1;
        }
    }


    /**
     * Fixing with replacing Integer.MAX_VALUE to (maxRetries + 1)
     * And combined retry from
     * - {@link SimpleReactiveRetryPolicy#retry()} &
     * - {@link FilteredReactiveRetryPolicy#retry()}
     *
     * @return Retry
     */
    @Override
    public Retry retry() {

        Retry retry =  Retry.from(errors -> errors
                // (maxRetries + 1) is the fix
                .zipWith(Flux.range(1, maxRetries + 1), (signal, index) -> {
                    long delay = retryDelay(signal.failure(), index);
                    if (delay >= 0) {
                        return Tuples.of(delay, signal);
                    } else {
                        throw Exceptions.propagate(signal.failure());
                    }
                }).concatMap(
                        tuple2 -> tuple2.getT1() > 0
                                ? Mono.delay(Duration.ofMillis(tuple2.getT1()), scheduler)
                                .map(time -> tuple2.getT2().failure())
                                : Mono.just(tuple2.getT2().failure())));


        // support for filtered exception retry only
        return new Retry() {
            @Override
            public Publisher<?> generateCompanion(Flux<RetrySignal> retrySignals) {
                return retry.generateCompanion(retrySignals.map(retrySignal -> {
                    if (toRetryOn.test(retrySignal.failure())) {
                        return retrySignal;
                    } else {
                        throw Exceptions.propagate(retrySignal.failure());
                    }
                }));
            }
        };
    }
}

[ChiragMoradiya]

Thanks @KamaniAman for the sharing.

We will give it a try.

[KamaniAman]

@kptfh any update on this?