Merge pull request #926 from launchableinc/error-message-capture

Make sure to report server side error message

Author: kohsuke

launchable/commands/subset.py

@@ -485,7 +485,7 @@ def run(self):
 
                     # The status code 422 is returned when validation error of the test mapping file occurs.
                     if res.status_code == 422:
-                        msg = "Error: {}".format(res.json().get("reason"))
+                        msg = "Error: {}".format(res.reason)
                         tracking_client.send_error_event(
                             event_name=Tracking.ErrorEvent.USER_ERROR,
                             stack_trace=msg,

launchable/utils/http_client.py

@@ -104,6 +104,18 @@ def request(
             "received response status:{} message:{} headers:{}".format(response.status_code, response.reason,
                                                                        response.headers)
         )
+
+        # because (I believe, though I could be wrong) HTTP/2 got rid of status message, our server side HTTP stack
+        # doesn't let us forward the status message (=response.reason), which would have been otherwise a very handy
+        # mechanism to reliably forward error messages. So instead, we forward JSON error response of the form
+        # {"reason": ...}. Backfill response.reason with this JSON error message if it exists, so that the exceptions
+        # thrown from response.raise_for_status() will have a meaningful message.
+        if response.status_code >= 400 and response.headers.get("Content-Type", "").startswith("application/json"):
+            try:
+                response.reason = response.json().get("reason", response.reason)
+            except json.JSONDecodeError:
+                pass
+
         return response
 
     def _headers(self, compress):

tests/utils/test_http_client.py

@@ -2,6 +2,8 @@
 import platform
 from unittest import TestCase, mock
 
+from requests import Session
+
 from launchable.utils.http_client import _HttpClient
 from launchable.version import __version__
 
@@ -43,3 +45,19 @@ def test_header(self):
                 "dummy",
             ),
         })
+
+    def test_reason(self):
+        '''make sure we correctly propagate error message from the server'''
+
+        # use new session to disable retry
+        cli = _HttpClient(session=Session())
+        # /error is an actual endpoint that exists on our service to test the behavior
+        res = cli.request("GET", "intake/error")
+        self.assertEqual(res.status_code, 500)
+        self.assertEqual(res.reason, "Welp")
+
+        try:
+            res.raise_for_status()
+            self.fail("should have raised")
+        except Exception as e:
+            self.assertIn("Welp", str(e))