Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve wording in RTN15a #170

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lawrence-forooghian
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian commented Oct 9, 2023

  • Replace "recovery key" (which misleadingly suggests RTN16 recovery) with "private connection key" (as used in RTN15b).
  • Add a link to RTN15b so it’s clear what is meant by "restore the connection state".

- Replace "recovery key" (which misleadingly suggests RTN16 recovery) with
  "private connection key" (as used in RTN15b).

- Add a link to RTN15b so it’s clear what is meant by "restore the connection
  state".
Copy link
Collaborator

@maratal maratal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -501,7 +501,7 @@ h3(#realtime-connection). Connection
*** @(RTN15h1)@ If the @DISCONNECTED@ message contains a token error (@statusCode@ value of 401 and error @code@ value in the range @40140 <= code < 40150@) and the library does not have a means to renew the token, the connection will transition to the @FAILED@ state and the @Connection#errorReason@ will be set
*** @(RTN15h2)@ If the @DISCONNECTED@ message contains a token error (@statusCode@ value of 401 and error @code@ value in the range @40140 <= code < 40150@) and the library has the means to renew the token, a single attempt to create a new token should be made and a new connection attempt initiated using the new token. If the token creation fails or the next connection attempt fails due to a token error, the connection will transition to the @DISCONNECTED@ state and the @Connection#errorReason@ will be set.
** @(RTN15i)@ If an @ERROR@ @ProtocolMessage@ is received, this indicates a fatal error in the connection. The server will close the transport immediately after. The client should transition to the @FAILED@ state triggering all attached channels to transition to the @FAILED@ state as well. Additionally the @Connection#errorReason@ should be set with the error received from Ably
** @(RTN15a)@ If a @Connection@ transport is disconnected unexpectedly or because a token has expired, then the @Connection@ manager will immediately attempt to reconnect and restore the connection state. Connection state recovery is provided by the Ably service and ensures that whilst the client is disconnected, all events are queued and channel state is retained on the Ably servers. When a new connection is made with the correct connection recovery key, the client is able to catch up by receiving the queued @ProtocolMessages@ from Ably.
** @(RTN15a)@ If a @Connection@ transport is disconnected unexpectedly or because a token has expired, then the @Connection@ manager will immediately attempt to reconnect and restore the connection state, by following the process described in "RTN15b":#RTN15b. Connection state recovery is provided by the Ably service and ensures that whilst the client is disconnected, all events are queued and channel state is retained on the Ably servers. When a new connection is made with the correct private connection key, the client is able to catch up by receiving the queued @ProtocolMessages@ from Ably.
Copy link
Member

@SimonWoolf SimonWoolf Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly, what even is the point of this spec item? it specifies that an unexpected disconnection should result in an immediate retry, sure, but then goes off on a tangent and starts vaguely waffling on about the point of connection recovery...?

tempted to say this spec item should just read: ** @(RTN15a)@ If the active transport is disconnected unexpectedly or because a token has expired, then the SDK must transition to the @CONNECTING@ state and make an immediate "RTN15b":#RTN15b reconnection attempt. and that's it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants