fix(email): mark missing-config as non-retryable + reject blank env vars (#40715)
Fold in the #40715 blank-env OOM fix on top of the host-resolution change: - connect() now sets a non-retryable fatal error when required settings are missing, so the gateway stops reconnecting against an empty host instead of looping forever and leaking memory until the host OOM-kills. - check_email_requirements() treats blank/whitespace-only EMAIL_* values as missing, so an abandoned setup with empty keys no longer enables the platform. Credits the parallel fixes by zerone0x (#40745) and liuhao1024 (#40829).
This commit is contained in:
parent
e921c4f826
commit
f79e0a7060
2 changed files with 51 additions and 13 deletions
|
|
@ -159,14 +159,16 @@ def _is_automated_sender(address: str, headers: dict) -> bool:
|
|||
return False
|
||||
|
||||
def check_email_requirements() -> bool:
|
||||
"""Check if email platform dependencies are available."""
|
||||
addr = os.getenv("EMAIL_ADDRESS")
|
||||
pwd = os.getenv("EMAIL_PASSWORD")
|
||||
imap = os.getenv("EMAIL_IMAP_HOST")
|
||||
smtp = os.getenv("EMAIL_SMTP_HOST")
|
||||
if not all([addr, pwd, imap, smtp]):
|
||||
return False
|
||||
return True
|
||||
"""Check if email platform settings are available and non-blank.
|
||||
|
||||
Treats blank/whitespace-only values as missing so an abandoned setup that
|
||||
left empty ``EMAIL_*`` keys in ``.env`` does not enable the platform (#40715).
|
||||
"""
|
||||
addr = os.getenv("EMAIL_ADDRESS", "").strip()
|
||||
pwd = os.getenv("EMAIL_PASSWORD", "").strip()
|
||||
imap = os.getenv("EMAIL_IMAP_HOST", "").strip()
|
||||
smtp = os.getenv("EMAIL_SMTP_HOST", "").strip()
|
||||
return all([addr, pwd, imap, smtp])
|
||||
|
||||
|
||||
def _decode_header_value(raw: str) -> str:
|
||||
|
|
@ -418,10 +420,19 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
if not value
|
||||
]
|
||||
if missing:
|
||||
logger.error(
|
||||
"[Email] Not configured — missing %s. Set it via `hermes gateway "
|
||||
"setup` (env) or platforms.email in config.yaml.",
|
||||
", ".join(missing),
|
||||
message = (
|
||||
"Not configured — missing "
|
||||
+ ", ".join(missing)
|
||||
+ ". Set it via `hermes gateway setup` (env) or platforms.email "
|
||||
"in config.yaml."
|
||||
)
|
||||
logger.error("[Email] %s", message)
|
||||
# Mark non-retryable so the gateway does NOT keep reconnecting against
|
||||
# an empty host. A blank-but-present env var (e.g. ``EMAIL_IMAP_HOST=``)
|
||||
# used to slip past the startup gate and drive an indefinite retry
|
||||
# loop that leaked memory until the host OOM-killed (#40715).
|
||||
self._set_fatal_error(
|
||||
"email_missing_configuration", message, retryable=False
|
||||
)
|
||||
return False
|
||||
|
||||
|
|
|
|||
|
|
@ -1436,7 +1436,8 @@ class TestConnectionConfigResolution(unittest.TestCase):
|
|||
self.assertEqual(adapter._address, "hermes@test.com")
|
||||
|
||||
def test_connect_aborts_without_attempting_imap_when_host_missing(self):
|
||||
"""A missing host returns False without the cryptic DNS error."""
|
||||
"""A missing host returns False without the cryptic DNS error, and marks
|
||||
the failure non-retryable so the gateway stops reconnecting (#40715)."""
|
||||
import asyncio
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
|
|
@ -1453,6 +1454,32 @@ class TestConnectionConfigResolution(unittest.TestCase):
|
|||
|
||||
self.assertFalse(result)
|
||||
mock_imap.assert_not_called()
|
||||
# The OOM fix (#40715): a blank host must NOT leave the platform in the
|
||||
# retryable reconnect loop — it is a permanent config error.
|
||||
self.assertTrue(adapter.has_fatal_error)
|
||||
self.assertEqual(adapter.fatal_error_code, "email_missing_configuration")
|
||||
self.assertFalse(adapter.fatal_error_retryable)
|
||||
self.assertIn("EMAIL_IMAP_HOST", adapter.fatal_error_message or "")
|
||||
|
||||
def test_blank_present_env_vars_are_not_required(self):
|
||||
"""Blank/whitespace EMAIL_* values must read as missing (#40715) — an
|
||||
abandoned setup with empty keys must not enable the platform."""
|
||||
from plugins.platforms.email.adapter import check_email_requirements
|
||||
for blank in ("", " ", "\n"):
|
||||
with patch.dict(os.environ, {
|
||||
"EMAIL_ADDRESS": blank, "EMAIL_PASSWORD": blank,
|
||||
"EMAIL_IMAP_HOST": blank, "EMAIL_SMTP_HOST": blank,
|
||||
}, clear=False):
|
||||
self.assertFalse(check_email_requirements())
|
||||
|
||||
def test_all_settings_present_satisfies_requirements(self):
|
||||
"""The connected check passes only when all four settings are non-blank."""
|
||||
from plugins.platforms.email.adapter import check_email_requirements
|
||||
with patch.dict(os.environ, {
|
||||
"EMAIL_ADDRESS": "hermes@test.com", "EMAIL_PASSWORD": "secret",
|
||||
"EMAIL_IMAP_HOST": "imap.test.com", "EMAIL_SMTP_HOST": "smtp.test.com",
|
||||
}, clear=False):
|
||||
self.assertTrue(check_email_requirements())
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue