Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

error-bot: Remove ERROR_BOT support. 

This isn't sufficiently useful to keep the added complexity.  Users
should use the email error reporting, or set up Sentry error
reporting.
This commit is contained in:
Alex Vandiver 2023-04-11 17:51:14 +00:00 committed by Tim Abbott
parent db07b8668f
commit d888bb3df2
11 changed files with 25 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
zerver/lib/email_mirror.py
View File

@ -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:

43
zerver/lib/error_notify.py
View File

@ -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)

2
zerver/lib/test_classes.py
View File

@ -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"

8
zerver/openapi/python_examples.py
View File

@ -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:

46
zerver/tests/test_email_mirror.py
View File

@ -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 <Address to stream id: {stream.id}>\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: {} <Address to stream id: {}>\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)

4
zerver/tests/test_realm.py
View File

@ -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)

4
zerver/tests/test_subs.py
View File

@ -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])

4
zerver/tests/test_user_groups.py
View File

@ -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")

1
zilencer/management/commands/populate_db.py
View File

@ -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"]):

3
zproject/default_settings.py
View File

@ -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

1
zproject/dev_settings.py
View File

@ -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"