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

Add support asynchronous authCallbacks #706

Open
ben-xD opened this issue Aug 30, 2021 · 2 comments
Open

Add support asynchronous authCallbacks #706

ben-xD opened this issue Aug 30, 2021 · 2 comments

Comments

@ben-xD
Copy link
Contributor

ben-xD commented Aug 30, 2021

To allow users to make a network request without blocking a thread, we should allow users to provide the result of their token auth server (TokenRequest, TokenDetails or JWT/ Token String) asynchronously/ without having to return the value.

Problem 1

Previously, a customer was confused when they could not find a way to make an asynchronous request in the authCallback. This is not possible in ably-java. I told them:

If you use a kotlin API which is asynchronous then it won’t get the response in the same callback. You can’t use that in the authCallback for Ably. You need to use a network request which is synchronous (which blocks the thread until the network response is returned/ it doesn’t do any work while it waits). This shouldn’t be called on the main thread, just make sure that client.auth.authorize() is not called on main thread.

This is the default behaviour in okhttp / retrofit :
For example, in the Okhttp example (from their. docs) below, the response is return’d from the function. Its synchronous, your code blocks until .execute() returns.

String post(String url, String json) throws IOException {
  RequestBody body = RequestBody.create(JSON, json);
  Request request = new Request.Builder()
      .url(url)
      .post(body)
      .build();
  try (Response response = client.newCall(request).execute()) {
    return response.body().string();
  }
}

Problem 2

In Flutter, Platform methods are asynchronous methods, and to receive the response a callback is handled. At the very least, we should allow the user to provide the results of their authCallback in a callback. You might also consider other options. It would be nicer in Ably-Flutter to call this asynchronous method. Currently, there is a related bug because Ably-java does not support async authCallbacks: ably/ably-flutter#156

┆Issue is synchronized with this Jira Story by Unito

@sacOO7
Copy link
Collaborator

sacOO7 commented Aug 31, 2021

I am sceptical as to whether asynchronous call is really needed. I think it kind of becomes language specific dependency ( either it can't do it, or we haven't explored enough options yet). As far as SDK implementation goes, it doesn't matter if call is synchronous/asynchronous, it can't proceed without proper token. So, it will be blocked for either way waiting for the result. I think better approach would be to handle asynchronous call synchronously and return the result the library wants as per spec.

@ben-xD
Copy link
Contributor Author

ben-xD commented Sep 1, 2021

@sacOO7 , it is definitely not needed, hence this issue is tagged with enhancement. I can use a latch as done in ably/ably-flutter#164, but:

  • we may want to help users (make their lives easier), and
  • as more users come from Kotlin apps, more people do not want to do threading themselves. Adding an async method is much nicer than asking users to block a thread manually.

It would be nice to have this, just like we have methods which end with async/ sync already. Of course, none were "really needed":
Screenshot 2021-09-01 at 19 18 54

@sync-by-unito sync-by-unito bot removed the enhancement New feature or improved functionality. label Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants