fix(scripts): add timeout and status handling to GitHub workflow cancellation script

scripts/cancel_github_workflows.py

@@ -46,18 +46,41 @@
 github_repo = os.environ.get("GITHUB_REPOSITORY", "apache/superset")
 
 
+REQUEST_TIMEOUT = 30  # seconds
+
+
 def request(
     method: Literal["GET", "POST", "DELETE", "PUT"], endpoint: str, **kwargs: Any
 ) -> dict[str, Any]:
-    resp = requests.request(  # noqa: S113
-        method,
-        f"https://api.github.com/{endpoint.lstrip('/')}",
-        headers={"Authorization": f"Bearer {github_token}"},
-        **kwargs,
-    ).json()
-    if "message" in resp:
-        raise ClickException(f"{endpoint} >> {resp['message']} <<")
-    return resp
+    try:
+        resp = requests.request(
+            method,
+            f"https://api.github.com/{endpoint.lstrip('/')}",
+            headers={"Authorization": f"Bearer {github_token}"},
+            timeout=REQUEST_TIMEOUT,
+            **kwargs,
+        )
+        resp.raise_for_status()
+    except requests.exceptions.Timeout as ex:
+        raise ClickException(f"{endpoint} >> request timed out <<") from ex
+    except requests.exceptions.HTTPError as ex:
+        try:
+            msg = ex.response.json().get("message", ex.response.text)
+        except (ValueError, AttributeError):
+            msg = getattr(ex.response, "text", None) or str(ex)
+        raise ClickException(f"{endpoint} >> {msg} <<") from ex
+
+    if not resp.content:
+        return {}
+
+    try:
+        data: dict[str, Any] = resp.json()
+    except ValueError as ex:
+        raise ClickException(f"{endpoint} >> invalid JSON in response <<") from ex
+
+    if "message" in data:
+        raise ClickException(f"{endpoint} >> {data['message']} <<")
+    return data
 
 
 def list_runs(

tests/unit_tests/scripts/cancel_github_workflows_test.py

@@ -0,0 +1,110 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from unittest.mock import MagicMock, patch
+
+import pytest
+import requests
+from click.exceptions import ClickException
+
+from scripts.cancel_github_workflows import request, REQUEST_TIMEOUT
+
+
+@patch("scripts.cancel_github_workflows.github_token", "fake-token")
+class TestRequest:
+    """Focused tests for the request() helper."""
+
+    @patch("scripts.cancel_github_workflows.requests.request")
+    def test_successful_json_response(self, mock_req: MagicMock) -> None:
+        resp = MagicMock()
+        resp.content = b'{"total_count": 0, "workflow_runs": []}'
+        resp.json.return_value = {"total_count": 0, "workflow_runs": []}
+        resp.raise_for_status.return_value = None
+        mock_req.return_value = resp
+
+        result = request("GET", "/repos/owner/repo/actions/runs")
+
+        assert result == {"total_count": 0, "workflow_runs": []}
+        mock_req.assert_called_once_with(
+            "GET",
+            "https://api.github.com/repos/owner/repo/actions/runs",
+            headers={"Authorization": "Bearer fake-token"},
+            timeout=REQUEST_TIMEOUT,
+        )
+
+    @patch("scripts.cancel_github_workflows.requests.request")
+    def test_api_error_message(self, mock_req: MagicMock) -> None:
+        resp = MagicMock()
+        http_error = requests.exceptions.HTTPError(response=resp)
+        resp.raise_for_status.side_effect = http_error
+        resp.json.return_value = {"message": "Not Found"}
+        resp.text = "Not Found"
+        mock_req.return_value = resp
+
+        with pytest.raises(ClickException, match="Not Found"):
+            request("GET", "/repos/owner/repo/pulls/999")
+
+    @patch("scripts.cancel_github_workflows.requests.request")
+    def test_non_json_http_error(self, mock_req: MagicMock) -> None:
+        resp = MagicMock()
+        http_error = requests.exceptions.HTTPError(response=resp)
+        resp.raise_for_status.side_effect = http_error
+        resp.json.side_effect = ValueError("No JSON")
+        resp.text = "502 Bad Gateway"
+        mock_req.return_value = resp
+
+        with pytest.raises(ClickException, match="502 Bad Gateway"):
+            request("GET", "/repos/owner/repo/actions/runs")
+
+    @patch("scripts.cancel_github_workflows.requests.request")
+    def test_timeout(self, mock_req: MagicMock) -> None:
+        mock_req.side_effect = requests.exceptions.Timeout()
+
+        with pytest.raises(ClickException, match="request timed out"):
+            request("GET", "/repos/owner/repo/actions/runs")
+
+    @patch("scripts.cancel_github_workflows.requests.request")
+    def test_empty_response_body(self, mock_req: MagicMock) -> None:
+        resp = MagicMock()
+        resp.content = b""
+        resp.raise_for_status.return_value = None
+        mock_req.return_value = resp
+
+        result = request("POST", "/repos/owner/repo/actions/runs/1/cancel")
+
+        assert result == {}
+
+    @patch("scripts.cancel_github_workflows.requests.request")
+    def test_success_with_message_field_raises(self, mock_req: MagicMock) -> None:
+        resp = MagicMock()
+        resp.content = b'{"message": "Bad credentials"}'
+        resp.json.return_value = {"message": "Bad credentials"}
+        resp.raise_for_status.return_value = None
+        mock_req.return_value = resp
+
+        with pytest.raises(ClickException, match="Bad credentials"):
+            request("GET", "/repos/owner/repo/actions/runs")
+
+    @patch("scripts.cancel_github_workflows.requests.request")
+    def test_invalid_json_body(self, mock_req: MagicMock) -> None:
+        resp = MagicMock()
+        resp.content = b"not-json"
+        resp.json.side_effect = ValueError("Expecting value")
+        resp.raise_for_status.return_value = None
+        mock_req.return_value = resp
+
+        with pytest.raises(ClickException, match="invalid JSON in response"):
+            request("GET", "/repos/owner/repo/actions/runs")