rpc: add client constructor with headers

[r2d4]

Adds a new constructor DialWebsocketWithHeaders that takes in http.Header to connect to the websocket with. Currently, some headers like basic auth and origin are added automagically through the existing constructor DialWebsocketWithDialer.

This doesn't change the behavior of that constructor at all, but code is shifted around slightly to add the minimal amount of new code and reuse existing constructors.

Happy to make this more extensible by added some sort of options pattern, but didn't want to change any of the existing APIs.

[r2d4]

@fjl Anything I can do to help get this merged?

[fjl]

Hey, sorry for the delay. I've been thinking about this issue a lot actually. The problem, as you have also noted, is that the existing API is not very extensible and cannot accommodate new options easily.

I don't really want to add yet another specialized client constructor for this. Instead, I would prefer one of the following solutions:

• Add a new general client constructor NewClient that creates the client but doesn't connect. Then, change Client.SetHeader to also work for WebSocket.

• Change Dial / DialContext to support options, i.e. change signature to

  func DialContext(ctx context.Context, rawurl string, options ...ClientOption) (*Client, error)
  

  and add an option for headers. If we decide to go this way, then we should also add other options that mirror the features available in transport-specific constructors.

I loosely prefer the second way because it is more future proof. At the same time, I think we will need NewClient eventually, so why not add it now.

[r2d4]

Thanks for the detailed response. I agree with both -- and happy to take on the NewClient work. Here are the general options I think might make sense to expose

WithHeaders (websocket, http)
WithHTTPClient (http)
WithDialer (websocket)
WithReader (io)
WithWriter (io)
WithBasicAuth (websocket, http)

A few concerns putting the options at the client level (vs transport-specific options)

1. Most options are transport specific. A few are shared between HTTP/websocket. Could error/ignore if an incorrect option is passed to the transport, but onus on the caller to know what options correspond to which transports.
2. Will need isHttp like flags on the client, and the client will need to know the underlying type and structure of the transport. More casts like this, and most options will be deeply tied to underlying impl

	if !c.isHTTP {
		return
	}
	conn := c.writeConn.(*httpConn)

So I think options need to be at the transport layer unfortunately. This will at least let us cut down on the number of constructors and code paths, but won't get us much code reuse (which I think is OK).

I took a first pass at implementing a few options for websockets/http (no tests/comments yet).

A path forward for NewClient to accept options could be be a generic ClientOptions that gets type switch'd to each transports options. That makes NewClient a little more robust against changes in the underlying transport struct impl.

Want to get your thoughts before I write any more code. Let me know what you think!

[fjl]

Still thinking about this.

[fjl]

I have implemented client options in #24911, which is now merged to the master branch. The way it's done is a bit different from this PR, but the result is the same. You can now set headers for websocket connections like this:

c, err := rpc.DialOptions(ctx, "ws://...", rpc.WithHeader("key", "value"))

There are also other options to choose from such as WithHeaders, WithHTTPAuth.