ethers.js icon indicating copy to clipboard operation
ethers.js copied to clipboard

WebSocketProvider constructor doesn't accept ConnectionInfo like its parent JsonRpcProvider

Open pfurini opened this issue 4 years ago • 1 comments

Describe the bug WebSocketProvider constructor signature reduces the types accepted by its parent class JsonRpcProvider, and makes it harder to use it in places where I was using a JsonRpcProvider before.

This is the constructor of the JsonRpcProvider class:

constructor(url?: ConnectionInfo | string, network?: Networkish)

It takes a ConnectionInfo object, useful for adding headers to each HTTP request (and I need this).

while this is the constructor of the WebSocketProvider class:

constructor(url: string, network?: Networkish)

It takes only a string, thus it's impossibile to pass additional options for the WS connection.

It's not a limitation of the underlying WebSocket lib, because it accepts a wealth of options, including custom headers. See its constructor here:

class WebSocket extends EventEmitter {
  /**
   * Create a new `WebSocket`.
   *
   * @param {(String|url.URL)} address The URL to which to connect
   * @param {(String|String[])} protocols The subprotocols
   * @param {Object} options Connection options
   */
  constructor(address, protocols, options) {
    ....

I've not checked every possible option, but I think most of the props in the ConnectionInfo type can be mapped to WS ones. At least I'm sure that the headers one can be mapped exactly as is for the WebSocket class. See this snippet from WebSocket as a proof:

  opts.headers = {
    'Sec-WebSocket-Version': opts.protocolVersion,
    'Sec-WebSocket-Key': key,
    Connection: 'Upgrade',
    Upgrade: 'websocket',
    ...opts.headers   // <-- these came from the `options` ctor param
  };

I propose to refactor the WebSocketProvider constructor to accept a ConnectionInfo param like its parent, and map it props to the corresponding WebSocket options.

Reproduction steps Try to construct a WebSocketProvider using a ConnectionInfo object, and of course it'll fail...

Environment: N/A

Search Terms WebSocketProvider

pfurini avatar May 28 '21 18:05 pfurini

It looks like according to this post headers aren't generally supported in WebSockets. The node.js implementation may allow additional options though. So, this might make sense in the future, but requires more investigation before making any changes to what is passed to the underlying library. It would require abstracting the browser-ws a bit too, so if attempting to override headers in the browser an error would be thrown.

I don't think the changes in the PR are sufficient though, since the values aren't actually passed through to the underlying WebSocket. But I'll look into it. :)

ricmoo avatar May 31 '21 22:05 ricmoo

I've secured my websocket with basic auth so would be nice to set custom headers... not this is not good practice, what else do you recommend?

mehranhydary avatar Jul 18 '23 16:07 mehranhydary

Oh wow. This is an old post, that I should close since this is now supported using a Socket creator function:

// A function that returns a websocket; it can configure anything it wishes and will be called whenever the Provider has to reconnect
const socketFunc = ()=> {
  // custom setup, headers, etc.
  const ws = new WebSocket(url, etc);
  // ws.on(somethingSpecial, etc)
  return ws;
};
const provider = new WebSocketProvider(socketFunc);

Does that work for you? :)

ricmoo avatar Jul 18 '23 17:07 ricmoo

(moving to discussions)

ricmoo avatar Jul 18 '23 17:07 ricmoo