Merge pull request #1 from alex-smile/fix_by_review_1120

不暴露 check_response_status
TencentBlueKing · Nov 20, 2023 · 11a6749 · 11a6749
2 parents ef6c403 + c58c718
commit 11a6749
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 40 deletions.
diff --git a/sdks/bkapi-client-core/CHANGE.md b/sdks/bkapi-client-core/CHANGE.md
@@ -2,9 +2,9 @@
 
 ## 1.2.0
 - BK_API_CLIENT_ENABLE_SSL_VERIFY 默认值设置为 False
-- BaseClient 处理响应内容时优先检查 json，添加辅助方法：check_response_apigateway_error, check_response_status
-- ResponseError 添加 response_status_code/response_text/response_json 等辅助方法
-- 日志中，curl 信息中不再携带请求头
+- Client 添加辅助方法：check_response_apigateway_error
+- ResponseError 添加辅助方法：response_status_code, response_text, response_json
+- 日志中，curl 信息不再携带请求头
 
 ### 1.1.8
 - 使用 `CurlRequest` 封装转换 curl 命令的逻辑以优化性能

diff --git a/sdks/bkapi-client-core/bkapi_client_core/client.py b/sdks/bkapi-client-core/bkapi_client_core/client.py
@@ -25,6 +25,7 @@
     EndpointNotSetError,
     HTTPResponseError,
     JSONResponseError,
+    ResponseError,
 )
 from bkapi_client_core.session import Session
 from bkapi_client_core.utils import CurlRequest, urljoin
@@ -209,24 +210,6 @@ def check_response_apigateway_error(
                 response_headers_representer=response_headers_representer,
             )
 
-    def check_response_status(
-        self,
-        response,  # type: Optional[Response]
-    ):
-        # type: (...) -> None
-        """检查响应状态码，即 requests raise_for_status 校验"""
-        if response is None:
-            return
-
-        try:
-            response.raise_for_status()
-        except HTTPError as err:
-            response_headers_representer = ResponseHeadersRepresenter(response.headers)
-            raise HTTPResponseError(
-                str(err),
-                response=response,
-                response_headers_representer=response_headers_representer,
-            )
 
     def update_headers(
         self,
@@ -300,7 +283,7 @@ def _handle_exception(
     ):
         # type: (...) -> Optional[Response]
         # log exception
-        if isinstance(exception, RequestException):
+        if isinstance(exception, ResponseError):
             response = exception.response
             response_headers_representer = ResponseHeadersRepresenter(response.headers if response is not None else None)
             logger.warning(
@@ -309,6 +292,15 @@ def _handle_exception(
                 response_headers_representer,
                 CurlRequest(exception.request),
             )
+        elif isinstance(exception, RequestException):
+            response = exception.response
+            response_headers_representer = ResponseHeadersRepresenter(response.headers if response is not None else None)
+            logger.exception(
+                "request bkapi error. status_code: %s, %s\n%s",
+                response.status_code if response is not None else None,
+                response_headers_representer,
+                CurlRequest(exception.request),
+            )
         else:
             logger.exception("request operation failed. operation: %s, context: %s", operation, context)
 
@@ -344,15 +336,16 @@ def _handle_response_content(
 
         self.check_response_apigateway_error(response)
 
-        # 先校验 json，状态码校验失败时，可以在 exception 中获取正常的 json 响应内容；
-        # 如此，方便调用者在同一层中处理 http 状态码和 json 两个异常
-        response_json = self._check_response_json(response)
-
-        self.check_response_status(response)
-
-        return response_json
+        try:
+            response.raise_for_status()
+        except HTTPError as err:
+            response_headers_representer = ResponseHeadersRepresenter(response.headers)
+            raise HTTPResponseError(
+                str(err),
+                response=response,
+                response_headers_representer=response_headers_representer,
+            )
 
-    def _check_response_json(self, response):
         try:
             return response.json()
         except (TypeError, json.JSONDecodeError):

diff --git a/sdks/bkapi-client-core/tests/test_client.py b/sdks/bkapi-client-core/tests/test_client.py
@@ -401,15 +401,6 @@ def test_check_response_apigateway_error(self, mocker, response, expected_error)
         with pytest.raises(expected_error):
             self.client.check_response_apigateway_error(mocker.MagicMock(**response))
 
-    def test_check_response_status(self, mocker):
-        self.client.check_response_status(None)
-        self.client.check_response_status(mocker.MagicMock())
-
-        response = {"raise_for_status.side_effect": HTTPError()}
-        with pytest.raises(HTTPResponseError):
-            self.client.check_response_status(mocker.MagicMock(**response))
-
-
     @pytest.mark.parametrize(
         "session_headers, headers, expected",
         [
@@ -498,8 +489,8 @@ def test_handle_response_content(self, mocker):
         "response",
         [
             {"headers": {"X-Bkapi-Error-Code": "error"}},
-            {"json.side_effect": TypeError},
             {"raise_for_status.side_effect": RequestException("error")},
+            {"json.side_effect": TypeError},
         ],
     )
     def test_handle_response_content_error(self, mocker, response):