temporalio / sdk-java

Temporal Java SDK
https://temporal.io
Apache License 2.0
211 stars 142 forks source link

Fix exception in GrpcRetryer. #2021

Closed chronos-tachyon closed 5 months ago

chronos-tachyon commented 6 months ago

What was changed

Why?

In the course of writing new gRPC retry tests for the features repo, it was discovered that any gRPC failures returned by the server in response to WorkflowService.getSystemInfo will cause an uncaught io.grpc.StatusRuntimeException that happens before the actual gRPC retry code gets the chance to run.

Checklist

  1. Closes internal JIRA issue SDK-1888. (Sorry, I should have opened it as a GitHub issue and let it sync to JIRA instead, I didn't learn this until after the fact.)

  2. How was this tested: using the new features repo tests in https://github.com/temporalio/features/pull/435 .

Quinn-With-Two-Ns commented 5 months ago

I don't think the CI failure is a result of any of your changes, I opened a separate issue to address this flake https://github.com/temporalio/sdk-java/issues/2038

Quinn-With-Two-Ns commented 5 months ago

As part of the PR can we please also add some unit tests to verify the changes address the issue? I understand the integration tests will live, so maybe just a targeted test of getServerCapabilitiesWithRetryOrThrow since that takes a channel and you can create a in processes server like this:

https://github.com/grpc/grpc-java/blob/master/examples/src/test/java/io/grpc/examples/helloworld/HelloWorldClientTest.java