Merge pull request #68 from nhooyr/docs

General Improvements

Author: nhooyr

.github/ISSUE_TEMPLATE/bug_report.md

@@ -4,7 +4,7 @@
 
 websocket is a minimal and idiomatic WebSocket library for Go.
 
-At minimum Go 1.12 is required as websocket uses a new [feature](https://github.com/golang/go/issues/26937#issuecomment-415855861) in net/http
+Go 1.12 is required as it uses a new [feature](https://github.com/golang/go/issues/26937#issuecomment-415855861) in net/http
 to perform WebSocket handshakes.
 
 This library is not final and the API is subject to change.
@@ -19,13 +19,12 @@ go get nhooyr.io/websocket
 
 ## Features
 
-- Full support of the WebSocket protocol
-- Zero dependencies outside of the stdlib
-- Very minimal and carefully considered API
-- context.Context is first class
-- net/http is used for WebSocket dials and upgrades
+- Minimal yet pragmatic API
+- First class context.Context support
 - Thoroughly tested, fully passes the [autobahn-testsuite](https://github.com/crossbario/autobahn-testsuite)
-- All returned errors include detailed context
+- Concurrent writes
+- Zero dependencies outside of the stdlib for the core library
+- JSON and ProtoBuf helpers in the wsjson and wspb subpackages
 
 ## Roadmap
 
@@ -97,45 +96,45 @@ c.Close(websocket.StatusNormalClosure, "")
 
 ## Design considerations
 
-- Minimal API is easier to maintain and for others to learn
+- Minimal API is easier to maintain and learn
 - Context based cancellation is more ergonomic and robust than setting deadlines
-- No pings or pongs because TCP keep alives work fine for HTTP/1.1 and they do not make
+- No ping support because TCP keep alives work fine for HTTP/1.1 and they do not make
   sense with HTTP/2 (see #1)
 - net.Conn is never exposed as WebSocket's over HTTP/2 will not have a net.Conn.
-- Functional options make the API very clean and easy to extend
+- Structures are nicer than functional options, see [google/go-cloud#908](https://github.com/google/go-cloud/issues/908#issuecomment-445034143)
 - Using net/http's Client for dialing means we do not have to reinvent dialing hooks
-  and configurations. Just pass in a custom net/http client if you want custom dialing.
+  and configurations like other WebSocket libraries
 
 ## Comparison
 
 While I believe nhooyr/websocket has a better API than existing libraries, 
 both gorilla/websocket and gobwas/ws were extremely useful in implementing the
-WebSocket protocol correctly so big thanks to the authors of both. In particular,
+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 make sure
-I implemented details correctly.
+I implemented details correctly and understood how people were using the package
+in production.
 
 ### gorilla/websocket
 
 https://github.com/gorilla/websocket
 
-This package is the community standard but it is very old and over time
-has accumulated cruft. There are many ways to do the same thing and the API
-is not clear. Just compare the godoc of
+This package is the community standard but it is 6 years old and over time
+has accumulated cruft. There are many ways to do the same thing, usage is not clear
+and there are some rough edges. Just compare the godoc of
 [nhooyr/websocket](godoc.org/github.com/nhooyr/websocket) side by side with
 [gorilla/websocket](godoc.org/github.com/gorilla/websocket).
 
 The API for nhooyr/websocket has been designed such that there is only one way to do things
-which makes using it correctly and safely much easier.
-
-In terms of lines of code, this library is around 2000 whereas gorilla/websocket is
-at 7000. So while the API for nhooyr/websocket is simpler, the implementation is also
-significantly simpler and easier to test which reduces the surface are of bugs.
+which makes it easy to use correctly.
 
 Furthermore, 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.
 gorilla/websocket writes its handshakes directly to a net.Conn which means
 it has to reinvent hooks for TLS and proxying and prevents support of HTTP/2.
 
+Another advantage of nhooyr/websocket is that it supports multiple concurrent writers out
+of the box.
+
 ### x/net/websocket
 
 https://godoc.org/golang.org/x/net/websocket
@@ -149,12 +148,12 @@ See https://github.com/golang/go/issues/18152
 https://github.com/gobwas/ws
 
 This library has an extremely flexible API but that comes at the cost of usability
-and clarity. Its not clear what the best way to do anything is.
+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.
+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,
-but for most users, the API provided by nhooyr/websocket will definitely fit better as it will
-be just as performant but much easier to use correctly.
+but for most users, the API provided by nhooyr/websocket will fit better as it is just as
+performant but much easier to use correctly and idiomatic.

.github/ISSUE_TEMPLATE/feature_request.md

@@ -34,8 +34,8 @@ type AcceptOptions struct {
 	// The only time you need this is if your javascript is running on a different domain
 	// than your WebSocket server.
 	// Please think carefully about whether you really need this option before you use it.
-	// If you do, remember if you store secure data in cookies, you wil need to verify the
-	// Origin header.
+	// If you do, remember that if you store secure data in cookies, you wil need to verify the
+	// Origin header yourself otherwise you are exposing yourself to a CSRF attack.
 	InsecureSkipVerify bool
 }
 

README.md

@@ -85,14 +85,14 @@ func benchConn(b *testing.B, stream bool) {
 			})
 		}
 
-		// runN(32)
-		// runN(128)
-		// runN(512)
-		// runN(1024)
+		runN(32)
+		runN(128)
+		runN(512)
+		runN(1024)
 		runN(4096)
 		runN(16384)
-		// runN(65536)
-		// runN(131072)
+		runN(65536)
+		runN(131072)
 
 		c.Close(websocket.StatusNormalClosure, "")
 	})

accept.go

@@ -9,7 +9,7 @@ go test --vet=off --run=^$ -bench=. \
 	-memprofile=profs/mem \
 	-blockprofile=profs/block \
 	-mutexprofile=profs/mutex \
-	./...
+	.
 
 set +x
 echo

bench_test.go

@@ -5,9 +5,11 @@ source ci/lib.sh || exit 1
 mkdir -p profs
 
 set +x
+echo
 echo "this step includes benchmarks for race detection and coverage purposes
 but the numbers will be misleading. please see the bench step for more
 accurate numbers"
+echo
 set -x
 
 go test -race -coverprofile=profs/coverage --vet=off -bench=. ./...

ci/bench/entrypoint.sh

@@ -22,8 +22,8 @@ type DialOptions struct {
 	// http.Transport does this correctly.
 	HTTPClient *http.Client
 
-	// Header specifies the HTTP headers included in the handshake request.
-	Header http.Header
+	// HTTPHeader specifies the HTTP headers included in the handshake request.
+	HTTPHeader http.Header
 
 	// Subprotocols lists the subprotocols to negotiate with the server.
 	Subprotocols []string
@@ -47,8 +47,11 @@ func dial(ctx context.Context, u string, opts DialOptions) (_ *Conn, _ *http.Res
 	if opts.HTTPClient == nil {
 		opts.HTTPClient = http.DefaultClient
 	}
-	if opts.Header == nil {
-		opts.Header = http.Header{}
+	if opts.HTTPClient.Timeout > 0 {
+		return nil, nil, xerrors.Errorf("please use context for cancellation instead of http.Client.Timeout; see issue nhooyr.io/websocket#67")
+	}
+	if opts.HTTPHeader == nil {
+		opts.HTTPHeader = http.Header{}
 	}
 
 	parsedURL, err := url.Parse(u)
@@ -67,7 +70,7 @@ func dial(ctx context.Context, u string, opts DialOptions) (_ *Conn, _ *http.Res
 
 	req, _ := http.NewRequest("GET", parsedURL.String(), nil)
 	req = req.WithContext(ctx)
-	req.Header = opts.Header
+	req.Header = opts.HTTPHeader
 	req.Header.Set("Connection", "Upgrade")
 	req.Header.Set("Upgrade", "websocket")
 	req.Header.Set("Sec-WebSocket-Version", "13")

ci/test/entrypoint.sh

@@ -2,8 +2,14 @@
 //
 // See https://tools.ietf.org/html/rfc6455
 //
+// Please see https://nhooyr.io/websocket for overview docs and a
+// comparison with existing implementations.
+//
+// Conn, Dial, and Accept are the main entrypoints into this package. Use Dial to dial
+// a WebSocket server, Accept to accept a WebSocket client dial and then Conn to interact
+// with the resulting WebSocket connections.
+//
 // The echo example is the best way to understand how to correctly use the library.
 //
-// Please see https://nhooyr.io/websocket for detailed design docs and a comparison with existing
-// libraries.
+// The wsjson and wspb packages contain helpers for JSON and ProtoBuf messages.
 package websocket

dial.go

@@ -1,4 +1,10 @@
-# Contributing Guidelines
+# Contributing
+
+## Issues
+
+Please be as descriptive as possible with your description.
+
+## Pull requests
 
 Please split up changes into several small descriptive commits.