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

Logging the final exception? #383

Closed
thurfir opened this issue Dec 13, 2017 · 7 comments
Closed

Logging the final exception? #383

thurfir opened this issue Dec 13, 2017 · 7 comments

Comments

@thurfir
Copy link

thurfir commented Dec 13, 2017

Hello,

In order to log the final exception after all retries have failed we are currently using the fallback event:

var fallbackPolicy = builder.FallbackAsync((token, context) => { return Task.CompletedTask; }, (ex, context) => LogFinalError(ex, context));

As suggested in another thread. However since the last update this operation now longer works, we are getting an exception:

You have executed the generic .Execute<TResult> method on a non-generic FallbackPolicy. A non-generic FallbackPolicy only defines a fallback action which returns void; it can never return a substitute TResult value. To use FallbackPolicy to provide fallback TResult values you must define a generic fallback policy FallbackPolicy<TResult>. For example, define the policy as Policy<TResult>.Handle<Whatever>.Fallback<TResult>(/* some TResult value or Func<..., TResult> */);

I suppose this is linked to the fix #294 ?

Is there a way to attach an event on fallback without having to handle the return type?

By the way this library is great, keep on the good work!

@reisenberger
Copy link
Member

reisenberger commented Dec 14, 2017

Hi @thurfir You are correct; the (necessary) fix #372 for #294 is related.

I can see options for supporting the logging scenario from the next release: proposals/suggestions will follow

@reisenberger
Copy link
Member

@thurfir My suggestion is going to be that a LoggingPolicy is implemented as a custom policy. #391, when released as part of Polly v5.7.0, will make custom policies possible. I will intend that the documentation for custom policies uses LoggingPolicy as an example.

@reisenberger
Copy link
Member

triage of recent issues: Another interim suggestion for logging the final exception/failure would be to use .ExecuteAndCapture/Async(...) overloads, and log when the PolicyResult.Outcome indicates the final result was a failure. Obviously this is not as concise as a native LoggingPolicy would be - though it could be made concise by wrapping in a method. Just adding other options to this thread.

I have not yet provided the promised LoggingPolicy example of a custom policy, as we are now working on Polly v6.0, where implementation for custom policies should be yet simpler than in v5.7+

@thurfir
Copy link
Author

thurfir commented Mar 14, 2018

Thanks @reisenberger , I took a look at custom policies but I'm not confident enough about Polly's internals to create a Policy without guidance.
About our specific case we are more than happy to stick with v5.5 for now, we will wait for Polly v6.0 before upgrading if this is what you recommend.

@reisenberger
Copy link
Member

reisenberger commented Sep 8, 2018

Closing, as we have proposed a number of possible solutions here.

To recap: the original issue was that we closed down the possibility of using a non-generic FallbackPolicy with generic .Execute<TResult>() methods, because the compiler gotcha (which Polly can't change) of silently assigning a Func<TResult> to an Action meant that the combination of non-generic FallbackPolicy with generic .Execute<TResult>() could lead to apparently silently dropped TResult values (see #294). However, closing down those overloads (#372) presented a problem for users like @thurfir using the non-generic FallbackPolicy as a generic exception-logging policy.

A further option

FallbackPolicy now requires a generic FallbackPolicy<TResult> for all TResult-returning executions. One could however continue to use FallbackPolicy as a logging-only policy by making a simple factory class to manufacture FallbackPolicy<TResult> dumb-logging policies as needed: [clarity: by dumb I mean dumb in the technical sense of not doing much, not a judgment!]

static class LoggingPolicyFactory
{
    static Policy<TResult> GetLoggingPolicyFor<TResult>() // Of course extra parameters could be used to refine the faults/exceptions handled
    {
        return Policy<TResult>.Handle<Exception>() // Of course extra parameters could be used to refine the faults/exceptions handled
            .Fallback(
                fallbackAction: (outcome, context, token) => default(TResult), // We never cared about this when we were using non-generic `FallbackPolicy` just for logging, so this should do.
                onFallback: (outcome, context) => { Log(outcome?.Exception?.ToString());} // Of course a logger could be injected.
                );
    }

    static void Log(string message) // Of course a logger could be injected rather than a hard method here; just for simplicity of this example.
    {

    }
}

@thurfir
Copy link
Author

thurfir commented Sep 19, 2018

Hello @reisenberger , wouldn't .Fallback with an Action prevent the Exception from actually bubbling?

Is it safe to rethrow the exception from onFallback, after logging of course?

Thank you.

@reisenberger
Copy link
Member

reisenberger commented Sep 19, 2018

Is it safe to rethrow the exception from onFallback, after logging of course?

Throwing an exception from within either onFallback or fallbackAction will cause that exception to propagate further outwards; the FallbackPolicy will not swallow or govern it in any way. (Policies govern the delegate passed to Execute(...), or all actions of a policy deeper inside a PolicyWrap. Policies do not guard their own actions (fallbackAction; onFallback; onRetry etc); that could lead to all manner of recursion.)

wouldn't .Fallback with an Action prevent the Exception from actually bubbling?

FallbackPolicy catches the exceptions it is configured to handle and passes control to the configured fallbackAction after calling onFallback. FallbackPolicy does not rethrow the exception it handled. If fallbackAction or onFallback rethrow it will bubble; if they do not, it will not.

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

No branches or pull requests

2 participants