2

I recently found out about a nuget called LanguageExt.Core and why it is not so efficient to throw exceptions while handling them via middleware or so.

Speaking of it, I would like to know what is the best way to simply check if the result has faulted, so I can throw the exception in order to trigger Polly's retry pattern logic.

The best I could think of:

private async Task RunAsync(Uri uri)
{
    await ConnectAsync(uri);

    var result = await ConnectAsync(uri);

    if (result.IsFaulted)
    {
        // Cannot access result.Exception because it's internal
    }

    _ = result.IfFail(ex =>
    {
        _logger.LogError(ex, "Error while connecting to the endpoint");
        throw ex;
    });

    ...
private async Task<Result<Unit>> ConnectAsync(Uri uri)
{
    if (_ws is not null)
    {
        return new Result<Unit>(new InvalidOperationException("The websocket client is already connected"));
    }

    var ws = Options.ClientFactory();
    try
    {
        using var connectTimeout = new CancellationTokenSource(Options.Timeout);
        await ws.ConnectAsync(uri, connectTimeout.Token).ConfigureAwait(false);
    }
    catch
    {
        ws.Dispose();
        return new Result<Unit>(new InvalidOperationException("Failed to connect to the endpoint"));
    }

    await _connectedEvent.InvokeAsync(new ConnectedEventArgs(uri));

    _ws = ws;
    IsRunning = true;

    return Unit.Default;
}
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
Hulkstance
  • 1,323
  • 9
  • 15
  • 1
    The question is unclear. `ws.ConnectAsync` is still throwing. You've already paid that cost. You gain nothing by hiding that exception and returning a different unrelated one. `Result` is used when you want to adopt a functional programming style, not for performance reasons. In this case though, nothing is gained because `ws.ConnectAsync` is still called. Your own `ConnectAsync` is called in an imperative, not functional, manner. – Panagiotis Kanavos Nov 14 '22 at 14:32
  • @Hulkstance As I can see the `ConnectAsync` can *return* with `InvalidOperationException`. Do you want to retry based on that exception? Can the `_connectedEvent.InvokeAsync` throw some other exception? – Peter Csala Nov 15 '22 at 10:22
  • @PeterCsala, yup. `_connectedEvent.InvokeAsync` cannot throw exceptions. – Hulkstance Nov 15 '22 at 11:04
  • @PanagiotisKanavos, you're right about that. The question was all about https://github.com/louthy/language-ext/issues/1141 but I guess I didn't make it clear on what I really need. – Hulkstance Nov 15 '22 at 11:19

2 Answers2

1

You can just handle a result with IsFaulted == true and not throw the exceptions yourself.

This is a snippet from one of my apps:

this.policy = Policy
  .TimeoutAsync(ctx => ((MyContext)ctx).Timeout)
  .WrapAsync(Policy
    .HandleResult<Result<string>>(result => result.IsFaulted)
    .WaitAndRetryForeverAsync( // Will get stopped by the total timeout. Fits as many retries as possible.
      (retry, _) => TimeSpan.FromSeconds(retry / 2d),
      (result, retry, wait, _) => result.Result.IfFail(
        exception => this.logger.Error(exception ?? result.Exception, "Error! {Retry}, {Wait}", retry, wait)
      )
    )
  );
var policyResult = await this.policy.ExecuteAndCaptureAsync(
  async (_, ct1) => await Prelude
    .TryAsync(this.ConnectAsync(ct1))
    .MapAsync(async _ => await this.SendAsync(mimeMessage, ct1))
    .Invoke(),
  new MyContext(timeout),
  ct
);

return policyResult.Result.Match(
  _ => policyResult.Outcome == OutcomeType.Failure
    ? new InfrastructureException("Error!", policyResult.FinalException).ToResult<Unit>()
    : Unit.Default,
  e => new InfrastructureException("Error!", e).ToResult<Unit>()
);
public static class LangExtExtensions
{
    [Pure, MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static TryAsync<T> TryAsync<T>(this Task<T> self) => Prelude.TryAsync(self);

    [Pure, MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static TryAsync<T> TryAsyncSucc<T>(this T self) => Prelude.TryAsyncSucc(self);

    [Pure, MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static TryAsync<T> TryAsyncFail<T>(this Exception self) => Prelude.TryAsyncFail<T>(self);

    [Pure, MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static Try<T> TrySucc<T>(this T self) => Prelude.TrySucc(self);

    [Pure, MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static Try<T> TryFail<T>(this Exception self) => Prelude.TryFail<T>(self);

    [Pure, MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static Result<T> ToResult<T>(this Exception self) => new(self);
}

Hope this helps a litte

EDIT: some more code

You can try something like this:

public ctor()
{
  this.policy = Policy
    // There will be no exceptions thrown outside the TryAsync monad
    // Handling faulted results only is enough
    .HandleResult<Result<string>>(result => result.IsFaulted)
    .WaitAndRetryAsync(
      10,
      (retry, _) => TimeSpan.FromSeconds(retry / 2d),
      (result, retry, wait, _) => result.Result.IfFail(
        exception => this.logger.Error(exception ?? result.Exception, "Error! {Retry}, {Wait}", retry, wait)
      )
    );
}

public Task<Result<Unit>> RetryAsync()
{
  var uri = ...;
  var policyResult = await this.policy.ExecuteAndCaptureAsync(
    async (_, ct1) => await this.ConnectAsync(uri) // Should use CancellationToken!
      .MapAsync(async _ => await this.RunAsync(ct1))
      .Do(result => this.logger.Log(result))
      .Invoke(),
    new Context(),
    ct
  );
  return policyResult.Result.Match(
    _ => policyResult.Outcome == OutcomeType.Failure
      ? new Exception("Retries did not complete successfully", policyResult.FinalException).ToResult<Unit>()
      : Unit.Default,
    e => new Exception("Error even after retries", e).ToResult<Unit>()
  );
}

private Task<string> RunAsync(CancellationToken ct = default)
{
  // Logic here
  return "Success"
}

private TryAsync<Unit> ConnectAsync(Uri uri)
{
  if (_ws is not null)
  {
    return new InvalidOperationException("...").TryAsyncFail<Unit>();
  }
  
  return Prelude
    .TryAsync(async () => {
      var ws = Options.ClientFactory();
      using var connectTimeout = new CancellationTokenSource(Options.Timeout);
      await ws.ConnectAsync(uri, connectTimeout.Token).ConfigureAwait(false);
      return ws;
    })
    .Do(async ws => {
      await _connectedEvent.InvokeAsync(new ConnectedEventArgs(uri));
      _ws = ws;
      IsRunning = true;
    })
    .Map(_ => Unit.Default);
}

// The _ws is long-lived so you can move dispose logic to here
// and let service lifetime handle that for you.
// You might want to add checks for being connected to the code above too
public async ValueTask DisposeAsync()
{
  if(_ws is { IsConnected: true })
  {
    await _ws.DisconnectAsync();
  }
  await _ws?.DisposeAsync();
}

As you can see, there is no throwing and catching of exceptions. Wrapping an exception in a result (and a delegate monad) is ~100x more efficient than throwing. The TryAsync monad lets you write short and declarative code overhead of which is inconsequent compared to classical exception workflow.

Heehaaw
  • 2,677
  • 17
  • 27
  • 1
    The exception is already thrown by `ws.ConnectAsync`. Nothing is gained by hiding it here. Wrapping the already thrown exception in a `Result` only to try and get back the exception only adds overhead – Panagiotis Kanavos Nov 14 '22 at 14:34
  • But you avoid the re-throw, which adds much more overhead. You can return a `Result` from your `Run` method and process the wrapped exception instead of catching. You can also refactor the `Connect` method to use `TryAsync` instead so that you do not need to care about any exceptions whatsoever. – Heehaaw Nov 14 '22 at 14:59
  • 2
    If you don't catch you don't need to rethrow. `Result` is used to switch to a functional style. It doesn't help if all the rest of the code is still imperative – Panagiotis Kanavos Nov 14 '22 at 15:02
  • 1
    @Heehaaw, thanks a lot for the effort you put in your answer! :) I ended up writing my own `Result` implementation, since all I needed was https://github.com/louthy/language-ext/issues/1141 which was unavailable in that library, i.e. the key points are the use of `MemberNotNullWhen` in order to calm down the IDE in situations like `if (result.IsSuccess) { var data = result.Data; }`. The working snippet can be found in the github issue link. :) – Hulkstance Nov 15 '22 at 11:42
  • @PanagiotisKanavos If there is an exception and you want to handle it, you have to catch it. In the OP's code there is a rethrow in the `IfFail` predicate; the exception gets caught inside the connect method. (Re)Throwing exceptions is an order of magnitude slower that returning results with exception instances wrapped inside them. Using all the LanguageExt stuff allows us to propagate external exceptions to the end of execution and avoid any overhead of rethrowing internally. With the added benefit of increased legibility. – Heehaaw Nov 16 '22 at 16:03
1

Based on the comments it seems like that ConnectAsync can fail with an InvalidOperationException inside a Result object. In this particular case the Result's IsFaulted is set to true only if an IOE is passed to the Result's ctor.

So, if you want to perform a retry then you should define your policy something like this:

var retryPolicy = Policy<Result<Unit>>
    .HandleResult(r => r.IsFaulted)
    .WaitAndRetryAsync(...)
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
  • 1
    Thanks, yours technically answered the question :) I failed to explain what I actually needed, it was more about the LangExt syntax - https://github.com/louthy/language-ext/issues/1141. However, after having a look at their code, I decided that it's not really worth using that library just because of that. In the end, I ended up writing a `Result` implementation myself, similar to the snippet I provided in the github issue. And yup, used what you actually wrote as answer to retry. – Hulkstance Nov 15 '22 at 11:30
  • 1
    @Hulkstance Cool :) Your `Result`'s implementation inside the github issue seems good to me. – Peter Csala Nov 15 '22 at 11:38