diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py index 840c64c206..9778088ddd 100644 --- a/zerver/lib/email_mirror.py +++ b/zerver/lib/email_mirror.py @@ -36,7 +36,6 @@ from zerver.models import ( UserProfile, get_client, get_display_recipient, - get_realm, get_stream_by_id_in_realm, get_system_bot, get_user, @@ -71,21 +70,7 @@ def redact_email_address(error_message: str) -> str: return re.sub(rf"\b(\S*?)(@{re.escape(domain)})", redact, error_message) -def report_to_zulip(error_message: str) -> None: - if settings.ERROR_BOT is None: - return - error_bot_realm = get_realm(settings.STAFF_SUBDOMAIN) - error_bot = get_system_bot(settings.ERROR_BOT, error_bot_realm.id) - error_stream = Stream.objects.get(name="errors", realm=error_bot_realm) - send_zulip( - error_bot, - error_stream, - "email mirror error", - f"""~~~\n{error_message}\n~~~""", - ) - - -def log_and_report(email_message: EmailMessage, error_message: str, to: Optional[str]) -> None: +def log_error(email_message: EmailMessage, error_message: str, to: Optional[str]) -> None: recipient = to or "No recipient found" error_message = "Sender: {}\nTo: {}\n{}".format( email_message.get("From"), recipient, error_message @@ -93,7 +78,6 @@ def log_and_report(email_message: EmailMessage, error_message: str, to: Optional error_message = redact_email_address(error_message) logger.error(error_message) - report_to_zulip(error_message) # Temporary missed message addresses @@ -503,7 +487,7 @@ def process_message(message: EmailMessage, rcpt_to: Optional[str] = None) -> Non # TODO: notify sender of error, retry if appropriate. logger.info(e.args[0]) except ZulipEmailForwardError as e: - log_and_report(message, e.args[0], to) + log_error(message, e.args[0], to) def validate_to_address(rcpt_to: str) -> None: diff --git a/zerver/lib/error_notify.py b/zerver/lib/error_notify.py index 2f1f572bdf..4ef8bcbaea 100644 --- a/zerver/lib/error_notify.py +++ b/zerver/lib/error_notify.py @@ -1,16 +1,15 @@ # System documented in https://zulip.readthedocs.io/en/latest/subsystems/logging.html from collections import defaultdict -from typing import Any, Dict, Literal +from typing import Any, Dict -from django.conf import settings from django.core.mail import mail_admins -from zerver.actions.message_send import internal_send_stream_message from zerver.filters import clean_data_from_query_parameters -from zerver.models import get_realm, get_stream, get_system_bot -def send_error(channel: Literal["email", "zulip"], report: Dict[str, Any]) -> None: +def do_report_error(report: Dict[str, Any]) -> None: + report = defaultdict(lambda: None, report) + topic = "{node}: {message}".format(**report).replace("\n", "\\n").replace("\r", "\\r") logger_str = "Logger {logger_name}, from module {log_module} line {log_lineno}:".format( @@ -45,8 +44,7 @@ Request info: else: request_repr = "Request info: none\n" - if channel == "email": - message = f"""\ + message = f"""\ {logger_str} Error generated by {user_info} @@ -56,33 +54,4 @@ Error generated by {user_info} {request_repr}""" - mail_admins(topic, message, fail_silently=True) - else: - assert settings.ERROR_BOT is not None - message = f"""{logger_str} -Error generated by {user_info} - -~~~~ pytb -{report['stack_trace']} - -~~~~ -{deployment} -{request_repr}""" - - error_bot_realm = get_realm(settings.STAFF_SUBDOMAIN) - error_bot = get_system_bot(settings.ERROR_BOT, error_bot_realm.id) - errors_stream = get_stream("errors", error_bot_realm) - - internal_send_stream_message( - error_bot, - errors_stream, - topic, - message, - ) - - -def do_report_error(report: Dict[str, Any]) -> None: - report = defaultdict(lambda: None, report) - send_error("email", report) - if settings.ERROR_BOT: - send_error("zulip", report) + mail_admins(topic, message, fail_silently=True) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 1ca9684b3b..a95e7be68b 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1455,7 +1455,7 @@ Output: This raises a failure inside of the try/except block of markdown.__init__.do_convert. """ - with self.settings(ERROR_BOT=None), mock.patch( + with mock.patch( "zerver.lib.markdown.timeout", side_effect=subprocess.CalledProcessError(1, []) ), self.assertLogs( level="ERROR" diff --git a/zerver/openapi/python_examples.py b/zerver/openapi/python_examples.py index 90c62388fc..03f2b8453d 100644 --- a/zerver/openapi/python_examples.py +++ b/zerver/openapi/python_examples.py @@ -87,11 +87,11 @@ def add_subscriptions(client: Client) -> None: validate_against_openapi_schema(result, "/users/me/subscriptions", "post", "200") - ensure_users([26], ["newbie"]) + ensure_users([25], ["newbie"]) # {code_example|start} # To subscribe other users to a stream, you may pass # the `principals` argument, like so: - user_id = 26 + user_id = 25 result = client.add_subscriptions( streams=[ {"name": "new stream", "description": "New stream for testing"}, @@ -633,13 +633,13 @@ def test_user_not_authorized_error(nonadmin_client: Client) -> None: @openapi_test_function("/streams/{stream_id}/members:get") def get_subscribers(client: Client) -> None: - ensure_users([11, 26], ["iago", "newbie"]) + ensure_users([11, 25], ["iago", "newbie"]) # {code_example|start} # Get the subscribers to a stream result = client.get_subscribers(stream="new stream") # {code_example|end} - assert result["subscribers"] == [11, 26] + assert result["subscribers"] == [11, 25] def get_user_agent(client: Client) -> None: diff --git a/zerver/tests/test_email_mirror.py b/zerver/tests/test_email_mirror.py index 7e96ded832..a49c8fa1b5 100644 --- a/zerver/tests/test_email_mirror.py +++ b/zerver/tests/test_email_mirror.py @@ -20,7 +20,7 @@ from zerver.lib.email_mirror import ( get_missed_message_token_from_address, is_forwarded, is_missed_message_address, - log_and_report, + log_error, process_message, process_missed_message, redact_email_address, @@ -1688,52 +1688,37 @@ class TestEmailMirrorProcessMessageNoValidRecipient(ZulipTestCase): incoming_valid_message["To"] = "address@wrongdomain, address@notzulip" incoming_valid_message["Reply-to"] = self.example_email("othello") - with mock.patch("zerver.lib.email_mirror.log_and_report") as mock_log_and_report: + with mock.patch("zerver.lib.email_mirror.log_error") as mock_log_error: process_message(incoming_valid_message) - mock_log_and_report.assert_called_with( + mock_log_error.assert_called_with( incoming_valid_message, "Missing recipient in mirror email", None ) class TestEmailMirrorLogAndReport(ZulipTestCase): - def test_log_and_report(self) -> None: + def test_log_error(self) -> None: user_profile = self.example_user("hamlet") self.login_user(user_profile) self.subscribe(user_profile, "errors") stream = get_stream("Denmark", user_profile.realm) stream_to_address = encode_email_address(stream) - address = Address(addr_spec=stream_to_address) - scrubbed_address = Address( - username="X" * len(address.username), domain=address.domain - ).addr_spec - incoming_valid_message = EmailMessage() incoming_valid_message.set_content("Test body") incoming_valid_message["Subject"] = "Test subject" incoming_valid_message["From"] = self.example_email("hamlet") incoming_valid_message["To"] = stream_to_address with self.assertLogs("zerver.lib.email_mirror", "ERROR") as error_log: - log_and_report(incoming_valid_message, "test error message", stream_to_address) + log_error(incoming_valid_message, "test error message", stream_to_address) self.assertEqual( error_log.output, [ f"ERROR:zerver.lib.email_mirror:Sender: hamlet@zulip.com\nTo: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX@testserver
\ntest error message" ], ) - message = most_recent_message(user_profile) - - self.assertEqual("email mirror error", message.topic_name()) - - msg_content = message.content.strip("~").strip() - expected_content = "Sender: {}\nTo: {}
\ntest error message" - expected_content = expected_content.format( - self.example_email("hamlet"), scrubbed_address, stream.id - ) - self.assertEqual(msg_content, expected_content) with self.assertLogs("zerver.lib.email_mirror", "ERROR") as error_log: - log_and_report(incoming_valid_message, "test error message", None) + log_error(incoming_valid_message, "test error message", None) self.assertEqual( error_log.output, [ @@ -1741,25 +1726,6 @@ class TestEmailMirrorLogAndReport(ZulipTestCase): ], ) - message = most_recent_message(user_profile) - self.assertEqual("email mirror error", message.topic_name()) - msg_content = message.content.strip("~").strip() - expected_content = "Sender: {}\nTo: No recipient found\ntest error message" - expected_content = expected_content.format(self.example_email("hamlet")) - self.assertEqual(msg_content, expected_content) - - def test_log_and_report_no_errorbot(self) -> None: - with self.settings(ERROR_BOT=None): - incoming_valid_message = EmailMessage() - incoming_valid_message.set_content("Test body") - incoming_valid_message["Subject"] = "Test subject" - incoming_valid_message["From"] = self.example_email("hamlet") - with self.assertLogs(logger_name, level="ERROR") as m: - log_and_report(incoming_valid_message, "test error message", None) - expected_content = "Sender: {}\nTo: No recipient found\ntest error message" - expected_content = expected_content.format(self.example_email("hamlet")) - self.assertEqual(m.output, [f"ERROR:{logger_name}:{expected_content}"]) - def test_redact_email_address(self) -> None: user_profile = self.example_user("hamlet") self.login_user(user_profile) diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index c34a02f787..b06b1b2901 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -997,9 +997,9 @@ class RealmTest(ZulipTestCase): realm=realm, name=UserGroup.FULL_MEMBERS_GROUP_NAME, is_system_group=True ) - self.assert_length(UserGroupMembership.objects.filter(user_group=members_system_group), 10) + self.assert_length(UserGroupMembership.objects.filter(user_group=members_system_group), 9) self.assert_length( - UserGroupMembership.objects.filter(user_group=full_members_system_group), 10 + UserGroupMembership.objects.filter(user_group=full_members_system_group), 9 ) self.assertEqual(realm.waiting_period_threshold, 0) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 5a2d19f594..d995c4e774 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -4431,7 +4431,7 @@ class SubscriptionAPITest(ZulipTestCase): ) self.assertNotIn(self.example_user("polonius").id, add_peer_event["users"]) - self.assert_length(add_peer_event["users"], 12) + self.assert_length(add_peer_event["users"], 11) self.assertEqual(add_peer_event["event"]["type"], "subscription") self.assertEqual(add_peer_event["event"]["op"], "peer_add") self.assertEqual(add_peer_event["event"]["user_ids"], [self.user_profile.id]) @@ -4462,7 +4462,7 @@ class SubscriptionAPITest(ZulipTestCase): # We don't send a peer_add event to othello self.assertNotIn(user_profile.id, add_peer_event["users"]) self.assertNotIn(self.example_user("polonius").id, add_peer_event["users"]) - self.assert_length(add_peer_event["users"], 12) + self.assert_length(add_peer_event["users"], 11) self.assertEqual(add_peer_event["event"]["type"], "subscription") self.assertEqual(add_peer_event["event"]["op"], "peer_add") self.assertEqual(add_peer_event["event"]["user_ids"], [user_profile.id]) diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index b4f917f241..323b04ef1d 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -333,11 +333,11 @@ class UserGroupAPITestCase(UserGroupTestCase): user_group = UserGroup.objects.get(name="support") # Test success self.assertEqual(UserGroup.objects.filter(realm=hamlet.realm).count(), 10) - self.assertEqual(UserGroupMembership.objects.count(), 47) + self.assertEqual(UserGroupMembership.objects.count(), 45) result = self.client_delete(f"/json/user_groups/{user_group.id}") self.assert_json_success(result) self.assertEqual(UserGroup.objects.filter(realm=hamlet.realm).count(), 9) - self.assertEqual(UserGroupMembership.objects.count(), 46) + self.assertEqual(UserGroupMembership.objects.count(), 44) # Test when invalid user group is supplied result = self.client_delete("/json/user_groups/1111") self.assert_json_error(result, "Invalid user group") diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py index cb40aa22cd..82d8bddcbf 100644 --- a/zilencer/management/commands/populate_db.py +++ b/zilencer/management/commands/populate_db.py @@ -543,7 +543,6 @@ class Command(BaseCommand): # These bots are directly referenced from code and thus # are needed for the test suite. zulip_realm_bots = [ - ("Zulip Error Bot", "error-bot@zulip.com"), ("Zulip Default Bot", "default-bot@zulip.com"), ] for i in range(options["extra_bots"]): diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 3318a536aa..6c70579a4b 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -321,9 +321,6 @@ REMINDER_BOT = "reminder-bot@zulip.com" # The following bots are optional system bots not enabled by # default. The default ones are defined in INTERNAL_BOTS, in settings.py. -# ERROR_BOT sends Django exceptions to an "errors" stream in the -# system realm. -ERROR_BOT: Optional[str] = None # These are extra bot users for our end-to-end Nagios message # sending tests. NAGIOS_STAGING_SEND_BOT = "nagios-staging-send-bot@zulip.com" if PRODUCTION else None diff --git a/zproject/dev_settings.py b/zproject/dev_settings.py index afe34456ec..b45085c30b 100644 --- a/zproject/dev_settings.py +++ b/zproject/dev_settings.py @@ -64,7 +64,6 @@ AUTHENTICATION_BACKENDS: Tuple[str, ...] = ( EXTERNAL_URI_SCHEME = "http://" EMAIL_GATEWAY_PATTERN = "%s@" + EXTERNAL_HOST_WITHOUT_PORT NOTIFICATION_BOT = "notification-bot@zulip.com" -ERROR_BOT = "error-bot@zulip.com" EMAIL_GATEWAY_BOT = "emailgateway@zulip.com" PHYSICAL_ADDRESS = "Zulip Headquarters, 123 Octo Stream, South Pacific Ocean" STAFF_SUBDOMAIN = "zulip"