Don't check for HTTP response body before checking for an error

skycoin · Feb 11, 2020 · ef3679f · ef3679f
1 parent 2a50608
commit ef3679f
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 78 deletions.
diff --git a/internal/httpauth/client.go b/internal/httpauth/client.go
@@ -133,17 +133,16 @@ func (c *Client) Nonce(ctx context.Context, key cipher.PubKey) (Nonce, error) {
 	req = req.WithContext(ctx)
 
 	resp, err := c.client.Do(req)
-	if resp != nil {
-		defer func() {
-			if err := resp.Body.Close(); err != nil {
-				log.WithError(err).Warn("Failed to close HTTP response body")
-			}
-		}()
-	}
 	if err != nil {
 		return 0, err
 	}
 
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.WithError(err).Warn("Failed to close HTTP response body")
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return 0, fmt.Errorf("error getting current nonce: status: %d <- %v", resp.StatusCode, extractError(resp.Body))
 	}

diff --git a/internal/utclient/client.go b/internal/utclient/client.go
@@ -64,17 +64,16 @@ func (c *httpClient) Get(ctx context.Context, path string) (*http.Response, erro
 // UpdateVisorUptime updates visor uptime.
 func (c *httpClient) UpdateVisorUptime(ctx context.Context) error {
 	resp, err := c.Get(ctx, "/update")
-	if resp != nil {
-		defer func() {
-			if err := resp.Body.Close(); err != nil {
-				log.WithError(err).Warn("Failed to close response body")
-			}
-		}()
-	}
 	if err != nil {
 		return err
 	}
 
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.WithError(err).Warn("Failed to close response body")
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body))
 	}

diff --git a/pkg/hypervisor/hypervisor_test.go b/pkg/hypervisor/hypervisor_test.go
@@ -129,13 +129,13 @@ func testCase(t *testing.T, addr string, client *http.Client, tc TestCase, testT
 	}
 
 	resp, err := client.Do(req)
-	if resp != nil {
-		defer func() {
-			assert.NoError(t, resp.Body.Close())
-		}()
-	}
 
 	require.NoError(t, err, testTag)
+
+	defer func() {
+		assert.NoError(t, resp.Body.Close())
+	}()
+
 	assert.Equal(t, tc.RespStatus, resp.StatusCode, testTag)
 
 	if tc.RespBody != nil {

diff --git a/pkg/transport-discovery/client/client.go b/pkg/transport-discovery/client/client.go
@@ -90,17 +90,16 @@ func (c *apiClient) RegisterTransports(ctx context.Context, entries ...*transpor
 	}
 
 	resp, err := c.Post(ctx, "/transports/", entries)
-	if resp != nil {
-		defer func() {
-			if err := resp.Body.Close(); err != nil {
-				log.WithError(err).Warn("Failed to close HTTP response body")
-			}
-		}()
-	}
 	if err != nil {
 		return err
 	}
 
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.WithError(err).Warn("Failed to close HTTP response body")
+		}
+	}()
+
 	if resp.StatusCode == http.StatusCreated {
 		return nil
 	}
@@ -111,17 +110,16 @@ func (c *apiClient) RegisterTransports(ctx context.Context, entries ...*transpor
 // GetTransportByID returns Transport for corresponding ID.
 func (c *apiClient) GetTransportByID(ctx context.Context, id uuid.UUID) (*transport.EntryWithStatus, error) {
 	resp, err := c.Get(ctx, fmt.Sprintf("/transports/id:%s", id.String()))
-	if resp != nil {
-		defer func() {
-			if err := resp.Body.Close(); err != nil {
-				log.WithError(err).Warn("Failed to close HTTP response body")
-			}
-		}()
-	}
 	if err != nil {
 		return nil, err
 	}
 
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.WithError(err).Warn("Failed to close HTTP response body")
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body))
 	}
@@ -137,17 +135,16 @@ func (c *apiClient) GetTransportByID(ctx context.Context, id uuid.UUID) (*transp
 // GetTransportsByEdge returns all Transports registered for the edge.
 func (c *apiClient) GetTransportsByEdge(ctx context.Context, pk cipher.PubKey) ([]*transport.EntryWithStatus, error) {
 	resp, err := c.Get(ctx, fmt.Sprintf("/transports/edge:%s", pk))
-	if resp != nil {
-		defer func() {
-			if err := resp.Body.Close(); err != nil {
-				log.WithError(err).Warn("Failed to close HTTP response body")
-			}
-		}()
-	}
 	if err != nil {
 		return nil, err
 	}
 
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.WithError(err).Warn("Failed to close HTTP response body")
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body))
 	}
@@ -163,16 +160,16 @@ func (c *apiClient) GetTransportsByEdge(ctx context.Context, pk cipher.PubKey) (
 // DeleteTransport deletes given transport by it's ID. A visor can only delete transports if he is one of it's edges.
 func (c *apiClient) DeleteTransport(ctx context.Context, id uuid.UUID) error {
 	resp, err := c.Delete(ctx, fmt.Sprintf("/transports/id:%s", id.String()))
-	if resp != nil {
-		defer func() {
-			if err := resp.Body.Close(); err != nil {
-				log.WithError(err).Warn("Failed to close HTTP response body")
-			}
-		}()
-	}
 	if err != nil {
 		return err
 	}
+
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.WithError(err).Warn("Failed to close HTTP response body")
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body))
 	}
@@ -187,17 +184,16 @@ func (c *apiClient) UpdateStatuses(ctx context.Context, statuses ...*transport.S
 	}
 
 	resp, err := c.Post(ctx, "/statuses", statuses)
-	if resp != nil {
-		defer func() {
-			if err := resp.Body.Close(); err != nil {
-				log.WithError(err).Warn("Failed to close HTTP response body")
-			}
-		}()
-	}
 	if err != nil {
 		return nil, err
 	}
 
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.WithError(err).Warn("Failed to close HTTP response body")
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return nil, fmt.Errorf("status: %d, error: %v", resp.StatusCode, extractError(resp.Body))
 	}

diff --git a/pkg/util/updater/updater.go b/pkg/util/updater/updater.go
@@ -240,18 +240,16 @@ func getChecksum(checksums, filename string) (string, error) {
 
 func downloadChecksums(url string) (checksums string, err error) {
 	resp, err := http.Get(url) // nolint:gosec
-	if resp != nil {
-		defer func() {
-			if closeErr := resp.Body.Close(); closeErr != nil && err == nil {
-				err = closeErr
-			}
-		}()
-	}
-
 	if err != nil {
 		return "", err
 	}
 
+	defer func() {
+		if closeErr := resp.Body.Close(); closeErr != nil && err == nil {
+			err = closeErr
+		}
+	}()
+
 	if resp.StatusCode != http.StatusOK {
 		return "", fmt.Errorf("received bad status code: %d", resp.StatusCode)
 	}
@@ -266,18 +264,16 @@ func downloadChecksums(url string) (checksums string, err error) {
 
 func downloadFile(url, filename string) (path string, err error) {
 	resp, err := http.Get(url) // nolint:gosec
-	if resp != nil {
-		defer func() {
-			if closeErr := resp.Body.Close(); closeErr != nil && err == nil {
-				err = closeErr
-			}
-		}()
-	}
-
 	if err != nil {
 		return "", err
 	}
 
+	defer func() {
+		if closeErr := resp.Body.Close(); closeErr != nil && err == nil {
+			err = closeErr
+		}
+	}()
+
 	tmpDir := os.TempDir()
 	path = filepath.Join(tmpDir, filename)
 
@@ -341,18 +337,16 @@ func currentVersion() (*Version, error) {
 
 func lastVersionHTML() (data []byte, err error) {
 	resp, err := http.Get(releaseURL)
-	if resp != nil {
-		defer func() {
-			if closeErr := resp.Body.Close(); closeErr != nil && err == nil {
-				err = closeErr
-			}
-		}()
-	}
-
 	if err != nil {
 		return nil, err
 	}
 
+	defer func() {
+		if closeErr := resp.Body.Close(); closeErr != nil && err == nil {
+			err = closeErr
+		}
+	}()
+
 	return ioutil.ReadAll(resp.Body)
 }