logging(app-context): adds more logging context for slack api errors (#15557)

Author: Stephen Cefali

src/sentry/integrations/slack/action_endpoint.py

@@ -31,8 +31,11 @@ class SlackActionEndpoint(Endpoint):
     authentication_classes = ()
     permission_classes = ()
 
-    def api_error(self, error):
-        logger.info("slack.action.api-error", extra={"response": error.body})
+    def api_error(self, error, action_type, logging_data):
+        logging_data = logging_data.copy()
+        logging_data["response"] = error.body
+        logging_data["action_type"] = action_type
+        logger.info("slack.action.api-error", extra=logging_data)
         return self.respond(
             {
                 "response_type": "ephemeral",
@@ -155,11 +158,15 @@ def post(self, request):
         channel_id = data.get("channel", {}).get("id")
         user_id = data.get("user", {}).get("id")
 
+        logging_data["channel_id"] = channel_id
+        logging_data["user_id"] = user_id
+
         integration = slack_request.integration
         logging_data["integration_id"] = integration.id
 
         # Determine the issue group action is being taken on
         group_id = slack_request.callback_data["issue"]
+        logging_data["group_id"] = group_id
 
         # Actions list may be empty when receiving a dialog response
         action_list = data.get("actions", [])
@@ -175,6 +182,8 @@ def post(self, request):
             logger.error("slack.action.invalid-issue", extra=logging_data)
             return self.respond(status=403)
 
+        logging_data["organization_id"] = group.organization.id
+
         # Determine the acting user by slack identity
         try:
             idp = IdentityProvider.objects.get(type="slack", external_id=slack_request.team_id)
@@ -205,7 +214,7 @@ def post(self, request):
             try:
                 self.on_status(request, identity, group, action, data, integration)
             except client.ApiError as e:
-                return self.api_error(e)
+                return self.api_error(e, "status_dialog", logging_data)
 
             group = Group.objects.get(id=group.id)
             attachment = build_group_attachment(group, identity=identity, actions=[action])
@@ -230,6 +239,7 @@ def post(self, request):
         defer_attachment_update = False
 
         # Handle interaction actions
+        action_type = None
         try:
             for action in action_list:
                 action_type = action["name"]
@@ -242,7 +252,7 @@ def post(self, request):
                     self.open_resolve_dialog(data, group, integration)
                     defer_attachment_update = True
         except client.ApiError as e:
-            return self.api_error(e)
+            return self.api_error(e, action_type, logging_data)
 
         if defer_attachment_update:
             return self.respond()

tests/sentry/integrations/slack/test_action_endpoint.py

@@ -5,6 +5,7 @@
 from six.moves.urllib.parse import parse_qs
 from mock import patch
 
+from sentry.api import client
 from sentry import options
 from sentry.models import (
     Integration,
@@ -335,6 +336,61 @@ def test_permission_denied(self):
         assert not resp.data["replace_original"]
         assert resp.data["text"] == "Sentry can't perform that action right now on your behalf!"
 
+    @responses.activate
+    @patch("sentry.api.client.put")
+    def test_handle_submission_fail(self, client_put):
+        status_action = {"name": "resolve_dialog", "value": "resolve_dialog"}
+
+        # Expect request to open dialog on slack
+        responses.add(
+            method=responses.POST,
+            url="https://slack.com/api/dialog.open",
+            body='{"ok": true}',
+            status=200,
+            content_type="application/json",
+        )
+
+        resp = self.post_webhook(action_data=[status_action])
+        assert resp.status_code == 200, resp.content
+
+        # Opening dialog should *not* cause the current message to be updated
+        assert resp.content == ""
+
+        data = parse_qs(responses.calls[0].request.body)
+        assert data["token"][0] == self.integration.metadata["access_token"]
+        assert data["trigger_id"][0] == self.trigger_id
+        assert "dialog" in data
+
+        dialog = json.loads(data["dialog"][0])
+        callback_data = json.loads(dialog["callback_id"])
+        assert int(callback_data["issue"]) == self.group1.id
+        assert callback_data["orig_response_url"] == self.response_url
+
+        # Completing the dialog will update the message
+        responses.add(
+            method=responses.POST,
+            url=self.response_url,
+            body='{"ok": true}',
+            status=200,
+            content_type="application/json",
+        )
+
+        # make the client raise an API error
+        client_put.side_effect = client.ApiError(
+            403, '{"detail":"You do not have permission to perform this action."}'
+        )
+
+        resp = self.post_webhook(
+            type="dialog_submission",
+            callback_id=dialog["callback_id"],
+            data={"submission": {"resolve_type": "resolved"}},
+        )
+
+        client_put.asser_called()
+
+        assert resp.status_code == 200, resp.content
+        assert resp.data["text"] == "Sentry can't perform that action right now on your behalf!"
+
     def test_invalid_token(self):
         resp = self.post_webhook(token="invalid")
         assert resp.status_code == 401