Use methods with generics to implement retry about dotnet-etcd

[shubhamranjan]

I totally agree on all the points mentioned. There's a lot of scope for refactoring. Thanks for pointing these out.

[shubhamranjan]

I have also changed 'throw ex' to simple 'throw' because 'throw ex' removes the original stack trace and creates a new one on the place of the throw while a simple 'throw' keeps the original stack trace that should provide more details. Especially with the helper function: without this change all stack traces would point to the helper function as the source of the exception.

@yoricksmeets Are you implementing these changes or should I proceed with their implementation ?

[yoricksmeets]

@shubhamranjan I'm not implementing them at this moment. I can make some time later this week to implement these changes.

[yoricksmeets]

I'm starting to work on this issue

[yoricksmeets]

I arrived at the methods that use a AsyncDuplexStreamingCall and I was reading back through the issues why there is a errorHandler action in the function signature.
I think the problem raised in #34 is the reason this errorHandler action is included, but I think that if the watch operations return a Task instead of void, as pointed out in #40, it will give the user the option to catch the exception by handling the task that is returned by the method.

This setup will not return the exception that is thrown within the task so the caller has no way to handle the exception:

        [Test]
        public void TestVoidVsTask()
        {
            try
            {
                Watcher();
            }
            catch (Exception e)
            {
                // Never reached because Watcher() will never throw
            }
        }

        public async void Watcher()
        {
            while (true)
            {
                try
                {
                    Task watcherTask = Task.Run(async () =>
                    {
                        // stuff
                        throw new Exception();
                    });
                    await watcherTask;
                    return;
                }
                catch (Exception ex)
                {
                    throw;
                }
            }
        }

The exception will now crash the entire program. And wrapping the call to 'Watcher()' in the test method with a try-catch will not help because the exception is not propagated.

This following code will propagate the error and gives the caller the option to handle the exceptions.

        [Test]
        public void TestVoidVsTask()
        {
            try
            {
                Watcher().GetAwaiter().GetResult();
            }
            catch (Exception e)
            {
                // Handle exception
            }
        }

        public async Task Watcher()
        {
            while (true)
            {
                try
                {
                    Task watcherTask = Task.Run(async () =>
                    {
                        // stuff
                        throw new Exception();
                    });
                    await watcherTask;
                    return;
                }
                catch (Exception ex)
                {
                    throw;
                }
            }
        }

I was hoping to remove the exceptionHandler and change the return type from 'async void' to 'async Task' but this will break the API, so I would like your view on this matter @shubhamranjan

[shubhamranjan]

Yeah, the exception handler can be removed with return type changed to Task. We can do this change, a breaking change but not a feature removal. Will be fine.