return errors instead of printing to logs

gorilla · Apr 2, 2024 · 58af150 · 58af150
1 parent e5f1a0a
commit 58af150
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 48 deletions.
diff --git a/client.go b/client.go
@@ -11,7 +11,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log"
 
 	"net"
 	"net/http"
@@ -294,10 +293,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
 			}
 			err = c.SetDeadline(deadline)
 			if err != nil {
-				if err := c.Close(); err != nil {
-					log.Printf("websocket: failed to close network connection: %v", err)
-				}
-				return nil, err
+				return nil, errors.Join(err, c.Close())
 			}
 			return c, nil
 		}
@@ -336,9 +332,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
 
 	defer func() {
 		if netConn != nil {
-			if err := netConn.Close(); err != nil {
-				log.Printf("websocket: failed to close network connection: %v", err)
-			}
+			netConn.Close() //#nosec:G104 (CWE-703)
 		}
 	}()
 
@@ -433,7 +427,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
 		return nil, nil, err
 	}
 	netConn = nil // to avoid close in defer.
-	return conn, resp, nil
+	return conn, resp, err
 }
 
 func cloneTLSConfig(cfg *tls.Config) *tls.Config {

diff --git a/client_server_test.go b/client_server_test.go
@@ -430,10 +430,10 @@ func TestDialBadOrigin(t *testing.T) {
 		ws.Close()
 		t.Fatalf("Dial: nil")
 	}
-	if resp == nil {
+	if resp == nil { // nolint:staticcheck
 		t.Fatalf("resp=nil, err=%v", err)
 	}
-	if resp.StatusCode != http.StatusForbidden {
+	if resp.StatusCode != http.StatusForbidden { // nolint:staticcheck
 		t.Fatalf("status=%d, want %d", resp.StatusCode, http.StatusForbidden)
 	}
 }
@@ -551,11 +551,11 @@ func TestRespOnBadHandshake(t *testing.T) {
 		t.Fatalf("Dial: nil")
 	}
 
-	if resp == nil {
+	if resp == nil { // nolint:staticcheck
 		t.Fatalf("resp=nil, err=%v", err)
 	}
 
-	if resp.StatusCode != expectedStatus {
+	if resp.StatusCode != expectedStatus { // nolint:staticcheck
 		t.Errorf("resp.StatusCode=%d, want %d", resp.StatusCode, expectedStatus)
 	}
 

diff --git a/proxy.go b/proxy.go
@@ -8,7 +8,6 @@ import (
 	"bufio"
 	"encoding/base64"
 	"errors"
-	"log"
 	"net"
 	"net/http"
 	"net/url"
@@ -58,29 +57,20 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error)
 	}
 
 	if err := connectReq.Write(conn); err != nil {
-		if err := conn.Close(); err != nil {
-			log.Printf("httpProxyDialer: failed to close connection: %v", err)
-		}
-		return nil, err
+		return nil, errors.Join(err, conn.Close())
 	}
 
 	// Read response. It's OK to use and discard buffered reader here becaue
 	// the remote server does not speak until spoken to.
 	br := bufio.NewReader(conn)
 	resp, err := http.ReadResponse(br, connectReq)
 	if err != nil {
-		if err := conn.Close(); err != nil {
-			log.Printf("httpProxyDialer: failed to close connection: %v", err)
-		}
-		return nil, err
+		return nil, errors.Join(err, conn.Close())
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		if err := conn.Close(); err != nil {
-			log.Printf("httpProxyDialer: failed to close connection: %v", err)
-		}
 		f := strings.SplitN(resp.Status, " ", 2)
-		return nil, errors.New(f[1])
+		return nil, errors.Join(errors.New(f[1]), conn.Close())
 	}
 	return conn, nil
 }
diff --git a/server.go b/server.go
@@ -8,7 +8,6 @@ import (
 	"bufio"
 	"errors"
 	"io"
-	"log"
 	"net/http"
 	"net/url"
 	"strings"
@@ -180,10 +179,10 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade
 	}
 
 	if brw.Reader.Buffered() > 0 {
-		if err := netConn.Close(); err != nil {
-			log.Printf("websocket: failed to close network connection: %v", err)
-		}
-		return nil, errors.New("websocket: client sent data before handshake is complete")
+		return nil, errors.Join(
+			errors.New("websocket: client sent data before handshake is complete"),
+			netConn.Close(),
+		)
 	}
 
 	var br *bufio.Reader
@@ -248,32 +247,20 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade
 
 	// Clear deadlines set by HTTP server.
 	if err := netConn.SetDeadline(time.Time{}); err != nil {
-		if err := netConn.Close(); err != nil {
-			log.Printf("websocket: failed to close network connection: %v", err)
-		}
-		return nil, err
+		return nil, errors.Join(err, netConn.Close())
 	}
 
 	if u.HandshakeTimeout > 0 {
 		if err := netConn.SetWriteDeadline(time.Now().Add(u.HandshakeTimeout)); err != nil {
-			if err := netConn.Close(); err != nil {
-				log.Printf("websocket: failed to close network connection: %v", err)
-			}
-			return nil, err
+			return nil, errors.Join(err, netConn.Close())
 		}
 	}
 	if _, err = netConn.Write(p); err != nil {
-		if err := netConn.Close(); err != nil {
-			log.Printf("websocket: failed to close network connection: %v", err)
-		}
-		return nil, err
+		return nil, errors.Join(err, netConn.Close())
 	}
 	if u.HandshakeTimeout > 0 {
 		if err := netConn.SetWriteDeadline(time.Time{}); err != nil {
-			if err := netConn.Close(); err != nil {
-				log.Printf("websocket: failed to close network connection: %v", err)
-			}
-			return nil, err
+			return nil, errors.Join(err, netConn.Close())
 		}
 	}
 
@@ -376,7 +363,7 @@ func bufioWriterBuffer(originalWriter io.Writer, bw *bufio.Writer) []byte {
 		panic(err)
 	}
 	if err := bw.Flush(); err != nil {
-		log.Printf("websocket: bufioWriterBuffer: Flush: %v", err)
+		panic(err)
 	}
 
 	bw.Reset(originalWriter)