Skip to content

Commit 3079171

Browse files
authored
fix: Stop processing data for logging when logging is disabled and cache response json (#824)
Closes: SDK-3230
1 parent ff19bc3 commit 3079171

File tree

1 file changed

+38
-34
lines changed

1 file changed

+38
-34
lines changed

boxsdk/network/default_network.py

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
from logging import getLogger
23
from pprint import pformat
34
import sys
@@ -74,10 +75,11 @@ def _log_request(self, method: str, url: str, **kwargs: Any) -> None:
7475
:param url:
7576
The URL for the request.
7677
"""
77-
self._logger.info(
78-
self.REQUEST_FORMAT,
79-
{'method': method, 'url': url, 'request_kwargs': pformat(sanitize_dictionary(kwargs))},
80-
)
78+
if self._logger.isEnabledFor(logging.INFO):
79+
self._logger.info(
80+
self.REQUEST_FORMAT,
81+
{'method': method, 'url': url, 'request_kwargs': pformat(sanitize_dictionary(kwargs))},
82+
)
8183

8284
def _log_exception(self, method: str, url: str, exc_info: Any) -> None:
8385
"""Log information at WARNING level about the exception that was raised when trying to make the request.
@@ -86,11 +88,12 @@ def _log_exception(self, method: str, url: str, exc_info: Any) -> None:
8688
:param url: The URL for the request.
8789
:param exc_info: The exception info returned from `sys.exc_info()`.
8890
"""
89-
exc_type, exc_value, _ = exc_info
90-
self._logger.warning(
91-
self.EXCEPTION_FORMAT,
92-
{'method': method, 'url': url, 'exc_type_name': exc_type.__name__, 'exc_value': exc_value},
93-
)
91+
if self._logger.isEnabledFor(logging.WARNING):
92+
exc_type, exc_value, _ = exc_info
93+
self._logger.warning(
94+
self.EXCEPTION_FORMAT,
95+
{'method': method, 'url': url, 'exc_type_name': exc_type.__name__, 'exc_value': exc_value},
96+
)
9497

9598

9699
class DefaultNetworkResponse(NetworkResponse):
@@ -119,26 +122,19 @@ class DefaultNetworkResponse(NetworkResponse):
119122
If the caller uses `Response.content`, then it is safe for
120123
:class:`DefaultNetwork` to also access it. But if the caller uses any of
121124
the streaming mechanisms, then it is not safe for :class:`DefaultNetwork`
122-
to ever read any of the content. Thus, the options available are:
123-
124-
- Never log the content of a response.
125-
- Make logging part of the :class:`Network` interface, and add an
126-
optional keyword argument that callers can use to specify when it is
127-
unsafe to log the content of a response.
128-
- Defer logging until it is possible to auto-detect which mechanism is
129-
being used.
130-
131-
This class implements the latter option. Instead of response
132-
logging taking place in `DefaultNetwork.request()`, it takes place in this
133-
`DefaultNetworkResponse` class, as soon as the caller starts reading the
134-
content. If `content` or `json()` are accessed, then the response will be
135-
logged with its content. Whereas if `response_as_stream` or
136-
`request_response` are accessed, then the response will be logged with a
137-
placeholder for the actual content.
138-
139-
Because most SDK methods immediately read the content on success, but not
140-
on errors (the SDK may retry the request based on just the status code),
141-
this class will log its content if it represents a failed request.
125+
to ever read any of the content.
126+
127+
The SDK logs only the response content of type JSON. Non-JSON responses, e.g.
128+
the content of the downloaded file should not be logged by the SDK at any time.
129+
In that case the response will be logged with a placeholder for the actual content.
130+
131+
:param: `log_response_content` specifies wheather the response content should be logged or not.
132+
Its value is determined by `BoxRequest.expect_json_response` field, which is set to False,
133+
only if explicitly specified inside SDK method, e.g. self._session.get(url, expect_json_response=False).
134+
So the contenet of the response will be read by the logger only when the call inside an SDK method
135+
expects a JSON response. In this case we can be sure that the SDK method will read the content
136+
of the response using non-streaming mechanism and accessing content of the response by logger
137+
with `response.json()` or 'response.content` is safe.
142138
"""
143139

144140
_COMMON_RESPONSE_FORMAT = '"%(method)s %(url)s" %(status_code)s %(content_length)s\n%(headers)s\n%(content)s\n'
@@ -151,11 +147,14 @@ def __init__(self, request_response: 'Response', access_token_used: str, log_res
151147
self._request_response = request_response
152148
self._access_token_used = access_token_used
153149
self._did_log = False
150+
self._json = None
154151
self.log(can_safely_log_content=log_response_content)
155152

156153
def json(self) -> dict:
157154
"""Base class override."""
158-
return self._request_response.json()
155+
if self._json is None:
156+
self._json = self._request_response.json()
157+
return self._json
159158

160159
@property
161160
def content(self) -> Optional[bytes]:
@@ -215,6 +214,15 @@ def log(self, can_safely_log_content: bool = False) -> None:
215214
if self._did_log:
216215
return
217216
self._did_log = True
217+
218+
if self.ok:
219+
logger_method, logger_level, response_format = self._logger.info, logging.INFO, self.SUCCESSFUL_RESPONSE_FORMAT
220+
else:
221+
logger_method, logger_level, response_format = self._logger.warning, logging.WARNING, self.ERROR_RESPONSE_FORMAT
222+
223+
if not self._logger.isEnabledFor(logger_level):
224+
return
225+
218226
content_length = self.headers.get('Content-Length', None)
219227
content = self.CONTENT_NOT_LOGGED
220228
if can_safely_log_content:
@@ -230,10 +238,6 @@ def log(self, can_safely_log_content: bool = False) -> None:
230238
content = pformat(sanitize_dictionary(content))
231239
if content_length is None:
232240
content_length = '?'
233-
if self.ok:
234-
logger_method, response_format = self._logger.info, self.SUCCESSFUL_RESPONSE_FORMAT
235-
else:
236-
logger_method, response_format = self._logger.warning, self.ERROR_RESPONSE_FORMAT
237241
logger_method(
238242
response_format,
239243
{

0 commit comments

Comments
 (0)