fix(api): improve validation error responses in InvolvedCancerStudyEx..#11937
Open
tim48-robot wants to merge 5 commits intocBioPortal:masterfrom
Open
fix(api): improve validation error responses in InvolvedCancerStudyEx..#11937tim48-robot wants to merge 5 commits intocBioPortal:masterfrom
tim48-robot wants to merge 5 commits intocBioPortal:masterfrom
Conversation
…tractorInterceptor - Return HTTP 400 Bad Request instead of silent failures on malformed JSON - Truncate technical Jackson details from error messages for cleaner feedback - Ensure consistency across all filter extraction methods
Author
|
on the video i havent tried the new localize docker after trruncating technical jackson details. |
Author
|
@inodb would love if you could do a review! |
There was a problem hiding this comment.
Pull request overview
This PR improves error handling in the InvolvedCancerStudyExtractorInterceptor to return proper HTTP 400 Bad Request responses for malformed JSON or invalid request bodies, instead of silently failing with HTTP 200 OK or causing internal server errors.
Changes:
- Added
sendBadRequestResponsehelper method to standardize HTTP 400 error responses with JSON error messages - Updated 25 filter extraction methods to accept
HttpServletResponseparameter and callsendBadRequestResponseon parsing errors - Implemented message truncation to remove technical Jackson source location details for cleaner error messages
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/org/cbioportal/legacy/web/util/InvolvedCancerStudyExtractorInterceptor.java
Outdated
Show resolved
Hide resolved
src/main/java/org/cbioportal/legacy/web/util/InvolvedCancerStudyExtractorInterceptor.java
Outdated
Show resolved
Hide resolved
src/main/java/org/cbioportal/legacy/web/util/InvolvedCancerStudyExtractorInterceptor.java
Outdated
Show resolved
Hide resolved
src/main/java/org/cbioportal/legacy/web/util/InvolvedCancerStudyExtractorInterceptor.java
Show resolved
Hide resolved
src/main/java/org/cbioportal/legacy/web/util/InvolvedCancerStudyExtractorInterceptor.java
Outdated
Show resolved
Hide resolved
Author
|
thanks for the review copilot, will check on it & resolve |
- Use ObjectMapper for proper JSON escaping (prevents injection) - Reuse ErrorResponse class for consistency with GlobalExceptionHandler - Check response.isCommitted() before writing error response - Catch IOException specifically instead of broad Exception - Fix log message typo (clinicalDataBinCountFilter -> clinicalDataCountFilter) - Add comprehensive unit tests for error handling scenarios
175de37 to
aa0ba5c
Compare
Exception.getMessage() can return null for some exceptions (e.g., NPE without message). Added null-safe handling in sendBadRequestResponse to prevent NPE when processing error messages.
63376d9 to
b0a78e4
Compare
Author
|
everything is done! i think its ready now |
Author
|
hey @haynescd ! could you take a review? |
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.
Summary
This PR improves the API validation error handling in
InvolvedCancerStudyExtractorInterceptor. Previously, malformed JSON or empty request bodies would result in a silent failure (HTTP 200 OK with no data) or sometimes an internal server error. This change ensures that a proper HTTP 400 Bad Request is returned with a descriptive JSON error message.Changes
InvolvedCancerStudyExtractorInterceptorto catch JSON parsing exceptions and return HTTP 400.sendBadRequestResponsehelper to standardize error responses.Verification
Verified locally via curl:
Checks
Videos
2026-02-04.10-45-38.mp4