Fix unchecked Close() errors

.github/workflows/build.yaml

@@ -21,8 +21,9 @@ jobs:
         go-version: 1.21
 
     - name: golangci-lint
-      uses: golangci/golangci-lint-action@v3
+      uses: golangci/golangci-lint-action@v6
       with:
+        version: v1.59
         working-directory: ./
         args: --timeout 5m
 

.golangci.yaml

@@ -12,3 +12,11 @@ linters:
     - ineffassign
     - govet
     - unused
+
+issues:
+  include:
+    # Include Close() errcheck https://golangci-lint.run/usage/false-positives/#exc0001
+    - EXC0001
+  # Show all issues
+  max-issues-per-linter: 0
+  max-same-issues: 0

example/preproxy/main.go

@@ -209,7 +209,10 @@ func main() {
 				masqueConn, err := HandleConnectMasque(relayClient, c, target, true, func(c net.Conn) {})
 				if err != nil {
 					logger.Error("Failed to open masque connection", "err", err)
-					c.Close()
+					if err := c.Close(); err != nil {
+						logger.Error("Failed to close masque connection", "err", err)
+					}
+
 				} else {
 					logger.Debug("Opened TCP masque connection for client", "c.LocalAddr", c.LocalAddr(), "c.RemoteAddr", c.RemoteAddr())
 					go Transfer(masqueConn, c)
@@ -254,15 +257,19 @@ func main() {
 								if err != nil {
 									logger.Error("Couldn't read from masque connection", "err", err)
 									delete(udpConns, addr.String())
-									masqueConn.Close()
+									if err := masqueConn.Close(); err != nil {
+										logger.Error("Failed to close masque connection", "err", err)
+									}
 									break
 								}
 								logger.Debug("Writing bytes to client over UDP", "n", n, "addr", addr)
 								_, err = l.WriteTo(bufFromMasque[:n], addr)
 								if err != nil {
 									logger.Error("Couldn't write to client connection", "err", err)
 									delete(udpConns, addr.String())
-									masqueConn.Close()
+									if err := masqueConn.Close(); err != nil {
+										logger.Error("Failed to close masque connection", "err", err)
+									}
 									break
 								}
 							}
@@ -290,8 +297,17 @@ func main() {
 
 // Transfer copies data from src to dst and closes both when done.
 func Transfer(dst io.WriteCloser, src io.ReadCloser) {
-	defer dst.Close()
-	defer src.Close()
+	defer func() {
+		if err := dst.Close(); err != nil {
+			logger.Error("Failed to close dst", "err", err)
+		}
+	}()
+
+	defer func() {
+		if err := src.Close(); err != nil {
+			logger.Error("Failed to close src", "err", err)
+		}
+	}()
 	_, err := io.Copy(dst, src)
 	if err != nil {
 		logger.Error("Failed to copy", "err", err)
@@ -309,22 +325,28 @@ func HandleConnectMasque(relayClient *masqueH3.Client, c net.Conn, target string
 	if err != nil {
 		logger.Error("Failed to split host and port", "err", err)
 		fail(c)
-		c.Close()
+		if err := c.Close(); err != nil {
+			logger.Error("Failed to close c", "err", err)
+		}
 		return nil, err
 	}
 
 	portInt, err := strconv.Atoi(port)
 	if err != nil {
 		logger.Error("Failed to convert port to int", "err", err)
 		fail(c)
-		c.Close()
+		if err := c.Close(); err != nil {
+			logger.Error("Failed to close c", "err", err)
+		}
 		return nil, err
 	}
 
 	if masque.IsDisallowedPort(uint16(portInt)) {
 		logger.Error("Disallowed port", "port", port)
 		fail(c)
-		c.Close()
+		if err := c.Close(); err != nil {
+			logger.Error("Failed to close c", "err", err)
+		}
 		return nil, fmt.Errorf("Disallowed port: %s", port)
 	}
 
@@ -334,15 +356,19 @@ func HandleConnectMasque(relayClient *masqueH3.Client, c net.Conn, target string
 		if err != nil {
 			logger.Error("Failed to create TCP stream", "err", err)
 			fail(c)
-			c.Close()
+			if err := c.Close(); err != nil {
+				logger.Error("Failed to close c", "err", err)
+			}
 			return nil, err
 		}
 	} else {
 		masqueConn, err = relayClient.CreateUDPStream(target)
 		if err != nil {
 			logger.Error("Failed to create UDP stream", "err", err)
 			fail(c)
-			c.Close()
+			if err := c.Close(); err != nil {
+				logger.Error("Failed to close c", "err", err)
+			}
 			return nil, err
 		}
 	}

example/relay-http-proxy/main.go

@@ -69,7 +69,9 @@ func handleConnectMasque(c net.Conn, req *http.Request, logger *slog.Logger) *ma
 		if err != nil {
 			logger.Error("Error calling disallowedRes.Write", "err", err)
 		}
-		c.Close()
+		if err := c.Close(); err != nil {
+			logger.Error("Error closing c", "err", err)
+		}
 		return nil
 	}
 
@@ -80,7 +82,9 @@ func handleConnectMasque(c net.Conn, req *http.Request, logger *slog.Logger) *ma
 		if err != nil {
 			logger.Error("Error calling disallowedRes.Write", "err", err)
 		}
-		c.Close()
+		if err := c.Close(); err != nil {
+			logger.Error("Error closing c", "err", err)
+		}
 		return nil
 	}
 
@@ -90,7 +94,9 @@ func handleConnectMasque(c net.Conn, req *http.Request, logger *slog.Logger) *ma
 		if err != nil {
 			logger.Error("Error calling disallowedRes.Write", "err", err)
 		}
-		c.Close()
+		if err := c.Close(); err != nil {
+			logger.Error("Error closing c", "err", err)
+		}
 		return nil
 	}
 
@@ -101,7 +107,9 @@ func handleConnectMasque(c net.Conn, req *http.Request, logger *slog.Logger) *ma
 		if err != nil {
 			logger.Error("Error calling disallowedRes.Write", "err", err)
 		}
-		c.Close()
+		if err := c.Close(); err != nil {
+			logger.Error("Error closing c", "err", err)
+		}
 		return nil
 	}
 
@@ -132,7 +140,9 @@ func handleReq(c net.Conn, logger *slog.Logger) {
 			if err != nil {
 				logger.Error("Error calling response.Write", "err", err)
 			}
-			c.Close()
+			if err := c.Close(); err != nil {
+				logger.Error("Error closing c", "err", err)
+			}
 			return
 		}
 
@@ -147,7 +157,9 @@ func handleReq(c net.Conn, logger *slog.Logger) {
 			if err != nil {
 				logger.Error("Error calling response.Write", "err", err)
 			}
-			c.Close()
+			if err := c.Close(); err != nil {
+				logger.Error("Error closing c", "err", err)
+			}
 			return
 		}
 	}
@@ -166,8 +178,16 @@ func handleReq(c net.Conn, logger *slog.Logger) {
 		}
 
 		if masqueConn := handleConnectMasque(c, req, logger); masqueConn != nil {
-			defer c.Close()
-			defer masqueConn.Close()
+			defer func() {
+				if err := c.Close(); err != nil {
+					logger.Error("Error closing c", "err", err)
+				}
+			}()
+			defer func() {
+				if err := masqueConn.Close(); err != nil {
+					logger.Error("Error closing masqueConn", "err", err)
+				}
+			}()
 			wg.Add(1)
 			go transfer(masqueConn, c, &wg, logger.WithGroup("connect-first-transfer"))
 			wg.Add(1)
@@ -184,8 +204,16 @@ func handleReq(c net.Conn, logger *slog.Logger) {
 		}
 
 		if masqueConn := handleConnectMasque(c, req, logger); masqueConn != nil {
-			defer c.Close()
-			defer masqueConn.Close()
+			defer func() {
+				if err := c.Close(); err != nil {
+					logger.Error("Error closing c", "err", err)
+				}
+			}()
+			defer func() {
+				if err := masqueConn.Close(); err != nil {
+					logger.Error("Error closing masqueConn", "err", err)
+				}
+			}()
 			// Replay the request to the masque connection.
 			err := req.Write(masqueConn)
 			if err != nil {
@@ -272,7 +300,11 @@ func main() {
 	if err != nil {
 		log.Fatalf("Error in net.Listen: %v", err)
 	}
-	defer l.Close()
+	defer func() {
+		if err := l.Close(); err != nil {
+			logger.Error("Error closing l", "err", err)
+		}
+	}()
 
 	var certData []byte
 	if *certDataFile != "" {

http2/client.go

@@ -480,7 +480,9 @@ loop:
 		}
 	}
 
-	udp.doClose()
+	if err := udp.doClose(); err != nil {
+		c.logger.Error("Error from udp.doClose in encodeLoopUDP", "err", err)
+	}
 }
 
 // decodeLoopUDP decodes data received from the proxied UDP stream.
@@ -520,7 +522,9 @@ loop:
 		}
 	}
 
-	udp.doClose()
+	if err := udp.doClose(); err != nil {
+		c.logger.Error("Error from udp.doClose in decodeLoopUDP", "err", err)
+	}
 }
 
 // CreateUDPStream creates a UDP stream to the given address using the client.
@@ -565,7 +569,9 @@ func (c *Client) CreateUDPStream(addr string) (*Conn, error) {
 	resp, err := http.ReadResponse(br, nil)
 
 	if err != nil {
-		tr.Close()
+		if err := tr.Close(); err != nil {
+			c.logger.Error("Error from tr.Close", "err", err)
+		}
 		return nil, fmt.Errorf("reading HTTP response from CONNECT-UDP to %s via proxy %s failed: %v",
 			addr, c.proxyAddr, err)
 	}

http2/client_test.go

@@ -106,7 +106,8 @@ func TestCreateTCPStream(t *testing.T) {
 	ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		assert.Equal(t, "", r.Header.Get("Proxy-Authorization"), "Should not pass the auth token to the end target server")
 		assert.Equal(t, http.MethodGet, r.Method, "Request to the end target server should be a GET")
-		fmt.Fprintf(w, expectedResponse)
+		_, err := fmt.Fprintf(w, expectedResponse)
+		require.NoError(t, err, "fmt.Fprintf")
 	}))
 	ts.EnableHTTP2 = true
 	// We want to listen on 0.0.0.0 because the proxy container will be on a different non-localhost network.
@@ -118,7 +119,7 @@ func TestCreateTCPStream(t *testing.T) {
 	}
 
 	// Swap out the default test server listener with our custom one listening on 0.0.0.0
-	ts.Listener.Close()
+	require.NoError(t, ts.Listener.Close(), "ts.Listener.Close()")
 	ts.Listener = l
 	ts.StartTLS()
 	defer ts.Close()
@@ -149,7 +150,9 @@ func TestCreateTCPStream(t *testing.T) {
 	dockerHostURL := fmt.Sprintf("%v:%v", containerGateway, port)
 	conn, err := c.CreateTCPStream(dockerHostURL)
 	require.NoError(t, err, "CreateTCPStream")
-	defer conn.Close()
+	defer func() {
+		require.NoError(t, conn.Close(), "conn.Close()")
+	}()
 
 	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://%v", dockerHostURL), nil)
 	require.NoError(t, err, "http.NewRequest")
@@ -178,7 +181,9 @@ func TestCreateTCPStream(t *testing.T) {
 	response, err := httpClient.Do(req)
 	require.NoError(t, err, "httpClient.Do")
 
-	defer response.Body.Close()
+	defer func() {
+		require.NoError(t, response.Body.Close(), "response.Body.Close()")
+	}()
 	data, err := io.ReadAll(response.Body)
 	require.NoError(t, err, "io.ReadAll response body")
 
@@ -234,7 +239,9 @@ func TestCreateUDPStream(t *testing.T) {
 
 	udpConn, err := c.CreateUDPStream(dockerHostURL)
 	require.NoError(t, err, "CreateUDPStream")
-	defer udpConn.Close()
+	defer func() {
+		require.NoError(t, udpConn.Close(), "udpConn.Close()")
+	}()
 
 	_, err = udpConn.Write([]byte(expectedRequest))
 	require.NoError(t, err, "udpConn.Write")

http2/conn.go

@@ -51,6 +51,7 @@ type Conn struct {
 }
 
 func (r *Conn) doClose() error {
+	var err error
 	defer r.cleanup(r.isTcp, r.sid)
 
 	if !r.alive {
@@ -60,18 +61,18 @@ func (r *Conn) doClose() error {
 	r.alive = false
 	// Close |r.IoOut| so that r.Read() returns immediately
 	if r.IoOut != nil {
-		r.IoOut.Close()
+		err = r.IoOut.Close()
 	}
 	// Close decode and encode loops for CONNECT-UDP stream
 	if r.connCancel != nil {
 		r.connCancel()
 	}
 	// Close the underlying tls conn if it is CONNECT-UDP
 	if r.transport != nil {
-		r.transport.Close()
+		err = r.transport.Close()
 	}
 
-	return nil
+	return err
 }
 
 // Sid returns the unique stream ID of the Conn.