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

Exception refinement: Adding additional information #2800

Merged
merged 6 commits into from
Sep 3, 2024

Conversation

iNinja
Copy link
Contributor

@iNinja iNinja commented Aug 29, 2024

Exception refinement: Adding additional information.

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • If any gains or losses in performance are possible, you've included benchmarks for your changes. More info
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Refining the agreed model to simplify where possible and match the original validation path's output.

Description

  • Removed generic error from Result type. All uses share the same constant error type. Removing it from the generic simplifies our interfaces and makes the code easier to read.
  • Restored use of Activator to instantiate the exceptions in an AOT compatible manner.
  • Added initial proposal of extending ExceptionDetail to assign additional information to exceptions

Notes:

For the additional data, 3 approaches were evaluated:

  • Creating specialised result types: this results in a lot of code duplication if we want to keep our value type result, or an extra allocation if we turn it into a class to use inheritance.
  • Passing a dictionary to ExceptionDetail that can then be used to extract the information in each exception results in an extra allocation of the dictionary.
  • Extending ExceptionDetail to allow the provision of specialised classes that receive the extra data and assign it only when the exception is instantiated. This was the chosen path forward for this PR since it avoids any new allocations and allows extensibility between ExceptionDetail subclasses and Exception subclasses. It makes no assumptions about the types.

Part of #2711

@iNinja iNinja marked this pull request as ready for review September 3, 2024 18:21
@iNinja iNinja requested a review from a team as a code owner September 3, 2024 18:21
Rename ExceptionDetail.Type to ExceptionDetail.ExceptionType
Adjust #nullable placement for consistency.
@brentschmaltz brentschmaltz merged commit 191329c into dev Sep 3, 2024
6 checks passed
@jennyf19 jennyf19 added this to the 8.0.3 milestone Sep 20, 2024
@brentschmaltz brentschmaltz deleted the iinglese/exception-refinement-new-data branch October 23, 2024 21:54
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.

4 participants