waitForSidecar should respect timeoutInMilliseconds

[artursouza]

Expected Behavior

When calling waitForSidecar(timeoutInMilliseconds), the expected behavior is that it will wait for the sidecar up to timeoutInMilliseconds, it does not matter how many retries are done behind the scenes.

Actual Behavior

The current implementation uses timeoutInMilliseconds but it does not apply after 5 retries of 500ms interval. See

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

Lines 224 to 278 in b1d0be7

	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);
	})
	*/
	.timeout(Duration.ofMillis(timeoutInMilliseconds))
	.onErrorResume(DaprException.class, e ->
	Mono.error(new RuntimeException(e)))
	.switchIfEmpty(DaprException.wrapMono(new RuntimeException("Health check timed out")))
	.then();
	}

Steps to Reproduce the Problem

Use the method with a value larger than 2.5s (the larger the better to visually identify the problem) without having a sidecar to connect to, forcing the timeout.

Release Note

RELEASE NOTE: FIXED handling of timeout in waitForSidecar

[artursouza]

Ideally, the integration test should repro this bug, so we avoid future regressions.

[olitomlinson]

It would be great to release a patch version of the SDK for this as it does cause us daily issues in production.

[olitomlinson]

@artursouza are you still planning to tackle this by the end of the week? If so that would be great because we're starting code freeze in my org on Monday and it would be awesome if we could have this resolved. Much appreciated!

[olitomlinson]

@artursouza please can we get this released under a patch release 1.12.3?