Skip to content

Commit

Permalink
chore: use reuseconn linter instead of bodyclose
Browse files Browse the repository at this point in the history
  • Loading branch information
atzoum committed Jan 7, 2023
1 parent f1d3d03 commit 0514bbf
Show file tree
Hide file tree
Showing 39 changed files with 84 additions and 88 deletions.
12 changes: 4 additions & 8 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,14 @@ run:

linters:
enable:
- deadcode
- errcheck
- gosimple
- govet
- ineffassign
- staticcheck
- structcheck
- typecheck
- unused
- varcheck
- bodyclose
- reuseconn
- decorder
- makezero
- nilnil
Expand All @@ -40,17 +37,16 @@ issues:
# False positive httptest.NewRecorder
- path: 'gateway/webhook/webhook_test.go'
linters:
- bodyclose
- reuseconn

# False positive .Close behind if
- path: 'processor/transformer/transformer.go'
linters:
- bodyclose
- reuseconn

# False positive httptest.NewRecorder
- path: 'gateway/gateway_test.go'
linters:
- bodyclose
- reuseconn

# Temporary disable until we fix the number of issues
- path: 'warehouse'
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ install-tools:

.PHONY: lint
lint: fmt ## Run linters on all go files
docker run --rm -v $(shell pwd):/app:ro -w /app golangci/golangci-lint:v1.49.0 bash -e -c \
docker run --rm -v $(shell pwd):/app:ro -w /app atzoum/golangci-lint:reuseconn bash -e -c \
'golangci-lint run -v --timeout 5m'

.PHONY: fmt
Expand Down
2 changes: 1 addition & 1 deletion cmd/devtool/commands/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func EventSend(c *cli.Context) error {
if err != nil {
return err
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
b, err := io.ReadAll(resp.Body)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion config/backend-config/namespace_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (nc *namespaceConfig) makeHTTPRequest(ctx context.Context, url string) ([]b
return nil, err
}

defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
respBody, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion config/backend-config/single_workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (wc *singleWorkspaceConfig) makeHTTPRequest(ctx context.Context, url string
return nil, err
}

defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
respBody, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion enterprise/reporting/reporting.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ func (r *HandleT) sendMetric(ctx context.Context, netClient *http.Client, client
httpStatTags["status"] = strconv.Itoa(resp.StatusCode)
stats.Default.NewTaggedStat(StatReportingHttpReq, stats.CountType, httpStatTags).Count(1)

defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
respBody, err := io.ReadAll(resp.Body)
if err != nil {
r.log.Error(err.Error())
Expand Down
2 changes: 1 addition & 1 deletion enterprise/suppress-user/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (s *Syncer) sync(token []byte) ([]model.Suppression, []byte, error) {
if err != nil {
return err
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)

// If statusCode is not 2xx, then returning empty regulations
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
Expand Down
4 changes: 2 additions & 2 deletions gateway/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func testGatewayByAppType(t *testing.T, appType string) {
resp, err := http.Get(healthEndpoint)
require.ErrorContains(t, err, "connection refused")
require.Nil(t, resp)
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)

// Checking now that the configuration has been processed and the server can start
t.Log("Checking health endpoint at", healthEndpoint)
Expand Down Expand Up @@ -317,7 +317,7 @@ func sendEvent(t *testing.T, httpPort int, payload *strings.Reader, callType, wr

res, err := httpClient.Do(req)
require.NoError(t, err)
defer func() { httputil.CloseResponse(res) }()
defer httputil.CloseResponse(res)

body, err := io.ReadAll(res.Body)
require.NoError(t, err)
Expand Down
4 changes: 1 addition & 3 deletions gateway/webhook/webhookTransformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,14 @@ func (bt *batchWebhookTransformerT) transform(events [][]byte, sourceType string
payload := misc.MakeJSONArray(events)
url := fmt.Sprintf(`%s/%s`, bt.sourceTransformerURL, strings.ToLower(sourceType))
resp, err := bt.webhook.netClient.Post(url, "application/json; charset=utf-8", bytes.NewBuffer(payload))

bt.stats.transformTimerStat.End()
if err != nil {
err := fmt.Errorf("JS HTTP connection error to source transformer: URL: %v Error: %+v", url, err)
return transformerBatchResponseT{batchError: err, statusCode: http.StatusServiceUnavailable}
}
defer httputil.CloseResponse(resp)

respBody, err := io.ReadAll(resp.Body)
func() { httputil.CloseResponse(resp) }()

if err != nil {
bt.stats.failedStat.Count(len(events))
statusCode := response.GetErrorStatusCode(response.RequestBodyReadFailed)
Expand Down
4 changes: 2 additions & 2 deletions integration_test/docker_test/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ func getEvent(url, method string) (string, error) {
if err != nil {
return "", err
}
defer func() { httputil.CloseResponse(res) }()
defer httputil.CloseResponse(res)

body, err := io.ReadAll(res.Body)
if err != nil {
Expand Down Expand Up @@ -720,7 +720,7 @@ func sendEvent(t *testing.T, payload *strings.Reader, callType, writeKey string)
t.Logf("sendEvent error: %v", err)
return
}
defer func() { httputil.CloseResponse(res) }()
httputil.CloseResponse(res)

body, err := io.ReadAll(res.Body)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions integration_test/multi_tentant_test/multi_tenant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func testMultiTenantByAppType(t *testing.T, appType string) {
require.ErrorContains(t, err, "connection refused")
require.Nil(t, resp)
if err == nil {
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
}

// Pushing valid configuration via ETCD
Expand Down Expand Up @@ -416,7 +416,7 @@ func sendEvent(t *testing.T, httpPort int, payload *strings.Reader, callType, wr

res, err := httpClient.Do(req)
require.NoError(t, err)
defer func() { httputil.CloseResponse(res) }()
defer httputil.CloseResponse(res)

body, err := io.ReadAll(res.Body)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion processor/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ func (proc *HandleT) makeFeaturesFetchCall() bool {
return true
}

defer func() { httputil.CloseResponse(res) }()
defer httputil.CloseResponse(res)
body, err := io.ReadAll(res.Body)
if err != nil {
return true
Expand Down
4 changes: 2 additions & 2 deletions processor/transformer/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func GetVersion() (transformerBuildVersion string) {
transformerBuildVersion = fmt.Sprintf("No response from transformer. %s", transformerBuildVersion)
return
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
if resp.StatusCode == http.StatusOK {
bodyBytes, err := io.ReadAll(resp.Body)
if err != nil {
Expand Down Expand Up @@ -391,7 +391,7 @@ func (trans *HandleT) doPost(ctx context.Context, rawJSON []byte, url string, ta
if reqErr != nil {
return reqErr
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
respData, reqErr = io.ReadAll(resp.Body)
return reqErr
},
Expand Down
2 changes: 1 addition & 1 deletion regulation-worker/cmd/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func startMinioServer(t *testing.T, pool *dockertest.Pool) {
if err != nil {
return err
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("status code not OK")
}
Expand Down
4 changes: 2 additions & 2 deletions regulation-worker/internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (j *JobAPI) Get(ctx context.Context) (model.Job, error) {
if err != nil {
return model.Job{}, err
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
pkgLogger.Debugf("obtained response code: %v", resp.StatusCode, "response body: ", resp.Body)

// if successful
Expand Down Expand Up @@ -132,7 +132,7 @@ func (j *JobAPI) UpdateStatus(ctx context.Context, status model.JobStatus, jobID
if err != nil {
return err
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)

pkgLogger.Debugf("response code: %v", resp.StatusCode, "response body: %v", resp.Body)
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
Expand Down
2 changes: 1 addition & 1 deletion regulation-worker/internal/delete/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (api *APIManager) deleteWithRetry(ctx context.Context, job model.Job, desti
}
return model.JobStatusFailed
}
defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)
bodyBytes, err := io.ReadAll(resp.Body)
if err != nil {
return model.JobStatusFailed
Expand Down
2 changes: 1 addition & 1 deletion router/batchrouter/batchrouter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,7 @@ func (brt *HandleT) postToWarehouse(batchJobs *BatchJobsT, output StorageUploadO
brt.logger.Errorf("BRT: Failed to route staging file URL to warehouse service@%v, error:%v", uri, err)
return
}
defer func() { httputil.CloseResponse(resp) }()
httputil.CloseResponse(resp)

if resp.StatusCode == http.StatusOK {
brt.logger.Infof("BRT: Routed successfully staging file URL to warehouse service@%v", uri)
Expand Down
2 changes: 1 addition & 1 deletion router/eventorder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ func TestEventOrderGuarantee(t *testing.T) {
req.SetBasicAuth(writeKey, "password")
resp, err := client.Do(req)
require.NoError(t, err, "should be able to send the request to gateway")
httputil.CloseResponse(resp)
require.Equal(t, http.StatusOK, resp.StatusCode, "should be able to send the request to gateway successfully", payload)
func() { httputil.CloseResponse(resp) }()
}
}()

Expand Down
2 changes: 1 addition & 1 deletion router/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (network *NetHandleT) SendPost(ctx context.Context, structData integrations
}
}

defer func() { httputil.CloseResponse(resp) }()
defer httputil.CloseResponse(resp)

respBody, err := io.ReadAll(resp.Body)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion router/router_dest_isolation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,9 @@ func Test_RouterDestIsolation(t *testing.T) {
req.SetBasicAuth(writeKey, "password")
resp, err := client.Do(req)
require.NoError(t, err, "should be able to send the request to gateway")
httputil.CloseResponse(resp)
require.Equal(t, http.StatusOK, resp.StatusCode)
func() { httputil.CloseResponse(resp) }()

}
require.Eventually(t, func() bool {
return atomic.LoadUint64(webhook2.count) == 100 && atomic.LoadUint64(webhook1.count) < 100
Expand Down
2 changes: 1 addition & 1 deletion router/router_throttling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ func Test_RouterThrottling(t *testing.T) {
req.SetBasicAuth(writeKey, "password")
resp, err := client.Do(req)
require.NoError(t, err, "should be able to send the request to gateway")
httputil.CloseResponse(resp)
require.Equal(t, http.StatusOK, resp.StatusCode)
func() { httputil.CloseResponse(resp) }()
}

require.Eventuallyf(t,
Expand Down
46 changes: 25 additions & 21 deletions router/transformer/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ func (trans *handle) Transform(transformType string, transformMessage *types.Tra
trans.logger.Debugf("[Router Transformer] :: input payload : %s", string(rawJSON))

retryCount := 0
var resp *http.Response
var respData []byte
var respStatus int
var respHeaders http.Header
// We should rarely have error communicating with our JS
reqFailed := false

Expand All @@ -128,18 +129,24 @@ func (trans *handle) Transform(transformType string, transformMessage *types.Tra
}

for {
s := time.Now()
resp, err = trans.client.Post(url, "application/json; charset=utf-8",
bytes.NewBuffer(rawJSON))

if err == nil {
// If no err returned by client.Post, reading body.
// If reading body fails, retrying.
respData, err = io.ReadAll(resp.Body)
}

if err != nil {
respData, respStatus, respHeaders, err = func() ([]byte, int, http.Header, error) {
var data []byte
var status int
var header http.Header
var err error

s := time.Now()
resp, err := trans.client.Post(url, "application/json; charset=utf-8", bytes.NewBuffer(rawJSON))
defer httputil.CloseResponse(resp)
trans.transformRequestTimerStat.SendTiming(time.Since(s))
if err == nil {
status = resp.StatusCode
header = resp.Header
data, err = io.ReadAll(resp.Body)
}
return data, status, header, err
}()
if err != nil {
reqFailed = true
trans.logger.Errorf("JS HTTP connection error: URL: %v Error: %+v", url, err)
if retryCount > maxRetry {
Expand All @@ -153,18 +160,16 @@ func (trans *handle) Transform(transformType string, transformMessage *types.Tra
if reqFailed {
trans.logger.Errorf("Failed request succeeded after %v retries, URL: %v", retryCount, url)
}

trans.transformRequestTimerStat.SendTiming(time.Since(s))
break
}

if resp.StatusCode != http.StatusOK {
trans.logger.Errorf("[Router Transfomrer] :: Transformer returned status code: %v reason: %v", resp.StatusCode, resp.Status)
if respStatus != http.StatusOK {
trans.logger.Errorf("[Router Transfomrer] :: Transformer returned status code: %d", respStatus)
}

var destinationJobs []types.DestinationJobT
if resp.StatusCode == http.StatusOK {
transformerAPIVersion, convErr := strconv.Atoi(resp.Header.Get(apiVersionHeader))
if respStatus == http.StatusOK {
transformerAPIVersion, convErr := strconv.Atoi(respHeaders.Get(apiVersionHeader))
if convErr != nil {
transformerAPIVersion = 0
}
Expand Down Expand Up @@ -230,7 +235,7 @@ func (trans *handle) Transform(transformType string, transformMessage *types.Tra
}
} else {
statusCode := 500
if resp.StatusCode == http.StatusNotFound {
if respStatus == http.StatusNotFound {
statusCode = 404
}
for i := range transformMessage.Data {
Expand All @@ -239,7 +244,6 @@ func (trans *handle) Transform(transformType string, transformMessage *types.Tra
destinationJobs = append(destinationJobs, resp)
}
}
func() { httputil.CloseResponse(resp) }()

return destinationJobs
}
Expand Down Expand Up @@ -338,6 +342,7 @@ func (trans *handle) doProxyRequest(ctx context.Context, proxyReqParams *ProxyRe

httpReqStTime := time.Now()
resp, err := trans.proxyClient.Do(req)
defer httputil.CloseResponse(resp)
reqRoundTripTime := time.Since(httpReqStTime)
// This stat will be useful in understanding the round trip time taken for the http req
// between server and transformer
Expand Down Expand Up @@ -386,7 +391,6 @@ func (trans *handle) doProxyRequest(ctx context.Context, proxyReqParams *ProxyRe
}

respData, err = io.ReadAll(resp.Body)
defer func() { httputil.CloseResponse(resp) }()
// error handling while reading from resp.Body
if err != nil {
respData = []byte(fmt.Sprintf(`failed to read response body, Error:: %+v`, err))
Expand Down
3 changes: 1 addition & 2 deletions services/alert/pagerduty.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@ func (ops *PagerDuty) Alert(message string) {
pkgLogger.Errorf("Alert: Failed to alert service: %s", err.Error())
return
}
defer httputil.CloseResponse(resp)

if resp.StatusCode != 200 && resp.StatusCode != 202 {
pkgLogger.Errorf("Alert: Got error response %d", resp.StatusCode)
}

body, err := io.ReadAll(resp.Body)
defer func() { httputil.CloseResponse(resp) }()
if err != nil {
pkgLogger.Errorf("Alert: Failed to read response body: %s", err.Error())
return
Expand Down
Loading

0 comments on commit 0514bbf

Please sign in to comment.