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.BadGateway:
case HttpStatusCode.GatewayTimeout:
await Task.Delay(2000);
response = await client.GetAsync("jobs/" + id);
if (response.StatusCode == HttpStatusCode.BadGateway || response.StatusCode == HttpStatusCode.GatewayTimeout)
{
await Task.Delay(7000);
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;
}
}
jobJson = await response.Content.ReadAsStringAsync();
}
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
implementsIDisposable
, so make sure to wrap it in ausing
block. You should also not be re-using theresponse
variable, it makes the code more difficult to read and prone to bugs. \$\endgroup\$ – Brad M Oct 19 '17 at 19:27