Repository navigation
refactor(api)!: migrate API errors to RFC 7807 ProblemDetail - #149
Open
hamdirahalpartner-del wants to merge 1 commit into
Open
hamdirahalpartner-del wants to merge 1 commit into
hamdirahalpartner-del wants to merge 1 commit into
Conversation
|
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
hamdirahalpartner-del
changed the title
Code Coverage OverviewLanguages: Java Java / code-coverage/jacocoThe overall line coverage in commit 76b4d3f in the Show a line coverage summary of the most impacted files.
Updated |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate findings affect error contracts, status codes, and instance URIs, with related specification and documentation updates pending.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (4)
What changed in this PR
This PR migrates REST API errors from ErrorResponse to Spring ProblemDetail/RFC 7807 while retaining legacy fields and updating contracts, tests, and documentation.
Changes:
- Refactors centralized exception handling with timestamps and legacy fields.
- Updates controller OpenAPI schemas and error-response tests.
- Documents the new error contract and migration considerations.
| File | Reviewed changes and final review notes |
|---|---|
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandlerTest.java |
Updates handler tests. nit (2 votes): pass the request through a framework override and assert ProblemDetail.instance. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityTemplateControllerTest.java |
Updates error assertions; no final review comments. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityDynamicMappingControllerTest.java |
Updates problem media-type assertions; no final review comments. |
src/test/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityControllerTest.java |
Updates error assertions; no final review comments. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java |
Implements ProblemDetail mappings. moderate (3 votes): inherited framework mappings can omit legacy fields (also at lines 107 and 543). moderate (1 vote): preserve Spring’s supplied status code for HandlerMethodValidationException. moderate (1 vote): strip only a leading uri= prefix when constructing the instance URI. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/InboundWebhookConfigurationController.java |
Updates ProblemDetail OpenAPI schemas; no final review comments. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityTemplateController.java |
Updates response annotations. nit (3 votes): regenerate docs/src/static/swagger.yaml so the published specification matches the generated contract. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityGraphController.java |
Updates ProblemDetail OpenAPI schemas; no final review comments. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityDynamicMappingController.java |
Updates ProblemDetail OpenAPI schemas; no final review comments. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityController.java |
Updates ProblemDetail OpenAPI schemas; no final review comments. |
src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/AuditController.java |
Updates ProblemDetail OpenAPI schemas; no final review comments. |
docs/src/contributing/code/exception-handling.md |
Documents the new error contract. nit (1 vote): update the architecture diagram’s stale ErrorResponse reference. nit (2 votes): address the breaking media-type change in the PR title or justify its classification. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+25
to
+26
| The API now returns `application/problem+json` responses. Standard RFC 7807 fields are | ||
| present alongside legacy compatibility fields: |
| @ApiResponse(responseCode = OK_CODE, description = RESPONSE_TEMPLATES_PAGINATED_SUCCESS, content = @Content(schema = @Schema(implementation = TemplatePageResponse.class))) | ||
| @ApiResponse(responseCode = BAD_REQUEST_CODE, description = RESPONSE_INVALID_PAGINATION, content = { | ||
| @Content(schema = @Schema(implementation = ErrorResponse.class))}) | ||
| @Content(schema = @Schema(implementation = ProblemDetail.class))}) |
hamdirahalpartner-del
force-pushed
the
CPDE-2969-idp-core-replace-error-response-by-problem-detail-rfc-7807
branch
from
September 24, 2026 14:31
e012ca6 to
3896ffd
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical and moderate issues remain in exception handling and the published OpenAPI contract.
9 open findings
2 resolved since last review
🧠 Review effort: Lite
Comment on lines
+548
to
+553
| public ResponseEntity<ErrorResponse> handleGenericException(Exception ex) { | ||
| log.error("Unexpected error occurred: {}", ex.getMessage(), ex); | ||
|
|
||
| String errorMessage = "An unexpected error occurred. Please try again later."; | ||
| return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) | ||
| .body(new ErrorResponse(HttpStatus.INTERNAL_SERVER_ERROR.name(), errorMessage)); |
Comment on lines
53
to
+56
| content: | ||
| "*/*": | ||
| schema: | ||
| "$ref": "#/components/schemas/ErrorResponse" | ||
| "$ref": "#/components/schemas/ProblemDetail" |
Comment on lines
+1189
to
+1191
| properties: | ||
| type: object | ||
| additionalProperties: {} |
| flows: | ||
| clientCredentials: | ||
| tokenUrl: https://example.com/as/token.oauth2 | ||
| tokenUrl: http://localhost:9999/mock/token |
Comment on lines
+118
to
+120
| : parseHttpMessageNotReadableError(ex.getMessage()); | ||
| } else { | ||
| errorMessage = parseHttpMessageNotReadableError(ex.getMessage()); |
Comment on lines
+596
to
+599
| if (request != null) { | ||
| problemDetail | ||
| .setInstance(java.net.URI.create(request.getDescription(false).replace("uri=", ""))); | ||
| } |
hamdirahalpartner-del
requested review from
brandPittCode and
evebrnd
and removed request for
evebrnd
October 13, 2026 15:06
hamdirahalpartner-del
changed the title
hamdirahalpartner-del
changed the title
hamdirahalpartner-del
changed the title
hamdirahalpartner-del
force-pushed
the
CPDE-2969-idp-core-replace-error-response-by-problem-detail-rfc-7807
branch
2 times, most recently
from
October 14, 2026 12:05
170f0c1 to
a33db4c
Compare
� Conflicts: � docs/src/static/swagger.yaml � src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/configuration/SwaggerConfiguration.java � src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/EntityController.java � src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/controller/PrincipalController.java � src/main/java/com/decathlon/idp_core/infrastructure/adapters/api/handler/ApiExceptionHandler.java
hamdirahalpartner-del
force-pushed
the
CPDE-2969-idp-core-replace-error-response-by-problem-detail-rfc-7807
branch
from
October 14, 2026 12:16
a33db4c to
76b4d3f
Compare
|
This branch has not been deployed
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
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.
PR Description
What this PR Provides
Fixes
Review
The reviewer must double-check these points:
!after the type/scope to identify the breakingchange in the release note and ensure we will release a major version.
How to test
Initial state: application running normally, no specific data setup required.
What and how to test:
GET /api/v1/entity_templates/{identifier}with a non-existent identifier (404), orPOST /api/v1/entity_templateswith an invalid payload (400)../gradlew test --tests "*ApiExceptionHandlerTest*"along with the impacted controller tests (EntityControllerTest,EntityTemplateControllerTest,EntityDynamicMappingControllerTest).docs/src/static/swagger.yaml/ Swagger UI to confirm error responses now reference theProblemDetailschema.Expected results:
ProblemDetailformat:{"type": ..., "title": ..., "status": ..., "detail": ..., "instance": ...}(Content-Typeapplication/problem+json), replacing the previous{"error": ..., "error_description": ...}format.Breaking changes (if any)
Context of the Breaking Change
The API error response format has been migrated from the internal
ErrorResponseclass ({"error": "...", "error_description": "..."}) to Spring's RFC 7807ProblemDetailstandard (application/problem+json), for all errors (400, 404, 409, 500, etc.) across all endpoints.Result of the Breaking Change
Clients consuming the IDP-Core API that parse error bodies on the
error/error_descriptionfields must be updated to read the newProblemDetailformat:type,title,status,detail,instance. TheContent-Typeof error responses changes toapplication/problem+json.