apache/gravitino

[Improvement] Catch-path request dereference can mask original exceptions in REST handlers

Open

#10,172 opened on Mar 4, 2026

View on GitHub
 (2 comments) (0 reactions) (0 assignees)Java (887 forks)auto 404
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 NullPointerException in catch hides the original exception context.

Scope

  • server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java register catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java register catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/MetalakeOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java create/testConnection catch paths
  • server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java add catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java add catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java create catch path
  • server/src/main/java/org/apache/gravitino/server/web/rest/PermissionOperations.java grant/revoke catch paths
  • server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java update/drop (+partition update/drop) catch paths
  • server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java register-job-template catch path

How should we improve?

Expected behavior

  • No request.get*() dereference inside affected catch blocks.
  • 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 ExceptionHandlers mapping 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());
}

Contributor guide