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")