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

   ### What changes were proposed in this pull request?
   
   This PR improves the error handling in `StatisticOperations` to avoid a 
possible
   `NullPointerException` in the catch path.
   
   Previously, in both table and partition statistics update methods,
   the catch block dereferenced `request.getUpdates()` to build an error
   context string. If the request payload or its `updates` field was null,
   this could trigger a second NPE during error handling, masking the original
   validation failure and resulting in an incorrect `500 Internal Server Error`.
   
   This PR makes the catch-path error formatting null-safe by:
   
   - Introducing helper methods (`getStatisticNames`, `getPartitionNames`)
     that defensively handle null `request` or null `updates`.
   - Preserving the original exception and passing it to `ExceptionHandlers`
     without re-triggering another NPE.
   
   ### Why are the changes needed?
   
   The previous implementation was fragile when request payloads were null
   or invalid. A secondary NPE in the catch block could mask the original
   validation error and produce incorrect server-side error responses.
   
   This change ensures:
   
   - The original validation exception is preserved.
   - Error formatting in the catch path is null-safe.
   - The API consistently returns client-facing validation errors (400)
     instead of incorrectly returning 500.
   
   Fix: #10124
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   This change only improves internal error handling logic.
   There are no API changes, configuration changes, or behavioral changes
   other than preventing incorrect 500 responses in invalid request scenarios.
   
   ### How was this patch tested?
   
   - Added a regression test in `TestStatisticOperations`.
   - The test explicitly sends a payload with `"updates": null`
     and verifies that the API returns `400 Bad Request`.
   - Confirmed that no secondary `NullPointerException`
     is thrown during error handling.
   
   I aligned the regression test with the existing Jersey `target(...)`
   style in this test class to match its convention.
   Please let me know if a white-box unit test style would be preferred instead.


-- 
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