[Feature] Unify REST API response format for server, pd and store modules #2975

[BrandaoFelipe]

Purpose of the PR

• close [Feature] Unify REST API response format for server,pd and store modules #2975

The purpose of this PR is to standardize and unify the REST API global error response format across the core modules (pd, server and store/hg-store-node) using the centralized ApiResponse envelope wrapper.

Currently, unhandled exceptions or module-specific errors could return inconsistent formats or expose raw stack traces to the client. This tracking change enforces a clean, predictable JSON error contract across different frameworks used in the ecosystem (Jersey and Spring Boot), enhancing API security and client-side error handling.

Main Changes

This PR introduces centralized exception mapping logic across two distinct sub-framework layers within the project:

1. hugegraph-server module (Jersey / JAX-RS)

• HugeExceptionMapper: Intercepts all HugeException occurrences. It safely resolves the underlying root cause using a recursive rootCause() lookup and maps internal core errors directly to appropriate HTTP Status Codes (e.g., IllegalArgumentException -> 400 Bad Request, NoSuchElementException -> 404 Not Found) wrapped cleanly inside ApiResponse.
• GenericExceptionMapper: Implemented as a top-level global catch-all provider for Exception.class to prevent infrastructure leakages, mapping unexpected exceptions to a secure HTTP 500 "An unexpected error occurred" message payload.

2. hugegraph-store module (hg-store-node sub-module / Spring Boot)

• StoreExceptionHandler: Created using Spring's @RestControllerAdvice to intercept HgStoreException. It dynamically parses the exception's internal error code to differentiate client errors from infrastructure faults based on explicit ranges:
  • Codes [1200, 1300) (e.g., RocksDB storage failures) -> 500 Internal Server Error (logged as SEVERE/ERROR with full stack trace).
  • Codes [1000, 1200) (e.g., Unsupported data formats) -> 400 Bad Request (logged cleanly as a WARNING message).
• GenericExceptionMapper: Added as a fallback interceptor for standard Exception.class instances inside the store node rest controllers to ensure uniform 500 error envelopes using the Log4j wrapper.

3. hugegraph-pd module (hg-pd-service sub-module / Spring Boot)

• PdExceptionHandler: Implemented via @RestControllerAdvice to handle metadata and coordination cluster exceptions (PDException). Centralizes cluster control plane failure logging (Raft state, partition routing) and converts them into the unified ApiResponse schema.
• Catch-All Filter: Includes standard Exception.class mapping to gracefully mask unhandled runtime infrastructure glitches with a generic "An unexpected error occurred in the cluster control plane" feedback text.

Verifying these changes

• Need tests and can be verified as follows:
  • Executed mvn clean compile -DskipTests from the root directory to confirm proper multi-module compile-time dependency alignment (validating ApiResponse cross-package visibility inside hg-store-node).
  • Validated error response payloads against the unified schema structure ensuring it adheres to the contract:

    {
      "status": 400,
      "message": "Specific validation/error message string",
      "data": null,
      "statusText": "Bad Request"
    }

Does this PR potentially affect the following parts?

• Dependencies
• Modify configurations
• The public API
• Other affects (typed here)
• Nope

Documentation Status

• Doc - TODO
• Doc - Done
• Doc - No Need

[imbajin]

I think the current implementation does not actually establish one unified runtime path yet. It adds new mappers/advice, but several existing paths either bypass them or now compete with them.

Current shape:

Expected:
  server / pd / store
        -> one clear response contract

Current PR:
  server -> old ExceptionFilter + new mapper providers coexist
  pd     -> many controller-level catch blocks still return old RestApiResponse
  store  -> HgStoreException business code is replaced by generic HTTP 400/500 in the body

The main design issues are:

1. PD exceptions are often caught before @RestControllerAdvice can see them. Many PD REST APIs already catch PDException and directly return RestApiResponse or the existing toJSON(e) format, so a new PDExceptionMapper only covers exceptions that escape the controller.

PD controller
  -> catch PDException
  -> return old RestApiResponse / old JSON
  -> new advice is bypassed

2. Server now has two competing Jersey exception-mapping paths. The server module already has ExceptionFilter with mappers for HugeException, IllegalArgumentException, NotFoundException, NoSuchElementException, WebApplicationException, HugeGremlinException, and unknown exceptions. This PR adds another provider package under org.apache.hugegraph.api, which is scanned by ApplicationConfig.

ApplicationConfig packages("org.apache.hugegraph.api")
        -> existing ExceptionFilter.*Mapper
        -> new exceptionshandler.*Mapper

That makes the effective behavior unclear. If the new HugeExceptionMapper wins, it also maps most HugeException cases to HTTP 400 by default, which can incorrectly classify server-side failures as client errors.

3. Store loses the original HgStoreException business code. HgStoreException has useful domain codes such as 1001, 1201, 1202, 1208, 1401, etc. But the new StoreExceptionHandler returns status.value() as the response code, so clients only see generic 400 or 500 instead of the actionable store error code.

Before:
  HgStoreException code = 1202

After this PR:
  response.code = 500

I suggest first defining the response contract and then updating the existing runtime paths instead of adding parallel ones:

PD:
  handle existing catch-and-return paths, or migrate/normalize RestApiResponse

Server:
  modify the existing ExceptionFilter path instead of adding competing mappers

Store:
  keep HgStoreException business code while using HTTP status for transport semantics

Tests:
  add REST-level tests that prove actual endpoints return the unified format

One unrelated item: hugegraph-pd/hg-pd-service/pom.xml adds <classifier>exec</classifier>, which changes the PD packaging output and does not seem related to REST response unification. Please split it out or explain why it is required here.

[imbajin]

A larger concern is that this PR changes the public REST API response contract. The server / pd / store REST APIs may already be consumed by external services, so a large response-format change can have significant downstream impact.

Current response shapes include fields such as:

server errors:
  exception / message / cause / trace

pd responses:
  message / data / status
  or status / error

store errors:
  module-specific business error codes

The new envelope introduces a different shape:

ApiResponse:
  code / message / data / status

That can break clients that parse existing fields like exception, error, status, or store-specific business codes. Even if the new format is cleaner, this kind of API contract change should be designed and discussed carefully before implementation.

I think we should first decide the compatibility and migration strategy, for example:

Questions to settle before implementation:
  - Is this intended to be a breaking public API response change?
  - If not, how do old clients keep receiving the old format?
  - Should the unified format be opt-in through a version/header/parameter?
  - Is there a deprecation period with compatibility fields?
  - Should normal responses also be wrapped, or only error responses?
  - Is `code` the HTTP status or the business error code?
  - What should be documented in REST docs / OpenAPI / release notes?

If this is intended to be a breaking change, it should probably go through community discussion and come with docs, migration notes, OpenAPI updates, and REST-level compatibility tests. If it is not intended to be breaking, then the implementation needs an explicit compatibility layer rather than replacing existing response bodies directly.

In short:

First:
  define API contract + compatibility/migration plan

Then:
  implement the unified response format against that contract

[BrandaoFelipe]

Hi @imbajin,
Thank you for the detailed review. You raised several important concerns regarding runtime consistency, exception flow coverage, and backward compatibility of the public REST API contract.
I agree that the current approach still leaves multiple competing exception-handling paths in place instead of establishing a single unified runtime contract.
The points you raised are all valid and need to be addressed before this can be considered a proper unification.
My current thinking is:

Server: integrate the unified formatting into the existing ExceptionFilter infrastructure.
PD: explore consolidating the existing controller-level catch-and-return patterns so exceptions can follow a consistent handling path.
Store: preserve existing HgStoreException business codes in the response body while keeping HTTP status codes strictly for transport semantics.
I also agree that the larger concern here is API compatibility. Since the current REST responses already expose module-specific response shapes and fields, introducing a new envelope format without a migration strategy could break downstream consumers.
Before continuing implementation, I think we should first align on:

• the target error response contract,
• compatibility expectations,
• migration/deprecation strategy,
• and whether this should be treated as a breaking API change.

Do you think it would be better to continue this discussion in the PR thread, or would this be more appropriate for a broader dev mailing list discussion first?
Thanks again for the thoughtful review — this was very helpful in clarifying the architectural direction the PR should take.

EDIT:
About the hugegraph-pd/hg-pd-service/pom.xml adding <classifier>exec</classifier> I added the <classifier>exec</classifier> during my local testing phase to resolve a multi-module classpath visibility issue where other modules couldn't properly locate classes packed inside the Spring Boot layout during test compilation.
Since this impacts the core packaging behavior and is indeed unrelated to the REST response unification contract, I will remove it from this PR, thanks for pointing that out.

[JisoLya]

IMO, a controller should only be responsible for receiving external requests and returning responses — exception handling logic does not belong there. We should remove the catch blocks from controllers and let a centralized handler take care of error formatting.

For the store business code, rather than replacing it with the HTTP status code, we could simply add a dedicated field (e.g. storeBizCode) to the response envelope to carry the original HgStoreException code alongside the HTTP status. That way transport semantics and domain error codes stay cleanly separated without losing any information.

That said, it's worth classifying controllers by their audience — user-facing, admin-facing, or internal module-facing. Internal module-facing interfaces may reasonably keep their existing response format unchanged, while the unified response format should at minimum be enforced across all user-facing endpoints.

On compatibility: could we treat this as a breaking change and land it in the next major/minor version with proper release notes and migration guidance?
cc @imbajin

[Copilot]

Pull request overview

This PR introduces a shared ApiResponse envelope and adds centralized REST exception handling across server (Jersey), PD, and store-node (Spring Boot) modules to make error responses more consistent and avoid leaking raw internal failures.

Changes:

• Adds ApiResponse<T> in hugegraph-common.
• Adds module-specific and generic exception handlers for server, PD, and store-node REST APIs.
• Adds PD mapper tests and changes PD service Spring Boot repackage output classifier.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rest/response/ApiResponse.java	Adds the shared response envelope type.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/exceptionshandler/HugeExceptionMapper.java	Adds Jersey handling for HugeException.
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/exceptionshandler/GenericExceptionMapper.java	Adds Jersey fallback handling for generic exceptions.
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/exceptionshandler/PDExceptionMapper.java	Adds Spring handling for PDException.
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/exceptionshandler/GenericExceptionMapper.java	Adds Spring fallback handling for PD service exceptions.
hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/exceptions/PDExceptionMapperTest.java	Adds unit coverage for PD exception mappers.
hugegraph-pd/hg-pd-service/pom.xml	Changes Spring Boot repackage artifact classifier.
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/exceptionhandlers/StoreExceptionHandler.java	Adds Spring handling for HgStoreException.
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/exceptionhandlers/GenericExceptionMapper.java	Adds Spring fallback handling for store-node exceptions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[VGalaxies]

Review summary

• Blocking: yes
• Summary: The PR can break PD distribution startup and introduces inconsistent error mappings in the new unified response handlers.
• Evidence:
  • git diff origin/master...HEAD
  • mvn -pl hugegraph-commons/hugegraph-common,hugegraph-server/hugegraph-api,hugegraph-pd/hg-pd-service,hugegraph-store/hg-store-node -am -DskipTests -Dmaven.javadoc.skip=true compile passed

[VGalaxies]

High: PD executable jar is no longer the artifact used by the distribution

hugegraph-pd/hg-pd-service/pom.xml:178

Evidence

• The new <classifier>exec</classifier> makes Spring Boot attach the repackaged executable jar as a classified artifact, while hugegraph-pd/hg-pd-dist/src/assembly/descriptor/server-assembly.xml:52 includes the unclassified hg-pd-service dependency and start-hugegraph-pd.sh:172-176 runs ${LIB}/hg-pd-service-*.jar with java -jar.

Impact

• The PD dist will package/run the plain unclassified jar instead of the executable Spring Boot jar, so packaged PD startup can fail or run the wrong artifact.

Requested fix

• Either remove the classifier so the main artifact remains executable, or update hg-pd-dist to depend on and include the exec classifier explicitly and make the startup glob select only that jar.

[VGalaxies]

Medium: Real PD error codes are mapped to HTTP 500

hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/exceptionshandler/PDExceptionMapper.java:67

Evidence

• resolveStatus() only treats exact HTTP codes or codes starting with 4 as client errors; actual PD codes include STORE_ID_NOT_EXIST = 101, NOT_FOUND = 103, and PD_UNREACHABLE = 104 in hugegraph-pd/hg-pd-grpc/src/main/proto/pdpb.proto:134-139, so those become 500.

Impact

• Uncaught domain errors such as not-found/store-not-found are reported as server failures, which breaks HTTP semantics and can mislead clients, retries, and monitoring.

Requested fix

• Map the existing Pdpb.ErrorType values explicitly to appropriate HTTP statuses, or preserve the previous REST status behavior while only changing the response body format.

[VGalaxies]

Medium: Store response body drops the HgStoreException code

hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/controller/exceptionhandlers/StoreExceptionHandler.java:57

Evidence

• The handler reads exception.getCode() at line 37, but constructs ApiResponse with status.value() at line 57. Store exceptions define machine-readable codes such as 1001 and 1201 in HgStoreException.java:23-40.

Impact

• Clients receive only HTTP codes like 400 or 500 in the unified code field and lose the store-specific error code needed to distinguish validation, closed-store, RocksDB, PD, and metric failures.

Requested fix

• Use errorCode for ApiResponse.code and keep the HTTP status only in the ResponseEntity status.