centrifugal / centrifuge-swift

Swift client SDK for bidirectional real-time communication with Centrifugo and Centrifuge-based server over WebSocket
MIT License
47 stars 41 forks source link

Make client status public #29

Closed danqing closed 4 years ago

danqing commented 4 years ago

Closes #27. This allows users to know the current status of the client.

danqing commented 4 years ago

(Also removed some trailing whitespaces as my editor automatically does so on save)

FZambia commented 4 years ago

@danqing hello!

Thanks for pr, I have some concerns about this.

First, could you describe how you are going to use this? This method can be pretty racy - the connection can be in connected status at the moment of check but be closed when you are performing actual operation after status check. Though it's pretty the same for current situation so you always need to handle errors.

If your use case is valid then maybe we can introduce a client method client.connected() which returns bool value to not expose statuses to public access (I'd like to leave statuses internal detail).

danqing commented 4 years ago

What I want to achieve is basically what all chat apps have: let the user know when the server is disconnected. For example, if you use FB messenger, you will see a "reconnecting" banner coming up in this case.

I want to do the same: when there is connection issue, show a yellow "reconnecting" banner or a red "no connection" banner, and dismiss the banner when connection resumes.

So I need to:

  1. Know the connection state when the view controller (chat view) first launches
  2. Be aware of state changes
danqing commented 4 years ago

Btw, this is related to #16 that I opened a while ago. It's useful to know initial connection error too.

danqing commented 4 years ago

So in summary, I'd like to know at any given time the status of the client:

I also want to understand better potential errors for subscription: if I have the permission and connection is successful, does that guarantee that subscription will succeed? If not, what's an example of failed subscription when connection itself is successful?

Thanks!

FZambia commented 4 years ago

To implement what you need you have to track status changes with connect, disconnect events anyway. Disconnect context has reconnect field which will tell you about whether client will reconnect or not. I don't understand how having status field will make your life much easier.

danqing commented 4 years ago

Well, why should I (i.e. the user) track status changes instead of the library itself? I'd say it's a common thing to do, so wouldn't it be better if the library already does this out of the box?

FZambia commented 4 years ago

Please post a code example on how you are planning to use public status without using connect and disconnect events. Maybe I am missing sth.

danqing commented 4 years ago

As I mentioned, I'd like to show a banner on viewDidAppear if status is not connected. To do so, I'd like to be able to check the status directly.

func viewDidAppear(_ animated: Bool) {
  super.viewDidAppear(animated)
  if centrifuge.status == .disconnected {
    // show banner
  }
}

Of course I can track the status myself, but it'd be better if centrifuge already does this.

FZambia commented 4 years ago

The reason why I don't want to make status public at this moment is because now it differs through client implementations. At moment internal statuses are:

1) In Swift client: NEW, CONNECTED, DISCONNECTED 2) In Java client: NEW, CONNECTED, DISCONNECTED 3) In Dart client: CONNECTED, DISCONNECTED, CONNECTING 4) In Go client: DISCONNECTED, CONNECTING, CONNECTED, RECONNECTING, CLOSED 5) In Javascript: disconnected, connected, connecting

While the public API is pretty much the same at this moment. I'd like to keep all clients consistent, so if you write code in one client it could be easily translated to another.

So if we make this part public we should carefully think on what other libraries do and make experience consistent. This is pretty big work so I prefer to avoid this at this moment until this is really necessary. Every change makes project maintenance harder, hope you can understand this.

danqing commented 4 years ago

Fair enough.