Retry the request if the first one failed

Question

Hi, I am using a xml web service on my web app and sometimes remote server fails to respond in time. I came up with the idea of re-request if first attempt fails. To prevent loop I want to limit concurrent request at 2. I want to get an opinion if what I have done below is ok and would work as I expect it.

public class ScEngine
{
    private int _attemptcount = 0;
    public int attemptcount
    {
        get
        {
            return _attemptcount;
        }
        set
        {
            _attemptcount = value;
        }
    }

    public DataSet GetStat(string q, string job)
    {

        try
        {
           //snip....
            attemptcount += attemptcount;
            return ds;

        }
        catch
        {
            if (attemptcount>=2)
            {
           return null;
            }
            else
            {
                return GetStat(q, job);
            }

        }

    }
}

Answer 1

You forgot to increment the attemptcount. Plus, if there's any error on the second run, it will not be caught (thus, becomes an unhandled exception).

Answer 2

I wouldn't recurse in order to retry. Also, I wouldn't catch and ignore all exceptions. I'd learn which exceptions indicate an error that should be retried, and would catch those. You will be ignoring serious errors, as your code stands.

Answer 3

public class ScEngine
{
    public DataSet GetStat(string q, string job)
    {
     int attemptCount;
     while(attemptCount < 2)
     {
            try
            {
                attemptCount++;
                var ds = ...//web service call
                return ds;
            }
            catch {}
        }
        //log the error
        return null;
    }
}

Answer 4

You don't want to solve it this way. You will just put more load on the servers and cause more timeouts.

You can increase the web service timeout via httpRuntime. Web services typically return a lot of data in one call, so I find myself doing this pretty frequently. Don't forget to increase how long the client is willing to wait on the client side.

Answer 5

Here's a version that doesn't use recursion but achieves the same result. It also includes a delay so you can give the server time to recover if it hiccups.

    /// <summary>
    /// The maximum amount of attempts to use before giving up on an update, delete or create
    /// </summary>
    private const int MAX_ATTEMPTS = 2;

    /// <summary>
    /// Attempts to execute the specified delegate with the specified arguments.
    /// </summary>
    /// <param name="operation">The operation to attempt.</param>
    /// <param name="arguments">The arguments to provide to the operation.</param>
    /// <returns>The result of the operation if there are any.</returns>
    public static object attemptOperation(Delegate operation, params object[] arguments)
    {
        //attempt the operation using the default max attempts
        return attemptOperation(MAX_ATTEMPTS, operation, arguments);
    }

    /// <summary>
    /// Use for creating a random delay between retry attempts.
    /// </summary>
    private static Random random = new Random();

    /// <summary>
    /// Attempts to execute the specified delegate with the specified arguments.
    /// </summary>
    /// <param name="operation">The operation to attempt.</param>
    /// <param name="arguments">The arguments to provide to the operation.</param>
    /// <param name="maxAttempts">The number of times to attempt the operation before giving up.</param>
    /// <returns>The result of the operation if there are any.</returns>
    public static object attemptOperation(int maxAttempts, Delegate operation, params object [] arguments)
    {
        //set our initial attempt count
        int attemptCount = 1;

        //set the default result
        object result = null;

        //we've not succeeded yet
        bool success = false;

        //keep trying until we get a result
        while (success == false)
        {
            try
            {
                //attempt the operation and get the result
                result = operation.DynamicInvoke(arguments);
                //we succeeded if there wasn't an exception
                success = true;
            }
            catch
            {
                //if we've got to the max attempts and still have an error, give up an rethrow it
                if (attemptCount++ == maxAttempts)
                {
                    //propogate the exception
                    throw;
                }
                else
                {
                    //create a random delay in milliseconds
                    int randomDelayMilliseconds = random.Next(1000, 5000);
                    //sleep for the specified amount of milliseconds
                    System.Threading.Thread.Sleep(randomDelayMilliseconds);
                }
            }
        }

        //return the result
        return result;
    }

Answer 6

Check out this link. It is great way of handling retries and timeouts. http://timross.wordpress.com/2008/02/10/implementing-the-circuit-breaker-pattern-in-c/