NFIBrokerage / slipstream

A slick WebSocket client for Phoenix Channels
https://hex.pm/packages/slipstream
Apache License 2.0
160 stars 18 forks source link

Rejected connections are not handled #36

Closed PhillippOhlandt closed 3 years ago

PhillippOhlandt commented 3 years ago

Hello,

when I connect to a socket that rejects the connection in the sockets connect function, i get a gun error

14:20:29.039 [error] unknown message {:gun_response, #PID<0.532.0>, #Reference<0.1111790471.536608770.196337>, :fin, 403, [{"cache-control", "max-age=0, private, must-revalidate"}, {"content-length", "0"}, {"date", "Fri, 18 Jun 2021 12:20:28 GMT"}, {"server", "Cowboy"}]} heard in Slipstream.Connection.Pipeline please open an issue in NFIBrokerage/slipstream with this message and any available information.

and the slipstream handle_disconnect callback is called several seconds (10 or more) later with the reason :closed_by_remote.

Proper handling via the handle_disconnect or maybe a new handle_reject callback would be nice.

the-mikedavis commented 3 years ago

Hi hi :wave:

Good catch! We totally missed this in testing.

37 should fix this. You should be able to catch these with something like this in a module-based client:

@impl Slipstream
def handle_disconnect({:error, {:connect_failure, %{status_code: 403}}}, socket) do
  # try reconnecting or stop the GenServer...
  {:stop, :shutdown, socket}
end

I like the idea of a c:Slipstream.handle_reject/2 callback though, I'll keep that in mind.

This should be released as v0.7.0, but 0.7.0 also has a breaking change with how leaves are handled. Once #37 is merged the CHANGELOG should have everything you need to adjust your code if you use Slipstream.leave/2 anywhere.

Thanks for the report!