From 251d56d8c9c27dfd19ec14e0d199fe7c084d5d22 Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Thu, 1 Oct 2026 00:58:23 +0530 Subject: [PATCH] fix: log failed Zoom responses whose body is not JSON create_request_log parses a string as JSON, and failed calls handed it response.text. A gateway error or an empty body then raised JSONDecodeError, which replaced Zoom's real error and left nothing in the Integration Request log. Failed calls now log the status code as data and the body as text, and their error messages carry the status code. --- zoom_integration/api.py | 20 ++++++++++++++---- .../tests/test_request_logging.py | 21 +++++++++++++++++++ .../doctype/zoom_webinar/zoom_webinar.py | 19 +++++------------ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/zoom_integration/api.py b/zoom_integration/api.py index 43c30a5..239cf17 100644 --- a/zoom_integration/api.py +++ b/zoom_integration/api.py @@ -21,8 +21,8 @@ def create_zoom_session(resource: str, body: dict) -> dict: create_request_log(data, is_remote_request=1, service_name="Zoom", status="Completed") return data - create_request_log(response.text, is_remote_request=1, service_name="Zoom", status="Failed") - frappe.throw(f"Failed to create {resource[:-1]} on Zoom: {response.text}") + log_failed_response(response) + frappe.throw(f"Failed to create {resource[:-1]} on Zoom ({response.status_code}): {response.text}") def update_zoom_session(resource: str, session_id: str, body: dict) -> None: @@ -53,8 +53,8 @@ def add_zoom_registrant(resource: str, session_id: str, body: dict) -> dict: response = requests.post(url, headers=headers, data=json.dumps(body)) if response.status_code not in (200, 201): - create_request_log(response.text, is_remote_request=1, service_name="Zoom", status="Failed") - frappe.throw(f"Failed to add registrant on Zoom: {response.text}") + log_failed_response(response) + frappe.throw(f"Failed to add registrant on Zoom ({response.status_code}): {response.text}") data = response.json() create_request_log(data, is_remote_request=1, service_name="Zoom", status="Completed") @@ -95,3 +95,15 @@ def get_zoom_participants(resource: str, session_uuid: str) -> list[dict]: encoded_uuid = quote(session_uuid, safe="") url = f"{ZOOM_API_BASE_PATH}/past_{resource}/{encoded_uuid}/participants?page_size={DEFAULT_PAGE_SIZE}" return _paginate(url, "participants") + + +def log_failed_response(response) -> None: + """Log a failed Zoom call. Its body is not always JSON (a gateway error is HTML or + empty), and `create_request_log` parses a string as JSON, so the body goes in as text.""" + create_request_log( + {"status_code": response.status_code}, + is_remote_request=1, + service_name="Zoom", + status="Failed", + error=response.text, + ) diff --git a/zoom_integration/tests/test_request_logging.py b/zoom_integration/tests/test_request_logging.py index bf0d684..b199c03 100644 --- a/zoom_integration/tests/test_request_logging.py +++ b/zoom_integration/tests/test_request_logging.py @@ -30,3 +30,24 @@ def test_access_token_is_never_written_to_the_request_log(self): self.assertTrue(logged, "expected the call to be logged") for row in logged: self.assertNotIn("Authorization", row.request_headers or "") + + def test_a_failure_with_a_non_json_body_is_logged_and_reported(self): + existing = set(frappe.get_all("Integration Request", pluck="name")) + + with ( + patch.object(api, "get_authenticated_headers_for_zoom", return_value=TEST_ZOOM_HEADERS), + patch.object(api, "requests") as mock_requests, + ): + mock_requests.post.return_value = mock_response(502, text="Bad Gateway") + with self.assertRaises(frappe.ValidationError) as raised: + api.create_zoom_session("meetings", {"topic": "Logging Test"}) + + self.assertIn("502", str(raised.exception)) + logged = frappe.get_all( + "Integration Request", + filters={"name": ["not in", list(existing)], "status": "Failed"}, + fields=["data", "error"], + ) + self.assertEqual(len(logged), 1) + self.assertIn("502", logged[0].data) + self.assertIn("Bad Gateway", logged[0].error) diff --git a/zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py b/zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py index 2c32589..b15d1cc 100644 --- a/zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py +++ b/zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py @@ -12,6 +12,7 @@ from frappe.utils import cint, format_datetime, getdate from frappe.utils.data import convert_utc_to_timezone, get_datetime, get_time +from zoom_integration.api import log_failed_response from zoom_integration.utils import ZOOM_API_BASE_PATH, get_authenticated_headers_for_zoom ATTENDANCE_SYNC_BATCH_SIZE = 300 # Zoom API allows max 300 per page @@ -83,13 +84,8 @@ def create_webinar_on_zoom(self): frappe.msgprint(_("Webinar created successfully on Zoom.")) create_request_log(data, is_remote_request=1, service_name="Zoom", status="Completed") else: - create_request_log( - response.text, - is_remote_request=1, - service_name="Zoom", - status="Failed", - ) - frappe.throw("Failed to create webinar on Zoom: {0}".format(response.text)) + log_failed_response(response) + frappe.throw(f"Failed to create webinar on Zoom ({response.status_code}): {response.text}") def on_update(self): if not self.zoom_webinar_id: @@ -227,13 +223,8 @@ def add_registrant( response = requests.post(url, headers=headers, data=body) if response.status_code not in (200, 201): - create_request_log( - response.text, - is_remote_request=1, - service_name="Zoom", - status="Failed", - ) - frappe.throw(frappe._(f"Failed to add registrant: {response.text}")) + log_failed_response(response) + frappe.throw(frappe._(f"Failed to add registrant ({response.status_code}): {response.text}")) data = response.json() create_request_log(data, is_remote_request=1, service_name="Zoom", status="Completed")