Error handling and logging #714
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
April 1, 2020 20:33
…lection of errors, instead of *being* one. Changed JsonApiException to contain a single error instead of a collection.
…st always inappropriate to set. Note it is intended to indicate on which query string parameter the error applies or on which json path (example: /data/attributes/lastName). These being strings may make users believe they can put custom erorr details in them, which violates the json:api spec. ErrorSource should only be filled from model validation or query string parsing, and that should not be an option that can be disabled.
- All errors are now directed to IExceptionHandler, which translates Exception into Error response and logs at level based on Exception type - Added example that overrides Error response and changes log level - Replaced options.DisableErrorStackTraces with IncludeExceptionStackTraceInErrors; no longer static -- this option is used by IExceptionHandler - Added IAlwaysRunResultFilter to replace NotFound() with NotFound(null) so it hits our output formatter (workaround for dotnet/aspnetcore#16969) - Fixes #655: throw instead of writing to ModelState if input invalid - Added test that uses [ApiController], which translates ActionResults into ProblemDetails
…nslated into a database constraint. This makes it impossible to toggle per test. So using regex constraint instead.
…r errors. Replaced class-name-to-query-string-parameter-name convention with pluggable logic. Parsers now have a CanParse phase that indicates whether the service supports parsing the parameter. Their IsEnabled method checks if the query parameter is blocked by a controller attribute (which now supports passing multiple parameters in a comma-separated list). Finally, support has been added for omitNull/omitDefault to be blocked on a controller. And instead of silently ignoring, a HTTP 400 is returned passing omitNull/omitDefault from query string when that is not enabled.
…ated custom exception, updated Errors to match with json:api spec and added error details validation to tests. Fixed missing lowerbound check on negative page numbers. Tweaks in query parameter name checks, to reduce chance of collisions.
…at is thrown when the operating system denies access because of...." Adding this as InnerException is not only wrong (this is not an OS error), but it adds no extra info because there is no nested call stack. If you want to catch this, create a custom exception that derives fromJsonApiException instead.
…ess object construction to central place.
…l and request body.
added 18 commits
April 6, 2020 14:52
…mpty string check from CurrentRequestMiddleware because I found now way to get there. The unit-test was misleading: throwing JsonApiException from CurrentRequestMiddleware does not hit any handler, so it always results in HTTP 500 without a body. Better not throw from there.
Fixed: having default page size of 0 is dangerous (returns complete tables)
Fixes in handling of missing relationships + extra tests
added 4 commits
April 8, 2020 12:09
Inlined TypeHelper.ConvertCollection
…cause they hide potential usages of property names instead of resource names, which only works with the default camel-case convention but breaks on kebab casing.
maurei
approved these changes
Apr 8, 2020
This was referenced Apr 8, 2020
This was referenced May 4, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is mostly about error handling and logging.
IExceptionHandler), which makes logging and exception handling customizable (seeCustomExceptionHandlerin tests).DefaultExceptionHandlerprovides virtual methods to adapt logging level, log message and the conversion from Exception to Error response objects.JsonApiExceptionobjects and added/updated tests to assert on the returned error objects.DisableQueryAttribute.Fixes #631
Fixes #689
Fixes #655
Fixes #709
Fixes #638