From 00158a55274609ffe2c91b9f4966da8ac34afe76 Mon Sep 17 00:00:00 2001
From: Matias Bordese <mbordese@gmail.com>
Date: Wed, 16 Oct 2024 12:33:06 -0300
Subject: [PATCH] Handle incident client general request errors (#5184)

Handle some (unexpected) incident request errors more gracefully.
---
 engine/common/incident_api/client.py          | 23 +++++++++++++---
 .../common/incident_api/tests/test_client.py  | 27 +++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/engine/common/incident_api/client.py b/engine/common/incident_api/client.py
index 7013b45a3c..ac5156fce7 100644
--- a/engine/common/incident_api/client.py
+++ b/engine/common/incident_api/client.py
@@ -4,6 +4,7 @@
 
 import requests
 from django.conf import settings
+from requests.exceptions import RequestException
 
 from common.constants.plugin_ids import PluginID
 
@@ -91,6 +92,18 @@ def __init__(self, api_url: str, api_token: str) -> None:
     def _request_headers(self):
         return {"User-Agent": settings.GRAFANA_COM_USER_AGENT, "Authorization": f"Bearer {self.api_token}"}
 
+    def _make_request(self, url, *args, **kwargs):
+        try:
+            response = requests.post(url, *args, **kwargs)
+        except RequestException as e:
+            raise IncidentAPIException(
+                status=e.response.status_code if e.response else 500,
+                url=e.response.request.url if e.response else url,
+                msg=e.response.text if e.response else "Unexpected error",
+                method=e.response.request.method if e.response else "POST",
+            )
+        return response
+
     def _check_response(self, response: requests.models.Response):
         message = ""
 
@@ -119,7 +132,7 @@ def create_incident(
         endpoint = "api/v1/IncidentsService.CreateIncident"
         url = self.api_url + endpoint
         # NOTE: invalid severity will raise a 500 error
-        response = requests.post(
+        response = self._make_request(
             url,
             json={
                 "title": title,
@@ -137,7 +150,9 @@ def create_incident(
     def get_incident(self, incident_id: str) -> typing.Tuple[IncidentDetails, requests.models.Response]:
         endpoint = "api/v1/IncidentsService.GetIncident"
         url = self.api_url + endpoint
-        response = requests.post(url, json={"incidentID": incident_id}, timeout=TIMEOUT, headers=self._request_headers)
+        response = self._make_request(
+            url, json={"incidentID": incident_id}, timeout=TIMEOUT, headers=self._request_headers
+        )
         self._check_response(response)
         return response.json().get("incident"), response
 
@@ -146,7 +161,7 @@ def get_severities(self) -> typing.Tuple[typing.List[SeverityDetails], requests.
         endpoint = "api/SeveritiesService.GetOrgSeverities"
         url = self.api_url + endpoint
         # pass empty json payload otherwise it will return a 500 response
-        response = requests.post(url, timeout=TIMEOUT, headers=self._request_headers, json={})
+        response = self._make_request(url, timeout=TIMEOUT, headers=self._request_headers, json={})
         self._check_response(response)
         return response.json().get("severities"), response
 
@@ -155,7 +170,7 @@ def add_activity(
     ) -> typing.Tuple[ActivityItemDetails, requests.models.Response]:
         endpoint = "api/v1/ActivityService.AddActivity"
         url = self.api_url + endpoint
-        response = requests.post(
+        response = self._make_request(
             url,
             json={"incidentID": incident_id, "activityKind": kind, "body": body},
             timeout=TIMEOUT,
diff --git a/engine/common/incident_api/tests/test_client.py b/engine/common/incident_api/tests/test_client.py
index 2ba6840d5b..ec82262d88 100644
--- a/engine/common/incident_api/tests/test_client.py
+++ b/engine/common/incident_api/tests/test_client.py
@@ -1,7 +1,9 @@
 import json
+from unittest.mock import patch
 
 import httpretty
 import pytest
+from requests.exceptions import RequestException
 from rest_framework import status
 
 from common.incident_api.client import (
@@ -185,3 +187,28 @@ def test_error_handling(endpoint, client_method_name, args):
         assert excinfo.value.msg == response_data["error"]
         assert excinfo.value.url == url
         assert excinfo.value.method == "POST"
+
+
+@pytest.mark.parametrize(
+    "endpoint, client_method_name, args",
+    [
+        ("api/v1/IncidentsService.CreateIncident", "create_incident", ("title",)),
+        ("api/v1/IncidentsService.GetIncident", "get_incident", ("incident-id",)),
+        ("api/SeveritiesService.GetOrgSeverities", "get_severities", ()),
+        ("api/v1/ActivityService.AddActivity", "add_activity", ("incident-id", "content")),
+    ],
+)
+@httpretty.activate(verbose=True, allow_net_connect=False)
+def test_unexpected_error_handling(endpoint, client_method_name, args):
+    stack_url = "https://foobar.grafana.net"
+    api_token = "asdfasdfasdfasdf"
+    client = IncidentAPIClient(stack_url, api_token)
+    url = f"{stack_url}{client.INCIDENT_BASE_PATH}{endpoint}"
+    with patch("common.incident_api.client.requests.post", side_effect=RequestException):
+        with pytest.raises(IncidentAPIException) as excinfo:
+            client_method = getattr(client, client_method_name)
+            client_method(*args)
+        assert excinfo.value.status == 500
+        assert excinfo.value.msg == "Unexpected error"
+        assert excinfo.value.url == url
+        assert excinfo.value.method == "POST"