juju/retry take 2 - looping

[roger peppe]

On 26 October 2016 at 04:46, John Meinel <john at arbash-meinel.com> wrote:
>> I think this is a hint that this is the wrong approach. The edge-cases
>> begin showing the cracks in the abstraction and end up making the code more
>> complex. Consider your example instead of:
>>
>> var finalResult Foo
>> for loop := retry.Loop(spec); loop.Next(); {
>>     result, err := SomeFunc(blah)
>>     if err != nil || resultNotGoodEnough(result) {
>>         continue
>>     }
>>
>>         finalResult = result
>>     break
>> }
>>
>> There are no special errors, no mixing of concerns, just a boring
>> imperative loop. It works like any other loop written in Go.
>
>
> The one things I'll specifically note about your "simple" example is that it
> doesn't actually handle errors. The fact that the "quick way to write it" is
> actually wrong is the specific argument of this thread.
> In your loop you have a way to generate a FinalResult, but no handling of
> the actual "I couldn't get a result". The way above that you can tell the
> loop failed would be to check if FinalResult doesn't have a valid value.
>
> This, at least, is the point I think Tim is bringing up of "hard to get
> wrong".

At least that's obvious from looking at the code, which is just normal
Go code like you might find anywhere. The looping construct doesn't
have anything to do with the error values that the code is dealing with.

In Tim's package, we've got *two* places that are storing the error,
inside the iterator and outside it. You can't always rely on the
error that's inside the iterator because you might not have passed it in.

Take this earlier example:

var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
     result, err = SomeFunc(blah)
     if isFatalErr(err) {
         break
     }
}
if err != nil {
   // what ever
}
// continue using result

In this case we sometimes want to use err (if the loop
hasn't run to completion) and sometimes loop.Error
(if it has), but in the latter case we probably want to
use err too because loop.Error returns the last-but-one
error not the most recent error.

And in this example, which I think Katherine adapted
for demo purposes, it's still not handling the error
in exactly the same way as Katherine's, so
it doesn't seem like it's "harder to get wrong" really:

var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
   result, err = SomeFunc(blah)
   if err == nil && resultNotGoodEnough(result) {
     err = retry.ErrTryAgain
   }
}
if err != nil {
   // what ever
}
// continue using result

Also, that example isn't fit for purpose because it
discards the error, so the code can't return what the most
recent issue was when the loop times out.

FWIW having evaluated the options, the Snappy team have decided to go
with the package I proposed. I landed it in gopkg.in/retry.v1.  Feel free
to use it if you think you can live with it.

  cheers,
    rog.