go-retry icon indicating copy to clipboard operation
go-retry copied to clipboard

Add ability to log message on retries

Open mortenlj opened this issue 9 months ago • 3 comments
trafficstars

We have noticed a quite common pattern, which is that we attempt some operation. It fails, and we want to inform the user about this, and that the operation will be retried. Except we don't know for sure if it will be retried, that is out of our hands.

Would it make sense to add some sort of logger to the retry mechanism, such that when a retryable error is returned and the operation will be retried, DoValue would log the error with "; retrying" (or something similar) appended?

I'm not entirely sure how to best design this, but figured it might be a common enough pattern to warrant support in go-retry.

mortenlj avatar Feb 13 '25 13:02 mortenlj

I'm not sure I understand. Why can't you just log within the callback function using your own logged?

if err := retry.DoValue(ctx, b, func(ctx context.Context) (string, error) {
  if err := db.PingContext(ctx); err != nil {
    myLogger.Warnf("failed to ping db (%s), retrying", err)
    return "", retry.RetryableError(err)
  }
  return "success", nil
}); err != nil {
  log.Fatal(err)
}

It you wanted something more sophisticated (like "retry attempt 1/5"), you'd build a custom Backoff to keep track of the attempts and log within the Next() function.

sethvargo avatar Feb 13 '25 14:02 sethvargo

That is what we do currently. The minor niggle that prompted this issue was that in cases where the problem doesn't actually resolve itself before the timeout, the last log will be a "lie". It will say that we are retrying, but in reality we give up and return an error.

It might just be me being overly concerned with the communication to the user. I guess it would also be possible to solve this by implementing a custom Backoff, so feel free to close the issue.

mortenlj avatar Feb 13 '25 14:02 mortenlj

It will say that we are retrying, but in reality we give up and return an error.

Only log if stop is false. Wouldn't that avoid the issue?

sethvargo avatar Feb 13 '25 18:02 sethvargo