wolfboys commented on code in PR #1811:
URL:
https://github.com/apache/incubator-streampark/pull/1811#discussion_r992898433
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/base/handler/GlobalExceptionHandler.java:
##########
@@ -56,32 +54,18 @@ public RestResponse handleException(Exception e) {
return RestResponse.fail("internal server error: " + e.getMessage(),
ResponseCode.CODE_FAIL);
}
- @ExceptionHandler(value = InternalException.class)
- @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
- public RestResponse handleParamsInvalidException(InternalException e) {
- log.info("Internal server error：{}", e.getMessage());
- return RestResponse.fail("internal server error: " + e.getMessage(),
ResponseCode.CODE_FAIL);
- }
-
@ExceptionHandler(value = HttpRequestMethodNotSupportedException.class)
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
public RestResponse handleException(HttpRequestMethodNotSupportedException
e) {
log.info("not supported request method，exception：{}", e.getMessage());
return RestResponse.fail("not supported request method，exception：" +
e.getMessage(), ResponseCode.CODE_FAIL);
}
- @ExceptionHandler(value = ApiAlertException.class)
+ @ExceptionHandler(value = AbstractApiException.class)
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
- public RestResponse handleException(ApiAlertException e) {
+ public RestResponse handleException(AbstractApiException e) {
log.info("api exception：{}", e.getMessage());
- return RestResponse.fail(e.getMessage(), ResponseCode.CODE_FAIL_ALERT);
- }
-
- @ExceptionHandler(value = ApiDetailException.class)
- @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
- public RestResponse handleException(ApiDetailException e) {
- log.info("detail exception：{}", e.getMessage());
- return RestResponse.fail("exception detail:\n" + e.getMessage(),
ResponseCode.CODE_FAIL_DETAIL);
+ return RestResponse.fail(e.getNotificationMessagePrefix() + "\n" +
e.getMessage(), e.getResponseCode());
Review Comment:
> Using the `getNotificationMessagePrefix` is convenient when we create new
`AbstractApiException` in the future.
>
> That's why create the `AbstractApiException`. The information of Exception
is saved inside the Exception, not in many places.
I agree with you. It's done well, But I still think method
`getNotificationMessagePrefix` is unnecessary, Because only two exceptions
alert message for front-end, and the role is clear, the method:
```
@ExceptionHandler(value = AbstractApiException.class)
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
public RestResponse handleException(AbstractApiException e) {
....
}
```
is the outlet for response to the front-end, Except this place will care,
other places will not be used, So we can just change it here:
```
@ExceptionHandler(value = AbstractApiException.class)
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
public RestResponse handleException(AbstractApiException e) {
log.info("api exception：{}", e.getMessage());
String prefix = (e instanceof ApiAlertException) ? "": "exception
detail:\n";
return RestResponse.fail(prefix + e.getMessage(), e.getResponseCode());
}
```
--
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]
