Skip to content

Commit 4528f68

Browse files
committed
Always try attempt on first try in order to get relevant context cancellation error. Cleanup err handling in balanced selector. Fallback to response status code if cgr error isn't defined.
1 parent dc1bf3b commit 4528f68

File tree

4 files changed

+26
-12
lines changed

4 files changed

+26
-12
lines changed

conjure-go-client/httpclient/client.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,10 @@ func (c *clientImpl) Do(ctx context.Context, params ...RequestParam) (*http.Resp
107107
}
108108
svc1log.FromContext(ctx).Debug("Retrying request", svc1log.Stacktrace(err))
109109
}
110-
return resp, err
110+
if err != nil {
111+
return nil, err
112+
}
113+
return resp, nil
111114
}
112115

113116
func (c *clientImpl) doOnce(

conjure-go-client/httpclient/internal/balanced_selector.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,15 @@ func (s *balancedSelector) RoundTrip(req *http.Request, next http.RoundTripper)
103103
defer s.updateInflight(baseURI, -1)
104104

105105
resp, err := next.RoundTrip(req)
106-
if resp == nil || err != nil {
107-
s.updateRecentFailures(baseURI, failureWeight)
108-
return nil, err
106+
errCode, ok := StatusCodeFromError(err)
107+
// fall back to the status code from the response
108+
if !ok && resp != nil {
109+
errCode = resp.StatusCode
109110
}
110-
statusCode := resp.StatusCode
111-
if isGlobalQosStatus(statusCode) || isServerErrorRange(statusCode) {
111+
112+
if isGlobalQosStatus(errCode) || isServerErrorRange(errCode) {
112113
s.updateRecentFailures(baseURI, failureWeight)
113-
} else if isClientError(statusCode) {
114+
} else if isClientError(errCode) {
114115
s.updateRecentFailures(baseURI, failureWeight/100)
115116
}
116117
return resp, err

conjure-go-client/httpclient/internal/request_retrier.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,14 @@ func (r *RequestRetrier) Next(resp *http.Response, err error) (bool, *url.URL) {
8484
return false, nil
8585
}
8686

87+
// should always try first request
88+
if r.attemptCount == 0 {
89+
// Trigger the first retry so later calls have backoff but ignore the returned value to ensure that the
90+
// client can instrument the request even if the context is done.
91+
r.retrier.Next()
92+
return true, nil
93+
}
94+
8795
// retry with backoff
8896
return r.retrier.Next(), nil
8997
}

conjure-go-client/httpclient/internal/stateful_uri_pool.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,18 @@ func (s *statefulURIPool) URIs() []string {
7878
// RoundTrip implements URIPool
7979
func (s *statefulURIPool) RoundTrip(req *http.Request, next http.RoundTripper) (*http.Response, error) {
8080
resp, err := next.RoundTrip(req)
81-
errCode, _ := StatusCodeFromError(err)
81+
errCode, ok := StatusCodeFromError(err)
82+
// fall back to the status code from the response
83+
if !ok && resp != nil {
84+
errCode = resp.StatusCode
85+
}
8286

8387
if isThrottle, ressurectAfter := isThrottleResponse(resp, errCode); isThrottle {
8488
s.markBackoffURI(req, ressurectAfter)
85-
}
86-
if isUnavailableResponse(resp, errCode) {
89+
} else if isUnavailableResponse(resp, errCode) {
8790
// 503: go to next node
8891
s.markBackoffURI(req, defaultResurrectDuration)
89-
}
90-
if resp == nil {
92+
} else if resp == nil {
9193
// if we get a nil response, we can assume there is a problem with host and can move on to the next.
9294
s.markBackoffURI(req, defaultResurrectDuration)
9395
}

0 commit comments

Comments
 (0)