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

fix: invoke result.success after pushReset #479

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

fix: invoke result.success after pushReset #479

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions android/src/main/java/io/ably/flutter/plugin/AblyMethodCallHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ private void pushReset(@NonNull MethodCall call, @NonNull MethodChannel.Result r
instanceStore.getPush(ablyMessage.handle)
.getActivationContext()
.reset();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem with reset is that it clears ably client from activationContext and it breaks everything until app is reloaded. There are 2 possible solutions:

  • Reinstall ably client in flutter (I've checked locally and it works)
instanceStore.getPush(ablyMessage.handle)
              .getActivationContext().setAbly((AblyRest) instanceStore.getAblyClient(ablyMessage.handle));
  • Do not clear ably client in reset (it looks for me that it is unnecessary, because ably client doesn't store any state) PR here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on my observations while reading the code, I understood that reset breaks activation/deactivation state machine. It was never been a proper solution to the problem. As per PR (#357), it was introduced as an experimental feature that was never tested as such and I couldn't see any test cases available for the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sacOO7 actually reset works in one particular very specific scenario: when ably environment has changed. If this happened activate/deactivate stop working. I agree that it's not very good solution, but it looks like it works. Basically it doesn't break state machine it simple delete it and recreate.

result.success(null);
} catch (AblyException e) {
handleAblyException(result, e);
}
Expand Down
Loading