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.

[xavierjohn]

I prefer the error class. I like the idea of message templates with parameters because it plays better with logging frameworks. I am also thinking if the validation error object from data annotations nugget can be used so that the.net framework knows how to deal with it.

[LetMeSleepAlready]

I recently started using this library and Result, and just as a thought "this error string is not enough", I did a (apparently correct) search and voila there is this thread.

As such, I will put in my 2 cents as well, for what it is worth.

My use case is a C# API back-end, angular front-end. Any views below are applicable to my project at hand, and in no way a means to invalidate your use-cases.

Regarding the message dictionary: Most validation errors are handled by model validations, which returns ValidationProblemDetails to the client, which includes error-to-field mapping. Any other validation errors are most likely not related to a specific field. As such, for the actual backend code, there is no hard need for differentiated validation errors. In line with the previous posts, having a dictionary in the standard error class is probably overkill, and could/would only result in more work/typing in other places.

Having a huge enum as suggested in "railway oriented programming" video ( https://www.youtube.com/watch?v=XFagoINwzHo) would mean that the list of error codes are "sealed". For my particular use case the string could be an "enum" value (as suggested in a comment before, thanks!), which would also allow me to translate the message. I can simply attach a translated label to the "enum" key and I would be i18n ready. The only part missing would be positional data, but as I do not need to 'parse' the result errors to handle certain errors differently (it is for human consumption, not for other systems), I could probably encode this somewhere in the "enum" string. maybe some json, or whatever.

My main issue with the error strings is differentiating hard or soft errors. "record not found" is a non-retryable error. "backend service down" however can be retried at a later time. In http status codes these would translate to either a 400 or a 500 error. Again, I don't care about actual http mapping (application domain <> http protocol), but hard/soft error differentiation is good to have for retying.

Actually, while typing out my thoughts, I believe I can easily handle hard/soft errors without too much impact, using only a string. Constraints:

• The error string does not need to be parsed outside the application itself (human consumption)
• In most cases, the error is a hard error due to bad input or state, with a static text.
• Only a few cases will have a soft error, one that can be retried (eg: backend service down)
• Errors are not the happy flow, and can be sub-optimal in terms of performance.

In the general use case: Use string as error. In the error case that indicates a retry is possible, put something such as { rety: true, message: "oops service down"} json in the Result error string

When pushing the error back to the client, I use a Result/Result => IActionResult helper. Some pseudo code: On Success: return OK(possibly with object) On failure: error-status = 400 error-text = result.Error

• if the error-text starts with { then extract error-text and set error-status = 500
• if the error-text can be found in the Labels collection, set error-text to translated text equivalent
• return error-text as ProblemData with status set to error-status

The above could be slightly different for logging, as one would not translate the messages then.

In actual code this would be used like:

return await GetTranslationRepo(parentTableName, AccessType.Create)
    .Bind(repo=> repo.CreateTranslation(parentId, languageTag, text))
    .Map(translation => MapEntity2Type(translation, parentTableName))
    .ToActionResult();

where ToActionResult() does all the above described magic, and acts as a Finally()

So for me, while I started thinking string is not enough for error, it will probably be enough :) At this point I don't believe having a forced Result<T,E> everywhere in my code would offset the extra benefits.

I guess using the above (json), one could handle all other cases described in other comments before. The key thing I got out of this rambling is that while an Error object is "nice" if you need to process errors actively, the reality is that in most cases I don't want to, and only really handle them at the end of the 'rail'. In case error handling is required, a Result extension method such as "result.CanRetry()", which then simply inspects the error string, possibly json and returns based on that, would be a trivial thing to make.

Maybe the above helps the discussion, maybe not...

I saw the proposed error class structure, and I think it has merits but whatever the standard error class is going to be, it will never satisfy all users and all cases. I think I prefer a simple type (eg, string) with some "string to error conversion" mappers in case the error needs to be parsed inside the app (eg, at the end of the rail). Writing your own extensions for this is trivial, and wont cause overhead when not used.

[vkhorikov]

Thanks @LetMeSleepAlready appreciate your input.

My main issue with the error strings is differentiating hard or soft errors. "record not found" is a non-retryable error. "backend service down" however can be retried at a later time. In http status codes these would translate to either a 400 or a 500 error. Again, I don't care about actual http mapping (application domain <> http protocol), but hard/soft error differentiation is good to have for retying.

I'd say this should be the responsibility of the application layer to map errors from the "enum" onto 400/500/404/401/etc. I wouldn't add this directly to Error, at least not in v1.

I'm calling it an "enum" because you can define all your errors in one place, just like with an enum, and use it in a similar way, for example: https://github.com/vkhorikov/ValidationInDDD/blob/master/src/DomainModel/Error.cs#L44

[CzechsMix]

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

My Error type has just one message, and I have an AggregateError : Error whose message is "Aggregate of {n} errors", and an additional property IEnumerable Messages.

public class AggregateError : Error
{
  public IEnumerable<string> Messages { get; }
  private AggregateError(IEnumerable<string> messages)
    : base($"Aggregate of {messages.Count()} errors.")
  {
    Messages = messages;
  }

  public static Result From(IEnumerable<Error> errors) => errors.Count() switch
  {
    0 => Result.Success,
    1 => errors.Single(),
    _ => new AggregateError(errors.Select(e => e.Message))
  };
}

It's a shame C# won't let me implicitly convert an IEnumerable to Result with this implementation.

[TomCumper]

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

@vkhorikov The option to have a collection of errors return with Result would be useful. A project I am working on consumes a number of external API's which often return one or more validation errors. Currently we are having to use Result<T, E> but as you mentioned it seems like a rather clunky way to handle multiple errors.

[zbarrier]

Create a different repo for the solution with the Error class. It will make it easier for others to fork and modify the Error class to be what they need and prevent breaking changes in the current library.

[nabenzine]

@vkhorikov any updates on this plz ? It would be a gamechanger feature !

[vbreuss]

@vkhorikov : I doubt you can foresee all possible use cases in any implementation. I think it would be better to allow users to enhance the implementation to match their use cases and keep the base implementation as simple as possible. Something along the way

    public class Error
    {
        public string Message { get; }

        private Error(string message)
        {
            Message = message;
        }

        public static implicit operator Error(string message)
        {
            return new Error(message);
        }
    }

By including an implicit operator from string, you would simplify migration for existing projects.

---

Users of the library can then extend the Error class as they need them, e.g. in an Web API environment

    public class HttpError: Error
    {
        public int StatusCode { get; }
        public HttpError(string message, int statusCode) : base(message)
        {
            StatusCode = statusCode;
        }
    }
    public class ClientError : Error
    {
        public ClientError(string message) : base(400, message)
        {
        }
    }
    public class ServerError: Error
    {
        public ClientError(string message) : base(500, message)
        {
        }
    }

By improving the extension methods to allow filtering for specific error classes, e.g.

    public static Result OnFailure<TError>(this Result result, Action<TError> action)
    {
        if (result.IsFailure && result.Failure is TError error)
        {
            action(error);
        }
        return result;
    }

users can then react to different error cases.

Or if users need to track a list of errors, they could implement a CombinedError:

    public class CombinedError : Error
    {
        public Error[] InnerErrors { get; }

        public CombinedError(IEnumerable<Error> errors)
        : this(";", errors.ToArray())
        {
        }

        public CombinedError(string separator, IEnumerable<Error> errors)
        : this(separator, errors.ToArray())
        {
        }

        public CombinedError(params Error[] errors)
        : this(";", errors)
        {
        }

        public CombinedError(string separator, params Error[] errors)
        {
            InnerErrors = errors;
            Message = string.Join(separator, InnerErrors.Select(x => x.Message));
        }
    }

[vkhorikov]

@zbarrier

Create a different repo for the solution with the Error class. It will make it easier for others to fork and modify the Error class to be what they need and prevent breaking changes in the current library.

A different repo would bring a whole new set of problems, I'd rather avoid this. The breaking changes shouldn't be that bad, I just haven't had time to work on this (and a couple other) feature yet.

@nabenzine I plan to work closely on this library this summer.

@vbreuss The issue with this approach is that you'd need to manually cast the library's error to your own:

MyError error = (MyError)result.Error;

because the library doesn't know about the MyError subclass. This is quite annoying and I'd rather avoid it. The Error class should have a structure that fits 99% of projects.

[APEX-JOAQUIN-ROVIRA]

Why are we trying to reinvent the wheel by implementing a new Error class when Exception is the standard way to represent errors?

public class Error : Exception { }

It would have the added benefit of allowing projects to gradually move from the traditional throw statements and flows to an FP error handling approach.

public Result<T> Foo<T>() 
{
    try 
    {
        T v = Bar<T>(); // some code that can throw
        return Result.Success<T>(v);
    } 
    catch (Exception e) 
    {
        return Result.Failue<T>(e);
    }
}

[vkhorikov]

I don't think this is a good idea because the 2 have different purposes. Exceptions convey technical information, including stack traces. Error describes errors from the business perspective. In terms of restful APIs, exceptions are bugs/5xx responses; Error is validation errors/4xx responses.

[APEX-JOAQUIN-ROVIRA]

I am 100% with you in the separation of business errors vs bugs/integration errors. However, I would still argue against reinventing the wheel when a simple if check would suffice to differentiate between them.

if (e is Error) return BadRequest(e);
else return InternalServerError(e);

Regarding the point about stack traces being an overhead, they are only captured by exceptions when there is a throw statement. If you create an new one with new Exception(), e.StackTrace will be null. The same thing applies to e.Source, e.TargetSite and e.InnerException.

As such, Exception already implements Message, Metadata (as e.Data) and InnerError (as e.InnerException) as suggested previously in this thread. This will probably adapt to most people's use cases where a simple string isn't enough.

As an added bonus, you get interoperability with the rest of the non-FP C# ecosystem and follows the KISS principle. However, this is just one man's opinion and other alternatives might be better.

[xavierjohn]

Interesting idea of returning exceptions instead of throwing. Except for the name of the exception class like "ArgumentException", etc, it sounds reasonable.

[hankovich]

Exceptions are not errors. Exceptions are not expected (like OutOfMemoryException), while errors are expected and documented (like validation errors). I lot of exception properties only make sense after exception was thrown. What's the benefit of retuning exceptions? I think it's even more counter-intuitive than returning results

[vkhorikov]

Yes, exactly. Here's an old article of mine where I wrote about the differences: https://enterprisecraftsmanship.com/posts/error-handling-exception-or-result/

[glen-84]

Some other result type libraries:

• Ardalis Result
• ErrorOr
• FluentResults
• LanguageExt

I like that ErrorOr doesn't require you to specify T when returning a failure.

---

public class Error {

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

1. I would make MessageArguments an IReadOnlyDictionary<string, object?>, to allow for naming the arguments (compatible with libraries like MessageFormat.NET.
2. I wouldn't force the use of string.Format, since a developer may wish to use another template syntax (f.e. ICU MessageFormat for localization), which may then throw a FormatException.
   • You could require the developer to set the Message in addition to the MessageTemplate.
   • Alternatively, some way to configure the message formatter (maybe a static property like Error.MessageFormatter = (messageTemplate, messageArguments) => string.Format(messageTemplate, messageArguments.Values);.
     • This would require a key in cases where you don't want/need it though, so perhaps there should be a constructor that takes just object?[] and stores the values in the dictionary with dummy keys?
3. I would consider adding a Metadata/Extensions property as suggested by @hankovich, to store any other data related to the error. Maybe make the values nullable as well.

public class Error
{
    public static Func<string, IReadOnlyDictionary<string, object?>, string> MessageFormatter
    { get; set; } = (messageTemplate, messageArguments) =>
        string.Format(messageTemplate, messageArguments.Values);

    public string Code { get; }

    public string MessageTemplate { get; }

    public IReadOnlyDictionary<string, object?> MessageArguments { get; }

    public IReadOnlyDictionary<string, object?> Extensions { get; }
}

[GarethOates]

Is this being worked on at all or is it still in the consideration/ideas phase?

[xavierjohn]

In my library also there is no need to specify Error type, my hope is the Error class I provide will suffice.

https://github.com/xavierjohn/FunctionalDDD/tree/main/RailwayOrientedProgramming/src/Errors