fix(rt): only retry client errors if metadata allows #560
Conversation
| @Test | ||
| fun testClientException() { | ||
| fun testNonRetryableClientException() { | ||
| val result = test(ClientException()) | ||
| assertEquals(RetryDirective.RetryError(RetryErrorType.ClientSide), result) | ||
| assertEquals(RetryDirective.TerminateAndFail, result) | ||
| } |
There was a problem hiding this comment.
Nit: Missing a new unit test for ClientException that is retryable.
There was a problem hiding this comment.
Yes I see that now, I thought thats what the other function you commented on was doing but I see now it's not.
| fun testClientSideException() { | ||
| fun testRetryableClientException() { |
There was a problem hiding this comment.
Question: Why did this name change? It's not technically testing for a ClientException where isRetryable is true...it's still testing a ServiceException with errorType == ErrorType.Client.
There was a problem hiding this comment.
Oh you're right I made the update here quickly thinking htis was testing a retryable client exception and missed the fact that this is a service side exception with error type of client (confused yet?). I'll update the name to something like testClientErrorType or something.
|
Kudos, SonarCloud Quality Gate passed!
|
Issue #
fixes #555
Description of changes
Do not retry
ClientExceptionunless metadata explicitly allows it. In general client exceptions are a class of exception that aren't retryable (serialization/deserialization errors, state errors, configuration errors, etc)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.