diff --git a/templates/zerver/api/fixtures.json b/templates/zerver/api/fixtures.json index 84a9b1100c..f451483f82 100644 --- a/templates/zerver/api/fixtures.json +++ b/templates/zerver/api/fixtures.json @@ -313,9 +313,10 @@ "var_name": "content" }, "nonexistent-stream-error": { - "code": "BAD_REQUEST", + "code": "STREAM_DOES_NOT_EXIST", "msg": "Stream 'nonexistent_stream' does not exist", - "result": "error" + "result": "error", + "stream": "nonexistent_stream" }, "private-message": { "id": 134, diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index b23dc4e144..a108ff81b4 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -36,6 +36,7 @@ from zerver.lib.cache import ( ) from zerver.lib.context_managers import lockfile from zerver.lib.emoji import emoji_name_to_emoji_code, get_emoji_file_name +from zerver.lib.exceptions import StreamDoesNotExistError from zerver.lib.hotspots import get_next_hotspots from zerver.lib.message import ( access_message, @@ -1869,8 +1870,7 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee, except Stream.DoesNotExist: send_pm_if_empty_stream(sender, None, stream_name, realm) - raise JsonableError(_("Stream '%(stream_name)s' " - "does not exist") % {'stream_name': escape(stream_name)}) + raise StreamDoesNotExistError(escape(stream_name)) recipient = get_stream_recipient(stream.id) if not stream.invite_only: diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py index 2e1e27d7a3..0091a63320 100644 --- a/zerver/lib/exceptions.py +++ b/zerver/lib/exceptions.py @@ -2,6 +2,7 @@ from enum import Enum from typing import Any, Dict, List, Optional, Text, Type from django.core.exceptions import PermissionDenied +from django.utils.translation import ugettext as _ class AbstractEnum(Enum): '''An enumeration whose members are used strictly for their names.''' @@ -29,6 +30,7 @@ class ErrorCode(AbstractEnum): BAD_IMAGE = () REALM_UPLOAD_QUOTA = () BAD_NARROW = () + STREAM_DOES_NOT_EXIST = () UNAUTHORIZED_PRINCIPAL = () BAD_EVENT_QUEUE_ID = () CSRF_FAILED = () @@ -134,6 +136,17 @@ class JsonableError(Exception): def __str__(self) -> str: return self.msg +class StreamDoesNotExistError(JsonableError): + code = ErrorCode.STREAM_DOES_NOT_EXIST + data_fields = ['stream'] + + def __init__(self, stream: str) -> None: + self.stream = stream + + @staticmethod + def msg_format() -> str: + return _("Stream '{stream}' does not exist") + class RateLimited(PermissionDenied): def __init__(self, msg: str="") -> None: super().__init__(msg) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 10f3103632..bb94d46035 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -652,7 +652,8 @@ class WebhookTestCase(ZulipTestCase): if content_type is not None: kwargs['content_type'] = content_type - msg = self.send_json_payload(self.test_user, self.url, payload, + sender = kwargs.get('sender', self.test_user) + msg = self.send_json_payload(sender, self.url, payload, stream_name=None, **kwargs) self.do_test_message(msg, expected_message) diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py index aa8e41328f..791571b39e 100644 --- a/zerver/lib/webhooks/common.py +++ b/zerver/lib/webhooks/common.py @@ -3,9 +3,11 @@ from typing import Optional, Text from zerver.lib.actions import check_send_stream_message, \ check_send_private_message +from zerver.lib.exceptions import StreamDoesNotExistError from zerver.lib.request import REQ, has_request_variables from zerver.models import UserProfile + @has_request_variables def check_send_webhook_message( request: HttpRequest, user_profile: UserProfile, @@ -20,5 +22,13 @@ def check_send_webhook_message( else: if user_specified_topic is not None: topic = user_specified_topic - check_send_stream_message(user_profile, request.client, - stream, topic, body) + + try: + check_send_stream_message(user_profile, request.client, + stream, topic, body) + except StreamDoesNotExistError: + # A PM will be sent to the bot_owner by check_message, notifying + # that the webhook bot just tried to send a message to a non-existent + # stream, so we don't need to re-raise it since it clutters up + # webhook-errors.log + pass diff --git a/zerver/webhooks/helloworld/tests.py b/zerver/webhooks/helloworld/tests.py index 4971658065..0067e173f9 100644 --- a/zerver/webhooks/helloworld/tests.py +++ b/zerver/webhooks/helloworld/tests.py @@ -1,7 +1,9 @@ # -*- coding: utf-8 -*- +from django.conf import settings from typing import Text from zerver.lib.test_classes import WebhookTestCase +from zerver.models import get_system_bot class HelloWorldHookTests(WebhookTestCase): STREAM_NAME = 'test' @@ -35,6 +37,16 @@ class HelloWorldHookTests(WebhookTestCase): self.send_and_test_private_message('goodbye', expected_message=expected_message, content_type="application/x-www-form-urlencoded") + def test_stream_error_pm_to_bot_owner(self) -> None: + # Note taht this is really just a test for check_send_webhook_message + self.STREAM_NAME = 'nonexistent' + self.url = self.build_webhook_url() + notification_bot = get_system_bot(settings.NOTIFICATION_BOT) + expected_message = "Hi there! We thought you'd like to know that your bot **Zulip Webhook Bot** just tried to send a message to stream `nonexistent`, but that stream does not yet exist. To create it, click the gear in the left-side stream list." + self.send_and_test_private_message('goodbye', expected_message=expected_message, + content_type='application/x-www-form-urlencoded', + sender=notification_bot) + def test_custom_topic(self) -> None: # Note that this is really just a test for check_send_webhook_message expected_subject = u"Custom Topic"