@Retry shouldn't trigger Uni publisher

[Dieken]

Describe the bug

import io.quarkus.test.junit.QuarkusTest;
import io.smallrye.mutiny.Uni;
import org.eclipse.microprofile.faulttolerance.Retry;
import org.junit.jupiter.api.Test;

@QuarkusTest
public class MyTest {

    @Test
    public void testIt() throws InterruptedException {
        Uni<Integer> u = retry();

        
        Thread.sleep(1000);
    }

    @Retry
    public Uni<Integer> retry() {
        return Uni.createFrom().item(1).invoke(() -> System.out.println("xxxxx"));
    }
}

The test prints "xxxxx", seems @Retry logic subscribes to the Uni publisher, this breaks time order:

Uni a = foo.doXxxWithRetry();
Uni b = bar.doYyy();

// expect a runs after b, but actually a may be triggered even before next line due to retry timeout.
return b.replaceWith(a);

Expected behavior

The retry uni doesn't take any effect, no output.

Actual behavior

The test outputs "xxxxx".

How to Reproduce?

Run the unit test.

Output of uname -a or ver

Darwin xxx.local 20.3.0 Darwin Kernel Version 20.3.0: Thu Jan 21 00:07:06 PST 2021; root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "11.0.11" 2021-04-20 OpenJDK Runtime Environment AdoptOpenJDK-11.0.11+9 (build 11.0.11+9) OpenJDK 64-Bit Server VM AdoptOpenJDK-11.0.11+9 (build 11.0.11+9, mixed mode)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

Client Version 2.2.1.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.1 (05c21c65bdfed0f71a2f2ada8b84da59348c4c5d)

Additional information

No response

[Dieken]

@Retry logic probably should use uni.onFailure().retry().

[quarkus-bot]

/cc @Ladicek

[Ladicek]

Fault Tolerance converts everything to CompletionStage internally, which is probably the reason. Fixing this properly will require quite some refactoring in Fault Tolerance.

[Ladicek]

Also CC @cescoffier @jponge in case they can advise how to use SmallRye Reactive Converters properly :-)

[cescoffier]

Where is the code handling this in FT?

[Ladicek]

Scattered over a few places, but the most interesting things are probably https://github.com/smallrye/smallrye-fault-tolerance/blob/main/implementation/fault-tolerance/src/main/java/io/smallrye/faulttolerance/AsyncTypes.java and https://github.com/smallrye/smallrye-fault-tolerance/blob/main/implementation/fault-tolerance/src/main/java/io/smallrye/faulttolerance/internal/AsyncTypesConversion.java

[Ladicek]

This is fixed in SmallRye Fault Tolerance 5.3.0, where reactive types support was refactored (dropped the dependency on SmallRye Reactive Converters, implemented our own conversion facility that supports lazy conversion).