diff --git a/swh/vault/backend.py b/swh/vault/backend.py
index b7d9bf3bcd1a6edf654be2f90e295ada1517cf50..bc51160fcb3ada9e7e1960e247d21e71dfb339e2 100644
--- 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.cache import VaultCache
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" ' "<bot@softwareheritage.org>"
@@ -75,7 +78,6 @@ class VaultBackend:
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 @@ class VaultBackend:
)
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
index 98c502562860b0916212e8724b4529e3c5f407f9..b06b435f3f532e5f98355ce25d2757a856ed4237 100644
--- 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 @@ def test_send_all_emails(swh_vault):
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 @@ def test_send_all_emails(swh_vault):
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 @@ def test_send_failure_email(swh_vault):
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"]