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

fix: do not clear ably client on reset #968

Closed
wants to merge 1 commit into from

Conversation

ttypic
Copy link
Contributor

@ttypic ttypic commented Sep 15, 2023

Looks like it unnecessary to delete ably client on reset, it breaks push logic

@sacOO7
Copy link
Collaborator

sacOO7 commented Sep 15, 2023

I would be honest, this push.reset() itself feels like breaking the state machine as mentioned here ably/ably-flutter#479 (comment).
Spec mentions proper flow of activation state machine. Every stateChange reflects in a flow that can go in different directions.
https://sdk.ably.com/builds/ably/specification/main/features/#RSH3.
I think right solution would be to provide a stateChange that is a part of activation machine and should resemble called deactivate

@sacOO7
Copy link
Collaborator

sacOO7 commented Sep 15, 2023

You can try and check if the hacky solution works!
It shouldn't introduce a state where it disrupts the natural flow of the activation machine though!

@ViniciusFSSilva
Copy link

ViniciusFSSilva commented Oct 3, 2023 via email

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 21, 2023

We planning to close this PR right @ttypic

@ttypic
Copy link
Contributor Author

ttypic commented Nov 21, 2023

Yes, this PR is just healing the symptom not the original problem

@ttypic ttypic closed this Nov 21, 2023
@ViniciusFSSilva
Copy link

ViniciusFSSilva commented Mar 30, 2024 via email

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