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

IllegalArgumentException: Synchronous ResponseHandler used in AsyncHttpClient after upgrade from 1.4.4 to 1.4.5 #751

Closed
bajtos opened this issue Dec 4, 2014 · 10 comments
Assignees
Labels
Milestone

Comments

@bajtos
Copy link

bajtos commented Dec 4, 2014

Hi, the change introduced in #507 seems to break our module depending on android-asycn-http, see strongloop-community/loopback-sdk-android#55.

java.lang.IllegalArgumentException: Synchronous ResponseHandler used in AsyncHttpClient. You should create your response handler in a looper thread or use SyncHttpClient instead.

This is how I create my handler (a simplified example):

   class CallbackHandler extends AsyncHttpResponseHandler {
        @Override
        public void onSuccess(int status, Header[] headers, byte[] body) {
          // ...
        }

         @Override
        public void onFailure(int statusCode,
                              org.apache.http.Header[] headers,
                              byte[] responseBody,
                              java.lang.Throwable error) {
          // ...
        }
    }

And this is the code using the callback:

var responseHandler = new CallbackHandler();
client = new HttpClient(context, url); // context and url provided in args
client.request(verb, path, parameters, parameterEncoding, responseHandler);

The code works correctly with v1.4.4 but throws with v1.4.5.

Could you please advise whether this is a bug in android-async-http or a problem in my code?

@smarek
Copy link
Member

smarek commented Dec 5, 2014

Hey @bajtos, as the error message says, it's quite simple.

Either use AsyncHttpResponseHandler subclass in combination with AsyncHttpClient(), or do use AsyncHttpResponseHandler where implemented getUseSynchronousMode() returns true in combination with SyncHttpClient.

And from your ticket I either cannot understand, why would this issue not popup in unit tests. Is it possible they use somehow customized version of loopback-sdk-android, custom Callback implementation or calling it from some strange Context/Looper ? If I were you, I'd request more information

I'm trying to dig deeper in your code, what I can see:

  1. We now support HEAD requests,
    https://github.com/strongloop/loopback-sdk-android/blob/4587dd82946c6a6647b60bd89ca74ad4ca66a481/src/main/java/com/strongloop/android/remoting/adapters/RestAdapter.java#L303

  2. Your BinaryHandler and CallbackHandler override specific methods, check if you're overriding the ones you want (there are few variants of every callback method, depending on what type of information you do want/need about the request/response processing.

  3. You could probably want to change declarations to require ResponseHandlerInterface instead of AsyncHttpResponseHandler

  4. I see the problem is reported from within user implementation, is it possible they use custom built AsyncHttpResponseHandler which returns getUseSynchronousMode = true?

@bajtos
Copy link
Author

bajtos commented Dec 5, 2014

Thanks for the reply.

I suspect the problem is in the fact that we use AsyncHttpResponseHandler together with sync HttpClient. It makes sense that such combination does not work.

And from your ticket I either cannot understand, why would this issue not popup in unit tests. Is it possible they use somehow customized version of loopback-sdk-android, custom Callback implementation or calling it from some strange Context/Looper ? If I were you, I'd request more information.

FWIW, you can run the test yourself if you have node.js installed on your machine - just run node test-server in background and then run unit-tests from Android Studio on an emulator.

Some of our tests use our own TestContext based on MockContext - see test/helpers/TestContext.java.

Other tests are using the context provided by ActivityTestCase from the Android SDK via this.getActivity().

I am not familiar with Android SDK internals, thus I cannot judge whether there is a difference in the test context that would explain the behaviour described above.

Thank you for suggestions on how to improve our code. Unfortunately I don't have bandwidth to address them right now, but I'll keep track of them in strongloop-community/loopback-sdk-android#56.

We can close the issue now, I'll reopen it if the change from HttpClient to AsyncHttpClient does not fix the problem.

@bajtos bajtos closed this as completed Dec 5, 2014
@smarek
Copy link
Member

smarek commented Dec 5, 2014

Great to hear that, good luck with fixing it. Would love to know, if you come up with problem on side of android-async-http library or not.

@smarek smarek added the question label Dec 5, 2014
@smarek smarek added this to the 1.4.7 milestone Dec 5, 2014
@smarek smarek self-assigned this Dec 5, 2014
@smarek
Copy link
Member

smarek commented Dec 5, 2014

Also latest stable is 1.4.6, you can see the changes in here: https://github.com/loopj/android-async-http/blob/master/CHANGELOG.md

bajtos pushed a commit to strongloop-community/loopback-sdk-android that referenced this issue Apr 1, 2015
The class name HttpClient suggests that we are using the sync variant,
while in reality we are using AsyncHttpClient under the hood.

See android-async-http/android-async-http#751
@bajtos
Copy link
Author

bajtos commented Apr 1, 2015

FWIW, our SDK is using AsyncHttpResponseHandler and AsyncHttpClient. However, we are subclassing AsyncHttpClient into a custom class that is called HttpClient, which I think caused some confusion. I have renamed the class to make the code easier to understand, see strongloop-community/loopback-sdk-android#70

@smarek
Copy link
Member

smarek commented Apr 2, 2015

Thank you for follow up

@SolomonBier
Copy link

@PalakSDarji
Copy link

In my case, I was calling post method from background.

@leisenhuang
Copy link

Is it true that we shouldn't call post method from background thread?

@houdangui
Copy link

same with @PalakSDarji , I got this issue calling it from a non main thread on android. Calling it from main thread fixed it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants