Exclude codes.DeadlineExceeded from the list of retryable gRPC codes

[alexshtin]

What was changed

Exclude codes.DeadlineExceeded from the list of retryable gRPC codes.

Why?

gRPC library doesn't retry "context errors" (codes.DeadlineExceeded and codes.Canceled), so remove them from the list of retryable codes to eliminate the confusion.

isRetriable: https://github.com/grpc-ecosystem/go-grpc-middleware/blob/main/interceptors/retry/retry.go#L269

isContextError: https://github.com/grpc-ecosystem/go-grpc-middleware/blob/main/interceptors/retry/retry.go#L283

[Quinn-With-Two-Ns]

This list of errors is common across SDKs, do we need to update them as well?

[alexshtin]

For that you need to check gRPC library implementation in those langs. I hope it is consistent there.

[cretz]

gRPC library doesn't retry "context errors" (codes.DeadlineExceeded and codes.Canceled)

DeadlineExceeded can be a client context error or a server error. I am a bit worried to assume they are only "context errors".

[alexshtin]

I think this should be checked/fixed on server. Server should never explicitly return CDE if user context is not timed out yet. I checked persistence layer, and CDE is always replaced with internal error (persistence.TimeoutError) which then becomes codes.Unknown on gRPC layer. I am pretty much sure there are other places, where server uses another context with shorter timeout, gets CDE, and returns it all the way up. This places need to be fixed.

[cretz]

To confirm this PR, since the interceptor skips retrying this code no matter what, this is just to remove the unnecessary item from our list? There should be no runtime effect, correct?

[cretz]

LGTM, would like @Quinn-With-Two-Ns approval before merge.

[alexshtin]

Yes, it doesn't change behaviour. CDE was never retried (I've just discovered this today). So I decided to align our codes with gRPC lib implementation.