error-or icon indicating copy to clipboard operation
error-or copied to clipboard

Adding IsError/Value to IErrorOr

Open wesley-sd opened this issue 1 year ago • 6 comments

Hi. I am writing LoggingPipelineBehavior using MediatR. Here's class:

internal class LoggingPipelineBehavior<TRequest, TResponse> :
        IPipelineBehavior<TRequest, TResponse>
        where TRequest : IRequest<TResponse>
        where TResponse : IErrorOr

And here's my Handle method:

public async Task<TResponse> Handle(TRequest request, CancellationToken _, RequestHandlerDelegate<TResponse> next)
{
    LogRequest(request);
    TResponse response = await next();

    if (response is IErrorOr errorOr && errorOr.Errors?.Count > 0)
    {
        LogErrors(errorOr.Errors);
        return response;
    }

    LogResponse(response);
    return response;
}

Unfortunately, if my handler is returning successful object (not errors), I'm still going inside that if is IErrorOr block, because if ErrorOr.IsError is false, Errors's Count is still getting 1 (NoErrors error). Would it be possible to add bool IsError { get; } to IErrorOr interface, so it can be checked by this flag if this object is successful or failed one? Edit: Also, would it be possible to add object Value { get; } to interface to retrieve successful instance? If there's other/better solution, please let me know. By the way - great package! Thanks, Sławek

wesley-sd avatar Aug 30 '22 09:08 wesley-sd

I'd also love to see this feature. My usecase is to consolidate a potential number of ErrorOr-Objects. The intended method is this one:

private static List<Error> AllErrors(params ErrorOr[] errorOrArray)
{
    var allErrors = new List<Error>();

    foreach (var errorOr in errorOrArray)
    {
        if (errorOr.HasError)
        {
            allErrors.AddRange(errorOr.Errors);
        }
    }
    return allErrors;
}

Currently it would require me to tryCast to ErrorOr which I don't like. If this an unintended usage of the package, I'd be thankful for advice on how to solve this. Thanks for help and the package itself!

janschreier avatar Sep 13 '22 04:09 janschreier

Same here, I can not understand why .Errors contains an element even if the the value is created without problem. It's ambiguous because a lot of time I want to do something like this:

var errors = new List<Error>();
errors.AddRange( myObject_0.Errors );
errors.AddRange( myObject_1.Errors );

if ( errors.Any() ) {
  return errors;
}

return Ok();

kikutano avatar Sep 19 '22 10:09 kikutano

@wesley-sd thanks for the comment. That's a great idea and an easy fix. I'll move the IsError property to the interface 👍Regarding the Value, this is not as straightforward, as it would box value types. Let me give it some thought.

amantinband avatar Sep 22 '22 14:09 amantinband

@kikutano I see what you mean. What do you think about a method similar to ErrorsOrEmptyList()?

Then, your code can look something like:

var errors = new List<Error>();
errors.AddRange( myObject_0.ErrorsOrEmptyList());
errors.AddRange( myObject_1.ErrorsOrEmptyList() );

if ( errors.Any() ) {
  return errors;
}

return Ok();

amantinband avatar Sep 22 '22 14:09 amantinband

@kikutano I see what you mean. What do you think about a method similar to ErrorsOrEmptyList()?

Then, your code can look something like:

var errors = new List<Error>();
errors.AddRange( myObject_0.ErrorsOrEmptyList());
errors.AddRange( myObject_1.ErrorsOrEmptyList() );

if ( errors.Any() ) {
  return errors;
}

return Ok();

Yeah, that ok for me! :) Because I can change this:

var errors = new List<Error>();
        var name = PlainText.Create( request.Name );

        if ( name.IsError ) 
            errors.AddRange( name.Errors );
        
        if ( errors.Any() ) {
            return errors;
        }

in this:

var errors = new List<Error>();
var name = PlainText.Create( request.Name );

errors.AddRange( name.ErrorsOrEmptyList() );

if ( errors.Any() ) {
    return errors;
}

kikutano avatar Sep 22 '22 14:09 kikutano

@wesley-sd thanks for the comment. That's a great idea and an easy fix. I'll move the IsError property to the interface 👍Regarding the Value, this is not as straightforward, as it would box value types. Let me give it some thought.

Regarding the Value, I'm open to suggestions on how to achieve what I want (log only successful instance). Right now, I'm logging it using ILogger (with NLog), and it looks like this:

private void LogResponseDebug(TResponse response)
{
    if (!Logger.IsEnabled(LogLevel.Debug))
    {
        return;
    }

    Logger.LogDebug(LoggingPipelineBehaviorEventId.ResponseDebug, "Response object: {@ResponseObject}.", response);
}

Using the @ formatting, ResponseObject is serialized to json, so right now I'm logging the whole ErrorOr<T> object, which is too much text, since I'm interested only in the successful Value, without the rest of properties (Errors etc).

wesley-sd avatar Sep 22 '22 21:09 wesley-sd