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

RequestFuture #16

Closed
iODroid opened this issue Mar 30, 2017 · 1 comment
Closed

RequestFuture #16

iODroid opened this issue Mar 30, 2017 · 1 comment

Comments

@iODroid
Copy link

iODroid commented Mar 30, 2017

Using the RequestFuture I realized that I can not handle network errors because doing future.get() forces me to implement ExecutionException, InterruptedException and TimeoutException.
By controlling the RequestFuture of the source code, I realized that the onRespoceError method as VolleyError parameter that would be perfect to know the type of network failure by VolleyError.NetworkResponse.
Instead, what is done is a throw new ExecutionException(mException) and then I can not have the VolleyError.NetworkResponse.

	RequestFuture<String> future = RequestFuture.newFuture();
    StringRequest testRequest = new StringRequest(this.mRequestMethod , url, future, future);
    queue.add(testRequest);

    try {
        String response = future.get(10, TimeUnit.SECONDS); // Blocks for at most 10 seconds.

        if(future.isDone()){
            callback.onSucces(response);
        }

    } catch (InterruptedException e) {
        // Exception handling
    } catch (ExecutionException e) {
	  
	  callback.onFail(9999 ,e.getMessage());
     
     } catch (TimeoutException e) {
        e.printStackTrace();
    }

I'm doing something wrong, or it was an oversight during the development of the future?

Thank you

@jpd236
Copy link
Collaborator

jpd236 commented Mar 30, 2017

Since RequestFuture implements the standard Java Future interface, get() has to throw an ExecutionException. Not an oversight, but an intentional design choice to be interoperable with Future.

It's not perfectly clean, but the safe thing to do is:

try {
  ...
} catch (ExecutionException e) {
  if (e.getCause() instanceof VolleyError) {
    VolleyError volleyError = (VolleyError) e.getCause();
    // Handle the VolleyError
  }
  // Handle other errors
}

It's actually guaranteed by the current implementation that e.getCause() can only be a VolleyError, so you could skip the getCause() check and other error handling as they're actually impossible, but this would be relying on a Volley implementation detail so I would suggest the instanceof check for safety's sake.

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

No branches or pull requests

2 participants