Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternate proposal for ASP.NET Core conversion #110

Draft
wants to merge 3 commits into
base: asp-net-core-nuget
Choose a base branch
from

Conversation

feO2x
Copy link
Contributor

@feO2x feO2x commented May 20, 2024

Hey Amichai,

sorry for taking so long, but I'm sick these days. I did a rather large refactoring regarding the conversion of List<Error> to IActionResult and IResult.

What is done

  • The ErrorOrOptions now has less members. With CustomToErrorActionResult and CustomToErrorResult, clients can completely circumvent our default implementation of the conversion.
  • Our default implementation now uses the prototype pattern. I introduced a new struct called ProblemDetailsPrototype which holds all properties of the problem details and the errors. This instance is then used to either create IActionResult via our default implementation or the ProblemDetailsFactory, or IResult via our default implementation. Clients can completely replace our creation of the prototype by using ErrorOrOptions.CustomCreatePrototype.
  • When the prototype is created by our default implementation and there are several errors in the list, we will now determine the leading error type. If all errors have the same type, it becomes the leading type. Otherwise, the leading type is 500 Internal Server Error by default. Clients can change this behavior by setting ErrorOrOptions.UseFirstErrorAsLeadingType to true so that the leading type is determined by the first error in the collection. This algorithm does not apply if there only is one error.
  • I have extended the ErrorTypes with more HTTP status codes. All the defaults for problem details can be found in class ErrorDefaults. Clients can customize this dictionary with the options.ErrorDefaults dictionary.
  • I also invested time so that Error instances are properly serializable. This allows us to fully recreate them on the calling side, which is pretty great IMO in distributed systems. A service could make a call to another service and then fully recreate potential error instances and use them after their call in memory - see MinimalApiTests for an example. For this to work, I implemented ErrorProblemDetails and a ErrorConverter (this was necessary for the metadata dictionary).
  • Our default implementation also tries to create appropriate IActionResult and IResult object - see CreateDefaultErrorResult and CreateDefaultErrorActionResult for details.

What still needs to be done

  • We need to write more tests. I only did the bare minimum so we have something to discuss.
  • We need XML comments.
  • How do we deal with Unauthorized and Forbidden? These results usually have no problem details - should we handle them completely differently?
  • The metadata object is currently pretty hard to serialize because TValue is object - it would be great if we could change Error.Metadata to Dictionary<string, string>. If we don't do that, we will have trouble with serialization as we either need to implement pretty complex conversion logic or throw runtime exceptions (as we do right now). Of course, this would be a breaking change.
  • I still need to tweek the ErrorOrOptions API, especially the delegates that exchange our default implementation should receive an options instance so they can act based on the info stored in there.

Conclusion

IMHO, the ability to move error instances across processes and transform them back to ErrorOr<TValue> has a huge potential. Consider that a proxy service receives an HTTP request and uses another technology like MassTransit or GRPC to communicate with other backend services. When we adopt all those technologies, we could be the library that allows for easy integration between these, saving clients a huge amount of work, while decoupling their domain logic from I/O. This would set this library apart from similar libraries like FluentResults.

feO2x added 2 commits May 18, 2024 21:25
… API IResult and MVC IActionResult

Signed-off-by: Kenny Pflug <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants