outgoing_webhook: Ignore the exception on failure if the stream is gone.

In the outgoing webhook handler, there is potentially several seconds
of trying between when a message triggering an outgoing webhook
arrives, and when it fails.  In the meantime, the stream the
triggering message was on may have been deleted, causing the
"Failure!" message to have no valid stream to be sent to.

Rather than raise an exception in the outgoing webhook worker, ignore
the exception and move on.
This commit is contained in:
Alex Vandiver
2022-11-03 10:35:07 -04:00
committed by Tim Abbott
parent eb7a2f2c38
commit d8ebbedbbb
2 changed files with 43 additions and 2 deletions

View File

@@ -11,7 +11,7 @@ from requests import Response
from version import ZULIP_VERSION
from zerver.actions.message_send import check_send_message
from zerver.lib.exceptions import JsonableError
from zerver.lib.exceptions import JsonableError, StreamDoesNotExistError
from zerver.lib.message import MessageDict
from zerver.lib.outgoing_http import OutgoingSession
from zerver.lib.queue import retry_event
@@ -232,7 +232,12 @@ def fail_with_message(event: Dict[str, Any], failure_message: str) -> None:
message_info = event["message"]
content = "Failure! " + failure_message
response_data = dict(content=content)
try:
send_response_message(bot_id=bot_id, message_info=message_info, response_data=response_data)
except StreamDoesNotExistError:
# If the stream has vanished while we were failing, there's no
# reasonable place to report the error.
pass
def get_message_url(event: Dict[str, Any]) -> str:

View File

@@ -7,11 +7,13 @@ import responses
from version import ZULIP_VERSION
from zerver.actions.create_user import do_create_user
from zerver.actions.streams import do_deactivate_stream
from zerver.lib.exceptions import JsonableError
from zerver.lib.outgoing_webhook import (
GenericOutgoingWebhookService,
SlackOutgoingWebhookService,
do_rest_call,
fail_with_message,
)
from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.topic import TOPIC_NAME
@@ -609,6 +611,40 @@ class TestOutgoingWebhookMessaging(ZulipTestCase):
display_recipient = get_display_recipient(stream_message.recipient)
self.assertEqual(display_recipient, "Denmark")
@responses.activate
def test_stream_message_failure_deactivated_to_outgoing_webhook_bot(self) -> None:
bot_owner = self.example_user("othello")
bot = self.create_outgoing_bot(bot_owner)
def wrapped(event: Dict[str, Any], failure_message: str) -> None:
do_deactivate_stream(get_stream("Denmark", get_realm("zulip")), acting_user=None)
fail_with_message(event, failure_message)
responses.add(
responses.POST,
"https://bot.example.com/",
body=requests.exceptions.Timeout("Time is up!"),
)
with mock.patch(
"zerver.lib.outgoing_webhook.fail_with_message", side_effect=wrapped
) as fail:
with self.assertLogs(level="INFO") as logs:
self.send_stream_message(
bot_owner, "Denmark", content=f"@**{bot.full_name}** foo", topic_name="bar"
)
self.assert_length(logs.output, 5)
fail.assert_called_once()
last_message = self.get_last_message()
self.assertIn("Request timed out after 10 seconds", last_message.content)
prev_message = self.get_second_to_last_message()
self.assertIn(
"tried to send a message to stream #**Denmark**, but that stream does not exist",
prev_message.content,
)
@responses.activate
def test_empty_string_json_as_response_to_outgoing_webhook_request(self) -> None:
"""