locnguyn opened a new pull request, #12590:
URL: https://github.com/apache/gravitino/pull/12590

   ### What changes were proposed in this pull request?
   
   - Precompute null-safe request identifiers before entering the affected REST 
`try/catch` blocks.
   - Use those precomputed values in exception handlers instead of 
dereferencing `request` from catch paths.
   - Cover create/register/grant/revoke/statistics/job-template handlers listed 
in #10172 while keeping the existing `ExceptionHandlers` mappings and endpoint 
contracts unchanged.
   - Add a regression test for a null group request body to verify that the 
original error is marshalled instead of being replaced by a secondary 
`NullPointerException`.
   
   ### Why are the changes needed?
   
   Several REST catch paths call methods such as `request.getName()`, 
`request.getRoleNames()`, or `request.getJobTemplate()` after an exception has 
already occurred. If the request itself is null, the catch block can throw 
another `NullPointerException`, masking the primary failure and preventing the 
existing exception handlers from returning the intended error response.
   
   Fix: #10172
   
   ### Does this PR introduce _any_ user-facing change?
   
   No API or configuration contract changes. For malformed/null request bodies, 
error responses now preserve the original exception-handling path instead of 
being replaced by a secondary catch-path `NullPointerException`.
   
   ### How was this patch tested?
   
   - `JAVA_HOME=/opt/homebrew/opt/openjdk@17 ./gradlew :server:compileJava 
-PskipITs --no-daemon`
   - `JAVA_HOME=/opt/homebrew/opt/openjdk@17 ./gradlew :server:test --tests 
org.apache.gravitino.server.web.rest.TestGroupOperations.testAddGroupWithNullRequestBodyDoesNotExposeNpe
 -PskipITs --no-daemon`
   - Ran the affected REST test classes as a wider targeted batch. 168/169 
passed; `TestTopicOperations.testRemoveTopicProperties` failed once with 404 vs 
200, then passed when rerun by itself, indicating an intermittent/shared-state 
test failure unrelated to this catch-path change.
   - `git diff --check`
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to