Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Host] Refactor 'Retry' delegate in IConsumerErrorHandler as response type #354

Merged
merged 1 commit into from
Dec 29, 2024

Conversation

EtherZa
Copy link
Contributor

@EtherZa EtherZa commented Dec 29, 2024

While the discussed 'pipeline reimagination' was a little too ambitious for the time that I have available; I have refactored the implementation to extract the retry delegate from IConsumerErrorHandler.OnHandleError and to rather include it as a response value. This does introduce a breaking change, but allows the handler to perform all of the previous logic albeit with a small change for retry.

The OnHandlerError method signature therefore changes from:

Task<ConsumerErrorHandlerResult> OnHandleError(T message, Func<Task<object>> retry, IConsumerContext consumerContext, Exception exception);

to:

// where `attempts` is the execution count
Task<ConsumerErrorHandlerResult> OnHandleError(T message, IConsumerContext consumerContext, Exception exception, int attempts);

In order to retry the execution (with a new scope as requried), the implemented method must return ConsumerErrorHandlerResult.Retry. Further to this, abstract implmentations of IConsumerErrorHandler have been added with methods to support Failure(), Success(object? response = null) and Retry() for a cleaner experience (similar to return Ok(); with asp.net controllers).

Example:

public class RetryHandler<T> : ConsumerErrorHandler<T>
{
    private static readonly Random _random = new();

    public override async Task<ConsumerErrorHandlerResult> OnHandleError(T message, IConsumerContext consumerContext, Exception exception, int attempts)
    {
        if (attempts <= 3)
        {
            var delay = (attempts * 1000) + (_random.Next(1000) - 500);

            await Task.Delay(delay, consumerContext.CancellationToken);
            return Retry();
        }

        return Failure();
    }
}

@zarusz
Copy link
Owner

zarusz commented Dec 29, 2024

This is an excellent and pragmatic approach to improving the error-handling pipeline while achieving the message scope recreation as discussed in issue #347. The implementation is mostly backward-compatible and could potentially outperform the Func<Task> next delegate approach.

I've suggested a few minor adjustments to the documentation for clarity and completeness.

Once those are addressed, I’ll be happy to approve the PR. Great work!

@zarusz zarusz added this to the 3.0.0 milestone Dec 29, 2024
…e instead of supplying a 'retry' delegate

Signed-off-by: Richard Pringle <[email protected]>
@EtherZa EtherZa force-pushed the bugfix/347-retry-scope branch from ac7df01 to 4828613 Compare December 29, 2024 11:45
@zarusz zarusz merged commit 2e5662d into zarusz:release/v3 Dec 29, 2024
1 check passed
@zarusz zarusz mentioned this pull request Dec 29, 2024
@zarusz
Copy link
Owner

zarusz commented Dec 29, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants