search

pusher / pusher-websocket-dotnet

Pusher Channels Client Library for .NET
MIT License
112 stars 113 forks source link

Client should not automatically reconnect after certain errors #74

Closed cdauphinee closed 3 years ago

cdauphinee commented 5 years ago

If I call ConnectAsync with an invalid application key, the connection will constantly try to reconnect with the invalid key until I call DisconnectAsync. This is easily seen by adding a while (true); to the end of the 'invalid key' acceptance test and watching the trace output.

As a work around, I can call DisconnectAsync in response to the error event being raised, but these two issues cause some worry: https://github.com/pusher/pusher-websocket-dotnet/issues/71 https://github.com/pusher/pusher-websocket-dotnet/issues/73

Catching and ignore the NRE in the first issue is fine, but if I ignore the NRE from the second issue, the thread in websocket_Closed will restart the connect loop and I'll end up with a socket leak. I can't tell where the NRE came from.

Ideally, I think the library should be responsible for stopping the reconnects on errors that indicate permanent failure.

damdo commented 4 years ago

Hey @cdauphinee. Yes! I agree that the client should not reconnect on errors that indicate permanent failure. This is also something that is specified by the Pusher Channels Protocol Spec here: https://pusher.com/docs/channels/library_auth_reference/pusher-websockets-protocol#connection-closure and that is present in other libs: https://github.com/pusher/pusher-websocket-java/blob/a025630abfc266a9afe29de9df11344b9378775f/src/main/java/com/pusher/client/connection/websocket/WebSocketConnection.java#L310-L314 Thank you for pointing this out!

elverkilde commented 3 years ago

This issue was fixed in #104 which was released as version 2.0.0. Please be aware that this is a major release, with several breaking changes. We have written a migration guide here:

https://github.com/pusher/pusher-websocket-dotnet#migrating-from-version-1-to-version-2