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.

Sign up for GitHub

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

chat: room attachment requirements #249

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chat: room attachment requirements #249

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0306c14
chat: add presence-leave conditions
AndyTWF Nov 28, 2024
1359ffb
chat: add room attachment requirements to typing
AndyTWF Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions textile/chat-features.textile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,14 @@ Presence allows chat room users to indicate to others that they're online, as we
*** @(CHA-PR10c2)@ This specification point has been removed.
** @(CHA-PR10d)@ @[Testable]@ If the room status is @Attaching@, then the operation shall invoke a @CHA-RL9@ pending room status operation and wait for it to complete. If this operation fails, then the error from @CHA-RL9@ must be raised. If this succeeds (i.e the room becomes @ATTACHED@), the library must invoke the underlying @presence.update()@ call.
** @(CHA-PR10h)@ @[Testable]@ If the room status is anything other than @Attached@ or @Attaching@, an @ErrorInfo@ using the @RoomInInvalidState@ error code from the "chat-specific error codes":#error-codes with a status code of @400@ must be thrown. The message shall explain that attach must be called first.
** @(CHA-PR10e)@ @[Testable]@ If the room status is @Attached@, then the @update@ call will invoke the underlying @presence.enter()@ call.
** @(CHA-PR10e)@ @[Testable]@ If the room status is @Attached@, then the @update@ call will invoke the underlying @presence.update()@ call.
** @(CHA-PR10f)@ This specification point has been removed. It was superseded by @CHA-PR10h@.
** @(CHA-PR10g)@ This specification point has been removed. It was superseded by @CHA-PR10h@.
* @(CHA-PR4)@ Users may leave presence.
** @(CHA-PR4a)@ @[Testable]@ Users may choose to leave presence, which results in them being removed from the Realtime presence set.
** @(CHA-PR4b)@ @[Testable]@ If the room status is @Attaching@, then the operation shall invoke a @CHA-RL9@ pending room status operation and wait for it to complete. If this operation fails, then the error from @CHA-RL9@ must be raised. If this succeeds (i.e the room becomes @ATTACHED@), the library must invoke the underlying @presence.leave()@ call.
** @(CHA-PR4c)@ @[Testable]@ If the room status is anything other than @Attached@ or @Attaching@, an @ErrorInfo@ using the @RoomInInvalidState@ error code from the "chat-specific error codes":#error-codes with a status code of @400@ must be thrown. The message shall explain that attach must be called first.
** @(CHA-PR4d)@ @[Testable]@ If the room status is @Attached@, then the @enter@ call will invoke the underlying @presence.leave()@ call.
* @(CHA-PR5)@ @[Testable]@ It must be possible to query if a given clientId is in the presence set.
* @(CHA-PR6)@ @[Testable]@ It must be possible to retrieve all the "@Members":#chat-structs-presence-member of the presence set. The behaviour depends on the current room status, as presence operations in a Realtime Client cause implicit attaches.
** @(CHA-PR6a)@ This clause has been replaced by @CHA-PR6b - CHA-PR6f@.
Expand Down Expand Up @@ -413,12 +416,17 @@ Typing Indicators allow chat room users to indicate to others that they are typi
* @(CHA-T3)@ @[Testable]@ Users may configure a timeout interval for when they are typing. This configuration is provided as part of the @RoomOptions@ @typing.timeoutMs@ property, or idiomatic equivalent. The default is 5000ms.
* @(CHA-T4)@ Users may indicate that they have started typing.
** @(CHA-T4a)@ @[Testable]@ If typing is not already in progress, per explicit cancellation or the timeout interval in (@CHA-T3@), then a new typing session is started.
*** @(CHA-T4a1)@ @[Testable]@ When a typing session is started, the client is entered into presence on the typing channel.
sacOO7 marked this conversation as resolved.
Show resolved Hide resolved
*** @(CHA-T4a2)@ @[Testable]@ When a typing session is started, a timeout is set according to the @CHA-T3@ timeout interval. When this timeout expires, the typing session is automatically ended by leaving presence.
*** @(CHA-T4a3)@ @[Testable]@ If the room status is @Attaching@, then the operation shall invoke a @CHA-RL9@ pending room status operation and wait for it to complete. If this operation fails, then the error from @CHA-RL9@ must be raised. If this succeeds (i.e the room becomes @ATTACHED@), the library will proceed per @CHA-T4a1@.
*** @(CHA-T4a4)@ @[Testable]@ If the room status is anything other than @Attached@ or @Attaching@, an @ErrorInfo@ using the @RoomInInvalidState@ error code from the "chat-specific error codes":#error-codes with a status code of @400@ must be thrown. The message shall explain that attach must be called first.
*** @(CHA-T4a1)@ @[Testable]@ If the room status is @Attached@, then the @start@ call will invoke the underlying @presence.enter()@ call.
*** @(CHA-T4a2)@ @[Testable]@ When a typing session is started per @CHA-T4a1@, a timeout is set according to the @CHA-T3@ timeout interval. When this timeout expires, the typing session is automatically ended by leaving presence.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
*** @(CHA-T4a2)@ @[Testable]@ When a typing session is started per @CHA-T4a1@, a timeout is set according to the @CHA-T3@ timeout interval. When this timeout expires, the typing session is automatically ended by leaving presence.
*** @(CHA-T4a2)@ @[Testable]@ When a typing session is started per @CHA-T4a1@, a timeout is set according to the @CHA-T3@ timeout interval. When this timeout expires, the typing session automatically ends by leaving presence as per @CHA-T5c@.

Copy link
Collaborator

@sacOO7 sacOO7 Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should implement an internal retry mechanism if the implicit presence.leave fails due to a timeout.
This can become important because we won't be notifying the user about errors produced by leave event due to implicit typing timeout.
Though, we do have an explicit stop method that takes notifies user whether typing stopped or not, most of the times user will rely on start method to produce stop event on timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - this is a known bug that we intend to fix at a later date, I would prefer not to spec for it until we know what that is

Copy link
Collaborator

@sacOO7 sacOO7 Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

if implicit leave event fails due to typing timeout, receivers will see eternal typing in progress : (
Unless, user disconnects explicitly. Also, on reconnection, presence members are entered again.

Copy link
Contributor Author

@AndyTWF AndyTWF Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes - it's a known bug tracked in a Chat JIRA ticket. Once someone fixes it in a satisfactory way (in any of the SDKs), we'll update the spec - I just don't want to write a spec point for it until we have a tried and tested fix :)

Copy link
Collaborator

@sacOO7 sacOO7 Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If we are tracking it somewhere then good 👍

  • Otherwise you might like to create separate github issue for this WDYT
  • I think use-case is pretty clear => if leave fails on timeout then we have eternal typing
    event on receiver side 🤷‍♂️

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just an idea,

  • All implicit ops that don't notify user about the error are retried for max_retries = 3 and then added to the internal queue of implicit failed operations.
  • All those operations will be performed again once the connection goes connected.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems as per RTP16b, presence events will be queued at channel level when channel in Initialized or Attaching state.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For other states, operation will result in error as per RTP16c

Copy link
Collaborator

Choose a reason for hiding this comment

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

You might like to add those points to JIRA ticket, I will approve the PR for now, to unblock the spec

** @(CHA-T4b)@ @[Testable]@ If typing is already in progress, the @CHA-T3@ timeout is extended to be @timeoutMs@ from now.
* @(CHA-T5)@ Users may indicate that they have stopped typing.
** @(CHA-T5a)@ @[Testable]@ If typing is not in progress, this operation is no-op.
** @(CHA-T5b)@ @[Testable]@ If typing is in progress, he @CHA-T3@ timeout is cancelled. The client then leaves presence.
sacOO7 marked this conversation as resolved.
Show resolved Hide resolved
** @(CHA-T5b)@ This specification point has been removed. It was superseded by @CHA-T5e@.
** @(CHA-T5c)@ @[Testable]@ If typing is in progress, and the room status is @Attaching@, then the operation shall invoke a @CHA-RL9@ pending room status operation and wait for it to complete. If this operation fails, then the error from @CHA-RL9@ must be raised. If this succeeds (i.e the room becomes @ATTACHED@), the library will proceed per @CHA-T5b@.
** @(CHA-T5d)@ @[Testable]@ If typing is in progress, and the room status is anything other than @Attached@ or @Attaching@, an @ErrorInfo@ using the @RoomInInvalidState@ error code from the "chat-specific error codes":#error-codes with a status code of @400@ must be thrown. The message shall explain that attach must be called first.
** @(CHA-T5e)@ @[Testable]@ If typing is in progress, and the room status is @Attached@, the client leaves presence and the @CHA-T3@ timeout is cancelled.
* @(CHA-T6)@ Users may subscribe to typing events - updates to a set of clientIDs that are typing. This operation, like all subscription operations, has no side-effects in relation to room lifecycle.
** @(CHA-T6a)@ @[Testable]@ Users may provide a listener to subscribe to "typing events":#chat-structs-typing-event in a room.
** @(CHA-T6b)@ @[Testable]@ A subscription to typing may be removed, after which it shall receive no further events.
Expand Down
Loading