Fix waitForSidecar to respect timeout.

Signed-off-by: Artur Souza <[email protected]>

Author: artursouza

sdk-tests/src/test/java/io/dapr/it/resiliency/WaitForSidecarIT.java

@@ -0,0 +1,107 @@
+/*
+ * Copyright 2024 The Dapr Authors
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package io.dapr.it.resiliency;
+
+import io.dapr.client.DaprClient;
+import io.dapr.client.DaprClientImpl;
+import io.dapr.client.resiliency.ResiliencyOptions;
+import io.dapr.it.BaseIT;
+import io.dapr.it.DaprRun;
+import io.dapr.it.ToxiProxyRun;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.nio.charset.StandardCharsets;
+import java.time.Duration;
+import java.util.Base64;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Test SDK resiliency.
+ */
+public class WaitForSidecarIT extends BaseIT {
+
+  // Use a number large enough to make sure it will respect the entire timeout.
+  private static final Duration LATENCY = Duration.ofSeconds(5);
+
+  private static final Duration JITTER = Duration.ofSeconds(0);
+
+  private static DaprRun daprRun;
+
+  private static ToxiProxyRun toxiProxyRun;
+
+  private static DaprRun daprNotRunning;
+
+  @BeforeAll
+  public static void init() throws Exception {
+    daprRun = startDaprApp(WaitForSidecarIT.class.getSimpleName(), 5000);
+    daprNotRunning = startDaprApp(WaitForSidecarIT.class.getSimpleName()+"NotRunning", 5000);
+    daprNotRunning.stop();
+
+    toxiProxyRun = new ToxiProxyRun(daprRun, LATENCY, JITTER);
+    toxiProxyRun.start();
+  }
+
+  @Test
+  public void waitSucceeds() throws Exception {
+    try(var client = daprRun.newDaprClient()) {
+      client.waitForSidecar(5000).block();
+    }
+  }
+
+  @Test
+  public void waitTimeout() {
+    int timeoutInMillis = (int)LATENCY.minusMillis(100).toMillis();
+    long started = System.currentTimeMillis();
+    assertThrows(RuntimeException.class, () -> {
+      try(var client = toxiProxyRun.newDaprClientBuilder().build()) {
+        client.waitForSidecar(timeoutInMillis).block();
+      }
+    });
+    long duration = System.currentTimeMillis() - started;
+    assertTrue(duration >= timeoutInMillis);
+  }
+
+  @Test
+  public void waitSlow() throws Exception {
+    int timeoutInMillis = (int)LATENCY.plusMillis(100).toMillis();
+    long started = System.currentTimeMillis();
+    try(var client = toxiProxyRun.newDaprClientBuilder().build()) {
+        client.waitForSidecar(timeoutInMillis).block();
+    }
+    long duration = System.currentTimeMillis() - started;
+    assertTrue(duration >= LATENCY.toMillis());
+  }
+
+  @Test
+  public void waitNotRunningTimeout() {
+    // Does not make this number too smaller since bug does not repro when <= 2.5s.
+    // This has to do with a previous bug in the implementation.
+    int timeoutMilliseconds = 5000;
+    long started = System.currentTimeMillis();
+    assertThrows(RuntimeException.class, () -> {
+      try(var client = daprNotRunning.newDaprClientBuilder().build()) {
+        client.waitForSidecar(timeoutMilliseconds).block();
+      }
+    });
+    long duration = System.currentTimeMillis() - started;
+    assertTrue(duration >= timeoutMilliseconds);
+  }
+}

sdk/src/main/java/io/dapr/client/DaprClientImpl.java

@@ -273,53 +273,13 @@ public <T extends AbstractStub<T>> T newGrpcStub(String appId, Function<Channel,
   @Override
   public Mono<Void> waitForSidecar(int timeoutInMilliseconds) {
     String[] pathSegments = new String[] { DaprHttp.API_VERSION, "healthz", "outbound"};
-    int maxRetries = 5;
-
-    Retry retrySpec = Retry
-        .fixedDelay(maxRetries, Duration.ofMillis(500))
-        .doBeforeRetry(retrySignal -> {
-          System.out.println("Retrying component health check...");
-        });
-
-    /*
-    NOTE: (Cassie) Uncomment this once it actually gets implemented:
-    https://github.com/grpc/grpc-java/issues/4359
-
-    int maxChannelStateRetries = 5;
-
-    // Retry logic for checking the channel state
-    Retry channelStateRetrySpec = Retry
-            .fixedDelay(maxChannelStateRetries, Duration.ofMillis(500))
-            .doBeforeRetry(retrySignal -> {
-              System.out.println("Retrying channel state check...");
-            });
-    */
 
     // Do the Dapr Http endpoint check to have parity with Dotnet
     Mono<DaprHttp.Response> responseMono = this.httpClient.invokeApi(DaprHttp.HttpMethods.GET.name(), pathSegments,
         null, "", null, null);
 
     return responseMono
-        .retryWhen(retrySpec)
-        /*
-        NOTE: (Cassie) Uncomment this once it actually gets implemented:
-        https://github.com/grpc/grpc-java/issues/4359
-        .flatMap(response -> {
-          // Check the status code
-          int statusCode = response.getStatusCode();
-
-          // Check if the channel's state is READY
-          return Mono.defer(() -> {
-            if (this.channel.getState(true) == ConnectivityState.READY) {
-              // Return true if the status code is in the 2xx range
-              if (statusCode >= 200 && statusCode < 300) {
-                return Mono.empty(); // Continue with the flow
-              }
-            }
-            return Mono.error(new RuntimeException("Health check failed"));
-          }).retryWhen(channelStateRetrySpec);
-        })
-        */
+        .retryWhen(Retry.indefinitely())
         .timeout(Duration.ofMillis(timeoutInMilliseconds))
         .onErrorResume(DaprException.class, e ->
             Mono.error(new RuntimeException(e)))

sdk/src/test/java/io/dapr/client/DaprClientHttpTest.java

@@ -133,16 +133,11 @@ public void waitForSidecarBadHealthCheck() throws Exception {
               .times(6)
               .respond(404, ResponseBody.create("Not Found", MediaType.get("application/json")));
 
-    // retry the max allowed retries (5 times)
+    // it will timeout.
     StepVerifier.create(daprClientHttp.waitForSidecar(5000))
             .expectSubscription()
-            .expectErrorMatches(throwable -> {
-              if (throwable instanceof RuntimeException) {
-                return "Retries exhausted: 5/5".equals(throwable.getMessage());
-              }
-              return false;
-            })
-            .verify(Duration.ofSeconds(20));
+            .expectError()
+            .verify(Duration.ofMillis(6000));
   }
 
   @Test