picker_wrapper: simplify picker error when timing out waiting for con… #8035
Conversation
| @@ -123,7 +123,7 @@ func (pw *pickerWrapper) pick(ctx context.Context, failfast bool, info balancer. | |||
| if lastPickErr != nil { | |||
| errStr = "latest balancer error: " + lastPickErr.Error() | |||
There was a problem hiding this comment.
@dfawley should we change this as well? or is it fine to mention balancer here?
There was a problem hiding this comment.
I think this is probably fine. The problem with below is not that it's mentioning LB policies, but that it's unclear to most users what might trigger an LB policy update to occur.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8035 +/- ##
==========================================
+ Coverage 82.17% 82.29% +0.11%
==========================================
Files 383 383
Lines 38776 38776
==========================================
+ Hits 31864 31909 +45
+ Misses 5585 5548 -37
+ Partials 1327 1319 -8
|
| @@ -123,7 +123,7 @@ func (pw *pickerWrapper) pick(ctx context.Context, failfast bool, info balancer. | |||
| if lastPickErr != nil { | |||
| errStr = "latest balancer error: " + lastPickErr.Error() | |||
There was a problem hiding this comment.
I think this is probably fine. The problem with below is not that it's mentioning LB policies, but that it's unclear to most users what might trigger an LB policy update to occur.
picker_wrapper.go
Outdated
| @@ -123,7 +123,7 @@ func (pw *pickerWrapper) pick(ctx context.Context, failfast bool, info balancer. | |||
| if lastPickErr != nil { | |||
| errStr = "latest balancer error: " + lastPickErr.Error() | |||
| } else { | |||
| errStr = fmt.Sprintf("received context error while waiting for new LB policy update: %s", ctx.Err().Error()) | |||
| errStr = fmt.Sprintf("received context error due to timing out waiting for connection: %s", ctx.Err().Error()) | |||
There was a problem hiding this comment.
How about:
fmt.Sprinft("%v while waiting for connections to become ready", ctx.Err())
This would result in either:
rpc error: code = DeadlineExceeded desc = context deadline exceeded while waiting for connections to become ready
OR
rpc error: code = Canceled desc = context canceled while waiting for connections to become ready
For reference we used to use ctx.Err().Error() directly, which resulted in, e.g.:
rpc error: code = DeadlineExceeded desc = context deadline exceeded
Fixes: #7983
RELEASE NOTES: None