diff --git a/swh/vault/backend.py b/swh/vault/backend.py --- a/swh/vault/backend.py +++ b/swh/vault/backend.py @@ -5,11 +5,13 @@ import collections from email.mime.text import MIMEText +import logging import smtplib from typing import Any, Dict, List, Optional, Tuple import psycopg2.extras import psycopg2.pool +import sentry_sdk from swh.core.db import BaseDb from swh.core.db.common import db_transaction @@ -21,6 +23,7 @@ from swh.vault.cookers import COOKER_TYPES, get_cooker_cls from swh.vault.exc import NotFoundExc +logger = logging.getLogger(__name__) cooking_task_name = "swh.vault.cooking_tasks.SWHCookingTask" NOTIF_EMAIL_FROM = '"Software Heritage Vault" ' "" @@ -75,7 +78,6 @@ self.cache = VaultCache(**config["cache"]) self.scheduler = get_scheduler(**config["scheduler"]) self.storage = get_storage(**config["storage"]) - self.smtp_server = smtplib.SMTP(**config.get("smtp", {})) if "db" not in self.config: raise ValueError( @@ -487,16 +489,29 @@ ) def _smtp_send(self, msg: MIMEText): - # Reconnect if needed + smtp_server = smtplib.SMTP(**self.config.get("smtp", {})) try: - status = self.smtp_server.noop()[0] + status = smtp_server.noop()[0] except smtplib.SMTPException: status = -1 if status != 250: - self.smtp_server.connect("localhost", 25) - - # Send the message - self.smtp_server.send_message(msg) + error_message = ( + f"Unable to send SMTP message '{msg['Subject']}' to " + f"{msg['To']}: cannot connect to server" + ) + logger.error(error_message) + sentry_sdk.capture_message(error_message, "error") + else: + try: + # Send the message + smtp_server.send_message(msg) + except smtplib.SMTPException as exc: + logger.exception(exc) + error_message = ( + f"Unable to send SMTP message '{msg['Subject']}' to " + f"{msg['To']}: {exc}" + ) + sentry_sdk.capture_message(error_message, "error") @db_transaction() def _cache_expire(self, cond, *args, db=None, cur=None) -> None: diff --git a/swh/vault/tests/test_backend.py b/swh/vault/tests/test_backend.py --- a/swh/vault/tests/test_backend.py +++ b/swh/vault/tests/test_backend.py @@ -5,12 +5,15 @@ import contextlib import datetime +import re +import smtplib from unittest.mock import MagicMock, patch import attr import psycopg2 import pytest +from swh.core.sentry import init_sentry from swh.model.model import Content from swh.model.swhids import CoreSWHID from swh.vault.exc import NotFoundExc @@ -212,10 +215,10 @@ swh_vault.set_status(TEST_TYPE, TEST_SWHID, "done") - with patch.object(swh_vault, "smtp_server") as m: + with patch.object(swh_vault, "_smtp_send") as m: swh_vault.send_notif(TEST_TYPE, TEST_SWHID) - sent_emails = {k[0][0] for k in m.send_message.call_args_list} + sent_emails = {k[0][0] for k in m.call_args_list} assert {k["To"] for k in sent_emails} == set(emails) for e in sent_emails: @@ -234,6 +237,64 @@ m.assert_not_called() +def test_send_email_error_no_smtp(swh_vault): + reports = [] + init_sentry("http://example.org", extra_kwargs={"transport": reports.append}) + + emails = ("a@example.com", "billg@example.com", "test+42@example.org") + with mock_cooking(swh_vault): + for email in emails: + swh_vault.cook(TEST_TYPE, TEST_SWHID, email=email) + swh_vault.set_status(TEST_TYPE, TEST_SWHID, "done") + swh_vault.send_notif(TEST_TYPE, TEST_SWHID) + + assert len(reports) == 6 + for i, email in enumerate(emails): + # first report is the logger.error + assert reports[2 * i]["level"] == "error" + assert reports[2 * i]["logger"] == "swh.vault.backend" + reg = re.compile( + "Unable to send SMTP message 'Bundle ready: gitfast [0-9a-f]{7}' " + f"to {email.replace('+', '[+]')}: cannot connect to server" + ) + assert reg.match(reports[2 * i]["logentry"]["message"]) + # second is the sentry_sdk.capture_message + assert reports[2 * i + 1]["level"] == "error" + assert reg.match(reports[2 * i + 1]["message"]) + + +def test_send_email_error_send_failed(swh_vault): + reports = [] + init_sentry("http://example.org", extra_kwargs={"transport": reports.append}) + + emails = ("a@example.com", "billg@example.com", "test+42@example.org") + with mock_cooking(swh_vault): + for email in emails: + swh_vault.cook(TEST_TYPE, TEST_SWHID, email=email) + swh_vault.set_status(TEST_TYPE, TEST_SWHID, "done") + + with patch("smtplib.SMTP") as MockSMTP: + smtp = MockSMTP.return_value + smtp.noop.return_value = [250] + smtp.send_message.side_effect = smtplib.SMTPHeloError(404, "HELO Failed") + + swh_vault.send_notif(TEST_TYPE, TEST_SWHID) + + assert len(reports) == 4 + # first one is the captured exception + assert reports[0]["level"] == "error" + assert reports[0]["exception"]["values"][0]["type"] == "SMTPHeloError" + + # the following 3 ones are the sentry_sdk.capture_message() calls + for i, email in enumerate(emails, start=1): + assert reports[i]["level"] == "error" + reg = re.compile( + "Unable to send SMTP message 'Bundle ready: gitfast [0-9a-f]{7}' " + f"to {email.replace('+', '[+]')}: [(]404, 'HELO Failed'[)]" + ) + assert reg.match(reports[i]["message"]) + + def test_available(swh_vault): assert not swh_vault.is_available(TEST_TYPE, TEST_SWHID) @@ -327,10 +388,10 @@ swh_vault.set_status(TEST_TYPE, TEST_SWHID, "failed") swh_vault.set_progress(TEST_TYPE, TEST_SWHID, "test error") - with patch.object(swh_vault, "smtp_server") as m: + with patch.object(swh_vault, "_smtp_send") as m: swh_vault.send_notif(TEST_TYPE, TEST_SWHID) - e = [k[0][0] for k in m.send_message.call_args_list][0] + e = [k[0][0] for k in m.call_args_list][0] assert e["To"] == "a@example.com" assert "bot@softwareheritage.org" in e["From"]