From a1e71e8639095fc22d415baa67ec313bd3e02adf Mon Sep 17 00:00:00 2001 From: madrix01 Date: Wed, 9 Feb 2022 21:19:46 +0530 Subject: [PATCH] topic: Return JsonableError for race condition in topic mute. To avoid an uncaught IntegrityError causing a 500 HTTP response in a race between two processes trying to mute a topic, we catch the integrity error and raise the error exception with status 400 we'd have gotten if the second request had been a bit later. Fixes #21011. --- zerver/tests/test_muting_topics.py | 17 +++++++++++++++++ zerver/views/muting.py | 6 +++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/zerver/tests/test_muting_topics.py b/zerver/tests/test_muting_topics.py index 1727080b77..2676fff8e5 100644 --- a/zerver/tests/test_muting_topics.py +++ b/zerver/tests/test_muting_topics.py @@ -113,6 +113,23 @@ class MutedTopicsTests(ZulipTestCase): topic_name="Verona3", ) + # Verify the error handling for the database level + # IntegrityError we'll get with a race between two processes + # trying to mute the topic. To do this, we patch the + # topic_is_muted function to always return False when trying + # to mute a topic that is already muted. + add_topic_mute( + user_profile=user, + stream_id=stream.id, + recipient_id=stream.recipient.id, + topic_name="Verona3", + date_muted=datetime(2020, 1, 1, tzinfo=timezone.utc), + ) + + with mock.patch("zerver.views.muting.topic_is_muted", return_value=False): + result = self.api_patch(user, url, data) + self.assert_json_error(result, "Topic already muted") + def test_remove_muted_topic(self) -> None: user = self.example_user("hamlet") realm = user.realm diff --git a/zerver/views/muting.py b/zerver/views/muting.py index 0dfa4db272..8520c643d4 100644 --- a/zerver/views/muting.py +++ b/zerver/views/muting.py @@ -1,6 +1,7 @@ import datetime from typing import Optional +from django.db import IntegrityError from django.http import HttpRequest, HttpResponse from django.utils.timezone import now as timezone_now from django.utils.translation import gettext as _ @@ -39,7 +40,10 @@ def mute_topic( if topic_is_muted(user_profile, stream.id, topic_name): raise JsonableError(_("Topic already muted")) - do_mute_topic(user_profile, stream, topic_name, date_muted) + try: + do_mute_topic(user_profile, stream, topic_name, date_muted) + except IntegrityError: + raise JsonableError(_("Topic already muted")) def unmute_topic(