apache/gravitino
View on GitHub[Improvement] Catch-path request dereference can mask original exceptions in REST handlers
Open
#10,172 opened on Mar 4, 2026
good first issueimprovement
Repository metrics
- Stars
- (3,058 stars)
- PR merge metrics
- (PR metrics pending)
Description
What would you like to be improved?
Multiple REST handlers dereference request inside catch (Exception e) (for example request.getName(), request.getRoleNames(), request.getUpdates(), request.getDrops(), request.getJobTemplate().name()). When request is null, catch-path code can throw a second NullPointerException, masking the real failure.
Representative failure:
- Primary exception occurs in method body.
- Catch block dereferences
request.get*(). - Secondary
NullPointerExceptionin catch hides the original exception context.
Scope
-
server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.javaregister catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.javaregister catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.javacreate/testConnection catch paths -
server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.javaadd catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.javaadd catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.javacreate catch path -
server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.javagrant/revoke catch paths -
server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.javaupdate/drop (+partition update/drop) catch paths -
server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.javaregister-job-template catch path
How should we improve?
Expected behavior
- No
request.get*()dereference inside affectedcatchblocks. - Catch blocks should always preserve/marshal the primary error through existing
ExceptionHandlers. - Endpoint APIs and response contracts remain unchanged.
Definition of done
- Replace catch-path request dereferences with null-safe precomputed identifiers or safe fallbacks.
- Keep existing
ExceptionHandlersmapping behavior. - Keep endpoint APIs unchanged.
- Add/adjust tests to ensure exception handling path is stable when request is null.
- Run and pass:
./gradlew test -PskipITs
Example unit test
@Test
public void testAddGroupWithNullRequestBodyDoesNotExposeNpe() {
Response resp =
target("/metalakes/metalake1/groups")
.request(MediaType.APPLICATION_JSON_TYPE)
.accept("application/vnd.gravitino.v1+json")
.post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE));
Assertions.assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), resp.getStatus());
ErrorResponse error = resp.readEntity(ErrorResponse.class);
Assertions.assertNotEquals(NullPointerException.class.getSimpleName(), error.getType());
}