Revert unnecessary nil request body checks (#49591)

This commit is contained in:
Petri-Johan Last 2023-03-17 16:32:04 +02:00 committed by GitHub
parent 8fd096e336
commit c8bc9341f6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 22 additions and 31 deletions

View File

@ -108,6 +108,7 @@ func (c *client) do(ctx context.Context, req *http.Request, urlOverride string,
queryParams.Set("api-version", apiVersion)
req.URL.RawQuery = queryParams.Encode()
req.URL = u.ResolveReference(req.URL)
var reqBody []byte
if req.Body != nil {
req.Header.Set("Content-Type", "application/json")
@ -115,8 +116,8 @@ func (c *client) do(ctx context.Context, req *http.Request, urlOverride string,
if err != nil {
return "", err
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
// Add authentication headers for authenticated requests.
if err := c.auth.Authenticate(req); err != nil {
@ -143,9 +144,7 @@ func (c *client) do(ctx context.Context, req *http.Request, urlOverride string,
for c.waitForRateLimit && resp.StatusCode == http.StatusTooManyRequests &&
numRetries < c.maxRateLimitRetries {
if c.externalRateLimiter.WaitForRateLimit(ctx, 1) {
if req.Body != nil {
req.Body = io.NopCloser(bytes.NewReader(reqBody))
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
resp, err = oauthutil.DoRequest(ctx, logger, c.httpClient, req, c.auth)
numRetries++
} else {

View File

@ -225,8 +225,8 @@ func (c *client) do(ctx context.Context, req *http.Request, result any) error {
if err != nil {
return err
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
req, ht := nethttp.TraceRequest(ot.GetTracer(ctx), //nolint:staticcheck // Drop once we get rid of OpenTracing
req.WithContext(ctx),
@ -234,7 +234,7 @@ func (c *client) do(ctx context.Context, req *http.Request, result any) error {
nethttp.ClientTrace(false))
defer ht.Finish()
if err := c.rateLimit.Wait(ctx); err != nil {
if err = c.rateLimit.Wait(ctx); err != nil {
return err
}
@ -258,9 +258,7 @@ func (c *client) do(ctx context.Context, req *http.Request, result any) error {
if sleepTime.Seconds() > 160 {
break
}
if req.Body != nil {
req.Body = io.NopCloser(bytes.NewReader(reqBody))
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
}
defer resp.Body.Close()

View File

@ -228,8 +228,8 @@ func (c *V3Client) request(ctx context.Context, req *http.Request, result any) (
if err != nil {
return nil, err
}
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
}
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
var resp *httpResponseState
resp, err = doRequest(ctx, c.log, c.apiURL, c.auth, c.externalRateLimiter, c.httpClient, req, result)
@ -243,9 +243,7 @@ func (c *V3Client) request(ctx context.Context, req *http.Request, result any) (
errors.As(err, &apiError) && apiError.Code == http.StatusForbidden {
// If we end up waiting because of an external rate limit, we need to retry the request.
if c.externalRateLimiter.WaitForRateLimit(ctx, 1) {
if req.Body != nil {
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
}
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
resp, err = doRequest(ctx, c.log, c.apiURL, c.auth, c.externalRateLimiter, c.httpClient, req, result)
numRetries++
} else {

View File

@ -251,8 +251,8 @@ func (c *Client) do(ctx context.Context, req *http.Request, result any) (respons
if err != nil {
return nil, 0, err
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
req.URL = c.baseURL.ResolveReference(req.URL)
respHeader, respCode, err := c.doWithBaseURL(ctx, req, result)
@ -260,9 +260,7 @@ func (c *Client) do(ctx context.Context, req *http.Request, result any) (respons
numRetries := 0
for c.waitForRateLimit && numRetries < c.maxRateLimitRetries && respCode == http.StatusTooManyRequests {
if c.externalRateLimiter.WaitForRateLimit(ctx, 1) {
if req.Body != nil {
req.Body = io.NopCloser(bytes.NewReader(reqBody))
}
req.Body = io.NopCloser(bytes.NewReader(reqBody))
respHeader, respCode, err = c.doWithBaseURL(ctx, req, result)
numRetries += 1
} else {

View File

@ -43,16 +43,6 @@ type TokenRefresher func(ctx context.Context, doer httpcli.Doer, oauthCtx OAuthC
// it will also attempt to refresh the token in case of a 401 response.
// If the token is updated successfully, the same request will be retried exactly once.
func DoRequest(ctx context.Context, logger log.Logger, doer httpcli.Doer, req *http.Request, auther auth.Authenticator) (*http.Response, error) {
// Store the body first in case we need to retry the request
var err error
var reqBody []byte
if req.Body != nil {
reqBody, err = io.ReadAll(req.Body)
if err != nil {
return nil, err
}
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
}
if auther == nil {
return doer.Do(req.WithContext(ctx))
}
@ -65,10 +55,20 @@ func DoRequest(ctx context.Context, logger log.Logger, doer httpcli.Doer, req *h
}
}
if err := auther.Authenticate(req); err != nil {
var err error
if err = auther.Authenticate(req); err != nil {
return nil, errors.Wrap(err, "authenticating request")
}
// Store the body first in case we need to retry the request
var reqBody []byte
if req.Body != nil {
reqBody, err = io.ReadAll(req.Body)
if err != nil {
return nil, err
}
}
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
// Do first request
resp, err := doer.Do(req.WithContext(ctx))
if err != nil {
@ -86,9 +86,7 @@ func DoRequest(ctx context.Context, logger log.Logger, doer httpcli.Doer, req *h
return nil, errors.Wrap(err, "authenticating request after token refresh")
}
// We need to reset the body before retrying the request
if req.Body != nil {
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
}
req.Body = io.NopCloser(bytes.NewBuffer(reqBody))
resp, err = doer.Do(req.WithContext(ctx))
}