Changing the Result signature from Result<Unit, string> to Result<Unit, Error>

[vkhorikov]

I want to gather feedback on the change I've been pondering lately.

The problem

In most of my projects, I use a custom Error class (with a code and a message) to denote application errors. The problem with this approach is that it requires the use of Result<T, E> or UnitResult<E>, which looks clunky and bloated.

I would like to use the simple Result<T> and Result in such situations. These classes are currently a shortcut for Result<T, string> and Result<Unit, string>, meaning that the error type is string in both of them (Unit means "nothing" or "void").

The solution

I propose to:

1. Introduce a new class Error with string Code and string Message fields
2. Change Result<T> and Result such that they use the Error class as the error type instead of string

This will allow to reduce the Result<T, E> signature to just Result<T> and UnitResult<E> to just Result.

Potential issues

1. This is a breaking change, though the consequences can be mitigated by introducing additional conversions between the string and Error classes.
2. I don't know how many people use custom error types vs simple strings, this change might be disruptive to all of them
3. I don't know whether the signature of Error is sufficient for everyone (i.e the two string fields: Code and Message). People might need additional fields and thus won't be able to use the built-in Error and therefore will have to use the custom Result<T, E> anyway

Personally, I feel that Error should have been introduced in the library from the very beginning. String is not a good type to represent errors anyway.

Please leave your thoughts in the comments.

cc @space-alien @hankovich @linkdotnet @JvSSD

[linkdotnet]

Personally I am very much in favor of an Error object. This is also the way golang handles error. The difference would be that golang returns a tuple.

Maybe it would be worth to return Result<T, TError> where TError : Error and only provide a basic Error - Implementation. Then the developer can either take the Error class from the library or does its own version.

[SamuelViesselman]

I agree that projects tend to outgrow using a simple string as an error response quickly, and that it should have been an Error object from the beginning.

I generally use Enums instead of strings for Error codes, so it wouldn't help me out immediately. If the proposed Error type was introduced into the library, I would strongly consider switching over to using strings for my error codes for the sake of convivence.

I like this proposed change, even if it is breaking, as I'd like to see this library keep expanding/improving. Thinking about my projects it would be tedious to make the change, but it wouldn't be complex or error prone.

[vkhorikov]

Maybe it would be worth to return Result<T, TError> where TError : Error and only provide a basic Error - Implementation. Then the developer can either take the Error class from the library or does its own version.

The main benefit of having a built-in Error is reducing the signature size (so essentially a syntax sugar). Result<T, TError> wouldn't add any benefits because I would still need to specify the error type (which I already can do now). The idea is to come up with an Error class that would meet most peoples needs, so that we don't have to use Result<T,E> most of the time.

I generally use Enums instead of strings for Error codes, so it wouldn't help me out immediately. If the proposed Error type was introduced into the library, I would strongly consider switching over to using strings for my error codes for the sake of convivence.

Yeah, enums is a better solution from a maintenance perspective (they are more strongly typed than strings), but they aren't extensible. We can't hard code all possible error types in the library and allowing users to do so would require specifying the type of the enum in the Result signature, which defeats the purpose.

There's probably a middle-ground option here: you could use string constants instead of enums. Another option is to predefine the list of possible Errors and use them instead of enums (similar to how enumerations currently work in the library, for example: https://github.com/vkhorikov/ValidationInDDD/blob/master/src/DomainModel/Error.cs#L46-L62 ).

[vbreuss]

When using it in a web environment I often have the "problem" of matching errors to http status codes. For this use case I often use an ErrorType property that either represents directly an Http Status code or at least is an enum specifying if it is a user- or server error (similar to the distinction between 4xx and 5xx status codes). I am not sure if providing a rough categorization makes sense for this library in general, but it would at least help in my use cases...

Overall I am much in favour of this change!

[vkhorikov]

Including an Http status in the error violates the Separation of Concerns principle (conflating application concerns with domain concerns), but that might be a needed concession still, this is a common scenario in a lot of projects.

[hankovich]

Personally, I never use Result, so I have nothing to say here.

I use Result<T> quite rarely (as a return type for methods which parse value objects, for example), for me it serves just as a replacement for Result<T, string> (for cases when I do not distinguish between different error types). But it seems to me that for most projects, plain Result<T> is enough (in many places, in the case of web applications, developers just want to return 400 Bad Request + some kind of error message). Again, the entire README focuses on Result<T> only, so it seems to me that changing the error type for Result<T> would break a lot of code.

Most projects don't need anything other than Result<T>. They are not going to do any compensation logic in case of errors (which they might need a strongly typed error representation for).

Therefore, it seems to me that such a breaking change will not benefit the library. It seems like the ideal solution to your problem would be to implement this proposal (https://github.com/dotnet/csharplang/issues/1239) so that you can write global using MyResult<T> = Result<T, MyError>. Unfortunately, we won't see such a feature in C# 10.

I would like to add that the proposed error format is unlikely to satisfy everyone. If, nevertheless, there is a desire to make this breaking change, then I could suggest such a format of the Error type:

public class Error // not sealed to allow consumers to create their own Errors
{
    public string Message { get; }

    public IDictionary <string, object> Metadata { get; } // Possible names: [Properties, Extensions].
    // It's mutable to allow different handlers to add their own properties during processing / error handling

    // I don't really thing Code should be here, it doent's sound like it's generic enought

    public Error? InnerError { get; } // Can be handy, but I'm not 100% it's required
}

By the way, I realized that we can take a look at the ProblemDetails structure (https://datatracker.ietf.org/doc/html/rfc7807), it was also designed as an error format suitable for everyone (however, we need to exclude everything that is specific to HTTP).

Personally, I now use Result<T, TError> 99% of the time, where TError is a very tricky self-written discriminated union that covers all possible outcomes (enum is not suitable, since you cannot add additional state to enum, and for some errors it is necessary to pass additional information). And TError is different for each operation.

P.S. If I were asked to design a library from scratch, then I would probably implement Result<T> as Result<T, Error>, and not as Result<T, string>, so my single concern is compatibility.

[vbreuss]

I didn't suggest to directly use an http status code in the library. It's just an example for the use case of an error categorization. But I am not sure if there is a generic enough approach...

I think the most important thing is to not make the error class sealed. Then I can enhance it with the categorization if needed...

[vkhorikov]

Thanks @hankovich . Could you elaborate on this?

Personally, I now use Result<T, TError> 99% of the time, where TError is a very tricky self-written discriminated union that covers all possible outcomes (enum is not suitable, since you cannot add additional state to enum, and for some errors it is necessary to pass additional information). And TError is different for each operation.

This is the only benefit of Result<T,E> over Result<T> IMO: that you can return different error types in different methods and make it more strongly typed (in a sense that the compiler won't allow you to use errors that are incompatible with what you specified in the return type), but I never tried it in my projects. Maybe you could give some code samples?

so my single concern is compatibility

It could be possible to preserve compatibility. After all, Error will contain a Message field and thus the new Result may preserve the existing APIs but work via Error behind the scenes. This needs more investigation, though.

My main concern is the implementation of Error. On one hand, it should be encompassing enough to cover most of peoples' use cases. But on the other -- I don't want to overcomplicate it.

@vbreuss I don't think it would be user-friendly to require people to enhance the Error class. This means that you would need to do something like this, which I'd rather avoid:

Result<User> result = RegisterUser();
MyError error = (MyError)result.Error;

Here, the conversion is required because the library doesn't know about the custom MyError class.

[lonix1]

I must often encapsulate a model's validation errors, where a single error string is insufficient. So I use a Dictionary<string, string[]> which denotes the invalid property, and a list of error messages for each one -- standard approach.

Example pseudocode:

var failures = ValidateModel(model);
if (failures.Any()) {
  var error = new Error(
    "Validation failed.",
    failures.GroupBy(x => x.PropertyName).ToDictionary(k => k.Key, v => v.Select(x => x.ErrorMessage).ToArray())
  );
  return error;  // or Result<Error> or Result<T,Error> or something like that (this is just pseudocode)
}

It would be nice if that could be considered in the design of the new Error class.

[vbreuss]

I don't mean to require users to enhance the error class, but to support it. This way you can create your own hierarchy of errors, but the library itself doesn't need to be aware of it. Possible examples would be

• a specific ValidationError with additional information about what kind of properties are incorrect or missing (as in the previous post )
• a ClientError, ServerError to distinguish between different error types. This way I could represent the categorization by its type and provide additional information if required. The library itself treats everything as an Error, but by extending it, I can adapt it to my needs.

So my suggestion would be to keep the error class as small as possible. Keep only the message string and leave everything else extendable to consumers. This way it would also be easier to implement implicit conversion between string and the Error class.

[lonix1]

Agree with @vkhorikov and @vbreuss so here's my quick stab at a middle-ground approach, UNTESTED:

[Serializable]
public partial class Error {        // not sealed; partial so can move factory methods to other files

  public Error(string message) : this(message, null) { }

  public Error(IDictionary<string, object> metadata) : this(null, metadata) { }

  public Error(string? message, IDictionary<string, object>? metadata) {
    if (message == null && metadata == null) throw new ArgumentNullException();
    Message = message;
    Metadata = metadata;
  }

  public string? Message { get; }

  public IDictionary<string, object>? Metadata { get; }

  // some users may want more, but what do we add? let's keep it simple
  // public string? Code { get; }                           // wanted by @vkhorikov
  // public IDictionary<string, object>? Metadata { get; }  // wanted by @hankovich, @lonix1
  // public Error? InnerError { get; }                      // wanted by @hankovich
  // public string? ErrorType { get; }                      // wanted by @vbreuss
  // public Severity Severity { get; }                      // common in many libraries
}

public interface IResult {
  bool IsSuccess { get; }
  bool IsFailure { get; }
  Error Error { get; }
}


public interface IResult<out T> : IResult { }

[Serializable]
public partial class Result : IResult {        // partial so can move factory methods to other files

  protected Result(bool isSuccess, Error? error) {
    if (isSuccess) {
      if (error != null) throw new ArgumentOutOfRangeException(nameof(error));
    }
    else {
      if (error == null) throw new ArgumentNullException(nameof(error));
    }
    IsSuccess = isSuccess;
    _error = error;
  }

  public bool IsSuccess { get; }
  public bool IsFailure => !IsSuccess;

  private readonly Error? _error;
  public Error Error => IsFailure ? _error! : throw new InvalidOperationException();

}

[Serializable]
public sealed class Result<T> : Result, IResult<T> {

  private Result(bool isSuccess, Error? error, T? value) : base(isSuccess, error) {
    if (isSuccess) {
      if (value == null) throw new ArgumentNullException(nameof(value));
    }
    else {
      if (value != null) throw new ArgumentOutOfRangeException(nameof(value));
    }
    _value = value;
  }

  private readonly T? _value;
  public T Value => IsSuccess ? _value! : throw new InvalidOperationException();

}

Yeah, enums is a better solution from a maintenance perspective ...We can't hard code all possible error types...

Maybe we could use a "typesafe enum" for that? Users could subclass it to add error codes:

  public interface IErrorCode {
    string Value { get; }
  }


  public class ErrorCode : IErrorCode {      // not sealed
    protected ErrorCode(string value) => Value = value;
    public string Value { get; }
    public static readonly IErrorCode Error = new ErrorCode(nameof(Error));
  }


  public sealed class MyErrorCode : ErrorCode {
    public MyErrorCode(string code) : base(code) { }
    public static readonly IErrorCode Unknown          = new MyErrorCode(nameof(Unknown));
    public static readonly IErrorCode ValidationFailed = new MyErrorCode(nameof(ValidationFailed));
    public static readonly IErrorCode ServerError      = new MyErrorCode(nameof(ServerError));
    public static readonly IErrorCode Foo              = new MyErrorCode(nameof(Foo));
    public static readonly IErrorCode Bar              = new MyErrorCode(nameof(Bar));
  }


  public class Test {

    public Result Callee() {
      // ...something fails
      var error = new Error(MyErrorCode.ValidationFailed, "Validation failed.", null);    // can add factory methods to make this prettier
      return new Result(false, error);
    }

    public void Caller() {
      var result = Callee();
      if (result.Error.ErrorCode.Value == MyErrorCode.ValidationFailed.Value) {
        // ...do something
      }
    }

  }

[hankovich]

Could you elaborate on this?

@vkhorikov Sure. It's kinda domain-specific, but I'll try to explain in a project-agnostic way.

Both server and client (Unity) parts are written in C#, but the server and client parts are developed by different teams. We want our system to be contract-driven and since both teams use C# to write code, we also use C# to define our contracts. Contracts are shared assemblies with interfaces and dtos. Those contracts are implemented twice: on the service side and on the client side (client libraries are used by Unity developers as well as by backend developers to make http calls between microservices).

We have a lot of operations, each of which has it's own set of possible error outcomes, but there are some common error outcomes, applicable to all operations:

• Authentication/Authorization problems
• Rate limiting problems
• Availability problems (i.e. 502-504 http status codes for clients and those situations when databases/queues are unreachable for services). We threat them as errors, not exceptions
• Unknown problems (mostly for bugs like validation stuff (format validation, not the domain one, i.e. missing query parameters, nulls for not-nullable properties, etc) and literally everything which is expected, but not domain specific, 500 status codes are also go here). This is also used as a fallback value for errors which we can't parse (like for situations when the client is using an obsolete contract and server returned some error with type that client is not yet aware of)

Those common outcomes have generic handlers:

• For Authentication/Authorization problems client performes a JWT update and tries to call the operation again. Server for each operation performes Authentication/Authorization and possibly return strongly-typed authorization error
• For Rate limiting problems client can wait a bit and call the operation again. Server for each call check whether client is allowed to call an operation and possibly return strongly-typed rate limiting error
• For some Availability issues client can return cached values
• For Unknown problems client can do almost nothing, but for such cases we want to log all possible information and append all available context to the log

All our operations are designed to be retryable (i.e. they are idempotent), so we can write generic decorators that handle retries.

We want all that common logic to be shared for all TError types and we also want out operations to have honest signatures with strongly-typed errors, each for each operation.

The ideal solution is to use something like inheritable discriminated unions (then we'll be able to extract common outcomes to the base type), but unfortunately we don't have such features in C#, so we decided to design our own solution for TErrors.

We decided to model error as combination of type and properties. Let's try to model type first:

public abstract class ErrorTypeBase
{
    protected string Type { get; set; } = string.Empty;
    // some additional stuff like equality operators
}

public abstract class ErrorType<TErrorType> : ErrorTypeBase // so called "recursive generics"
    where TErrorType : ErrorType<TErrorType>, new()
{
    private static readonly Dictionary<string, TDerived> KnownTypes = new();

    public static readonly TDerived Unauthorized = Register();
    public static readonly TDerived TooManyRequests = Register();
    public static readonly TDerived Unknown = Register();
    // other common types

    public static TDerived Parse(string? type)
    {
        // Get from KnownTypes
    }

    protected static TDerived Register([CallerMemberName] string type = "")
    {
        // GetOrAdd to KnownTypes
    }
}

public sealed class ErrorType : ErrorType<ErrorType> // simple type which contains only shared values
{
}

Those classes help us to model enum-like error types. Let's model a simple 'get entity by id' operation.

public sealed class GetBookErrorType : ErrorType<GetBookErrorType>
{
    public static readonly GetBookErrorType NotFound = Register();
}

// possible values
GetBookErrorType type1 = GetBookErrorType.Unknown;
GetBookErrorType type2 = GetBookErrorType.Unauthorized;
GetBookErrorType type3 = GetBookErrorType.TooManyRequests;
GetBookErrorType type4 = GetBookErrorType.NotFound;

So far so good. Sometimes we also have to pass parameters of an error, to let's add a wrapper on top of error types:

public class ErrorBase
{
    public ErrorBase()
    {
        Type = ErrorType.Unknown;
        Reason = Type.ToString();
        Details = new JObject();
    }

    public ErrorTypeBase Type { get; protected set; }

    public string Reason { get; protected set; }

    public JObject Details { get; protected set; }
}

public class Error<TError, TErrorType> : ErrorBase    // O
    where TError : Error<TError, TErrorType>, new()   // M
    where TErrorType : ErrorType<TErrorType>, new()   // G
{
    public Error()
    {
        Type = ErrorType<TErrorType>.Unknown;
        Reason = Type.ToString();
    }

    public new TErrorType Type
    {
        get => (TErrorType)base.Type;
        protected set => base.Type = value;
    }

    public static TError Unknown(string reason)
    {
        return new()
        {
            Type = ErrorType<TErrorType>.Unknown,
            Reason = reason,
        };
    }

    // factory methods for other common values
    
    public static TError TooManyRequests(TimeSpan retryAfter, string? reason = null)
    {
        return new()
        {
            Type = ErrorType<TErrorType>.TooManyRequests,
            Reason = reason ?? "You have sent too many requests in a given amount of time.",
            Details = JObject.FromObject(new Dictionary<string, object>
            {
                [RetryAfterValueName] = retryAfter,
            })
        };
    }

    private const string RetryAfterValueName = "RetryAfter";
    
    // additional methods to extract parameters
    public Result<(TimeSpan RetryAfter, string Reason)> GetTooManyRequestsParameters()
    {
        if (Type != ErrorType<TErrorType>.TooManyRequests)
        {
            return Result.Failure<(TimeSpan RetryAfter, string Reason)>("detailed error message");
        }
        
        return (Details[RetryAfterValueName].ToObject<TimeSpan>(), Reason);
    }
}

And we're finally ready to model our TError:

public sealed class GetBookError : Error<GetBookError, GetBookErrorType>
{
    public GetBookError NotFound(BookId bookId, string? reason = null)
    {
        return new()
        {
            Type = ErrorType<TErrorType>.NotFound,
            Reason = reason ?? "The requested book wasn't found.",
            Details = JObject.FromObject(new Dictionary<string, object>
            {
                [BookIdValueName] = bookId,
            })
        }
    }
    
    private const string BookIdValueName = "BookId";
    
    public Result<(BookId BookId, string Reason)> GetNotFoundParameters()
    {
        if (Type != GetBookError.NotFound)
        {
            return Result.Failure<(TimeSpan RetryAfter, string Reason)>("detailed error message");
        }
        
        return (Details[BookIdValueName].ToObject<BookId>(), Reason);
    }
}

And our operation can look like:

public async Task<Result<Book, GetBookError>> GetBookAsync(BookId id, CancellationToken cancellationToken)
{
    if (IsFirstRequest)
    {
        IsFirstRequest = false;
        return GetBookError.TooManyRequests(TimeSpan.FromSeconds(1));
    }

    return GetBookError.NotFound(id);
}

// and on the client side we can do

public async Task ConsumeAsync(BookId id, CancellationToken cancellationToken)
{
    var result = await GetBookAsync(id, cancellationToken).OnFailureCompensate(async error =>
    {
        if (error.Type == GetBookError.TooManyRequests)
        {
            var span = error.GetTooManyRequestsParameters().Map(t => t.RetryAfter).Match(t => t, _ => TimeSpan.Zero);
            await Task.Delay(span, cancellationToken);
        }
        
        return await GetBookAsync(id, cancellationToken);
    });
}

And we're also able to write generic decorators:

public Task<Result<T, TError>> RetryAsync<T, TError>(Func<Task<Result<T, TError>>> func)
    where TError : ErrorBase
{
    var policy = Policy // from the Polly nuget package
        .HandleResult<Result<T, TError>>(error => error.IsFailure && error.Error.Type == ErrorType.Unknown)
        .Or<Exception>()
        .RetryAsync(5);

    return policy.ExecuteAsync(func);
}

In addition, we can utilize source generators to generate code and omit some boilerplate. And on the server side we can also use DispatchProxy to perform generic authentication/authorization, rate limiting and for much more other coolness! That's basically how we model errors, huh.

Our solution may seem convoluted, but we have achieved all our goals, we can have strongly-typed errors and reuse all common logic with no pain.

[vkhorikov]

Thanks @vbreuss , this looks reasonable, yes. My point was that you'll need to do additional conversion at the points of consumption of failed result instances, but that shouldn't be a huge deal, there are few of those places anyway.

@lonix1 , this is roughly how I see the implementation too, though the additional class for ErrorCode looks excessive, I would rather keep the whole solution as simple as possible, at least for V1.

@hankovich thanks for the in-depth answer. In your example, the string Type property looks similar to the Code property that I was suggesting.

Your solution looks very solid overall, but I'm wondering if it can be simplified. I have a couple of questions:

1. The TooManyRequests error handler would seem more appropriate in a generic decorator that would process responses from all API endpoints. If so, would it make more sense to place it in a separate ErrorType class (e.g. GenericErrorType), rather than "copying" it to all other error types? Is it solely to enable discriminated-union-esque approach where you can list all possible errors for each method?

2. I see that the client needs strong typing to process TooManyRequests and Unknown error types. Do you have any use cases where the client (or the server) reacts to non-generic (e.g. NotFound) error types in a similar way? E.g. not just showing that error to the user.

3. Regarding metadata. The use case with TooManyRequests is clear and quite nice. Do you have any other usage examples?

I'm trying to understand what kind of error definition will be minimally sufficient for us. So far, I'm thinking of this one (borrowing the code from @lonix1's sample):

[Serializable]
public partial class Error {        // not sealed; partial so can move factory methods to other files

  public string Code { get; } // Or Type?
  public string? Message { get; }
  public IDictionary<string, object>? Metadata { get; } // I'd personally rather skip this property, though
}

[lonix1]

@vkhorikov I've been playing with a similar design for a few days and learned that the Metadata property is not as useful as it first appears, and you'll run into cases where a strongly-typed dictionary will be insufficient. It should be an object?, if included at all. And an ErrorCode class or valueobject adds no value.

So the design from your original post is the best one: a string code, a string message, and maybe a metadata object.

[vkhorikov]

@lonix1 good to know. That was my experience too, though I'd like to take into account other people's use case.

[hankovich]

@vkhorikov Sorry, completely missed your questions.

1. The example with the TooManyRequests handler was added just for demo purposes indeed. We have generic decorators extracted to be reusable across all endpoints. Yeap, we want each operation to explicitly list all possible outcomes. Our decorators are created to be reusable and easy opt-in/opt-out, but in the end they only try to handle errors. It's totally possible that even after retrying (or updating the JWT), the operation will still fail. And we want the client developers to handle all possible outcomes (well, at least we want developers to know all of them).

2. Yes! There are quite a lot of them. Mostly it's very straightforward mapping (like when the nested microservice returned NotFound, we map the result to NotFound of a different type), but some of them are quite interesting. We have a lot of different products/projects, so we created a bunch a product-agnostic Lego-brick-style microservices and then we write separate product-specific ervices which contain business logic that unique for each product. And usually product-specific services contain a lot of error handling logic.

For example:

async Task<Result<GuildId, CreateGuildError>> CreateGuildAsync(UserId originatorId, string name)
{
    var groupId = GroupId.GenerateNew(); // GuildId, UserId, GroupId are simple wrappers on top of System.Guid
	
    var result = await _nestedService.CreateAsync(new CreateGroupRequest
    {
        GroupId = groupId,
        Name = name,
        LeaderId = originatorId,
        GroupNamesAreUnique = true,
        UserCanOnlyBeInOneGroup = true,
    }); // nested service is project-agnostic and does not know how to handle conflicts
	
    if (result.IsSuccess) // we use extensions of course :) just for demo purposes
    {
        return groupId;
    }
	
    if (result.Error.Type == CreateGroupError.NameIsNotUnique)
    {
        var (existingGroupId, existingLeaderId) = result.Error.GetNameConflictParameters();
	
        if (existingLeaderId == originatorId)
        {
            return existingGroupId;
        }
	
        return CreateGuildError.NameConflict(existingLeaderId);
    }

    if (result.Error.Type == CreateGroupError.UserInAnotherGroup)
    {
        var (userGroupId, userGroupName, userGroupLeaderId) = result.Error.GetUserInAnotherGroupParameters();
	
        if (userGroupName == name && userGroupLeaderId == originatorId)
        {
            return userGroupId;
        }

        return CreateGuildError.GuildConflict(userGroupId);
    }

    // another logic

    static GuildId ToGuildId(GroupId groupId) => GuildId.Parse(groupId.ToString());
}

Here we see our product-specific Guild service which uses the nested Group service and knows that

• guilds are just groups
• guild names must be unique, users are members of no more than one guild
• the method must be idempotent/retryable and be as fail-tolerant as we can, i.e. if the user is already in the group they wants to be, we just return the existing group id.

3. You can get two additional examples in my previous answer. Many mutating operations (POSTs/PUTs) have similar outcomes (409 Conflict) and we provide as much context as possible. Some metadata values are just valuable for our clients, i.e. is processed (or at least shown) on the client side.

I can provide additional examples:

Task<Result<MissionDescriptor, StartMissionError>> StartNextMissionAsync(GuildId guildId, UserId originatorId);

public interface StartMissionError // I wrote all the interesting results as interface members (just to force github to render them nicely) 
{
    // not very interesting errors

    void NotEnoughMoney(Money balance, Money price); // also used to actualize balance (users can play from two devices simultaneously) 
    void MissionSkipped(string reason, DateTimeOffset nextMissionAvailableAt, Money missionPrice);
    void YouAreBlacklisted(string reason, UserId blacklisterId, DateTimeOffset blacklistedAt, DateTimeOffset? blacklistedTo);
}

[vkhorikov]

@hankovich Thanks for another in-depth answer. These are really nice examples. I haven't come across such use cases in my own projects, hence I was curious too see more samples.

It's interesting, looks that such a rich error infrastructure makes sense primarily in scenarios with rich clients as in your Unity/gamedev project. I don't think a lot of typical business-line applications (e.g React on the frontend and .NET at the backend) would need the same level of detail when it comes to errors.

[leksts]

The Error class could provide a way to pass additional arguments for localized error messages. For instance, the value of courseName in this line: https://github.com/vkhorikov/ValidationInDDD/blob/master/src/DomainModel/Error.cs#L52

The application receiving this error could then construct a localized version of the error message using the error code and the arguments provided.

[seangwright]

I'm in favor of this change because it think it's the correct architectural approach, however it will be painful for me to make the upgrade as we've used Result and Result<T> quite a bit. 🤷‍♂️ It is what it is...

[vkhorikov]

@leksts Yeah, good point.

@seangwright The breaking change should be minimal. We'll need to change the Error property's type from string to Error and introduce a new ErrorMessage property so that the current clients of Result can be easily retargeted, but all the remaining APIs will remain the same. We can still accept strings in Result, but instead of putting them directly to ErrorMessage, those strings will be put into a "generic" error to conform with the new Result structure.

[vkhorikov]

I have a question for everyone. For context, here's the current plan for the Error class:

public class Error {

  public string Code { get; }
  public string MessageTemplate { get; }
  public object[] MessageArguments { get; }
  public string Message => string.Format(MessageTemplate, MessageArguments);
}

The question is: should Result contain just one Error or a collection or errors? Would appreciate if everyone chimes in @hankovich @seangwright @leksts @lonix1 @linkdotnet @SamuelViesselman @vbreuss

[linkdotnet]

For me a typical use case is validation where having a collection of errors might be more handy. For convenience one can write itself an extension method which always retrieves the first error if existing.

[lonix1]

I'm using an Error class with a single error - and extensions methods to map various failures (validation, etc.) to collections of Errors.

Works for me and is simple. The more I tried to get clever in this problem, the messier it became, and the more edge cases I found that required a redesign. Eventually I discovered that for this problem, the absolute simplest solution was the best.

[SamuelViesselman]

I would echo what Ionix1 said. I normally use enums instead of an error code string, but I would switch to using the string purely for the convenience of it being built into the library. I understand the impossibility of using an enum for the library's Error type. Vladimir has pretty clearly laid out in his blog how to enumerate the list of possible error strings statically

With the proposal, you're also able to define compound errors pretty easily for somewhat complex processes. For example error codes like; customer.duplicate.name, customer.duplicate.number, and customer.duplicate.name.and.number

When I get into the more complex scenarios where I need collections of errors I can always use the custom error type. I would prefer to not default the library to the most complex scenario.

[seangwright]

I might be the exception here, but I use string errors almost all the time. Maybe my use cases don't require anything more complex?

I have used custom error types for complex workflows where rules are defined on how to handle specific kinds of failures, but most of the time it's enough to log the string error and compensate in a simplistic way when there is a failure.

I guess I use this library more as an expression-based alternative to statements/conditionals rather than a way to model a complex domain, while also being glad that it supports both use cases.

I agree with those that have said to default to simple use cases and let library consumers provide their own more complex solutions when needed.

[leksts]

My vote would go to the collection of errors, so you could tell your consumer email.invalid and email.toolong. Main reason for this is I want to be able to aggregate the errors from 1 or more results and have my frontend code loop over these so it can display the errors with the corresponding inputs.

Using and error like email.toolong.and.invalid and splitting that into two separate localized messages would mean I can't just blindly loop over the errors, meaning my frontend code isn't dumb anymore.

To be honest, creating a combined localized message is an acceptable workaround in 99% of the cases. And for the other 1%, there is always the custom error type, as stated by Samuel. But it would be cool if the workaround wasn't need, though.

While I understand the reasoning behind returning a single error, if it's not too much trouble, it would be nice if the default was a collection of errors. Especially if the collection is a specialized collection which handles the single error variation very well.

[leksts]

I couldn't resist. 😊 Here is an example of what I think the ErrorCollection might look like: https://github.com/leksts/ErrorCollectionPoc

[vkhorikov]

Thanks everyone, appreciate your input!

I personally gravitate toward the simpler solution with just one error in Result, but the use case with aggregating all errors from 1 value object is indeed too common to ignore. I'll experiment with solutions for this that preserve the simplicity and don't require too much of additional code.

One such solution could be to include a built-in ErrorCollection (or ErrorList) in the library. It would still require Result<T. E> but at least you don't need to write your own collection implementation.

@leksts thanks for the sample implementation :)

[leksts]

👍 It got me thinking though; my main reason for wanting the default implementation to match my use-case is so I can write slightly less bulky method signatures (public async Task<UnitResult<ErrorList>> Handle(...) becomes public async Task<Result> Handle(...)), plus other derived advantages.

Simultaneously, I see an ongoing debate/investigation (#151, #186) whether or not classes should be used for Result. I'm not sure if moving to classes is still an option. If it is, that would make this discussion less important. Because then it doesn't matter what the default is, everyone can create their own default implementation by inheriting from Result<T, E>.

[vkhorikov]

Because then it doesn't matter what the default is, everyone can create their own default implementation by inheriting from Result<T, E>.

No, that wouldn't work because the existing extensions will still return library's Result, so you'd have to manually cast those to your custom implementation.