Return errors instead of printing to logs

client.go

@@ -11,7 +11,6 @@ import (
  "errors"
  "fmt"
  "io"
- "log"
 
  "net"
  "net/http"
@@ -294,9 +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)
- }
+ c.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, err
  }
  return c, nil
@@ -336,9 +333,9 @@ 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)
- }
+ // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098:
+ // It's safe to ignore the errors for netconn.Close()
+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
  }
  }()
 
@@ -429,10 +426,8 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
  resp.Body = io.NopCloser(bytes.NewReader([]byte{}))
  conn.subprotocol = resp.Header.Get("Sec-Websocket-Protocol")
 
- if err := netConn.SetDeadline(time.Time{}); err != nil {
- return nil, nil, err
- }
- netConn = nil // to avoid close in defer.
+ netConn.SetDeadline(time.Time{}) //#nosec G104 (CWE-703): Errors unhandled
+ netConn = nil // to avoid close in defer.
  return conn, resp, nil
 }
 

client_server_test.go

@@ -92,6 +92,7 @@ func (t cstHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
  }
  ws, err := cstUpgrader.Upgrade(w, r, http.Header{"Set-Cookie": {"sessionID=1234"}})
  if err != nil {
+ t.Logf("Upgrade: %v", err)
  return
  }
  defer ws.Close()
@@ -103,16 +104,20 @@ func (t cstHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
  }
  op, rd, err := ws.NextReader()
  if err != nil {
+ t.Logf("NextReader: %v", err)
  return
  }
  wr, err := ws.NextWriter(op)
  if err != nil {
+ t.Logf("NextWriter: %v", err)
  return
  }
  if _, err = io.Copy(wr, rd); err != nil {
+ t.Logf("Copy: %v", err)
  return
  }
  if err := wr.Close(); err != nil {
+ t.Logf("Close: %v", err)
  return
  }
 }
@@ -433,7 +438,7 @@ func TestDialBadOrigin(t *testing.T) {
  if resp == nil {
  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)
  }
 }
@@ -539,9 +544,7 @@ func TestRespOnBadHandshake(t *testing.T) {
 
  s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
  w.WriteHeader(expectedStatus)
- if _, err := io.WriteString(w, expectedBody); err != nil {
- t.Fatalf("WriteString: %v", err)
- }
+ io.WriteString(w, expectedBody) // nolint:errcheck
  }))
  defer s.Close()
 
@@ -551,11 +554,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)
  }
 
@@ -574,6 +577,7 @@ type testLogWriter struct {
 }
 
 func (w testLogWriter) Write(p []byte) (int, error) {
+ w.t.Logf("%s", p)
  return len(p), nil
 }
 
@@ -793,10 +797,7 @@ func TestSocksProxyDial(t *testing.T) {
  }
  defer c1.Close()
 
- if err := c1.SetDeadline(time.Now().Add(30 * time.Second)); err != nil {
- t.Errorf("set deadline failed: %v", err)
- return
- }
+ c1.SetDeadline(time.Now().Add(30 * time.Second)) // nolint:errcheck
 
  buf := make([]byte, 32)
  if _, err := io.ReadFull(c1, buf[:3]); err != nil {
@@ -835,15 +836,10 @@ func TestSocksProxyDial(t *testing.T) {
  defer c2.Close()
  done := make(chan struct{})
  go func() {
- if _, err := io.Copy(c1, c2); err != nil {
- t.Errorf("copy failed: %v", err)
- }
+ io.Copy(c1, c2) // nolint:errcheck
  close(done)
  }()
- if _, err := io.Copy(c2, c1); err != nil {
- t.Errorf("copy failed: %v", err)
- return
- }
+ io.Copy(c2, c1) // nolint:errcheck
  <-done
  }()
 

compression.go

@@ -33,9 +33,7 @@ func decompressNoContextTakeover(r io.Reader) io.ReadCloser {
  "\x01\x00\x00\xff\xff"
 
  fr, _ := flateReaderPool.Get().(io.ReadCloser)
- if err := fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil); err != nil {
- panic(err)
- }
+ fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil) //#nosec G104 (CWE-703): Errors unhandled
  return &flateReadWrapper{fr}
 }
 
@@ -134,7 +132,7 @@ func (r *flateReadWrapper) Read(p []byte) (int, error) {
  // Preemptively place the reader back in the pool. This helps with
  // scenarios where the application does not call NextReader() soon after
  // this final read.
- _ = r.Close()
+ r.Close() //#nosec G104 (CWE-703): Errors unhandled
  }
  return n, err
 }

compression_test.go

@@ -23,9 +23,7 @@ func TestTruncWriter(t *testing.T) {
  if m > n {
  m = n
  }
- if _, err := w.Write(p[:m]); err != nil {
- t.Fatal(err)
- }
+ w.Write(p[:m]) // nolint:errcheck
  p = p[m:]
  }
  if b.String() != data[:len(data)-len(w.p)] {
@@ -49,9 +47,7 @@ func BenchmarkWriteNoCompression(b *testing.B) {
  messages := textMessages(100)
  b.ResetTimer()
  for i := 0; i < b.N; i++ {
- if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil {
- b.Fatal(err)
- }
+ c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck
  }
  b.ReportAllocs()
 }
@@ -64,9 +60,7 @@ func BenchmarkWriteWithCompression(b *testing.B) {
  c.newCompressionWriter = compressNoContextTakeover
  b.ResetTimer()
  for i := 0; i < b.N; i++ {
- if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil {
- b.Fatal(err)
- }
+ c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck
  }
  b.ReportAllocs()
 }

conn.go

@@ -934,9 +934,7 @@ func (c *Conn) advanceFrame() (int, error) {
  }
 
  if c.readLimit > 0 && c.readLength > c.readLimit {
- if err := c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)); err != nil {
- return noFrame, err
- }
+ c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled
  return noFrame, ErrReadLimit
  }
 
@@ -997,9 +995,7 @@ func (c *Conn) handleProtocolError(message string) error {
  if len(data) > maxControlFramePayloadSize {
  data = data[:maxControlFramePayloadSize]
  }
- if err := c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)); err != nil {
- return err
- }
+ c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled
  return errors.New("websocket: " + message)
 }
 

proxy.go

@@ -8,7 +8,6 @@ import (
  "bufio"
  "encoding/base64"
  "errors"
- "log"
  "net"
  "net/http"
  "net/url"
@@ -58,9 +57,9 @@ 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)
- }
+ // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098:
+ // It's safe to ignore the errors for conn.Close()
+ conn.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, err
  }
 
@@ -69,16 +68,12 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error)
  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)
- }
+ conn.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, err
  }
 
  if resp.StatusCode != http.StatusOK {
- if err := conn.Close(); err != nil {
- log.Printf("httpProxyDialer: failed to close connection: %v", err)
- }
+ conn.Close() //#nosec G104 (CWE-703): Errors unhandled
  f := strings.SplitN(resp.Status, " ", 2)
  return nil, errors.New(f[1])
  }

server.go

@@ -180,9 +180,9 @@ 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)
- }
+ // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098:
+ // It's safe to ignore the errors for netconn.Close()
+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, errors.New("websocket: client sent data before handshake is complete")
  }
 
@@ -248,31 +248,23 @@ 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)
- }
+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, err
  }
 
  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)
- }
+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, err
  }
  }
  if _, err = netConn.Write(p); err != nil {
- if err := netConn.Close(); err != nil {
- log.Printf("websocket: failed to close network connection: %v", err)
- }
+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, err
  }
  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)
- }
+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
  return nil, err
  }
  }