fix: empty req body after retry

http/http.go

@@ -18,6 +18,8 @@
 package http
 
 import (
+	"bytes"
+	"io"
 	"net/http"
 	"time"
 
@@ -93,7 +95,18 @@ func (s *httpClient) Do(req *http.Request) (response *http.Response, err error)
 
 func (s *httpClient) httpCall(req *http.Request) (*http.Response, error) {
 	log := s.logger.With().Str("method", "http.httpCall").Logger()
+
+	// store the request body so that after retrying it can be read again
+	var copyReqBody io.ReadCloser
+	if req.Body != nil {
+		buf, _ := io.ReadAll(req.Body)
+		reqBody := io.NopCloser(bytes.NewBuffer(buf))
+		copyReqBody = io.NopCloser(bytes.NewBuffer(buf))
+		req.Body = reqBody
+	}
 	response, err := s.clientFactory().Do(req)
+	req.Body = copyReqBody
+
 	if err != nil {
 		log.Error().Err(err).Msg("got http error")
 		s.errorReporter.CaptureError(err, observability.ErrorReporterOptions{ErrorDiagnosticPath: req.RequestURI})

http/http_test.go

@@ -16,6 +16,7 @@
 package http_test
 
 import (
+	"fmt"
 	"io"
 	"net/http"
 	"strings"
@@ -34,17 +35,25 @@ import (
 // dummyTransport is a transport struct that always returns the response code specified in the constructor
 type dummyTransport struct {
 	responseCode int
-	responseBody string
 	status       string
 	calls        int
 }
 
-func (d *dummyTransport) RoundTrip(_ *http.Request) (*http.Response, error) {
+func (d *dummyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	d.calls++
+	if req.Body != nil {
+		fmt.Println("read body")
+		body, err := io.ReadAll(req.Body)
+		if err != nil {
+			return nil, err
+		}
+		if string(body) == "" {
+			return nil, fmt.Errorf("body is empty")
+		}
+	}
 	return &http.Response{
 		StatusCode: d.responseCode,
 		Status:     d.status,
-		Body:       io.NopCloser(strings.NewReader(d.responseBody)),
 	}, nil
 }
 
@@ -74,6 +83,32 @@ func TestSnykCodeBackendService_DoCall_shouldRetry(t *testing.T) {
 	assert.Equal(t, 3, d.calls)
 }
 
+func TestSnykCodeBackendService_DoCall_shouldRetryWithARequestBody(t *testing.T) {
+	d := &dummyTransport{responseCode: 502, status: "502 Bad Gateway"}
+	dummyClientFactory := func() *http.Client {
+		return &http.Client{
+			Transport: d,
+		}
+	}
+
+	ctrl := gomock.NewController(t)
+	mockSpan := mocks.NewMockSpan(ctrl)
+	mockSpan.EXPECT().GetTraceId().AnyTimes()
+	mockInstrumentor := mocks.NewMockInstrumentor(ctrl)
+	mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).Times(1)
+	mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(1)
+	mockErrorReporter := mocks.NewMockErrorReporter(ctrl)
+
+	req, err := http.NewRequest(http.MethodGet, "https://httpstat.us/500", io.NopCloser(strings.NewReader("body")))
+	require.NoError(t, err)
+
+	s := codeClientHTTP.NewHTTPClient(newLogger(t), dummyClientFactory, mockInstrumentor, mockErrorReporter)
+	res, err := s.Do(req)
+	assert.NoError(t, err)
+	assert.NotNil(t, res)
+	assert.Equal(t, 3, d.calls)
+}
+
 func TestSnykCodeBackendService_doCall_rejected(t *testing.T) {
 	dummyClientFactory := func() *http.Client {
 		return &http.Client{}

internal/deepcode/client_test.go

@@ -95,6 +95,39 @@ func TestSnykCodeBackendService_GetFilters(t *testing.T) {
 	assert.Equal(t, 1, len(filters.ConfigFiles))
 }
 
+func TestSnykCodeBackendService_GetFilters_Failure(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	mockSpan := mocks.NewMockSpan(ctrl)
+	mockSpan.EXPECT().GetTraceId().AnyTimes()
+	mockSpan.EXPECT().Context().AnyTimes()
+	mockConfig := confMocks.NewMockConfig(ctrl)
+	mockConfig.EXPECT().Organization().AnyTimes().Return("")
+	mockConfig.EXPECT().IsFedramp().AnyTimes().Return(false)
+	mockConfig.EXPECT().SnykCodeApi().AnyTimes().Return("http://localhost")
+
+	mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl)
+	mockHTTPClient.EXPECT().Do(
+		mock.MatchedBy(func(i interface{}) bool {
+			req := i.(*http.Request)
+			return req.URL.String() == "http://localhost/filters" &&
+				req.Method == "GET" &&
+				req.Header.Get("Cache-Control") == "private, max-age=0, no-cache" &&
+				req.Header.Get("Content-Type") == "application/json"
+		}),
+	).Return(&http.Response{
+		StatusCode: http.StatusBadRequest,
+	}, nil).Times(1)
+
+	mockInstrumentor := mocks.NewMockInstrumentor(ctrl)
+	mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).Times(1)
+	mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(1)
+	mockErrorReporter := mocks.NewMockErrorReporter(ctrl)
+
+	s := deepcode.NewSnykCodeClient(newLogger(t), mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+	_, err := s.GetFilters(context.Background())
+	assert.Error(t, err)
+}
+
 func TestSnykCodeBackendService_CreateBundle(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	mockSpan := mocks.NewMockSpan(ctrl)
@@ -136,6 +169,43 @@ func TestSnykCodeBackendService_CreateBundle(t *testing.T) {
 	assert.Equal(t, 1, len(missingFiles))
 }
 
+func TestSnykCodeBackendService_CreateBundle_Failure(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	mockSpan := mocks.NewMockSpan(ctrl)
+	mockSpan.EXPECT().GetTraceId().AnyTimes()
+	mockSpan.EXPECT().Context().AnyTimes()
+	mockConfig := confMocks.NewMockConfig(ctrl)
+	mockConfig.EXPECT().Organization().AnyTimes().Return("")
+	mockConfig.EXPECT().IsFedramp().AnyTimes().Return(false)
+	mockConfig.EXPECT().SnykCodeApi().AnyTimes().Return("http://localhost")
+	mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl)
+	mockHTTPClient.EXPECT().Do(
+		mock.MatchedBy(func(i interface{}) bool {
+			req := i.(*http.Request)
+			return req.URL.String() == "http://localhost/bundle" &&
+				req.Method == "POST" &&
+				req.Header.Get("Cache-Control") == "private, max-age=0, no-cache" &&
+				req.Header.Get("Content-Encoding") == "gzip" &&
+				req.Header.Get("Content-Type") == "application/octet-stream"
+		}),
+	).Return(&http.Response{
+		StatusCode: http.StatusBadRequest,
+	}, nil).Times(1)
+
+	mockInstrumentor := mocks.NewMockInstrumentor(ctrl)
+	mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).Times(1)
+	mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(1)
+	mockErrorReporter := mocks.NewMockErrorReporter(ctrl)
+
+	s := deepcode.NewSnykCodeClient(newLogger(t), mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+
+	files := map[string]string{}
+	randomAddition := fmt.Sprintf("\n public void random() { System.out.println(\"%d\") }", time.Now().UnixMicro())
+	files[path1] = util.Hash([]byte(content + randomAddition))
+	_, _, err := s.CreateBundle(context.Background(), files)
+	assert.Error(t, err)
+}
+
 func TestSnykCodeBackendService_ExtendBundle(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	mockSpan := mocks.NewMockSpan(ctrl)
@@ -190,6 +260,57 @@ func TestSnykCodeBackendService_ExtendBundle(t *testing.T) {
 	assert.NotEmpty(t, bundleHash)
 }
 
+func TestSnykCodeBackendService_ExtendBundle_Failure(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	mockSpan := mocks.NewMockSpan(ctrl)
+	mockSpan.EXPECT().GetTraceId().AnyTimes()
+	mockSpan.EXPECT().Context().AnyTimes()
+	mockConfig := confMocks.NewMockConfig(ctrl)
+	mockConfig.EXPECT().Organization().AnyTimes().Return("")
+	mockConfig.EXPECT().IsFedramp().AnyTimes().Return(false)
+	mockConfig.EXPECT().SnykCodeApi().AnyTimes().Return("http://localhost")
+	mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl)
+	mockHTTPClient.EXPECT().Do(
+		mock.MatchedBy(func(i interface{}) bool {
+			req := i.(*http.Request)
+			return req.URL.String() == "http://localhost/bundle" &&
+				req.Method == "POST" &&
+				req.Header.Get("Cache-Control") == "private, max-age=0, no-cache" &&
+				req.Header.Get("Content-Encoding") == "gzip" &&
+				req.Header.Get("Content-Type") == "application/octet-stream"
+		}),
+	).Return(&http.Response{
+		StatusCode: http.StatusBadRequest,
+	}, nil).Times(1)
+	mockHTTPClient.EXPECT().Do(
+		mock.MatchedBy(func(i interface{}) bool {
+			req := i.(*http.Request)
+			return req.URL.String() == "http://localhost/bundle/bundleHash" &&
+				req.Method == "PUT" &&
+				req.Header.Get("Cache-Control") == "private, max-age=0, no-cache" &&
+				req.Header.Get("Content-Encoding") == "gzip" &&
+				req.Header.Get("Content-Type") == "application/octet-stream"
+		}),
+	).Return(&http.Response{
+		StatusCode: http.StatusOK,
+		Body:       io.NopCloser(bytes.NewReader([]byte(`{"bundleHash":   "bundleHash", "missingFiles": []}`))),
+	}, nil).Times(1)
+	mockInstrumentor := mocks.NewMockInstrumentor(ctrl)
+	mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).Times(2)
+	mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(2)
+	mockErrorReporter := mocks.NewMockErrorReporter(ctrl)
+
+	s := deepcode.NewSnykCodeClient(newLogger(t), mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+	var removedFiles []string
+	files := map[string]string{}
+	files[path1] = util.Hash([]byte(content))
+	bundleHash, _, _ := s.CreateBundle(context.Background(), files)
+	filesExtend := createTestExtendMap()
+
+	_, _, err := s.ExtendBundle(context.Background(), bundleHash, filesExtend, removedFiles)
+	assert.Error(t, err)
+}
+
 func Test_Host(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	mockConfig := confMocks.NewMockConfig(ctrl)