Skip to content

Commit 46399d8

Browse files
fix: Fix handling responses with invalid JSON (box/box-codegen#667) (#485)
Closes: #470
1 parent bd7fefa commit 46399d8

File tree

3 files changed

+64
-20
lines changed

3 files changed

+64
-20
lines changed

.codegen.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{ "engineHash": "8a9cc1d", "specHash": "f20ba3f", "version": "1.11.1" }
1+
{ "engineHash": "22f85cc", "specHash": "f20ba3f", "version": "1.11.1" }

box_sdk_gen/networking/box_network_client.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,7 @@ def fetch(self, options: 'FetchOptions') -> FetchResponse:
101101
url=network_response.url,
102102
status=network_response.status_code,
103103
headers=dict(response.network_response.headers),
104-
data=(
105-
json_to_serialized_data(network_response.text)
106-
if network_response.text
107-
else None
108-
),
104+
data=(self._read_json_body(network_response.text)),
109105
content=io.BytesIO(network_response.content),
110106
)
111107

@@ -254,11 +250,7 @@ def _raise_on_unsuccessful_request(
254250
)
255251

256252
network_response = response.network_response
257-
258-
try:
259-
response_json = network_response.json()
260-
except ValueError:
261-
response_json = {}
253+
response_json = BoxNetworkClient._read_json_body(network_response.text)
262254

263255
raise BoxAPIError(
264256
message=f'{network_response.status_code} {response_json.get("message", "")}; Request ID: {response_json.get("request_id", "")}',
@@ -307,6 +299,15 @@ def _validate_seekable(stream: ByteStream, raised_exception: Optional[Exception]
307299
error=raised_exception,
308300
)
309301

302+
@staticmethod
303+
def _read_json_body(response_body: str) -> dict:
304+
if not response_body:
305+
return {}
306+
try:
307+
return json_to_serialized_data(response_body)
308+
except (ValueError, TypeError):
309+
return {}
310+
310311
def _reset_stream(
311312
self,
312313
stream: ByteStream,

test/box_network_client.py

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
Authentication,
1313
BoxSDKError,
1414
BoxClient,
15+
ResponseFormat,
1516
)
1617
from box_sdk_gen.networking.box_network_client import (
1718
BoxNetworkClient,
@@ -467,7 +468,7 @@ def test_fetch_get_json_format_response_success(
467468
method="get",
468469
url="https://example.com",
469470
network_session=network_session_mock,
470-
response_format="json",
471+
response_format=ResponseFormat.JSON,
471472
)
472473
)
473474

@@ -488,7 +489,7 @@ def test_fetch_get_binary_format_response_success(
488489
method="get",
489490
url="https://example.com",
490491
network_session=network_session_mock,
491-
response_format="binary",
492+
response_format=ResponseFormat.BINARY,
492493
)
493494
)
494495

@@ -526,6 +527,37 @@ def test_retryable_status_codes(
526527
assert mock_requests_session.request.call_count == 3
527528

528529

530+
@pytest.mark.parametrize("retryable_status_code", [429, 500, 503])
531+
def test_retryable_status_codes_with_invalid__response_body(
532+
network_client,
533+
mock_requests_session,
534+
network_session_mock,
535+
response_200,
536+
retryable_status_code,
537+
response_failure_no_status,
538+
):
539+
response_failure_no_status.status_code = retryable_status_code
540+
response_failure_no_status.text = 'Invalid JSON'
541+
response_200.text = '{"id": "123456"}'
542+
mock_requests_session.request.side_effect = [
543+
response_failure_no_status,
544+
response_failure_no_status,
545+
response_200,
546+
]
547+
548+
fetch_response = network_client.fetch(
549+
FetchOptions(
550+
method="get",
551+
url="https://example.com",
552+
network_session=network_session_mock,
553+
response_format=ResponseFormat.JSON,
554+
)
555+
)
556+
assert fetch_response.status == 200
557+
assert fetch_response.data == {"id": "123456"}
558+
assert mock_requests_session.request.call_count == 3
559+
560+
529561
def test_status_code_202_with_no_retry_after_header(
530562
network_client, mock_requests_session, network_session_mock, response_202
531563
):
@@ -539,7 +571,7 @@ def test_status_code_202_with_no_retry_after_header(
539571
)
540572
)
541573
assert fetch_response.status == 202
542-
assert fetch_response.data == None
574+
assert fetch_response.data == {}
543575

544576

545577
def test_retryable_status_code_202(
@@ -590,7 +622,7 @@ def test_202_should_be_returned_if_retry_limit_is_reached(
590622
)
591623

592624
assert fetch_response.status == 202
593-
assert fetch_response.data == None
625+
assert fetch_response.data == {}
594626

595627

596628
@pytest.mark.parametrize("not_retryable_status_code", [404, 403, 400])
@@ -871,15 +903,13 @@ def test_raising_api_error_with_valid_json_body(network_client):
871903
assert e.name == "BoxAPIError"
872904

873905

874-
def test_raising_api_error_without_valid_json_body(network_client):
906+
@pytest.mark.parametrize('response_body', ['', 'Invalid json', 123])
907+
def test_raising_api_error_without_valid_json_body(network_client, response_body):
875908
client_error_response = Mock(Response)
876909
client_error_response.status_code = 400
877910
client_error_response.ok = False
878911
client_error_response.headers = {}
879-
client_error_response.text = ""
880-
client_error_response.json.side_effect = json.JSONDecodeError(
881-
"Expecting value: line 1 column 1 (char 0)", "", 0
882-
)
912+
client_error_response.text = response_body
883913

884914
request = APIRequest(
885915
method="POST",
@@ -960,6 +990,19 @@ def test_proxy_config():
960990
assert requests_session.proxies["https"] == "http://user:[email protected]:3128/"
961991

962992

993+
@pytest.mark.parametrize(
994+
"response_body, expected_json",
995+
[
996+
('', {}),
997+
('Invalid json', {}),
998+
(123, {}),
999+
('{"name": "John"}', {'name': 'John'}),
1000+
],
1001+
)
1002+
def test_read_json_body(response_body, expected_json):
1003+
assert BoxNetworkClient._read_json_body(response_body) == expected_json
1004+
1005+
9631006
def test_get_options_stream_position(network_client, mock_byte_stream):
9641007
mock_byte_stream.seek(1)
9651008
options = FetchOptions(

0 commit comments

Comments
 (0)