bugfix: 依赖的其他系统（CMDB、IAM等）接口异常时报错信息优化 #1430 #1691

liuliaozhong · 2023-01-12T07:03:43Z

No description provided.

jsonwan · 2023-01-12T07:39:21Z

src/backend/commons/cmdb-sdk/src/main/java/com/tencent/bk/job/common/cc/sdk/BizCmdbClient.java

-        } catch (ExecutionException e) {
-            throw new InternalException(e, ErrorCode.CMDB_API_DATA_ERROR, null);
+        } catch (Throwable e) {
+            throw new CmdbClientException(e, ErrorCode.CMDB_API_DATA_ERROR, null);


这里异常捕获的范围太大了，会将真实的异常全部视为CMDB异常，比如因代码bug原因导致的空指针异常，只要把外部缓存层包装的内部cause异常放出来就可以了

这里的问题没解决，还是会把所有的异常都转为Cmdb异常，有个e.getCause()可以用，老代码的逻辑不一定是合理的，可以批判性地思考下

确实，没考虑周全，Throwable是异常超类，会把所有异常都捕获，都当成CMDB异常

jsonwan · 2023-01-12T09:11:26Z

.../main/java/com/tencent/bk/job/common/web/exception/handler/EsbExceptionControllerAdvice.java

+    @ExceptionHandler(CmdbClientException.class)
+    @ResponseBody
+    @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
+    ResponseEntity<?> handleInternalException(HttpServletRequest request, CmdbClientException ex) {


该方法可以省略，处理逻辑跟其父类是一样的，将来有特殊的处理逻辑时再添加

jsonwan · 2023-01-12T09:16:58Z

...nd/commons/esb-sdk/src/main/java/com/tencent/bk/job/common/esb/sdk/AbstractEsbSdkClient.java

@@ -28,7 +28,7 @@
 import com.tencent.bk.job.common.constant.ErrorCode;
 import com.tencent.bk.job.common.esb.model.EsbReq;
 import com.tencent.bk.job.common.esb.model.EsbResp;
-import com.tencent.bk.job.common.exception.InternalException;
+import com.tencent.bk.job.common.exception.CmdbClientException;


这个类是个抽象类，不止是调用CMDB的API，还可能是通过ESB调用其他系统，这个改成Cmdb专有异常是不对的，可以考虑改为EsbClientException

jsonwan · 2023-01-12T09:18:23Z

.../main/java/com/tencent/bk/job/common/web/exception/handler/WebExceptionControllerAdvice.java

@@ -183,4 +184,13 @@ ResponseEntity<?> handleResourceExhaustedException(HttpServletRequest request, R
        return new ResponseEntity<>(Response.buildCommonFailResp(ex), HttpStatus.TOO_MANY_REQUESTS);
    }

+    @ExceptionHandler(CmdbClientException.class)


该方法可以省略，处理逻辑跟其父类是一样的，将来有特殊的处理逻辑时再添加

jsonwan · 2023-01-12T09:19:22Z

...n/java/com/tencent/bk/job/common/web/exception/handler/ServiceExceptionControllerAdvice.java

@@ -173,4 +174,13 @@ ResponseEntity<?> handleResourceExhaustedException(HttpServletRequest request, R
        return new ResponseEntity<>(InternalResponse.buildCommonFailResp(ex), HttpStatus.TOO_MANY_REQUESTS);
    }

+    @ExceptionHandler(CmdbClientException.class)


该方法可以省略，处理逻辑跟其父类是一样的，将来有特殊的处理逻辑时再添加

jsonwan

3类问题需要处理下

jsonwan

第1个问题需要再处理下

liuliaozhong · 2023-01-12T12:09:33Z

3类问题需要处理下

多处地方我都没考虑周全，犯低级错误，针对修复意见定好好修改

jsonwan

待验证通过

jsonwan · 2023-01-12T12:53:41Z

src/backend/commons/cmdb-sdk/src/main/java/com/tencent/bk/job/common/cc/sdk/BizCmdbClient.java

+        } catch (ExecutionException | UncheckedExecutionException e) {
+            Throwable cause = e.getCause();
+            if (cause instanceof RuntimeException) {
+                throw (RuntimeException) e;


e应当改为cause

jsonwan · 2023-02-02T03:36:36Z

src/backend/commons/cmdb-sdk/src/main/java/com/tencent/bk/job/common/cc/sdk/BizCmdbClient.java

@@ -263,11 +264,16 @@ public InstanceTopologyDTO getBizInstCompleteTopology(long bizId) {
        return completeTopologyDTO;
    }

-    public InstanceTopologyDTO getCachedBizInstCompleteTopology(long bizId) {
+    public InstanceTopologyDTO getCachedBizInstCompleteTopology(long bizId){


代码格式规范里面是有空格的，确认下提交前是否对代码进行了格式化，如果有，看下是idea的格式设置问题还是误删除了空格

多名开发者之间的代码格式规范需要保持一致，否则容易引起不必要的代码改动和合并冲突

jsonwan · 2023-02-02T03:45:32Z

...kend/commons/common-iam/src/main/java/com/tencent/bk/job/common/iam/client/EsbIamClient.java

 import com.tencent.bk.job.common.esb.model.EsbReq;
 import com.tencent.bk.job.common.esb.model.EsbResp;
 import com.tencent.bk.job.common.esb.sdk.AbstractEsbSdkClient;
+import com.tencent.bk.job.common.exception.InternalCmdbException;


这里为什么会引入CMDB的异常，检查下是否格式化代码，去除无效引用

jsonwan · 2023-02-02T03:54:26Z

...s/common-iam/src/main/java/com/tencent/bk/job/common/iam/interceptor/AuthAppInterceptor.java

+                        throw new PermissionDeniedException(authResult);
+                    }
+                } catch (InternalIamException e) {
+                    throw e;


捕获异常后又立即抛出，是否是多余的代码？

jsonwan · 2023-02-02T03:57:34Z

...mons/common/src/main/java/com/tencent/bk/job/common/exception/InternalUserInfoException.java

+ */
+@Getter
+@ToString
+public class InternalUserInfoException extends InternalException {


UserInfo这个名称不太合适，这个异常的目的是要指出依赖的哪一个第三方系统出了问题，叫UserManage更合适

jsonwan · 2023-02-02T04:06:52Z

.../job-gateway/src/main/java/com/tencent/bk/job/gateway/web/service/impl/LoginServiceImpl.java

+            if (cause instanceof RuntimeException) {
+                throw (RuntimeException) cause;
+            } else {
+                throw new InternalUserInfoException("Query userinfo from paas fail", e,


逻辑不对，会将其他类型异常全部当做InternalUserInfoException，这里应该抛内部服务器错误的异常

jsonwan · 2023-02-02T04:07:42Z

...-manage/src/main/java/com/tencent/bk/job/manage/dao/notify/impl/NotifyEsbChannelDAOImpl.java

            String errorMsg = "Fail to load EsbChannel from cache";
            logger.error(errorMsg, e);
-            throw new InternalException(errorMsg, e, ErrorCode.CMSI_MSG_CHANNEL_DATA_ERROR);
+            throw new InternalCmsiException(errorMsg, e, ErrorCode.CMSI_MSG_CHANNEL_DATA_ERROR);


逻辑不对，会将其他类型异常全部当做InternalCmsiException，这里参考其他缓存异常的处理方式

jsonwan · 2023-02-02T04:15:21Z

src/backend/commons/cmdb-sdk/src/main/java/com/tencent/bk/job/common/cc/sdk/BizCmdbClient.java

+            if (cause instanceof RuntimeException) {
+                throw (RuntimeException) cause;
+            } else {
+                throw new InternalCmdbException(e, ErrorCode.INTERNAL_ERROR, null);


逻辑不对，会将其他类型异常全部当做InternalCmdbException，这里应该抛内部服务器错误的异常

这样的话，原本CmdbException异常也变成内部服务器异常了

这样行不行呢？

已在群内沟通

jsonwan · 2023-02-02T04:17:33Z

...kend/commons/common-iam/src/main/java/com/tencent/bk/job/common/iam/client/EsbIamClient.java

@@ -179,6 +183,8 @@ private <R> EsbResp<R> requestIamApi(String method,
            HttpMetricUtil.setHttpMetricName(CommonMetricNames.ESB_IAM_API_HTTP);
            HttpMetricUtil.addTagForCurrentMetric(Tag.of("api_name", uri));
            return getEsbRespByReq(method, uri, reqBody, typeReference);
+        } catch (Exception e) {
+            throw new InternalIamException(e, ErrorCode.IAM_API_DATA_ERROR, null);


当前IAM_API_DATA_ERROR对应的错误码信息不准确，改为：IAM接口数据异常

jsonwan

几个问题需要处理下

jsonwan · 2023-02-02T09:35:35Z

src/backend/commons/cmdb-sdk/src/main/java/com/tencent/bk/job/common/cc/sdk/BizCmdbClient.java

        try {
            return bizInstCompleteTopologyCache.get(bizId);
        } catch (ExecutionException | UncheckedExecutionException e) {
            Throwable cause = e.getCause();
            if (cause instanceof RuntimeException) {
                throw (RuntimeException) cause;
            } else {
-                throw new InternalCmdbException(e, ErrorCode.INTERNAL_ERROR, null);
+                throw new InternalException(e, ErrorCode.CMDB_API_DATA_ERROR, null);


errorCode应该是ErrorCode.INTERNAL_ERROR

jsonwan · 2023-02-02T09:37:21Z

...-manage/src/main/java/com/tencent/bk/job/manage/dao/notify/impl/NotifyEsbChannelDAOImpl.java

+            if (cause instanceof RuntimeException) {
+                throw (RuntimeException) cause;
+            } else {
+                throw new InternalException(errorMsg, e, ErrorCode.CMSI_MSG_CHANNEL_DATA_ERROR);


errorCode应该是ErrorCode.INTERNAL_ERROR

jsonwan

2处需要继续改下

liuliaozhong requested a review from jsonwan January 12, 2023 07:03

jsonwan reviewed Jan 12, 2023

View reviewed changes

jsonwan requested changes Jan 12, 2023

View reviewed changes

jsonwan approved these changes Jan 12, 2023

View reviewed changes

jsonwan requested changes Jan 12, 2023

View reviewed changes

jsonwan reviewed Jan 12, 2023

View reviewed changes

jsonwan reviewed Feb 2, 2023

View reviewed changes

jsonwan requested changes Feb 2, 2023

View reviewed changes

jsonwan reviewed Feb 2, 2023

View reviewed changes

jsonwan requested changes Feb 2, 2023

View reviewed changes

jsonwan approved these changes Feb 2, 2023

View reviewed changes

bugfix: 依赖的其他系统（CMDB、IAM等）接口异常时报错信息优化 TencentBlueKing#1430

b16fb8b

liuliaozhong force-pushed the 3.5.x_issue1430 branch from 5ef83ea to b16fb8b Compare February 2, 2023 13:28

jsonwan approved these changes Feb 2, 2023

View reviewed changes

jsonwan merged commit 36eb109 into TencentBlueKing:3.5.x Feb 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfix: 依赖的其他系统（CMDB、IAM等）接口异常时报错信息优化 #1430 #1691

bugfix: 依赖的其他系统（CMDB、IAM等）接口异常时报错信息优化 #1430 #1691

liuliaozhong commented Jan 12, 2023

jsonwan Jan 12, 2023 •

edited

Loading

jsonwan Jan 12, 2023

liuliaozhong Jan 12, 2023

jsonwan Jan 12, 2023 •

edited

Loading

jsonwan Jan 12, 2023 •

edited

Loading

jsonwan Jan 12, 2023 •

edited

Loading

jsonwan Jan 12, 2023

jsonwan left a comment

jsonwan left a comment

liuliaozhong commented Jan 12, 2023

jsonwan left a comment

jsonwan Jan 12, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

liuliaozhong Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan Feb 2, 2023

jsonwan left a comment

jsonwan Feb 2, 2023

liuliaozhong Feb 2, 2023

jsonwan Feb 2, 2023

liuliaozhong Feb 2, 2023

jsonwan left a comment

bugfix: 依赖的其他系统（CMDB、IAM等）接口异常时报错信息优化 #1430 #1691

bugfix: 依赖的其他系统（CMDB、IAM等）接口异常时报错信息优化 #1430 #1691

Conversation

liuliaozhong commented Jan 12, 2023

jsonwan Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsonwan Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

jsonwan Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

jsonwan Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsonwan left a comment

Choose a reason for hiding this comment

jsonwan left a comment

Choose a reason for hiding this comment

liuliaozhong commented Jan 12, 2023

jsonwan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsonwan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsonwan left a comment

Choose a reason for hiding this comment

jsonwan Jan 12, 2023 •

edited

Loading

jsonwan Jan 12, 2023 •

edited

Loading

jsonwan Jan 12, 2023 •

edited

Loading

jsonwan Jan 12, 2023 •

edited

Loading