Merge pull request #25 from hertz-contrib/fix/cli-conn-close

fix(client): keep ref to resp to avoid conn closed

Author: GuangmingLuo

.github/workflows/pr-check.yml

@@ -1,46 +1,36 @@
 name: Pull Request Check
 
-on: [pull_request]
+on: [ pull_request ]
 
 jobs:
   compliant:
-    runs-on: self-hosted
+    runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Check License Header
-        uses: apache/skywalking-eyes@header@v0.4.0
+        uses: apache/skywalking-eyes/header@v0.4.0
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
-      - name: typos-action
+      - name: Check Spell
         uses: crate-ci/typos@master
 
-  staticcheck:
-    runs-on: self-hosted
+  golangci-lint:
+    runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
-
+      - uses: actions/checkout@v4
       - name: Set up Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: 1.16
-
-      - uses: actions/cache@v3
+        uses: actions/setup-go@v5
         with:
-          path: ~/go/pkg/mod
-          key: reviewdog-${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
-          restore-keys: |
-            reviewdog-${{ runner.os }}-go-
-
-      - uses: reviewdog/action-staticcheck@v1
+          go-version: stable
+          # for self-hosted, the cache path is shared across projects
+          # and it works well without the cache of github actions
+          # Enable it if we're going to use Github only
+          cache: true
+
+      - name: Golangci Lint
+        # https://golangci-lint.run/
+        uses: golangci/golangci-lint-action@v6
         with:
-          github_token: ${{ secrets.github_token }}
-          # Change reviewdog reporter if you need [github-pr-check,github-check,github-pr-review].
-          reporter: github-pr-review
-          # Report all results.
-          filter_mode: nofilter
-          # Exit with 1 when it find at least one finding.
-          fail_on_error: true
-          # Set staticcheck flags
-          staticcheck_flags: -checks=inherit,-SA1029,-SA6002
+          version: latest

.github/workflows/release-check.yml

@@ -1,30 +1,31 @@
 name: Tests
 
-on: [push, pull_request]
+on: [ push, pull_request ]
 
 jobs:
-  lint-and-ut:
-    runs-on: self-hosted
+  benchmark:
+    runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
-
+      - uses: actions/checkout@v4
       - name: Set up Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: 1.18
-
-      - uses: actions/cache@v3
+        uses: actions/setup-go@v5
         with:
-          path: ~/go/pkg/mod
-          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
-          restore-keys: |
-            ${{ runner.os }}-go-
+          go-version: stable
 
-      - name: Lint
-        run: |
-          go vet -stdmethods=false $(go list ./...)
-          go install mvdan.cc/gofumpt@v0.2.0
-          test -z "$(gofumpt -l -extra .)" 
+      - name: Benchmark
+        run: go test -bench=. -benchmem ./...
 
+  uinttest:
+    strategy:
+      matrix:
+        go: [ "1.18", "1.19", "1.20", "1.21", "1.22", "1.23" ]
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ matrix.go }}
+          cache: true # set false for self-hosted
       - name: Unit Test
-        run: go test -race -covermode=atomic -coverprofile=coverage.out ./...
+        run: go test -race ./...

.github/workflows/tests.yml

@@ -0,0 +1,10 @@
+linters: # https://golangci-lint.run/usage/linters/
+  disable-all: true
+  enable:
+    # - errcheck # can not skip _test.go ?
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+    - unconvert

.golangci.yaml

@@ -1,3 +1,10 @@
+// Copyright 2017 The Gorilla WebSocket Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+//
+// This file may have been modified by CloudWeGo authors. All CloudWeGo
+// Modifications are Copyright 2022 CloudWeGo Authors.
+
 package websocket
 
 import (
@@ -86,5 +93,6 @@ func (p *ClientUpgrader) UpgradeResponse(req *protocol.Request, resp *protocol.R
 		conn.newCompressionWriter = compressNoContextTakeover
 		conn.newDecompressionReader = decompressNoContextTakeover
 	}
+	conn.resp = resp
 	return conn, nil
 }

client.go

@@ -1,3 +1,10 @@
+// Copyright 2017 The Gorilla WebSocket Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+//
+// This file may have been modified by CloudWeGo authors. All CloudWeGo
+// Modifications are Copyright 2022 CloudWeGo Authors.
+
 package websocket
 
 import (

client_test.go

@@ -282,6 +282,10 @@ type Conn struct {
 
 	readDecompress         bool // whether last read frame had RSV1 set
 	newDecompressionReader func(io.Reader) io.ReadCloser
+
+	// keep reference to the resp to make sure the underlying conn will not be closed.
+	// see: https://github.com/cloudwego/hertz/pull/1214 for the details.
+	resp interface{} // *protocol.Response
 }
 
 func newConn(conn net.Conn, isServer bool, readBufferSize, writeBufferSize int, writeBufferPool BufferPool, br *bufio.Reader, writeBuf []byte) *Conn {

conn.go

@@ -86,7 +86,7 @@ func TestFraming(t *testing.T) {
 			return w.Write(writeBuf[:n])
 		}},
 		{"string", func(w io.Writer, n int) (int, error) {
-			return io.WriteString(w, string(writeBuf[:n]))
+			return io.WriteString(w, string(writeBuf[:n])) // nolint: staticcheck
 		}},
 	}
 

conn_test.go

@@ -211,7 +211,9 @@ func (u *HertzUpgrader) Upgrade(ctx *app.RequestContext, handler HertzHandler) e
 		handler(c)
 
 		writeBuf = writeBuf[0:0]
-		poolWriteBuffer.Put(writeBuf)
+
+		// FIXME: argument should be pointer-like to avoid allocations (staticcheck)
+		poolWriteBuffer.Put(writeBuf) // nolint: staticcheck
 	})
 
 	return nil