diff --git a/.circleci/config.yml b/.circleci/config.yml index b34f651f..65b17aa0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,54 +2,59 @@ version: 2 jobs: fmt: docker: - - image: golang:1 + - image: nhooyr/websocket-ci steps: - checkout - restore_cache: keys: - - go-{{ checksum "go.sum" }} + - go-v3-{{ checksum "go.sum" }} # Fallback to using the latest cache if no exact match is found. - - go- + - go-v3- - run: ./ci/fmt.sh - save_cache: paths: - - /go + - /root/gopath - /root/.cache/go-build - key: go-{{ checksum "go.sum" }} + key: go-v3-{{ checksum "go.sum" }} lint: docker: - - image: golang:1 + - image: nhooyr/websocket-ci steps: - checkout - restore_cache: keys: - - go-{{ checksum "go.sum" }} + - go-v3-{{ checksum "go.sum" }} # Fallback to using the latest cache if no exact match is found. - - go- + - go-v3- - run: ./ci/lint.sh - save_cache: paths: - - /go + - /root/gopath - /root/.cache/go-build - key: go-{{ checksum "go.sum" }} + key: go-v3-{{ checksum "go.sum" }} test: docker: - - image: golang:1 + - image: nhooyr/websocket-ci steps: - checkout - restore_cache: keys: - - go-{{ checksum "go.sum" }} + - go-v3-{{ checksum "go.sum" }} # Fallback to using the latest cache if no exact match is found. - - go- + - go-v3- - run: ./ci/test.sh + - store_artifacts: + path: ci/out + destination: out - save_cache: paths: - - /go + - /root/gopath - /root/.cache/go-build - key: go-{{ checksum "go.sum" }} + key: go-v3-{{ checksum "go.sum" }} + - store_test_results: + path: ci/out workflows: version: 2 diff --git a/README.md b/README.md index d30ee5f5..f3bd6a84 100644 --- a/README.md +++ b/README.md @@ -42,15 +42,15 @@ http.HandlerFunc(func (w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), time.Second*10) defer cancel() - + var v interface{} err = wsjson.Read(ctx, c, &v) if err != nil { // ... } - + log.Printf("received: %v", v) - + c.Close(websocket.StatusNormalClosure, "") }) ``` @@ -93,7 +93,7 @@ c.Close(websocket.StatusNormalClosure, "") ## Comparison Before the comparison, I want to point out that both gorilla/websocket and gobwas/ws were -extremely useful in implementing the WebSocket protocol correctly so *big thanks* to the +extremely useful in implementing the WebSocket protocol correctly so _big thanks_ to the authors of both. In particular, I made sure to go through the issue tracker of gorilla/websocket to ensure I implemented details correctly and understood how people were using WebSockets in production. @@ -111,7 +111,7 @@ Just compare the godoc of The API for nhooyr/websocket has been designed such that there is only one way to do things which makes it easy to use correctly. Not only is the API simpler, the implementation is only 1700 lines whereas gorilla/websocket is at 3500 lines. That's more code to maintain, - more code to test, more code to document and more surface area for bugs. +more code to test, more code to document and more surface area for bugs. Moreover, nhooyr/websocket has support for newer Go idioms such as context.Context and also uses net/http's Client and ResponseWriter directly for WebSocket handshakes. @@ -151,7 +151,7 @@ and clarity. This library is fantastic in terms of performance. The author put in significant effort to ensure its speed and I have applied as many of its optimizations as -I could into nhooyr/websocket. Definitely check out his fantastic [blog post](https://medium.freecodecamp.org/million-websockets-and-go-cc58418460bb) +I could into nhooyr/websocket. Definitely check out his fantastic [blog post](https://medium.freecodecamp.org/million-websockets-and-go-cc58418460bb) about performant WebSocket servers. If you want a library that gives you absolute control over everything, this is the library, diff --git a/ci/.gitignore b/ci/.gitignore new file mode 100644 index 00000000..1fcb1529 --- /dev/null +++ b/ci/.gitignore @@ -0,0 +1 @@ +out diff --git a/ci/bench.sh b/ci/bench.sh deleted file mode 100755 index a3d81b26..00000000 --- a/ci/bench.sh +++ /dev/null @@ -1,16 +0,0 @@ -#!/usr/bin/env bash - -set -euo pipefail -cd "$(dirname "${0}")" -source ./lib.sh - -go test --vet=off --run=^$ -bench=. -o=ci/out/websocket.test \ - -cpuprofile=ci/out/cpu.prof \ - -memprofile=ci/out/mem.prof \ - -blockprofile=ci/out/block.prof \ - -mutexprofile=ci/out/mutex.prof \ - . - -echo -echo "Profiles are in ./ci/out/*.prof -Keep in mind that every profiler Go provides is enabled so that may skew the benchmarks." diff --git a/ci/fmt.sh b/ci/fmt.sh index 52ef3fd1..a4e7ff02 100755 --- a/ci/fmt.sh +++ b/ci/fmt.sh @@ -2,11 +2,7 @@ set -euo pipefail cd "$(dirname "${0}")" -source ./lib.sh - -unstaged_files() { - git ls-files --other --modified --exclude-standard -} +cd "$(git rev-parse --show-toplevel)" gen() { # Unfortunately, this is the only way to ensure go.mod and go.sum are correct. @@ -21,17 +17,33 @@ fmt() { gofmt -w -s . go run go.coder.com/go-tools/cmd/goimports -w "-local=$(go list -m)" . go run mvdan.cc/sh/cmd/shfmt -i 2 -w -s -sr . + # shellcheck disable=SC2046 + npx prettier \ + --write \ + --print-width 120 \ + --no-semi \ + --trailing-comma all \ + --loglevel silent \ + $(git ls-files "*.yaml" "*.yml" "*.md") +} + +unstaged_files() { + git ls-files --other --modified --exclude-standard +} + +check() { + if [[ ${CI:-} && $(unstaged_files) != "" ]]; then + echo + echo "Files need generation or are formatted incorrectly." + echo "Please run:" + echo "./ci/fmt.sh" + echo + git status + git diff + exit 1 + fi } gen fmt - -if [[ $CI && $(unstaged_files) != "" ]]; then - echo - echo "Files either need generation or are formatted incorrectly." - echo "Please run:" - echo "./ci/fmt.sh" - echo - git status - exit 1 -fi +check diff --git a/ci/image/Dockerfile b/ci/image/Dockerfile new file mode 100644 index 00000000..d435e949 --- /dev/null +++ b/ci/image/Dockerfile @@ -0,0 +1,13 @@ +FROM golang:1 + +ENV DEBIAN_FRONTEND=noninteractive +ENV GOPATH=/root/gopath +ENV GOFLAGS="-mod=readonly" +ENV PAGER=cat + +RUN apt-get update && \ + apt-get install -y shellcheck python-pip npm && \ + pip2 install autobahntestsuite && \ + npm install -g prettier + +RUN git config --global color.ui always \ No newline at end of file diff --git a/ci/image/push.sh b/ci/image/push.sh new file mode 100755 index 00000000..1cbae979 --- /dev/null +++ b/ci/image/push.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash + +set -euo pipefail +cd "$(dirname "${0}")" +cd "$(git rev-parse --show-toplevel)" + +docker build -f ./ci/image/Dockerfile -t nhooyr/websocket-ci . +docker push nhooyr/websocket-ci diff --git a/ci/lib.sh b/ci/lib.sh deleted file mode 100644 index 590e7908..00000000 --- a/ci/lib.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/usr/bin/env bash - -set -euo pipefail - -# Ensures $CI can be used if it's set or not. -export CI=${CI:-} -if [[ $CI ]]; then - export GOFLAGS=-mod=readonly - export DEBIAN_FRONTEND=noninteractive -fi - -cd "$(git rev-parse --show-toplevel)" diff --git a/ci/lint.sh b/ci/lint.sh index 65655451..b7268c55 100755 --- a/ci/lint.sh +++ b/ci/lint.sh @@ -2,14 +2,9 @@ set -euo pipefail cd "$(dirname "${0}")" -source ./lib.sh - -if [[ $CI ]]; then - apt-get update -qq - apt-get install -qq shellcheck > /dev/null -fi +cd "$(git rev-parse --show-toplevel)" # shellcheck disable=SC2046 -shellcheck -e SC1091 -x $(git ls-files "*.sh") +shellcheck -x $(git ls-files "*.sh") go vet ./... go run golang.org/x/lint/golint -set_exit_status ./... diff --git a/ci/out/.gitignore b/ci/out/.gitignore deleted file mode 100644 index 72e8ffc0..00000000 --- a/ci/out/.gitignore +++ /dev/null @@ -1 +0,0 @@ -* diff --git a/ci/run.sh b/ci/run.sh index 53f04278..56da2d93 100755 --- a/ci/run.sh +++ b/ci/run.sh @@ -4,8 +4,8 @@ set -euo pipefail cd "$(dirname "${0}")" -source ./lib.sh +cd "$(git rev-parse --show-toplevel)" -./fmt.sh -./lint.sh -./test.sh +./ci/fmt.sh +./ci/lint.sh +./ci/test.sh diff --git a/ci/test.sh b/ci/test.sh index f08c2ddf..ab101e91 100755 --- a/ci/test.sh +++ b/ci/test.sh @@ -2,29 +2,23 @@ set -euo pipefail cd "$(dirname "${0}")" -source ./lib.sh +cd "$(git rev-parse --show-toplevel)" -echo "This step includes benchmarks for race detection and coverage purposes -but the numbers will be misleading. please see the bench step or ./bench.sh for -more accurate numbers." -echo +mkdir -p ci/out/websocket +testFlags=( + -race + "-vet=off" + "-bench=." + "-coverprofile=ci/out/coverage.prof" + "-coverpkg=./..." +) +# https://circleci.com/docs/2.0/collect-test-data/ +go run gotest.tools/gotestsum \ + --junitfile ci/out/websocket/testReport.xml \ + --format=short-verbose \ + -- "${testFlags[@]}" -if [[ $CI ]]; then - apt-get update -qq - apt-get install -qq python-pip > /dev/null - # Need to add pip install directory to $PATH. - export PATH="/home/circleci/.local/bin:$PATH" - pip install -qqq autobahntestsuite -fi - -go test -race -coverprofile=ci/out/coverage.prof --vet=off -bench=. -coverpkg=./... "$@" ./... -go tool cover -func=ci/out/coverage.prof - -if [[ $CI ]]; then +go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html +if [[ ${CI:-} ]]; then bash <(curl -s https://codecov.io/bash) -f ci/out/coverage.prof -else - go tool cover -html=ci/out/coverage.prof -o=ci/out/coverage.html - - echo - echo "Please open ci/out/coverage.html to see detailed test coverage stats." fi diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index ade2013f..2d9104d1 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -12,17 +12,26 @@ Please capitalize the first word in the commit message title. The commit message title should use the verb tense + phrase that completes the blank in -> This change modifies websocket to ___________ +> This change modifies websocket to \_\_\_\_\_\_\_\_\_ Be sure to link to an existing issue if one exists. In general, try creating an issue before making a PR to get some discussion going and to make sure you do not spend time on a PR that may be rejected. -Run `ci/run.sh` to test your changes. You'll need [shellcheck](https://github.com/koalaman/shellcheck#installing), the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite) and Go. +You can run tests normally with `go test`. +You'll need the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite). +In the future this dependency will be removed. See [#117](https://github.com/nhooyr/websocket/issues/117). -See [../ci/lint.sh](../ci/lint.sh) and [../ci/lint.sh](../ci/test.sh) for the -installation of shellcheck and the Autobahn test suite on Debian or Ubuntu. +Please ensure CI passes for your changes. If necessary, you may run CI locally. +The various steps are located in `ci/*.sh`. -For Go, please refer to the [offical docs](https://golang.org/doc/install). +`ci/fmt.sh` requires node (specifically prettier). +`ci/lint.sh` requires [shellcheck](https://github.com/koalaman/shellcheck#installing). +`ci/test.sh` requires the [Autobahn Test suite pip package](https://github.com/crossbario/autobahn-testsuite). +`ci/run.sh` runs everything in the above order and requires all of their dependencies. -You can benchmark the library with `./ci/benchmark.sh`. You only need Go to run that script. +See [../ci/image/Dockerfile](../ci/image/Dockerfile) for the installation of the CI dependencies on Ubuntu. + +For CI coverage, you can use either [codecov](https://codecov.io/gh/nhooyr/websocket) or click the +`coverage.html` artifact on the test step in CI. +For coverage details locally, please see `ci/out/coverage.html` after running `ci/run.sh` or `ci/test.sh`. diff --git a/go.mod b/go.mod index cc9a865d..58d79bf1 100644 --- a/go.mod +++ b/go.mod @@ -13,5 +13,8 @@ require ( golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 golang.org/x/tools v0.0.0-20190429184909-35c670923e21 golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522 + gotest.tools/gotestsum v0.3.5 mvdan.cc/sh v2.6.4+incompatible ) + +replace gotest.tools/gotestsum => github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b diff --git a/go.sum b/go.sum index 7965958d..98b766bf 100644 --- a/go.sum +++ b/go.sum @@ -1,22 +1,63 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/fatih/color v1.6.0 h1:66qjqZk8kalYAvDRtM1AdAJQI0tj4Wrue3Eq3B3pmFU= +github.com/fatih/color v1.6.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= +github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= +github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= +github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1 h1:YF8+flBXS5eO826T4nzqPrxfhQThhXl0YzfuUPu4SBg= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= +github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= +github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= +github.com/jonboulle/clockwork v0.1.0 h1:VKV+ZcuP6l3yW9doeqz6ziZGgcynBVQO+obU0+0hcPo= +github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/mattn/go-colorable v0.0.9 h1:UVL0vNpWh04HeJXV0KLcaT7r06gOH2l4OW6ddYRUIY4= +github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= +github.com/mattn/go-isatty v0.0.3 h1:ns/ykhmWi7G9O+8a448SecJU3nSMBXJfqQkl0upE1jI= +github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= +github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b h1:t6DbmxEtGMM72Uhs638nBOyK7tjsrDwoMfYO1EfQdFE= +github.com/nhooyr/gotestsum v0.3.6-0.20190821172136-aaabbb33254b/go.mod h1:Mnf3e5FUzXbkCfynWBGOwLssY7gTQgCHObK9tMpAriY= +github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= +github.com/onsi/ginkgo v1.8.0 h1:VkHVNpR4iVnU8XQR6DBm8BqYjN7CRzw+xKUbVVbbW9w= +github.com/onsi/ginkgo v1.8.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= +github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU= +github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= +github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw= +github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/sirupsen/logrus v1.0.5 h1:8c8b5uO0zS4X6RPl/sd1ENwSkIc0/H2PaHxE3udaE8I= +github.com/sirupsen/logrus v1.0.5/go.mod h1:pMByvHTf9Beacp5x1UXfOR9xyW/9antXMhjMPG0dEzc= +github.com/spf13/pflag v1.0.1 h1:aCvUg6QPl3ibpQUxyLkrEkCHtPqYJL4x9AuhqVqFis4= +github.com/spf13/pflag v1.0.1/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= go.coder.com/go-tools v0.0.0-20190317003359-0c6a35b74a16 h1:3gGa1bM0nG7Ruhu5b7wKnoOOwAD/fJ8iyyAcpOzDG3A= go.coder.com/go-tools v0.0.0-20190317003359-0c6a35b74a16/go.mod h1:iKV5yK9t+J5nG9O3uF6KYdPEz3dyfMyB15MN1rbQ8Qw= +golang.org/x/crypto v0.0.0-20180426230345-b49d69b5da94/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/lint v0.0.0-20190409202823-959b441ac422 h1:QzoH/1pFpZguR8NrRHLcO6jKqfv2zpuSqZLgdm7ZmjI= golang.org/x/lint v0.0.0-20190409202823-959b441ac422/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/net v0.0.0-20181102091132-c10e9556a7bc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190424112056-4829fb13d2c6 h1:FP8hkuE6yUEaJnK7O2eTuejKWwW+Rhfj80dQ2JcKxCU= golang.org/x/net v0.0.0-20190424112056-4829fb13d2c6/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv3hCI0z56oJR5vAMgBU= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -30,5 +71,19 @@ golang.org/x/tools v0.0.0-20190429184909-35c670923e21 h1:Kjcw+D2LTzLmxOHrMK9uvYP golang.org/x/tools v0.0.0-20190429184909-35c670923e21/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522 h1:bhOzK9QyoD0ogCnFro1m2mz41+Ib0oOhfJnBp5MR4K4= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/airbrake/gobrake.v2 v2.0.9 h1:7z2uVWwn7oVeeugY1DtlPAy5H+KYgB1KeKTnqjNatLo= +gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4= +gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= +gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2 h1:OAj3g0cR6Dx/R07QgQe8wkA9RNjB2u4i700xBkIT4e0= +gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKWaSkCsqBpgog8nAV2xsGOxlo= +gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= +gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= +gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE= +gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gotest.tools v2.1.0+incompatible h1:5USw7CrJBYKqjg9R7QlA6jzqZKEAtvW82aNmsxxGPxw= +gotest.tools v2.1.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= mvdan.cc/sh v2.6.4+incompatible h1:eD6tDeh0pw+/TOTI1BBEryZ02rD2nMcFsgcvde7jffM= mvdan.cc/sh v2.6.4+incompatible/go.mod h1:IeeQbZq+x2SUGBensq/jge5lLQbS3XT2ktyp3wrt4x8= diff --git a/netconn.go b/netconn.go index 25787703..d28eeb84 100644 --- a/netconn.go +++ b/netconn.go @@ -33,7 +33,8 @@ import ( // The Addr methods will return a mock net.Addr that returns "websocket" for Network // and "websocket/unknown-addr" for String. // -// A received StatusNormalClosure close frame will be translated to EOF when reading. +// A received StatusNormalClosure or StatusGoingAway close frame will be translated to +// io.EOF when reading. func NetConn(c *Conn, msgType MessageType) net.Conn { nc := &netConn{ c: c, @@ -93,7 +94,7 @@ func (c *netConn) Read(p []byte) (int, error) { typ, r, err := c.c.Reader(c.readContext) if err != nil { var ce CloseError - if xerrors.As(err, &ce) && (ce.Code == StatusNormalClosure) { + if xerrors.As(err, &ce) && (ce.Code == StatusNormalClosure) || (ce.Code == StatusGoingAway) { c.eofed = true return 0, io.EOF } diff --git a/tools.go b/tools.go index c78042b7..a6f0268e 100644 --- a/tools.go +++ b/tools.go @@ -7,5 +7,6 @@ import ( _ "go.coder.com/go-tools/cmd/goimports" _ "golang.org/x/lint/golint" _ "golang.org/x/tools/cmd/stringer" + _ "gotest.tools/gotestsum" _ "mvdan.cc/sh/cmd/shfmt" ) diff --git a/websocket.go b/websocket.go index ee61f543..393ea547 100644 --- a/websocket.go +++ b/websocket.go @@ -46,6 +46,7 @@ type Conn struct { closeErr error closed chan struct{} + // messageWriter state. // writeMsgLock is acquired to write a data message. writeMsgLock chan struct{} // writeFrameLock is acquired to write a single frame. @@ -56,6 +57,8 @@ type Conn struct { // read limit for a message in bytes. msgReadLimit int64 + // Used to ensure a previous writer is not used after being closed. + activeWriter *messageWriter // messageWriter state. writeMsgOpcode opcode writeMsgCtx context.Context @@ -63,18 +66,18 @@ type Conn struct { // Used to ensure the previous reader is read till EOF before allowing // a new one. - previousReader *messageReader + activeReader *messageReader // readFrameLock is acquired to read from bw. readFrameLock chan struct{} readClosed int64 readHeaderBuf []byte controlPayloadBuf []byte - // messageReader state - readMsgCtx context.Context - readMsgHeader header - readFrameEOF bool - readMaskPos int + // messageReader state. + readerMsgCtx context.Context + readerMsgHeader header + readerFrameEOF bool + readerMaskPos int setReadTimeout chan context.Context setWriteTimeout chan context.Context @@ -358,7 +361,7 @@ func (c *Conn) Reader(ctx context.Context) (MessageType, io.Reader, error) { } func (c *Conn) reader(ctx context.Context) (MessageType, io.Reader, error) { - if c.previousReader != nil && !c.readFrameEOF { + if c.activeReader != nil && !c.readerFrameEOF { // The only way we know for sure the previous reader is not yet complete is // if there is an active frame not yet fully read. // Otherwise, a user may have read the last byte but not the EOF if the EOF @@ -371,7 +374,7 @@ func (c *Conn) reader(ctx context.Context) (MessageType, io.Reader, error) { return 0, nil, err } - if c.previousReader != nil && !c.previousReader.eof { + if c.activeReader != nil && !c.activeReader.eof() { if h.opcode != opContinuation { err := xerrors.Errorf("received new data message without finishing the previous message") c.Close(StatusProtocolError, err.Error()) @@ -382,7 +385,7 @@ func (c *Conn) reader(ctx context.Context) (MessageType, io.Reader, error) { return 0, nil, xerrors.Errorf("previous message not read to completion") } - c.previousReader.eof = true + c.activeReader = nil h, err = c.readTillMsg(ctx) if err != nil { @@ -394,16 +397,16 @@ func (c *Conn) reader(ctx context.Context) (MessageType, io.Reader, error) { return 0, nil, err } - c.readMsgCtx = ctx - c.readMsgHeader = h - c.readFrameEOF = false - c.readMaskPos = 0 + c.readerMsgCtx = ctx + c.readerMsgHeader = h + c.readerFrameEOF = false + c.readerMaskPos = 0 c.readMsgLeft = c.msgReadLimit r := &messageReader{ c: c, } - c.previousReader = r + c.activeReader = r return MessageType(h.opcode), r, nil } @@ -430,8 +433,11 @@ func (c *Conn) CloseRead(ctx context.Context) context.Context { // messageReader enables reading a data frame from the WebSocket connection. type messageReader struct { - c *Conn - eof bool + c *Conn +} + +func (r *messageReader) eof() bool { + return r.c.activeReader != r } // Read reads as many bytes as possible into p. @@ -449,7 +455,7 @@ func (r *messageReader) Read(p []byte) (int, error) { } func (r *messageReader) read(p []byte) (int, error) { - if r.eof { + if r.eof() { return 0, xerrors.Errorf("cannot use EOFed reader") } @@ -463,8 +469,8 @@ func (r *messageReader) read(p []byte) (int, error) { p = p[:r.c.readMsgLeft] } - if r.c.readFrameEOF { - h, err := r.c.readTillMsg(r.c.readMsgCtx) + if r.c.readerFrameEOF { + h, err := r.c.readTillMsg(r.c.readerMsgCtx) if err != nil { return 0, err } @@ -475,34 +481,34 @@ func (r *messageReader) read(p []byte) (int, error) { return 0, err } - r.c.readMsgHeader = h - r.c.readFrameEOF = false - r.c.readMaskPos = 0 + r.c.readerMsgHeader = h + r.c.readerFrameEOF = false + r.c.readerMaskPos = 0 } - h := r.c.readMsgHeader + h := r.c.readerMsgHeader if int64(len(p)) > h.payloadLength { p = p[:h.payloadLength] } - n, err := r.c.readFramePayload(r.c.readMsgCtx, p) + n, err := r.c.readFramePayload(r.c.readerMsgCtx, p) h.payloadLength -= int64(n) r.c.readMsgLeft -= int64(n) if h.masked { - r.c.readMaskPos = fastXOR(h.maskKey, r.c.readMaskPos, p) + r.c.readerMaskPos = fastXOR(h.maskKey, r.c.readerMaskPos, p) } - r.c.readMsgHeader = h + r.c.readerMsgHeader = h if err != nil { return n, err } if h.payloadLength == 0 { - r.c.readFrameEOF = true + r.c.readerFrameEOF = true if h.fin { - r.eof = true + r.c.activeReader = nil return n, io.EOF } } @@ -593,9 +599,11 @@ func (c *Conn) writer(ctx context.Context, typ MessageType) (io.WriteCloser, err } c.writeMsgCtx = ctx c.writeMsgOpcode = opcode(typ) - return &messageWriter{ + w := &messageWriter{ c: c, - }, nil + } + c.activeWriter = w + return w, nil } // Write is a convenience method to write a message to the connection. @@ -622,8 +630,11 @@ func (c *Conn) write(ctx context.Context, typ MessageType, p []byte) (int, error // messageWriter enables writing to a WebSocket connection. type messageWriter struct { - c *Conn - closed bool + c *Conn +} + +func (w *messageWriter) closed() bool { + return w != w.c.activeWriter } // Write writes the given bytes to the WebSocket connection. @@ -636,7 +647,7 @@ func (w *messageWriter) Write(p []byte) (int, error) { } func (w *messageWriter) write(p []byte) (int, error) { - if w.closed { + if w.closed() { return 0, xerrors.Errorf("cannot use closed writer") } n, err := w.c.writeFrame(w.c.writeMsgCtx, false, w.c.writeMsgOpcode, p) @@ -658,10 +669,10 @@ func (w *messageWriter) Close() error { } func (w *messageWriter) close() error { - if w.closed { + if w.closed() { return xerrors.Errorf("cannot use closed writer") } - w.closed = true + w.c.activeWriter = nil _, err := w.c.writeFrame(w.c.writeMsgCtx, true, w.c.writeMsgOpcode, nil) if err != nil { diff --git a/websocket_test.go b/websocket_test.go index 06e0fc6d..cd6bdaf5 100644 --- a/websocket_test.go +++ b/websocket_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "io/ioutil" + "net" "net/http" "net/http/cookiejar" "net/http/httptest" @@ -781,12 +782,28 @@ func discardLoop(ctx context.Context, c *websocket.Conn) { } } +func unusedListenAddr() (string, error) { + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + return "", err + } + l.Close() + return l.Addr().String(), nil +} + // https://github.com/crossbario/autobahn-python/blob/master/wstest/testee_client_aio.py func TestAutobahnClient(t *testing.T) { t.Parallel() + serverAddr, err := unusedListenAddr() + if err != nil { + t.Fatalf("failed to get unused listen addr for wstest: %v", err) + } + + wsServerURL := "ws://" + serverAddr + spec := map[string]interface{}{ - "url": "ws://localhost:9001", + "url": wsServerURL, "outdir": "ci/out/wstestClientReports", "cases": []string{"*"}, // See TestAutobahnServer for the reasons why we exclude these. @@ -814,9 +831,10 @@ func TestAutobahnClient(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, time.Minute*10) defer cancel() - args := []string{"--mode", "fuzzingserver", "--spec", specFile.Name()} - if os.Getenv("CI") == "" { - args = append([]string{"--debug"}, args...) + args := []string{"--mode", "fuzzingserver", "--spec", specFile.Name(), + // Disables some server that runs as part of fuzzingserver mode. + // See https://github.com/crossbario/autobahn-testsuite/blob/058db3a36b7c3a1edf68c282307c6b899ca4857f/autobahntestsuite/autobahntestsuite/wstest.py#L124 + "--webport=0", } wstest := exec.CommandContext(ctx, "wstest", args...) err = wstest.Start() @@ -835,9 +853,9 @@ func TestAutobahnClient(t *testing.T) { var cases int func() { - c, _, err := websocket.Dial(ctx, "ws://localhost:9001/getCaseCount", websocket.DialOptions{}) + c, _, err := websocket.Dial(ctx, wsServerURL+"/getCaseCount", websocket.DialOptions{}) if err != nil { - t.Fatalf("failed to dial: %v", err) + t.Fatal(err) } defer c.Close(websocket.StatusInternalError, "") @@ -862,17 +880,17 @@ func TestAutobahnClient(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, time.Second*45) defer cancel() - c, _, err := websocket.Dial(ctx, fmt.Sprintf("ws://localhost:9001/runCase?case=%v&agent=main", i), websocket.DialOptions{}) + c, _, err := websocket.Dial(ctx, fmt.Sprintf(wsServerURL+"/runCase?case=%v&agent=main", i), websocket.DialOptions{}) if err != nil { - t.Fatalf("failed to dial: %v", err) + t.Fatal(err) } echoLoop(ctx, c) }() } - c, _, err := websocket.Dial(ctx, fmt.Sprintf("ws://localhost:9001/updateReports?agent=main"), websocket.DialOptions{}) + c, _, err := websocket.Dial(ctx, fmt.Sprintf(wsServerURL+"/updateReports?agent=main"), websocket.DialOptions{}) if err != nil { - t.Fatalf("failed to dial: %v", err) + t.Fatal(err) } c.Close(websocket.StatusNormalClosure, "") @@ -915,9 +933,7 @@ func checkWSTestIndex(t *testing.T, path string) { if failed { path = strings.Replace(path, ".json", ".html", 1) if os.Getenv("CI") == "" { - t.Errorf("wstest found failure, please see %q", path) - } else { - t.Errorf("wstest found failure, please run test.sh locally to see %q", path) + t.Errorf("wstest found failure, please see %q (output as an artifact in CI)", path) } } } @@ -944,7 +960,7 @@ func benchConn(b *testing.B, echo, stream bool, size int) { c, _, err := websocket.Dial(ctx, wsURL, websocket.DialOptions{}) if err != nil { - b.Fatalf("failed to dial: %v", err) + b.Fatal(err) } defer c.Close(websocket.StatusInternalError, "") @@ -1019,7 +1035,7 @@ func BenchmarkConn(b *testing.B) { b.Run("echo", func(b *testing.B) { for _, size := range sizes { b.Run(strconv.Itoa(size), func(b *testing.B) { - benchConn(b, false, false, size) + benchConn(b, true, true, size) }) } })