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

[ECO-4780] Fix Connection closed error when using usePresence hook #1761

Merged
merged 1 commit into from
May 29, 2024

Conversation

VeskeR
Copy link
Contributor

@VeskeR VeskeR commented May 14, 2024

Resolves #1753
Also seems to help with #1759 , ideally would confirm with the user that this fix helps with the problem once released.

ably.connection.state dependency in onUnmount callback caused the next issue:

When ably.connection.state is added as a dependency to the onUnmount callback, changes to the connection state will update this callback, which in turn triggers our mounting useEffect hook. This became problematic on page reloads: when the connection state changes to closed, the onUnmount callback updates, and this triggers our mounting useEffect hook, which calls onMount and tries to enter presence. Since the connection is closed at that point, we get a "Connection closed" error. Although it's not critical and doesn't seem to cause any application behavior issues, polluting the log with "Connection closed" errors should be avoided.

@VeskeR VeskeR requested a review from ttypic May 14, 2024 06:09
@github-actions github-actions bot temporarily deployed to staging/pull/1761/features May 14, 2024 06:09 Inactive
@VeskeR VeskeR changed the title Fix Connection closed error when using usePresence hook [ECO-4780] Fix Connection closed error when using usePresence hook May 14, 2024
@github-actions github-actions bot temporarily deployed to staging/pull/1761/bundle-report May 14, 2024 06:10 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1761/typedoc May 14, 2024 06:10 Inactive
Copy link
Collaborator

@ttypic ttypic left a comment

Choose a reason for hiding this comment

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

Nice catch!

@VeskeR VeskeR force-pushed the 1753/fix-usePresence-connection-closed branch from e295b66 to 78633ff Compare May 22, 2024 12:09
@github-actions github-actions bot temporarily deployed to staging/pull/1761/features May 22, 2024 12:09 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1761/bundle-report May 22, 2024 12:10 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1761/typedoc May 22, 2024 12:10 Inactive
@VeskeR VeskeR force-pushed the 1753/fix-usePresence-connection-closed branch from 78633ff to 2743834 Compare May 22, 2024 12:13
@github-actions github-actions bot temporarily deployed to staging/pull/1761/features May 22, 2024 12:13 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1761/bundle-report May 22, 2024 12:13 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1761/typedoc May 22, 2024 12:14 Inactive
src/platform/react-hooks/src/hooks/usePresence.ts Outdated Show resolved Hide resolved
src/platform/react-hooks/src/hooks/usePresence.ts Outdated Show resolved Hide resolved
@VeskeR VeskeR force-pushed the 1753/fix-usePresence-connection-closed branch from 2743834 to 2f9fadd Compare May 24, 2024 04:39
@github-actions github-actions bot temporarily deployed to staging/pull/1761/features May 24, 2024 04:39 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1761/bundle-report May 24, 2024 04:40 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1761/typedoc May 24, 2024 04:40 Inactive
@VeskeR VeskeR requested a review from ttypic May 24, 2024 04:46
Copy link
Collaborator

@ttypic ttypic left a comment

Choose a reason for hiding this comment

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

LGTM

@VeskeR VeskeR merged commit a3270b3 into main May 29, 2024
12 checks passed
@VeskeR VeskeR deleted the 1753/fix-usePresence-connection-closed branch May 29, 2024 00:56
VeskeR added a commit that referenced this pull request Jun 3, 2024
…usePresence`

Similar to the fix in #1761 with
`Connection closed` errors, we should avoid entering presence for
specific channel states.

Resolves #1780
VeskeR added a commit that referenced this pull request Jun 3, 2024
…usePresence`

Similar to the fix in #1761 with
`Connection closed` errors, we should avoid entering presence for
specific channel states.

Resolves #1780
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.

usePresence error - Uncaught (in promise) Error: Connection closed
2 participants