Skip to content

Document why Dial returns the http response in the success caseΒ #281

@jawnsy

Description

@jawnsy

Thanks so much for your excellent WebSocket library!

websocket.Dial has the following signature and documentation (snippet):

// Dial performs a WebSocket handshake on url.
//
// The response is the WebSocket handshake response from the server.
// You never need to close resp.Body yourself.
//
// If an error occurs, the returned response may be non nil.
// However, you can only read the first 1024 bytes of the body.
func Dial(ctx context.Context, u string, opts *websocket.DialOptions) (*websocket.Conn, *http.Response, error)

The bodyclose linter, designed to check whether we consistently defer resp.Body.Close() on returned *http.Response objects, will flag these instances as problematic, even though (per the godoc), callers are not responsible for reading and closing the body.

Users can manually suppress these linter warnings, but I think it would be better to re-design the API in some way to avoid returning the raw http.Response directly. For example, if the http.Response only needs to be returned if there is an error in the websocket handshake, then we can wrap it in a struct that implements the error interface. This also generally improves usability of the API as well.

My interpretation of the Godoc, and please correct me if I misunderstood, is that users need code like this to correctly handle errors:

ws, resp, err := ws.Dial(...)
if err != nil {
  // handle & close resp, it's non-nil in this case
  // the user needs to distinguish between an http.Client error (resp will be nil)
  // and a websocket library error (resp will not be nil) before calling resp.Body.Close()
}
// use the socket

If maintaining API backward compatibility is a requirement, then an alternative is to provide a "new" Dial function that wraps the existing one and handles wrapping the http.Response into an error internally.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions