MrDave1999 / SimpleResults

A simple library to implement the Result pattern for returning from services
https://mrdave1999.github.io/SimpleResults
MIT License
103 stars 2 forks source link

Overload to allow Exceptions #60

Closed StevenBlair123 closed 6 months ago

StevenBlair123 commented 6 months ago

More of a question, than a request. What would be your opinion on adding an overload on the ErrorResult to accept to Exception (and maybe even a String,Exception)

Something like this:

try
{
    var state = await domainEventHandler.Handle(domainEvent, domainEventMetaData,cancellationToken);
    return new Result.Success(state);
}
catch(Exception e)
{
    return new ErrorResult(e);
}
MrDave1999 commented 6 months ago

Hello @StevenBlair123,

I don't see any benefit in adding an overload that accepts an exception (maybe you have an idea?). The purpose of the result pattern is so that the consumer does not have to deal with exception handling and in this example I see that it does not follow that purpose. Honestly, I would avoid using try-catch.

StevenBlair123 commented 6 months ago

It's maybe a specific use case for use, but when a "WrongExpectedException" occurs it informs the Command Handler that it can retry a specific command. I guess we could look at changing it to check for a String value (i.e. result.Message.Contains("WrongExpected") We still need to use try catch for libraries that don't support returning a Result.

MrDave1999 commented 6 months ago

What is "WrongExpectedException"? It is a generic exception. What specific error does it represent?

We still need to use try catch for libraries that don't support returning a Result.

Are they third-party libraries or custom libraries?

StevenBlair123 commented 6 months ago

Third party unfortunately. WrongExpectedException in this example is when we write to our Eventstore and an optimistic concurrency issue has occured. It's not a fatal issue, and in some cases, retrying it works, rather than having to do a full round trip again.

MrDave1999 commented 6 months ago

If there is an overload to allow exceptions, then the caller can access the exception object, right? Although I think it would be simple that instead of accepting an exception, it would accept a message:

try
{
    var state = await domainEventHandler.Handle(domainEvent, domainEventMetaData, cancellationToken);
    return new Result.Success(state);
}
catch(Exception ex)
{
    return new ErrorResult(ex.Message);
}

What matters to the caller is the error message. Because it does not need to expose the entire exception object as the stack trace. Because the stack trace would only be necessary if it is a fatal issue but you just mentioned that WrongExpectedException is not.

StevenBlair123 commented 6 months ago

Yeah, that's exactly what we have done to integrate using your library. We had our own version of "Result" prior to this (but your version is better)

MrDave1999 commented 6 months ago

Okay, so I think I've solved your question, or is there something else?

StevenBlair123 commented 6 months ago

Yeah, i think we can close this. Thanks for the quick response.

MrDave1999 commented 6 months ago

You're welcome!