Altinn / Altinn/app-lib-dotnet
ADR for Exception handling in C# for Altinn.App.Api
- Dominant language
- C#
- Stars
- 8
- Forks
- 27
- Avg merge
- 1h 21m
- Merged PRs (30d)
- 7
Description
### Description
Handling unexpected error conditions in apps and ensure that we return helpful information in a `ProblemDetails` response that makes troubleshooting easier. Logging errors to console, makes them less accessible to lots of devs (different window locally, and in application insights in tt02/prod). Common issues with known causes should still be explicitly returned from controller, to make it more explicit for API documentation what kind of responses a client should look for.
As this is impacting global config for the app, we should make a conscious decision and to improve consistency of tha app-lib code.
### In scope
Errors for
* Inconsistent configuration that should be fixed in by app developers
* References to unknown tasks/pages/components/expression functions ...
* Infrastructure, networking or configuration issues with the (local)app platform.
* Human-readable messages to API client developers about mistakes in their code (e.g. when they typically would fix their code to not make the mistake instead of writing code to handle the response)
* Missing parameters
* Missing auth
* Invalid commands in the current state.
In short issues that we (or api users) would typically investigate when we see them in logs.
### Out of scope
* Checks/validation in the beginning of controller actions to ensure that auth is correct and
* Issues that API clients tries to call `/process/next` on an instance in a task with validation issues
In short, any errors/issues that can be handled gracefully by an api client (e.g. issues that an end user might understand and try again)
### Additional Information
Currently, app-lib does not have a consistent way of handling error conditions. Sometimes it throws an exception which looks fairly nice in development, but results in an empty 500 error in production. Other times various levels of error handling is implemented in the controller or service level to improve the response and make it more useful.
### Analysis
### 1. Don't handle general exceptions in controllers, but register an `IExceptionHandler` to rewrite exceptions to a proper `ProblemDetails` response
1. Create custom exception types for various kinds of errors, capturing the information that we want to expose in the `ProblemDetails` response.
2. and create cases in the `IExceptionHandler` that maps exception info to a `ProblemDetails` response.
#### Pros
1. Common pattern where you can simply throw an exception anywhere to both fail the request and include info for the ``ProblemDetails`` response.
#### Cons
1. Interferes with global configuration of the app. App developers can't have their own totally separate way of handling exceptions in middleware. This is likely not a very common use case, and if someone overrides exception handling in middleware, they should be aware that this might cause issues. The only issue is that some debug information might be lost, so concecuences
### 2. Wrap all of our controller actions in try/catch and forward exceptions to our own `ErrorHandlerService`
This is the same as alternative 1. except that the location of the try/catch is moved from middleware to the controller action so that we don't interfere with global configuration of the app.
#### Pros
1. Does not interfere with global configuration of the app, so that app developers don't need to agree with our approaches.
#### Cons
More code in controller actions, and more likely that we
### 3. Continue with an ad hoc approach with some controller actions having a try/catch and decide what to return for unexpected exceptions.
#### Pros
A more explicit approach makes it more likely that errors that should be exposed in openApi doc are actually exposed, so that clients know they can expect them.
#### Cons
This will require much more code for each case that we want to add custom ``ProblemDetails.Description`` for, witch will reduce the number of cases that we will actually implement.
### Conclusion
TBD
Contributor guide
Assessment
This issue has not been assessed yet.