fix(mail): preserve send results when SMTP cleanup fails (#42903)

Co-authored-by: Zhangchenhao-111 <285911435+Zhangchenhao-111@users.noreply.github.com>

api/libs/smtp.py

@@ -60,4 +60,12 @@ class SMTPClient:
             raise
         finally:
             if smtp:
-                smtp.quit()
+                # Cleanup must not replace a send failure or turn an accepted send into a failure.
+                try:
+                    smtp.quit()
+                except Exception:
+                    logger.warning("Failed to quit SMTP connection", exc_info=True)
+                    try:
+                        smtp.close()
+                    except Exception:
+                        logger.warning("Failed to close SMTP connection", exc_info=True)

api/tests/unit_tests/libs/test_smtp_client.py

@@ -1,3 +1,4 @@
+import smtplib
 from typing import Any
 from unittest.mock import ANY, MagicMock, patch
 
@@ -95,3 +96,57 @@ def test_smtp_smtplib_exception_in_login(mock_smtp_cls: MagicMock):
     with pytest.raises(smtplib.SMTPException):
         client.send(_mail())
     mock_smtp.quit.assert_called_once()
+
+
+@pytest.mark.parametrize("use_tls", [False, True])
+@pytest.mark.parametrize(
+    "send_error_type",
+    [
+        None,
+        smtplib.SMTPDataError,
+        TimeoutError,
+        RuntimeError,
+    ],
+)
+@pytest.mark.parametrize("close_fails", [False, True])
+def test_smtp_cleanup_preserves_send_result(
+    use_tls: bool,
+    send_error_type: type[Exception] | None,
+    close_fails: bool,
+    caplog: pytest.LogCaptureFixture,
+):
+    send_error: Exception | None = None
+    if send_error_type is smtplib.SMTPDataError:
+        send_error = smtplib.SMTPDataError(451, b"temporary send failure")
+    elif send_error_type is not None:
+        send_error = send_error_type("send failed")
+
+    smtp_class = "libs.smtp.smtplib.SMTP_SSL" if use_tls else "libs.smtp.smtplib.SMTP"
+    with patch(smtp_class, autospec=True) as mock_smtp_cls:
+        mock_smtp = mock_smtp_cls.return_value
+        mock_smtp.sendmail.side_effect = send_error
+        mock_smtp.quit.side_effect = smtplib.SMTPServerDisconnected("connection lost during QUIT")
+        if close_fails:
+            mock_smtp.close.side_effect = OSError("close failed")
+        client = SMTPClient(
+            server="smtp.example.com",
+            port=465 if use_tls else 25,
+            username="",
+            password="",
+            _from="noreply@example.com",
+            use_tls=use_tls,
+        )
+
+        if send_error is None:
+            assert client.send(_mail()) is None
+        else:
+            with pytest.raises(type(send_error)) as exc_info:
+                client.send(_mail())
+            assert exc_info.value is send_error
+
+        mock_smtp.sendmail.assert_called_once()
+        mock_smtp.quit.assert_called_once()
+        mock_smtp.close.assert_called_once()
+        assert "Failed to quit SMTP connection" in caplog.text
+        if close_fails:
+            assert "Failed to close SMTP connection" in caplog.text