feat(errors): complete refactor Phase 1 with exception filter tests a…#61
Conversation
…nd documentation - Add tests verifying JsonApiExceptionFilter serializes code, source, and meta fields - Complete JSON:API spec compliance audit (no deviations found) - Update upgrade-guide.md with v1.3.0 changes - Update enhanced-error-handling.md with factory method examples
🤖 I have created a release *beep* *boop* --- ## [1.3.0](Intility.JsonApiToolkit-v1.2.5...Intility.JsonApiToolkit-v1.3.0) (2026-01-24) ### Features * ✨ `AllowedIncludesAttribute` to whitelist allowed include paths ([6a26c29](6a26c29)) * ✨ `JsonApiOkAsync` ([d22466d](d22466d)) * ✨ `JsonApiOkAsync` ([bc26940](bc26940)) * ✨ add filtering support for included resources ([4e81c99](4e81c99)) * ✨ add support for filtering in primary resource with included r… ([0b0bb87](0b0bb87)) * ✨ add support for filtering in primary resource with included relationships ([5c90d41](5c90d41)) * ✨ add too many reqyests exeption ([94a810d](94a810d)) * ✨ Allow collections and json columns to be mapped ([6a096bc](6a096bc)) * ✨ Code cleanup and standardization of error handling ([fec75f5](fec75f5)) * ✨ Enhance QueryHelpers with enum support and additional types ([4949c26](4949c26)) * ✨ general-purpose exception class ([0cdf9a5](0cdf9a5)) * ✨ general-purpose exception class ([e222bb4](e222bb4)) * ✨ Overall project cleanup ([c8c10f4](c8c10f4)) * ✨ Remove IncludeAsAttribute and related logic ([a1593be](a1593be)) * ✨ Support complex JsonCols ([b744b8e](b744b8e)) * 📚 add comprehensive debugging guide and enhance logging for better troubleshooting ([0ecc0cf](0ecc0cf)) * 🚀 add ApplyFiltersOnly method for pre-aggregation filtering and add documentation on statistics and aggregations ([b9c6546](b9c6546)) * 🚀 enhance query processing with AsSingleQuery for pagination and add detailed logging for inclusion processing ([7824da9](7824da9)) * **errors:** add JsonApiErrorCodes and JsonApiErrors factory methods ([#60](#60)) ([8531ad3](8531ad3)) * **errors:** complete refactor Phase 1 with exception filter tests a… ([#61](#61)) ([e5b50be](e5b50be)) ### Bug Fixes * 🐛 single included resources are no longer ignored ([1e2e4b6](1e2e4b6)) * 🚑️ `[JsonIgnore]` not being respected ([af12b0b](af12b0b)) * 🚑️ adds support for filtering on included collection fields ([ee2eb19](ee2eb19)) * 🚑️ adds support for filtering on included collection fields ([1194fd6](1194fd6)) * 🚑️ adjust query processing order for filtered and regular includes to enhance EF Core compatibility ([8d21509](8d21509)) * 🚑️ bracket nested filtering without the nessesary includes breaking main filtering ([e1e5785](e1e5785)) * 🚑️ correct version number in project file to match release version ([a0a51dd](a0a51dd)) * 🚑️ error responses for forbidden includes did not include meta information ([9610878](9610878)) * 🚑️ filtering on includes not working on 2-level ([c658327](c658327)) * 🚑️ Fixed the filtering issue for included resources. ([86cab81](86cab81)) * 🚑️ improve error messages for forbidden includes to clarify not found status ([07ea15e](07ea15e)) * 🚑️ Initial working fix. Needs further testing and validation. ([0fa5628](0fa5628)) * 🚑️ JsonApiOk and JsonApiCreated methods not adding includes ([903eda3](903eda3)) * 🚑️ refactor querying files and fix single resource relationship issues ([962d4d4](962d4d4)) * 🚑️ reorder query processing to apply sorting before includes for better EF Core compatibility ([20bf0d9](20bf0d9)) * 🚑️ three level nested values and collection include filters ([7f9a336](7f9a336)) * 🚑️ three level nested values and collection include filters ([044aaf0](044aaf0)) * 🚑️ use single query mode to prevent EF Core split query correlation issues with filtered includes ([ff48615](ff48615)) * add defensive reflection checks with ReflectionMethodCache ([#57](#57)) ([75eb978](75eb978)) * **mapping:** remove dead AddIncludedResourcesRecursive method ([#55](#55)) ([bbc8c17](bbc8c17)) * **pagination:** guard against division by zero when Size is 0 ([#59](#59)) ([0863dee](0863dee)) * **parsing:** guard unsafe string parsing in filter parsers ([#58](#58)) ([9fb463d](9fb463d)) * **security:** prevent log forging and add workflow permissions ([#51](#51)) ([5fbbaba](5fbbaba)) * **security:** prevent log forging and update tooling ([#52](#52)) ([52d73ce](52d73ce)) * support JsonPropertyName attribute and fix many-to-many collecti… ([634abff](634abff)) * support JsonPropertyName attribute and fix many-to-many collection filtering ([6f1d961](6f1d961)) ### Refactoring * 🔨 follow ts-package renaming ([4cd1e7e](4cd1e7e)) * 🔨 optimize logging and add XML documentation ([8c14bc0](8c14bc0)) * 🔨 remove Microsoft.Identity.Abstractions package reference ([55933b7](55933b7)) * 🔨 remove the OR max count ([65107d5](65107d5)) * 🔨 remove the OR max count ([5a3aa87](5a3aa87)) * 🔨 Update JsonApiOk function and docs to align with what it actually does ([bfe7635](bfe7635)) ### Documentation * 📝 update stats docs ([549743c](549743c)) * 📜 add too many request exeption to docs ([872ae2a](872ae2a)) * 📜 Clarify that filtering is only on main entity ([5ee3568](5ee3568)) * 📜 Update Claude.md ([88502bb](88502bb)) * 📜 update error message for forbidden includes to clarify not found status ([95ab6ce](95ab6ce)) ### Dependencies * **actions:** bump actions/checkout from 4 to 6 ([#47](#47)) ([a16ab53](a16ab53)) * **actions:** bump actions/setup-dotnet from 4 to 5 ([#45](#45)) ([db8c0d1](db8c0d1)) * **actions:** bump actions/upload-pages-artifact from 3 to 4 ([#44](#44)) ([c5e35fb](c5e35fb)) * **actions:** bump github/codeql-action from 3 to 4 ([#46](#46)) ([4bad70c](4bad70c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: intility-release-bot[bot] <175299729+intility-release-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds documentation and tests to support the Phase 1 error-handling refactor, focusing on richer JSON:API error payloads (code/source/meta) and updated v1.3.0 guidance.
Changes:
- Adds new unit tests ensuring
JsonApiExceptionFiltercarries throughcode,source, andmetafields (including factory-generated exceptions). - Extends enhanced error-handling documentation with
JsonApiErrorsfactory method guidance and examples. - Updates the v1.3.0 upgrade guide checklist items to marked-complete.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/docs/upgrade-guide.md | Marks v1.3.0 bug fixes and features as completed in the upgrade checklist. |
| docs/docs/enhanced-error-handling.md | Documents JsonApiErrors factory methods with usage examples and sample responses. |
| JsonApiToolkit.Tests/Filters/JsonApiExceptionFilterTests.cs | Adds tests verifying error code, source, and meta are propagated into the filter’s response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _filter.OnException(context); | ||
|
|
||
| var result = Assert.IsType<ObjectResult>(context.Result); | ||
| var errorResponse = Assert.IsType<JsonApiErrorResponse>(result.Value); | ||
| var error = errorResponse.Errors[0]; |
There was a problem hiding this comment.
This test doesn’t assert context.ExceptionHandled or that exactly one error was produced. Adding Assert.True(context.ExceptionHandled) and Assert.Single(errorResponse.Errors) (and optionally verifying the 400 status) would align with the other tests and ensure regressions in exception handling/multi-error behavior are caught.
| _filter.OnException(context); | ||
|
|
||
| var result = Assert.IsType<ObjectResult>(context.Result); | ||
| Assert.Equal(404, result.StatusCode); | ||
|
|
||
| var errorResponse = Assert.IsType<JsonApiErrorResponse>(result.Value); | ||
| var error = errorResponse.Errors[0]; | ||
|
|
There was a problem hiding this comment.
This test indexes errorResponse.Errors[0] without asserting the collection size or that the exception was marked handled. Please add Assert.True(context.ExceptionHandled) and Assert.Single(errorResponse.Errors) before accessing [0] to make the test fail with clearer intent if behavior changes.
| _filter.OnException(context); | ||
|
|
||
| var result = Assert.IsType<ObjectResult>(context.Result); | ||
| Assert.Equal(409, result.StatusCode); | ||
|
|
||
| var errorResponse = Assert.IsType<JsonApiErrorResponse>(result.Value); | ||
| var error = errorResponse.Errors[0]; | ||
|
|
There was a problem hiding this comment.
This test should assert context.ExceptionHandled and Assert.Single(errorResponse.Errors) before accessing errorResponse.Errors[0], consistent with the earlier tests, so it also verifies the filter doesn’t emit multiple errors.
| var errorResponse = Assert.IsType<JsonApiErrorResponse>(result.Value); | ||
| var error = errorResponse.Errors[0]; | ||
|
|
||
| Assert.Equal("RESOURCE_NOT_FOUND", error.Code); |
There was a problem hiding this comment.
Consider using JsonApiErrorCodes.ResourceNotFound instead of the string literal "RESOURCE_NOT_FOUND" to avoid duplicating the constant value and keep the test aligned with the library’s public API.
| Assert.Equal("RESOURCE_NOT_FOUND", error.Code); | |
| Assert.Equal(JsonApiErrorCodes.ResourceNotFound, error.Code); |
…nd documentation