ably / specification

The Ably features spec for client library SDKs.
Apache License 2.0
0 stars 4 forks source link

Change presence event methods from `on` to `subscribe`, change error handling for `on` #164

Closed mattheworiordan closed 3 months ago

mattheworiordan commented 1 year ago

As we've developed our our Spaces API, we've leaned towards a convention that makes it clearer what on and subscribe are for.

The new Spaces API for presence does not emit enter, leave, and update events with on, but instead exposes a subscribe handler, which is more consistent with how channel event handlers work. In addition, by using this convention, Spaces Cursors can expose state changes on the cursor channel with on and can emit cursor events with subscribe.

In addition, we currently overload our on event handler in Connections and Channels to emit an error event, which is not a state change. This presents two problems:

I propose we consider doing the following in v2+ of the protocol:

  1. Change presence.on to presence.subscribe
  2. Emit errors on channels and connections with a new handler such as onError, and stop emitting errors altogether with the on handler unless the error is embedded in the state change.
  3. Aim for on to be used exclusively for state changes, and subscribe to be used for listening to the intended events that the object relates to i.e. presence updates for presence, messages for pub/sub channels, cursors for cursors, lock updates for locks etc.
sync-by-unito[bot] commented 1 year ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3827

SimonWoolf commented 1 year ago

This issue is nostalgic but confusing. All the changes you're suggesting were made years ago.

Change presence event methods from on to subscribe

Presence event subscriptions already use subscribe. https://ably.com/docs/api/realtime-sdk/presence?lang=javascript#subscribe, spec is RTP6. Ably-js also supports on as a deprecated alias, it's been deprecated since 2016.

In addition, we currently overload our on event handler in Connections and Channels to emit an error event, which is not a state change.

No, we don't. An error event was proposed for the 0.9 spec but was removed in the final version in favour of update.

Are you accidentally playing with ably-js 0.8 or something?

mattheworiordan commented 3 months ago

Urr, brain fart. I have no idea where I came across .on that led me to believe this had not been resolved. I am also disappointed in myself for clearly not spending some time to recheck the spec and current implenentations.

Sorry for the noise. Closing this ticket.