vkhorikov / CSharpFunctionalExtensions

Functional extensions for C#
MIT License
2.43k stars 303 forks source link

Proposal: An Error class which has an inner error. #533

Open opcodewriter opened 7 months ago

opcodewriter commented 7 months ago

With the result pattern, I often feel the need to have code which is able to create an error based on another error:

public UnitResult<Error> SaveItem(Item item)
{  
     Result<Stream, Error> getFileResult  = OpenFile(...);
     if(getFileResult.IsFailure)
     {
           var error = .... ; // Create a new error based on an the getFileResult.Error

           return UniResult.Failed(error2);
    }

     ...
}

In order for Error class to be able to support inner errors, I created it like this;

    public class Error(string message, Exception? exception, Error? innerError)
    {
        public string Message { get; } = message;
        public Exception? Exception { get; } = exception;
        public Error? InnerError { get; } = innerError;

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

        public static implicit operator Error(string message) => new(message);
    }

which allows me to write:

 return UnitResult.Failed(error: new("Add item failed.", getFileResult.Error));

But since this Error class does not already exist in CSharpFunctionalExtensions, am I the first who needs this concept of inner errors?

I think it's important to keep a stack of inner errors, similar to how exceptions work.

Looking forward to hear your thoughts!

vkhorikov commented 7 months ago

I'm personally on the fence about this one. I see the value, but struggle to see if it's really worth putting into the library, given that I personally haven't had a lot of use for it.

Could you please describe the use cases where you use the inner errors? That would help put it into proper context.

opcodewriter commented 7 months ago

@vkhorikov Thanks for your reply.

The main idea is to be able to have errors which contain inner errors, similar to how the inner exceptions work. This allows maintaining a stack of errors.

If you have a flow where method A calls method B which in turn calls method C, and all the three methods return a Result, when method A returns, you will be able to see all the error stack, instead of just the error returned by C.

Without an error which can contain inner errors, how do you have a good context of what went wrong in a chained call?

vkhorikov commented 7 months ago

Sorry for the late reply.

Without an error which can contain inner errors, how do you have a good context of what went wrong in a chained call?

In my experience, once you encounter an error, you just short-circuit and exit from all further processing. I personally never needed inner errors to be part of the error I'm returning (multiple errors shown side-by-side in a list -- yes, but not multiple errors where one includes another one).

That's why I asked for an example from your past projects. That would help put this feature into proper context. Could you describe one?

tecnocrata commented 7 months ago

I agree with your perspective regarding the InnerError; however, I still believe that support for an Error class would be valuable. There are scenarios where an error string might not be sufficient, and I have encountered situations where it was necessary to raise business errors that eventually manifest as HTTP errors. These errors require localization into various languages, making an Error class with a code or localization code property beneficial for handling them at the front end. I hope this clarifies my viewpoint.

vkhorikov commented 6 months ago

Yeah, there's a separate open issue for the Error class, which I agree is a good idea to add to the library.