Problem
I have made the following system to automatically retry network calls if some exception is thrown. (Earlier posted on stackOverFlow, solution inspired by what @Boris answered)
public static <T> Optional<T> autoRetry(@NonNull final DoWork<T> task, @NonNull final Optional<Predicate<T>> resultRejector) {
for (int retry = 0; retry <= StaticData.NETWORK_RETRY; ++retry) {
try {
Thread.sleep(retry * StaticData.NETWORK_CALL_WAIT);
final T resultAfterWork = task.doWork();
/**
* If the result was not
* desirable we RETRY.
*/
if(resultRejector.isPresent() && resultRejector.get().apply(resultAfterWork))
continue;
/**
* Else we return
*/
return Optional.fromNullable(resultAfterWork);
} catch (InterruptedException | UnknownHostException e) {
e.printStackTrace(); //To be replaced by proper logging
return Optional.absent();
} catch (IOException e) {
e.printStackTrace(); //To be replaced by proper logging
}
}
return Optional.absent();
}
- I take care to not retry after InterruptedException &
UnknownHostException. - I retry 5 times. After each failure I perform an exponential back
off, starting from 300ms going upto 1500ms. - The predicate is used to verify the validity of the expected result.
The DoWork class :
public abstract class DoWork<T> {
protected abstract T doWork() throws IOException;
}
I did not use Supplier because I need to throw an IOException.
Here is an example of how I use it. (this example updates the GCM Id)
//Context is android specific stuff, basically if app dies, reference will return null.
public static boolean updateGCM(final long id, final WeakReference<Context> reference) {
final String regId = autoRetry(new DoWork<String>() {
@Override
protected String doWork() throws IOException {
final Context context = reference.get();
if(context == null)
return "QUIT";
return GoogleCloudMessaging.getInstance(context)
.register("XXXXXXXXXX");
}
}, Optional.<Predicate<String>>of(new Predicate<String>() {
@Override
public boolean apply(@Nullable String input) {
return TextUtils.isEmpty(input); //I need to retry if the returned GCM ID was null/empty
}
})).orNull();
//Context becomes null when the application is exited
//If retires failed or application exited, we return
if(TextUtils.isEmpty(regId) || regId.equals("QUIT"))
return false;
Log.i("Ayush", "Uploading newGcmId to server");
final Boolean result = autoRetry(new DoWork<Boolean>() {
@Override
protected Boolean doWork() throws IOException {
//if everything is fine, send id to server
return true;
}
}, Optional.<Predicate<Boolean>>absent()).orNull(); //No need of predicate hence send absent
return !(result == null || !result);
}
- So on a scale of 1 to 10 how professional is this approach ? 🙂
- Also I do not want to retry if the network is genuinely failing, like
UnknownHostException, or something bad happens which wont get
resolved with retrying. Any possibility of improving exception handling ?
Solution
public static <T> Optional<T> autoRetry(@NonNull final DoWork<T> task, @NonNull final Optional<Predicate<T>> predicate) {
No predicate, please. Call it something like acceptResult or resultRejector… you know what I mean.
Thread.sleep(retry * StaticData.NETWORK_CALL_WAIT);
This doesn’t do what you write, as the first time retry=0.
I take care to not retry after InterruptedException & UnknownHostException.
The latter may be retryable as it may be a temporary network or DNS problem. But it’s your choice.
final String regId = autoRetry(new DoWork<String>() {...
I’m too lazy to copy it in my Eclipse and without it, it’s unreadable as I have no idea where the expressions ends. What about a private final DoWork<String> regIdDoWork = ... or alike?
Optional.<Predicate<Boolean>>absent()).orNull(); //No need of predicate hence send absent
How nicer would the world be if there was no Optional? 😀 Just null.
Note that even if you like Optional, you should probably use it for as return types only. This is the place where it may prevent bugs and where it doesn’t add so much verbosity.