Mechanism to retry failed requests, with throttling

Question

I have a simple event that the client sends to the server, the server can then respond saying if it was successful, or if it failed for some reason.

The client has to send a auth code, which is inside their config class. Below I attempt to throttle it, allowing 2 attempts, then reloading the config after the 2nd failed attempt.

If it still fails 2 more attempts after reloading the config, I throttle it for 60 seconds. If it fails after reloading the config 3 or more times (12 requests), then I just refuse to resend it.

Please note that I understand just leaving it and not warning the user why it failed to connect to the server is wrong, but I haven't finished with that code yet, and will adapt later on.

I'm just looking to see if its okay, and if I can improve it in any way.

public class HandshakeFailedEvent : IIncomingPacketEvent
{
    private static int ATTEMPTS_BEFORE_CONFIG_RELOAD = 2;
    private static int ATTEMPTS_BEFORE_THROTTLE = 4;
    private static int SECONDS_TO_THROTTLE = 60;

    private int _timesFailed;
    private int _timesSinceReset;

    public void Process(ICoreContext coreContext, IIncomingPacket packet)
    {
        if (packet.GetInteger("error_code") == 1) // WRONG_PASSWORD
        {
            CheckForThrottle(coreContext);
        }

        // Other error codes may need fixing, not throttling, handle here.

        coreContext.SocketHandler.SendPacket(new HandshakeRequestComposer(coreContext.ConfigHandler));
    }

    private void CheckForThrottle(ICoreContext coreContext)
    {
        _timesFailed++;
        _timesSinceReset++;

        if (_timesFailed >= ATTEMPTS_BEFORE_THROTTLE * 3)
        {
            // Application.Restart();
            return;
        }

        if (_timesSinceReset >= ATTEMPTS_BEFORE_CONFIG_RELOAD && _timesSinceReset < ATTEMPTS_BEFORE_THROTTLE)
        {
            coreContext.ConfigHandler.Download();
        }
        else if (_timesSinceReset >= ATTEMPTS_BEFORE_THROTTLE)
        {
            Task.Delay(SECONDS_TO_THROTTLE * 1000).ContinueWith((t) => { _timesSinceReset = 0; });
        }
    }
}

You should use a TimeSpan instead of integers for Task.Delay etc.: docs.microsoft.com/en-us/dotnet/api/… — BCdotWEB
– BCdotWEB, Commented Aug 17, 2018 at 14:04
How does Process method get fired again? Does the sendpacket somehow cycle back to it? I see your fields for counting are non static so it would need to hit the same class again for those to increase — CharlesNRice
– CharlesNRice, Commented Aug 17, 2018 at 15:20

IEatBagels · Accepted Answer · 2018-08-14 14:28:56Z

Prefixing field members with _ is more of an "old school" C# approach. It's a preference, I guess, but I think it's not really used anymore.

If the only thing you do with SECONDS_TO_THROTTLE is Task.Wait(SECONDS_TO_THROTTLE * 1000), then SECONDS_TO_THROTTLE should be equal to 60000, not 60, it's kind of confusing. At first I thought maybe there was something related to milliseconds, but no.

Reading your explanation, I understand why you do : if (_timesFailed >= ATTEMPTS_BEFORE_THROTTLE * 3), but if I only had your code, I wouldn't. Consider either adding another constant or maybe a comment.

CheckForThrottle's name is confusing. Reading this, I'd expect the method to tell me if I need to throttle, not to do the throttling itself. I'd either rename it to something else. But then again, I'd notice that a name like MaybeThrottle is a poor name. I'd keep the "should I throttle?" approach in this method, but I'd move Task.Delay(SECONDS_TO_THROTTLE * 1000).ContinueWith((t) => { _timesSinceReset = 0; }); some place else.

I think it's not really used anymore (Prefixing field members with _). - this is not true, it's widely used ;-) what's not used is upper-case constant names. — t3chb0t
– t3chb0t, Commented Aug 14, 2018 at 15:46

Stack Exchange Network

Mechanism to retry failed requests, with throttling

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Mechanism to retry failed requests, with throttling

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions