Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with this
* work for additional information regarding copyright ownership. The ASF
* licenses this file to You under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package org.apache.hugegraph.rest.response;

public class ApiResponse<T> {
private int code;
private String message;
private T data;
private String status;

public ApiResponse(){

}

public ApiResponse(int code, String message, T data, String status) {
this.code = code;
this.message = message;
this.data = data;
this.status = status;
}

public int getCode() {
return code;
}

public void setCode(int code) {
this.code = code;
}

public String getMessage() {
return message;
}

public void setMessage(String message) {
this.message = message;
}

public T getData() {
return data;
}

public void setData(T data) {
this.data = data;
}

public String getStatus() {
return status;
}

public void setStatus(String status) {
this.status = status;
}

}
1 change: 1 addition & 0 deletions hugegraph-pd/hg-pd-service/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@
<mainClass>
org.apache.hugegraph.pd.boot.HugePDServer
</mainClass>
<classifier>exec</classifier>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

</configuration>
<!-- should configure explicitly without spring-boot-starter-parent -->
<goals>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hugegraph.pd.rest.exceptionshandler;

import org.apache.hugegraph.rest.response.ApiResponse;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;

@RestControllerAdvice
public class GenericExceptionMapper {

private static final Logger logger = LogManager.getLogger(GenericExceptionMapper.class);

@ExceptionHandler(Exception.class)
public ResponseEntity<ApiResponse<Object>>handleGenericException(Exception exception) {

HttpStatus springStatus = HttpStatus.INTERNAL_SERVER_ERROR;

logger.error("Unexpected error ", exception);

ApiResponse<Object> apiResponse = new ApiResponse<>(
500,
"An unexpected error occurred",
null,
springStatus.getReasonPhrase());

return ResponseEntity.status(springStatus).body(apiResponse);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hugegraph.pd.rest.exceptionshandler;

import org.apache.hugegraph.pd.common.PDException;
import org.apache.hugegraph.rest.response.ApiResponse;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;

@RestControllerAdvice
public class PDExceptionMapper {

private static final Logger logger = LogManager.getLogger(PDExceptionMapper.class);

@ExceptionHandler(PDException.class)
public ResponseEntity<ApiResponse<Object>> toResponse(PDException exception) {

logger.error(exception.getMessage(), exception);

HttpStatus status = resolveStatus(exception.getErrorCode());
String reasonPhrase = status.getReasonPhrase();

ApiResponse<Object> apiResponse = new ApiResponse<>(
exception.getErrorCode(),
exception.getMessage(),
null,
reasonPhrase);

return ResponseEntity
.status(status)
.body(apiResponse);

}

private HttpStatus resolveStatus(int code) {
try {
// Tenta mapear códigos HTTP exatos (ex: 400, 404, 500)
return HttpStatus.valueOf(code);
} catch (IllegalArgumentException e) {
// Se falhar (ex: 4001), extraímos o primeiro dígito para descobrir a família do erro
String codeStr = String.valueOf(code);

if (codeStr.startsWith("4")) {
return HttpStatus.BAD_REQUEST; // Erros de cliente -> 400
}

// Tudo que for da família 5000 ou não reconhecido vira Erro de Servidor -> 500
return HttpStatus.INTERNAL_SERVER_ERROR;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}
Comment on lines +54 to +69
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hugegraph.pd.rest.exceptions;

import org.junit.Test;
import org.junit.Assert;

import org.springframework.http.ResponseEntity;

import org.apache.hugegraph.pd.common.PDException;
import org.apache.hugegraph.pd.rest.exceptionshandler.GenericExceptionMapper;
import org.apache.hugegraph.pd.rest.exceptionshandler.PDExceptionMapper;
import org.apache.hugegraph.rest.response.ApiResponse;

public class PDExceptionMapperTest {

@Test
public void testPDExceptionMapping() {
PDExceptionMapper mapper = new PDExceptionMapper();

PDException exception = new PDException(4001, "test error");

ResponseEntity<ApiResponse<Object>> response =
mapper.toResponse(exception);

Assert.assertEquals(400, response.getStatusCodeValue());
Assert.assertEquals(4001, response.getBody().getCode());
Assert.assertEquals("test error", response.getBody().getMessage());
Assert.assertEquals("Bad Request", response.getBody().getStatus());
}

@Test
public void testGenericExceptionFallback() {
PDExceptionMapper mapper = new PDExceptionMapper();

PDException exception = new PDException(9999, "Erro desconhecido");

ResponseEntity<ApiResponse<Object>> response =
mapper.toResponse(exception);

Assert.assertEquals(500, response.getStatusCodeValue());
Assert.assertEquals(9999, response.getBody().getCode());
Assert.assertEquals("Internal Server Error", response.getBody().getStatus());
}

@Test
public void testGenericExceptionMapping() {
GenericExceptionMapper mapper = new GenericExceptionMapper();

Exception exception = new RuntimeException("Erro genérico");

ResponseEntity<ApiResponse<Object>> response =
mapper.handleGenericException(exception);

Assert.assertEquals(500, response.getStatusCodeValue());
Assert.assertEquals(500, response.getBody().getCode());
Assert.assertEquals("An unexpected error occurred", response.getBody().getMessage());
Assert.assertEquals("Internal Server Error", response.getBody().getStatus());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hugegraph.api.exceptionshandler;

import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;
import org.apache.hugegraph.rest.response.ApiResponse;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

@Provider
public class GenericExceptionMapper implements ExceptionMapper<Exception> {

private static final Logger LOG = LogManager.getLogger(GenericExceptionMapper.class);

@Override
public Response toResponse(Exception exception) {
LOG.error("Unexpected error occurred", exception);

int code = 500;
ApiResponse<Object> apiResponse = new ApiResponse<>(
code,
"An unexpected error occurred",
null,
Response.Status.INTERNAL_SERVER_ERROR.getReasonPhrase());

return Response.status(code)
.type(MediaType.APPLICATION_JSON)
.entity(apiResponse)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hugegraph.api.exceptionshandler;

import com.alipay.sofa.rpc.log.Logger;
import com.alipay.sofa.rpc.log.LoggerFactory;

import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;
import org.apache.hugegraph.HugeException;
import org.apache.hugegraph.rest.response.ApiResponse;

@Provider
public class HugeExceptionMapper implements ExceptionMapper<HugeException> {
Comment on lines +30 to +31

private static final Logger LOG =
LoggerFactory.getLogger(HugeExceptionMapper.class);

@Override
public Response toResponse(HugeException exception) {

Throwable rootCause = HugeException.rootCause(exception);
int code;

if (rootCause instanceof InterruptedException) {
code = 500;
} else if (rootCause instanceof IllegalArgumentException) {
code = 400;
} else if (rootCause instanceof java.util.NoSuchElementException) {
code = 404;
} else {
code = 400;
}

if (code >= 500) {
LOG.error("Unexpected server error", exception);
} else {
LOG.warn("Request error: {}", exception.getMessage());
}

Response.Status statusCode = Response.Status.fromStatusCode(code);
String statusText = (statusCode != null ) ? statusCode.getReasonPhrase() : "BAD REQUEST";

ApiResponse<Object> apiResponse = new ApiResponse<>(
code,
exception.getMessage(),
null,
statusText
);

return Response.status(code)
.type(MediaType.APPLICATION_JSON)
.entity(apiResponse)
.build();
}
}

Loading
Loading