diff --git a/runtime/client_http_request.go b/runtime/client_http_request.go index c20e5d339..7aa190a03 100644 --- a/runtime/client_http_request.go +++ b/runtime/client_http_request.go @@ -215,11 +215,11 @@ func (req *ClientHTTPRequest) Do() (*ClientHTTPResponse, error) { var retryCount int64 = 1 var res *http.Response - //when timeoutAndRetryOptions per request is not configured, use default client level timeout + // when timeoutAndRetryOptions per request is not configured, use default client level timeout if req.timeoutAndRetryOptions == nil || req.timeoutAndRetryOptions.MaxAttempts == 0 { res, err = req.client.Client.Do(req.httpReq.WithContext(ctx)) } else { - res, retryCount, err = req.executeDoWithRetry(ctx) //new code for retry and timeout per ep level + res, retryCount, err = req.executeDoWithRetry(ctx) // new code for retry and timeout per ep level } span.Finish() @@ -237,7 +237,7 @@ func (req *ClientHTTPRequest) Do() (*ClientHTTPResponse, error) { return req.res, nil } -//executeDoWithRetry will execute executeDo with retries +// executeDoWithRetry will execute executeDo with retries func (req *ClientHTTPRequest) executeDoWithRetry(ctx context.Context) (*http.Response, int64, error) { var err error var res *http.Response @@ -264,14 +264,14 @@ func (req *ClientHTTPRequest) executeDoWithRetry(ctx context.Context) (*http.Res zap.Int("maxAttempts", req.timeoutAndRetryOptions.MaxAttempts), zap.Bool("shouldRetry", shouldRetry)) - //TODO (future releases) - make retry conditional, inspect error/response and then retry + // TODO (future releases) - make retry conditional, inspect error/response and then retry - //reassign body + // reassign body if req.rawBody != nil && len(req.rawBody) > 0 { req.httpReq.Body = io.NopCloser(bytes.NewBuffer(req.rawBody)) } - //Break loop if no retries + // Break loop if no retries if !shouldRetry { break } @@ -279,11 +279,20 @@ func (req *ClientHTTPRequest) executeDoWithRetry(ctx context.Context) (*http.Res return nil, retryCount, err } -//executeDo will send the request out with a timeout +// executeDo will send the request out with a timeout func (req *ClientHTTPRequest) executeDo(ctx context.Context) (*http.Response, error) { - attemptCtx, cancelFn := context.WithTimeout(ctx, req.timeoutAndRetryOptions.RequestTimeoutPerAttemptInMs) - defer cancelFn() - return req.client.Client.Do(req.httpReq.WithContext(attemptCtx)) + ctx, cancel := context.WithTimeout(ctx, req.timeoutAndRetryOptions.RequestTimeoutPerAttemptInMs) + defer cancel() + res, err := req.client.Client.Do(req.httpReq.WithContext(ctx)) + // when no error, read body and capture before closing the connection + if err == nil { + req.res.setRawHTTPResponse(res) + _, err = req.res.ReadAll() + if err != nil { + return nil, err + } + } + return res, err } // InjectSpanToHeader will inject span to request header diff --git a/runtime/client_http_request_test.go b/runtime/client_http_request_test.go index e5f310936..c77eb9d5f 100644 --- a/runtime/client_http_request_test.go +++ b/runtime/client_http_request_test.go @@ -270,6 +270,60 @@ func TestMakingClientCallWithHeadersWithRequestLevelTimeoutAndRetries(t *testing "the request should have failed due to context deadline (timeout)") } +func TestMakingClientCallWithHeadersWithRequestLevelTimeoutAndRetriesSuccess(t *testing.T) { + gateway, err := benchGateway.CreateGateway( + defaultTestConfig, + defaultTestOptions, + exampleGateway.CreateGateway, + ) + if !assert.NoError(t, err) { + return + } + defer gateway.Close() + + bgateway := gateway.(*benchGateway.BenchGateway) + + bgateway.HTTPBackends()["bar"].HandleFunc( + "POST", "/bar-path", + func(w http.ResponseWriter, r *http.Request) { + bodyBytes, _ := io.ReadAll(r.Body) + w.WriteHeader(200) + response := map[string]string{"Example-Header": r.Header.Get("Example-Header"), "body": string(bodyBytes)} + responseBytes, _ := json.Marshal(response) + _, _ = w.Write(responseBytes) + // Check that the default header got set and actually sent to the server. + assert.Equal(t, r.Header.Get("X-Client-ID"), "bar") + assert.Equal(t, r.Header.Get("Accept"), "application/test+json") + }, + ) + + deps := bgateway.Dependencies.(*exampleGateway.DependenciesTree) + barClient := deps.Client.Bar + client := barClient.HTTPClient() + + retryOptionsCopy := retryOptions + retryOptionsCopy.RequestTimeoutPerAttemptInMs = 500 * time.Millisecond + retryOptionsCopy.MaxAttempts = 2 + + ctx := context.Background() + req := zanzibar.NewClientHTTPRequest(ctx, "bar", "Normal", "bar::Normal", client, &retryOptionsCopy) + + err = req.WriteJSON( + "POST", + client.BaseURL+"/bar-path", + map[string]string{ + "Example-Header": "Example-Value", + "Accept": "application/test+json", + }, + "dummy body", + ) + assert.NoError(t, err) + + _, err = req.Do() + + assert.NoError(t, err) +} + func TestBarClientWithoutHeaders(t *testing.T) { gateway, err := benchGateway.CreateGateway( defaultTestConfig, diff --git a/runtime/client_http_response.go b/runtime/client_http_response.go index 30eb32e40..428aa57a8 100644 --- a/runtime/client_http_response.go +++ b/runtime/client_http_response.go @@ -42,6 +42,7 @@ type ClientHTTPResponse struct { finishTime time.Time finished bool rawResponse *http.Response + responseRead bool rawResponseBytes []byte StatusCode int Duration time.Duration @@ -68,6 +69,16 @@ func (res *ClientHTTPResponse) setRawHTTPResponse(httpRes *http.Response) { // ReadAll reads bytes from response. func (res *ClientHTTPResponse) ReadAll() ([]byte, error) { + // if response is already read + if res.responseRead { + return res.GetRawBody(), nil + } + + // mark response read to true + defer func() { + res.responseRead = true + }() + rawBody, err := io.ReadAll(res.rawResponse.Body) cerr := res.rawResponse.Body.Close() diff --git a/scripts/cover.sh b/scripts/cover.sh index 196b2159a..0814b560c 100755 --- a/scripts/cover.sh +++ b/scripts/cover.sh @@ -128,7 +128,7 @@ cat ./coverage/istanbul.json | jq '[ ] | from_entries' > ./coverage/istanbul-runtime.json echo "Checking code coverage for runtime folder" -./node_modules/.bin/istanbul check-coverage --statements 99.51 \ +./node_modules/.bin/istanbul check-coverage --statements 99.45 \ ./coverage/istanbul-runtime.json cat ./coverage/istanbul.json | jq '[