Too restrictive ws_opts type

[michalwski]

Currently the ws_opts type is defined in gun.erl:152

-type ws_opts() :: #{
    compress => boolean()
}.

From what I understand and from what is in fact needed when using gun for WebSockets with sec-websocket-protocol specified, the type needs to also allow protocols key. See how I need to call the ws_upgrade in esl/escalus in order to successfully upgrade the connection.

WSUpgradeHeaders = [{<<"sec-websocket-protocol">>, <<"xmpp">>}],
StreamRef = gun:ws_upgrade(ConnPid, Resource, WSUpgradeHeaders,
                                                 #{protocols => [{<<"xmpp">>, gun_ws_h}]}),

From what I understand, the protocols proplist is required because in gun_http:ws_handshake_protocol, gun will try to find the protocol on the list in case the server incudes the sec-websocket-protocol header in its response.

ws_handshake_protocols(Buffer, State, StreamRef, Headers, Extensions, Opts) ->
    case lists:keyfind(<<"sec-websocket-protocol">>, 1, Headers) of
        false ->
            ws_handshake_end(Buffer, State, StreamRef, Headers, Extensions,
                maps:get(default_protocol, Opts, gun_ws_h), Opts);
        {_, Proto} ->
            ProtoOpt = maps:get(protocols, Opts, []),
            case lists:keyfind(Proto, 1, ProtoOpt) of
                {_, Handler} ->
                    ws_handshake_end(Buffer, State, StreamRef,
                        Headers, Extensions, Handler, Opts);
                false ->
                    close
            end
end.

I'm wondering what's the correct way to fix the issue? The simplest would be to modify the ws_opts type. On the other hand, maybe gun could use the default gun_ws_h in case there is no handler specified for the protocol? This handler works in my case perfectly.

[essen]

Websocket handlers and protocol-handling are currently an undocumented feature, hence why protocols is not documented. I plan to document it in a future release.

I do not think there should be a specific handler by default when the protocol is not recognized, the current behavior is fine, it just needs documentation.

I suggest you silence Dialyzer for now and I will try to add the documentation for this in Gun 1.3 next week. If I can't make it then I'll take some time after that to document the few experimental features we got into a Gun 1.4.

[michalwski]

All right, thanks! Is there sth I can help you with regarding the issue? I'd really want to see this fixed in 1.3 and I may be able to spent some time on preparing a PR.

[essen]

Feel free to extend ws_opts() and add the relevant documentation in the manual.