davidstump / SwiftPhoenixClient

Connect your Phoenix and iOS applications through WebSockets!
MIT License
506 stars 146 forks source link

Automatic success for `Channel.leave()` attempts when offline #241

Closed msandel6 closed 4 months ago

msandel6 commented 1 year ago

Issue:

This week, I encountered unexpected behavior when a user triggered join and leave events on various topics while unable to connect to the internet.

Cause:

After some investigation, I found that Channel.leave() automatically returns a success if canPush is false, even though the phx_leave event was never pushed and the channel instance was never removed from the socket.

Screenshot 2023-05-12 at 5 04 00 PM

Question:

Was there a specific use case for that line to be added (presumably with a different reason for canPush == false)? If not, can it be safely removed?

The bug in my app was caused by a local array of joined channels being out of sync with the socket after receiving these "ok" events without a channel ever actually being removed from the socket. I made a PR that will allow clients to access the Socket's list of channels directly, which will resolve my problem for now. That said, an app could listen for successful leave events for reasons other than keeping track of open channels, in which case this "fake" success would still lead to bugs.

dsrees commented 1 year ago

This is a behavior in the phoenix js client that SwiftPhoenixClient is porting. I'm not sure why that behavior was decided upon.

I believe that the reasoning is because the leave push is stored in the message queue and will be sent to the sever once the connection is established. The client assumes that leave will be successful and allows the app the discard the channel since that's really the only thing to do after a channel is left. It's likely an optimization to prevent potentially waiting for an internet connection to simply close a channel.

msandel6 commented 1 year ago

The client assumes that leave will be successful and allows the app the discard the channel

Is this a safe assumption? If so, then I think this PR should unblock my use case.

dsrees commented 1 year ago

I don't see why channels can't be made public as you proposed in your PR