# Simple web service call with retries

This code is actually pretty basic. Basically, I'm doing a large number of web calls. I currently do up to 3 Web Service calls simultaneously, one of which is below.

There were 3 errors I saw in testing: Bad Gateway, Gateway Timeout, and basically "Couldn't establish a secure connection" (not the exact error text). In each case, a single retry appears to have fixed the issue.

"Bad Gateway" and "Gateway Timeout" didn't result in an exception, but "Couldn't establish a secure connection" did.

If I get either of the first two errors, it'll wait 2 seconds and try again. If it still didn't work, it tries 7 seconds and tries again. After that, it "gives up" and just returns null. (I've never seen that happen in testing, and I've never even gotten to the second retry). The 2-second and 7-second delays are somewhat arbitrary.

The code is below:

public static async Task<Job> GetJob(string id)
{
int attempts = 1;

tryAgain:

string jobJson;

try
{
using (HttpClient client = HttpClientConstructor.GetHttpClient(false, "application/json"))
{
HttpResponseMessage response = await client.GetAsync("jobs/" + id);

if (!response.IsSuccessStatusCode)
{
switch (response.StatusCode)
{
case HttpStatusCode.GatewayTimeout:
response = await client.GetAsync("jobs/" + id);

if (response.StatusCode == HttpStatusCode.BadGateway || response.StatusCode == HttpStatusCode.GatewayTimeout)
{
response = await client.GetAsync("jobs/" + id);
}
break;
}

if (!response.IsSuccessStatusCode)
{
string errorMessage = "Failed I have. Reason: " + response.ReasonPhrase;
Console.WriteLine(errorMessage);

System.Diagnostics.Trace.TraceError(errorMessage);

// If multiple retries couldn't get it, just give up
return null;
}
}

}

Job job = JsonConvert.DeserializeObject<Job>(jobJson);

return job;
}
catch (Exception e)
{
Console.WriteLine(e.ToString());

System.Diagnostics.Trace.TraceError(e.ToString());

// Go for two retries
// This actually worked in my testing
// In the rare case where I got exceptions, a retry worked
if (attempts <= 3)
{
attempts++;

// Is goto evil here?
goto tryAgain;
}

return null;
}
}


Obviously, this will be running asynchronously. As mentioned above, I will run up to 3 calls at once. It's fixed at 3 to avoid being "throttled" by rate limits on the API.

Is this a good way to solve this?

Could this code be improved?

• HttpResponseMessage implements IDisposable, so make sure to wrap it in a using block. You should also not be re-using the response variable, it makes the code more difficult to read and prone to bugs. – Brad M Oct 19 '17 at 19:27
• stackoverflow.com/a/19650002/1798889 is an answer to make it so the call doesn't need to be wrapped. – CharlesNRice Oct 20 '17 at 18:54
• Required reading: aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong – Jesse C. Slicer Apr 11 '18 at 14:45

In general if you have a goto statement, you should reorder your code to not need it. In this case, your code actually tries 9 times, not three, because you await client.GetAsync 3 times, and then jump back to your goto label up to 3 times.

The simple fix in this case is to remove the extra response=await... calls, enclose the entire try/except block in a loop and break or return if the call is successful. Something like:

for (int i = 0; i < 3; i++){
try
{
HttpResponseMessage response = await client.GetAsync("jobs/" + id);
}
catch (CantConnectExcept as ex){
continue;
}

if (!response.IsSuccessStatusCode){
continue;
}

return JsonConvert.DeserializeObject<Job>(jobJson);
}
return null


It doesn't sound like waiting more seconds on later failures is important. If you find it is important, you can do something like:

List<int> waitTime = new List<int> {2000, 2000, 7000};
...


Retry logic is usually a bit of a code smell. It's not very common to have a transient error that is common and resolved in the space of a few seconds of retry logic. In such a scenario it may be better to have a queue of some sort in place that can poison messages can be sent to an error store.

I would like to add few more suggestions to the previous answer. The idea is to set minimum delay, maximum delay and step delay, this will help us in increasing of delay progressively, irrespective of the retry count. If the requirement comes up to increase the number of retries then we don't need to change the code/logic

1. Declare the below properties, these values can be taken from configuration files, for simplicity I have declared value directly inside the class.

private int _maximumInterval = 10000;
private int _minimumDelay = 1000;
private int _stepDelay = 500;
private int _retryCount = 3;

2. In the function you can do the following

public async Task.....
{
int _currentDelay = _minimumDelay;
for (int i = 0; i < _retryCount; i++)
{
try
{
HttpResponseMessage response = await client.GetAsync("jobs/" + id);
}
catch (Exception  ex)
{
await Task.Delay(_currentDelay = _currentDelay >= _maximumInterval ? _maximumInterval : (_currentDelay + _stepDelay));
continue;
}

if (!response.IsSuccessStatusCode)
{
await Task.Delay(_currentDelay = _currentDelay >= _maximumInterval ? _maximumInterval : (_currentDelay + _stepDelay));
continue;
}