From d448b751763e01ea6b52d943c61115a2507d17fa Mon Sep 17 00:00:00 2001 From: Aditya Kumar Kasaudhan Date: Fri, 18 Oct 2024 17:11:48 +0530 Subject: [PATCH] slack_incoming: Add ok=false to JSON in case of error. Previously, errors were returned using Zulip's default format, which did not match Slack's expected response structure. This change ensures that errors in the Slack incoming webhook handler return JSON responses in Slack's expected format: {ok: false, error: "error string"}. Fixes: #31878. --- zerver/webhooks/slack_incoming/tests.py | 3 ++- zerver/webhooks/slack_incoming/view.py | 27 ++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/zerver/webhooks/slack_incoming/tests.py b/zerver/webhooks/slack_incoming/tests.py index cbf96e5e98..0b2ca20968 100644 --- a/zerver/webhooks/slack_incoming/tests.py +++ b/zerver/webhooks/slack_incoming/tests.py @@ -66,7 +66,8 @@ Hello, world. def test_message_without_payload(self) -> None: self.url = self.build_webhook_url() result = self.client_post(self.url) - self.assert_json_error(result, "Missing 'payload' argument") + self.assertEqual(result.json()["error"], "Missing 'payload' argument") + self.assertEqual(result.json()["ok"], False) def test_message_with_actions(self) -> None: expected_topic_name = "C1H9RESGL" diff --git a/zerver/webhooks/slack_incoming/view.py b/zerver/webhooks/slack_incoming/view.py index 79adaca429..3db74ac526 100644 --- a/zerver/webhooks/slack_incoming/view.py +++ b/zerver/webhooks/slack_incoming/view.py @@ -1,10 +1,13 @@ # Webhooks for external integrations. import re +from collections.abc import Callable +from functools import wraps from itertools import zip_longest from typing import Literal, TypedDict, cast -from django.http import HttpRequest, HttpResponse +from django.http import HttpRequest, HttpResponse, JsonResponse from django.utils.translation import gettext as _ +from typing_extensions import ParamSpec from zerver.decorator import webhook_view from zerver.lib.exceptions import JsonableError @@ -25,9 +28,31 @@ from zerver.lib.validator import ( from zerver.lib.webhooks.common import OptionalUserSpecifiedTopicStr, check_send_webhook_message from zerver.models import UserProfile +ParamT = ParamSpec("ParamT") + + +def slack_error_handler(view_func: Callable[..., HttpResponse]) -> Callable[..., HttpResponse]: + """ + A decorator that catches JsonableError exceptions and returns a + Slack-compatible error response in the format: + {ok: false, error: "error message"}. + """ + + @wraps(view_func) + def wrapped_view( + request: HttpRequest, *args: ParamT.args, **kwargs: ParamT.kwargs + ) -> HttpResponse: + try: + return view_func(request, *args, **kwargs) + except JsonableError as error: + return JsonResponse({"ok": False, "error": error.msg}, status=error.http_status_code) + + return wrapped_view + @webhook_view("SlackIncoming") @typed_endpoint +@slack_error_handler def api_slack_incoming_webhook( request: HttpRequest, user_profile: UserProfile,