# HttpClient error handling

I would be happy to hear feedback on my implementation of HttpClient. I am not so strong with error handling so I want to improve. My thought process behind this was that IOExceptions can be recoverable and should be tried again. Where as client exception isn't. Hope to hear your thoughts.

private HttpResponse execute(int httpVerb, boolean authHeader) throws NoInternetConnectionError, IOException, ClientProtocolException {

try {
switch (httpVerb) {
case HTTP_POST:
}
return httpClient.execute(httpPost);
case HTTP_PUT:
}
return httpClient.execute(httpPut);
case HTTP_GET:
}
return httpClient.execute(httpGet);
default:
return null;
}
} catch (IOException e) {
InternetChecker internetChecker = new InternetChecker(context);
if (internetChecker.isInternetConnected() == false) {
throw new NoInternetConnectionError("internet not connected");
} else {
numberOfTries ++;
if (numberOfTries > 2) {
throw new IOException(e.getMessage());
} else {
Log.i(TAG, "executing for the " + String.valueOf(numberOfTries) + " time");
}
}
} finally {
numberOfTries = 0;
Log.i(TAG, "finally block run");
}
}

-

1. I would create a common executeRequest method:

private HttpResponse executeRequest(final HttpUriRequest request,
}
return httpClient.execute(request);
}


then remove the code duplication from the case branches.

switch (httpVerb) {
case HTTP_POST:
case HTTP_PUT:
case HTTP_GET:
default:
return null;
}


Anyway, it doesn't seem too good that you have httpPost, httpGet and httpPut at the same time.

2. Wouldn't it worth to check the result of isInternetConnected() at the beginning of the method? I'm not too familiar with Android development, maybe it's the best to put into the catch block. Anyway, if it is not a slow/costly service I would call at the beginning of the method too.

3. Calling String.valueOf(numberOfTries) seem unnecessary. I could be simply

Log.i(TAG, "executing for the " + numberOfTries + " time");

4. I'd use a loop instead of recursion. Two links on the topic:

-
Thanks palacsint. Just a couple of questions if you don't mind. 1. Why is it bad to have all 3 of them at the same time, after all I call the same method (execute) for each 2. The internet connection can be lost at anytime. It could be there at the start when I check then gone once I do the work 3. Didn't know I could put an in in there –  jiduvah Apr 30 '12 at 15:32
I have modified my code and its fairly obvious why I shouldn't be using the case. Thanks a lot! –  jiduvah Apr 30 '12 at 15:52
1. Because two of the three always unnecessary. Post those parts (or the full code) as a new question and I'll check it. I think only one reference with HttpUriRequest type would be enough but it's hard to say more without the code. 2. Check the update please. –  palacsint Apr 30 '12 at 20:50

In the end there was no need to retry manually httpclient has a retry handler.

    HttpRequestRetryHandler retryhandler = new DefaultHttpRequestRetryHandler(6, true);

httpClient.setHttpRequestRetryHandler(retryhandler);

-