[FEATURE] Reduce support issues related to concurrent writes

[hulkingshtick]

Is there an existing feature request for this?

• [X] I have searched the existing feature requests

Is your feature request related to a problem? Please describe.

The documentation says:

Applications are responsible for ensuring that no more than one goroutine calls the write methods (NextWriter, SetWriteDeadline, WriteMessage, WriteJSON, EnableWriteCompression, SetCompressionLevel) concurrently

To help developers detect concurrency errors in their application, the connection makes a best effort to detect concurrent calls to WriteMessage, WriteJSON and Write on the writer returned from NextWriter. When a concurrent calls is detected, the connection panics with the string concurrent write to websocket connection. This panic is better than the alternatives such as commingling messages on the underlying network connection or a nil pointer exception.

The concurrent write to websocket connection panic is the most common support issue for the package. The issue arises because developers incorrectly assume that concurrency is allowed or wrongly think they covered all the bases with regards to ensuring a single writer.

Describe the solution that you would like.

Here are options for improving the situation:

Improve the panic string

The terse panic string does not sufficiently describe the problem or a fix. Create a markdown file on this repository explaining the concurrency restriction and suggested fixes. Include a link to the markdown file in the panic string.

Improve the detector

The current detector is:

if c.isWriting {
	panic("concurrent write to websocket connection")
}
c.isWriting = true

err := c.write(w.frameType, c.writeDeadline, c.writeBuf[framePos:w.pos], extra)

if !c.isWriting {
	panic("concurrent write to websocket connection")
}
c.isWriting = false

The second concurrent writer panics, but not the first. The stack trace for the first concurrent writer may be the key to finding the bug in the application. Cause the first writer to panic using the following code. In this code, the field c.writing is an int and replaces the field c.isWriting.

if c.Writing != 0 {
            c.Writing = 2
	panic("concurrent write to websocket connection")
}
c.Writing = 1

err := c.write(w.frameType, c.writeDeadline, c.writeBuf[framePos:w.pos], extra)

if c.Writing != 1 {
	panic("concurrent write to websocket connection")
}
c.Writing = 0

(this code requires more review and testing).

Return errors instead of panicking

Return an error instead of panicking. Include the stack trace of the caller in the error string.

Describe alternatives you have considered.

I considered and rejected making the existing methods thread safe. It's possible to make the individual methods thread safe, but the SetWriteDeadline, EnableWriteCompression and SetCompressionLevel methods require synchronization at the application level to be useful. Because robust applications should call SetWriteDeadline, a robust application will implement synchronization at the application level. It follows that there's no point in making the individual methods thread safe.

I also considered changing the API to accept a WriteOptions struct for NextWriter, WriteMessage and WriteJSON. The options struct replaces the SetWriteDeadline, EnableWriteCompression and SetCompressionLevel methods. I rejected this idea because it's a breaking change to the API.

Anything else?

No response