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

Expose the current attempt number to the `recoverFromFailure` callback

Open edigaryev opened this issue 10 months ago • 2 comments

This would allow the swift-retry package users to avoid printing extraneous information if this is the last possible retry attempt.

Here's a real-world example: https://github.com/cirruslabs/tart/pull/788/files#diff-52d106b7150fe2a5d777ea88144bbf0a07f23d521a973fa1fd92133ab223fec3R196.

Here's a similar library for Golang that has this feature: https://pkg.go.dev/github.com/avast/retry-go/v4#OnRetryFunc.

P.S. thanks for making such a great package ❤️

edigaryev avatar Apr 12 '24 14:04 edigaryev

Hi @edigaryev! If I understood correctly, you are adding logging for the retries. If so, then have you considered passing a logger object to the retry function? The implementation already handles the situation you described.

For example, here are some example log messages when using an os.Logger object:

Attempt 0 failed with error of type `MyError`: `Some error description.`. Will wait 1.0 seconds before retrying.
Attempt 1 failed with error of type `MyError`: `Some error description.`. Will wait 2.0 seconds before retrying.
Attempt 2 failed with error of type `MyError`: `Some error description.`. No remaining attempts.

fumoboy007 avatar Apr 13 '24 06:04 fumoboy007

If I understood correctly, you are adding logging for the retries

Exactly.

If so, then have you considered passing a logger object to the retry function?

I see that there are two types of loggers that the retry function accepts:

The appleLogger uses os.Logger which is a structure for which one cannot change the logging level. It seems that the only way to force it to log to the standard output/error and log at the debug log level (which swift-retry uses) is to run the program via XCode, which is not what our users normally do.

The logger uses Logging.Logger which is a protocol for which the Logging.LogHandler can also be re-defined to get the access to the metadata, however, swift-retry package does not seem to set the (1) number of attempts left nor (2) the error that was occurred in the metadata:

https://github.com/fumoboy007/swift-retry/blob/9edd186966f69b4edff52e0eecfcc03440c6ceb2/Sources/Retry/Retry.swift#L328-L330

Taking into account the issues above, it seems to me that passing the missing bits to the recoverFromFailure callback is better approach anyways, because it is more type-safe and solves the public vs private data problem above, since the user can decide whether to log the error in question on their own.

edigaryev avatar Apr 17 '24 08:04 edigaryev

(Sorry for the delay. I missed your reply.)

swift-retry package does not seem to set the (1) number of attempts left nor (2) the error that was occurred in the metadata:

I just released version 0.2.4, which logs the maximum allowed number of attempts and the full error. Thanks for the suggestions!

Taking into account the issues above, it seems to me that passing the missing bits to the recoverFromFailure callback is better approach anyways, because it is more type-safe and solves the public vs private data problem above, since the user can decide whether to log the error in question on their own.

Hopefully with the above changes, this shouldn’t be necessary!

fumoboy007 avatar Jun 05 '24 16:06 fumoboy007

@fumoboy007 thank you!

I haven't tested it yet, but with the changes you've mentioned this should be possible, so closing the issue.

edigaryev avatar Jun 07 '24 12:06 edigaryev