Skip to content

Commit 2474d10

Browse files
fix: no proper handling of JSON server errors (#685)
On server errors with JSON responses, the CLI was using response.text instead of response.json() to get the detail, causing string formatting conflicts. Now, it properly parses JSON responses before error handling.
1 parent 0a99f94 commit 2474d10

File tree

4 files changed

+53
-11
lines changed

4 files changed

+53
-11
lines changed

safety/auth/utils.py

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,27 @@ def is_email_verified(info: Dict[str, Any]) -> Optional[bool]:
7676
return True
7777

7878

79+
def extract_detail(response: requests.Response) -> Optional[str]:
80+
"""
81+
Extract the reason from an HTTP response.
82+
83+
Args:
84+
response (requests.Response): The response.
85+
86+
Returns:
87+
Optional[str]: The reason.
88+
"""
89+
detail = None
90+
91+
try:
92+
detail = response.json().get("detail")
93+
except Exception:
94+
LOG.debug("Failed to extract detail from response: %s",
95+
response.status_code)
96+
97+
return detail
98+
99+
79100
def parse_response(func: Callable) -> Callable:
80101
"""
81102
Decorator to parse the response from an HTTP request.
@@ -105,12 +126,8 @@ def wrapper(*args, **kwargs):
105126
# TODO: Handle content as JSON and fallback to text for all responses
106127

107128
if r.status_code == 403:
108-
reason = None
109-
try:
110-
reason = r.json().get("detail")
111-
except Exception:
112-
LOG.debug("Failed to parse 403 response: %s", r.text)
113-
129+
reason = extract_detail(response=r)
130+
114131
raise InvalidCredentialError(
115132
credential="Failed authentication.", reason=reason
116133
)
@@ -130,7 +147,10 @@ def wrapper(*args, **kwargs):
130147
raise SafetyError(message=reason, error_code=error_code)
131148

132149
if r.status_code >= 500:
133-
raise ServerError(reason=f"{r.reason} - {r.text}")
150+
reason = extract_detail(response=r)
151+
LOG.debug("ServerError %s -> Response returned: %s",
152+
r.status_code, r.text)
153+
raise ServerError(reason=reason)
134154

135155
data = None
136156

safety/errors.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,6 @@ class ServerError(DatabaseFetchError):
281281
"""
282282
def __init__(self, reason: Optional[str] = None,
283283
message: str = "Sorry, something went wrong.\n"
284-
"Safety CLI cannot connect to the server.\n"
285284
"Our engineers are working quickly to resolve the issue."):
286285
info = f" Reason: {reason}"
287286
self.message = message + (info if reason else "")

safety/scan/render.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ def print_wait_project_verification(console: Console, project_id: str, closure:
303303
LOG.exception(f'Unable to verify the project, reason: {e}')
304304
reason = "We are currently unable to verify the project, " \
305305
"and it is necessary to link the scan to a specific " \
306-
f"project. Reason: {e}"
306+
f"project. \n\nAdditional Information: \n{e}"
307307
raise SafetyException(message=reason)
308308

309309
if not status:

tests/auth/test_auth_utils.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import unittest
22
from unittest.mock import MagicMock, Mock, patch, call
3-
from safety.auth.utils import initialize
3+
4+
from safety.auth.utils import initialize, extract_detail, FeatureType, \
5+
str_to_bool, get_config_setting, save_flags_config
46
from safety.errors import InvalidCredentialError
5-
from safety.auth.utils import FeatureType, str_to_bool, get_config_setting, save_flags_config
7+
68

79
class TestUtils(unittest.TestCase):
810

@@ -137,3 +139,24 @@ def test_initialize_with_server_response(self,
137139

138140
# Verify number of calls matches number of features
139141
self.assertEqual(mock_setattr.call_count, len(FeatureType))
142+
143+
def test_extract_detail(self):
144+
# Test valid JSON with detail
145+
response = Mock()
146+
response.json.return_value = {"detail": "Error message"}
147+
detail = extract_detail(response)
148+
self.assertEqual(detail, "Error message")
149+
150+
# Test valid JSON without detail
151+
response.json.return_value = {"message": "Something else"}
152+
detail = extract_detail(response)
153+
self.assertIsNone(detail)
154+
155+
# Test invalid JSON
156+
response.json.side_effect = ValueError()
157+
detail = extract_detail(response)
158+
self.assertIsNone(detail)
159+
160+
# Test empty response
161+
response.json.side_effect = None
162+
response.json.return_value = {}

0 commit comments

Comments
 (0)