diff --git a/CHANGELOG.md b/CHANGELOG.md index 62b1593..0a8926f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ What counts as a breaking change — and what does not — is defined in [API st - **`jasil.providers.MAIL_SEND_DEADLINE_SECONDS`** is public: the upper bound on one SMTP `send` (90 seconds, DNS lookups excepted). Import it to size a lease or timeout around a send; the value may change in a minor release. `jasil.container.SMTP_SEND_DEADLINE_SECONDS`, which was not covered, is removed. ([#57](https://github.com/endurain-project/jasil/issues/57)) +- **`MailError` says why a send failed and whether it may have been delivered.** `kind` is a new `jasil.providers.MailFailure` naming the failed step (`auth_failed`, `sender_refused`, `recipient_refused`, `message_rejected`, `unreachable`, `connection_lost`, `deadline`, ...). `delivery_unknown` is `True` only when the connection was lost, or the deadline fired, after the message body started going out. `message_id` is set on every failure after the message was built. All three are keyword-only with defaults, so existing code is unaffected. See [MailProvider](docs/providers-and-backends.md#mailprovider). ([#52](https://github.com/endurain-project/jasil/issues/52), [#53](https://github.com/endurain-project/jasil/issues/53)) + +### Changed + +- `SmtpMail` sends `MAIL FROM`, `RCPT TO` and `DATA` itself instead of through `smtplib`'s `send_message`, so it knows which step failed. The conversation on the wire is unchanged, including the `SIZE` declaration and success with a warning when only some recipients are refused. A refused `MAIL FROM` is now reported as a sender refusal, no longer as a rejected message. + ## [0.17.0] - 2026-11-04 This release fixes the first audit write on a fresh SQLite database failing with `database is locked`. Migration `rev0010` creates the `jasil_instance` row, and `ensure_instance_id` is now public. diff --git a/docs/api-stability.md b/docs/api-stability.md index 2723065..89465c5 100644 --- a/docs/api-stability.md +++ b/docs/api-stability.md @@ -45,8 +45,11 @@ modules: ### Provider protocols -`MailProvider.send` returns the `Message-ID` as a `str`, and `MailError` / -`MailNotConfiguredError` are covered. Adding a field to `MailMessage` with a +`MailProvider.send` returns the `Message-ID` as a `str`, and `MailError` (with +`status`, `permanent`, `kind`, `delivery_unknown` and `message_id`) / +`MailNotConfiguredError` are covered. `MailFailure` is covered like +`MailProblemCode`: members may be added in a minor release, and a member is never +renamed or reused. Which `kind` a given failure maps to is documented, not frozen. Adding a field to `MailMessage` with a default is additive. `RealtimeProvider`'s four methods and its contract are covered: best-effort, diff --git a/docs/durable-jobs.md b/docs/durable-jobs.md index f2b2a08..f404b62 100644 --- a/docs/durable-jobs.md +++ b/docs/durable-jobs.md @@ -559,7 +559,10 @@ failure. `MailError`, so a job on a deployment with no `mail.uri` uses its whole retry budget before dead-lettering. Catch it in the handler if that is not wanted. - **Delivery is at-least-once.** A retry after a lost acknowledgement sends the - message twice. + message twice. A `MailError` with `delivery_unknown` set lost its connection + after the body went out, so the message may already have been delivered; a + handler that must not send twice catches it, records `message_id`, and does + not retry. - **Size `jobs.max_runtime_seconds` for the whole handler, not one send.** One SMTP send can take up to `jasil.providers.MAIL_SEND_DEADLINE_SECONDS` (90 seconds), and the DNS lookups before it are not counted against that deadline. A handler that sends N messages can take N times diff --git a/docs/providers-and-backends.md b/docs/providers-and-backends.md index 8d1810c..7428e9f 100644 --- a/docs/providers-and-backends.md +++ b/docs/providers-and-backends.md @@ -532,6 +532,33 @@ CR/LF in a header value). `MemoryMail` validates through the same builder as `MailError` messages carry a failure category and SMTP status code only — never an address, subject, body or server text. +A `MailError` also tells a host what to do next without parsing its message: + +| Attribute | Meaning | +|---|---| +| `status` / `permanent` | The SMTP status code, and whether it is a 5xx. | +| `kind` | A `MailFailure` naming the step that failed (table below). | +| `delivery_unknown` | `True` when the connection was lost, or the send deadline fired, after the body started going out. The server may have accepted the message, so a retry may deliver it twice; reconcile with the relay's logs instead. `False` means the message was not accepted. | +| `message_id` | The `Message-ID` of the message that failed, set on every failure after the message was built. Look it up in the relay's or provider's logs. It is never part of `str(error)`. | + +| `MailFailure` | Raised when | Typical reaction | +|---|---|---| +| `auth_failed` | The server refused the credentials. | Fix the configuration; alert an operator. | +| `unsupported` | The server lacks STARTTLS, AUTH, or an AUTH mechanism `smtplib` implements. | Fix the configuration. | +| `sender_refused` | `MAIL FROM` was refused. | When `permanent`, stop using that sender; a 4xx is usually rate limiting. | +| `recipient_refused` | Every recipient was refused, or the server closed the session (421) during `RCPT`. | When `permanent`, record it against the recipient. | +| `message_rejected` | The server refused the message at `DATA` or at its end (content, size, policy). | When `permanent`, do not retry the same content. | +| `unreachable` | DNS, a refused or failed connect, a greeting other than 220, or a cleartext relay that no longer resolves to a private address. | Retry later. | +| `tls_failed` | The TLS handshake failed. | Check the relay's certificate and name. | +| `connection_lost` | The connection broke after the session was established. | Retry, unless `delivery_unknown`. | +| `deadline` | The send ran past `MAIL_SEND_DEADLINE_SECONDS`. | Retry, unless `delivery_unknown`. | +| `lease_lost` | The durable job lost its lease; nothing was sent. | None: another worker owns the job. | +| `settings_unavailable` | Runtime mail settings could not be loaded or verified. | Retry later. | +| `not_configured` | No backend is configured (`MailNotConfiguredError`). | Configure `mail.uri`. | +| `other` | Anything else, and a `MailError` raised without a kind. | Treat by `status`. | + +Members may be added in a minor release; a member is never renamed or reused. + One `SmtpMail` send is bounded by `MAIL_SEND_DEADLINE_SECONDS` (90 seconds; DNS lookups are not counted). Import it to size a lease or timeout around a send rather than copying the value, which may change in a minor release. diff --git a/jasil/__init__.py b/jasil/__init__.py index e3ccdec..daaf25e 100644 --- a/jasil/__init__.py +++ b/jasil/__init__.py @@ -28,6 +28,7 @@ EventBusProvider, LockProvider, MailError, + MailFailure, MailMessage, MailNotConfiguredError, MailProvider, @@ -70,6 +71,7 @@ "JobWorkerMode", "LockProvider", "MailError", + "MailFailure", "MailMessage", "MailNotConfiguredError", "MailProvider", diff --git a/jasil/_reloading.py b/jasil/_reloading.py index d6c6e84..b993567 100644 --- a/jasil/_reloading.py +++ b/jasil/_reloading.py @@ -21,6 +21,7 @@ GeocodedPlace, GeocodingProvider, MailError, + MailFailure, MailMessage, MailNotConfiguredError, MailProvider, @@ -181,9 +182,9 @@ def send(self, message: MailMessage) -> str: outcome = self._backend.resolve() except SourceUnavailableError: # Not chained: the source's exception is the host's, and its text may carry a secret. - raise MailError("Mail settings could not be loaded") from None + raise MailError("Mail settings could not be loaded", kind=MailFailure.SETTINGS_UNAVAILABLE) from None if outcome.transient: - raise MailError("Mail settings could not be verified; retry") + raise MailError("Mail settings could not be verified; retry", kind=MailFailure.SETTINGS_UNAVAILABLE) if outcome.backend is None: # The reason carries no setting's value (see ``ReloadingBackend``). raise MailNotConfiguredError(f"Mail settings are invalid: {outcome.rejection}") diff --git a/jasil/backends/mail_memory.py b/jasil/backends/mail_memory.py index 4c26b6f..14e40f5 100644 --- a/jasil/backends/mail_memory.py +++ b/jasil/backends/mail_memory.py @@ -12,7 +12,7 @@ from jasil._core.job_context import lease_lost from jasil._core.mail_message import build_mime -from jasil.providers import ClockProvider, MailError, MailMessage, MailNotConfiguredError +from jasil.providers import ClockProvider, MailError, MailFailure, MailMessage, MailNotConfiguredError # Used only when ``memory://`` is configured without a ``from_address``. _DEFAULT_SENDER = "jasil@localhost" @@ -67,7 +67,7 @@ def send(self, message: MailMessage) -> str: :class:`~jasil.backends.mail_smtp.SmtpMail` would send nothing. """ if lease_lost(): - raise MailError("Job lease lost; send abandoned") + raise MailError("Job lease lost; send abandoned", kind=MailFailure.LEASE_LOST) mime = build_mime(message, sender=self._from_address, now=self._clock.now()) # Recipients copied as sent: the caller's list may be reused or mutated # after ``send`` returns, which would rewrite what the test asserts on. A diff --git a/jasil/backends/mail_smtp.py b/jasil/backends/mail_smtp.py index 29deb93..bf7dbcb 100644 --- a/jasil/backends/mail_smtp.py +++ b/jasil/backends/mail_smtp.py @@ -17,17 +17,19 @@ import contextlib import functools import logging +import re import smtplib import socket import ssl import threading import time from email.message import EmailMessage +from typing import Literal import jasil._core.network as network from jasil._core.job_context import lease_lost from jasil._core.mail_message import build_mime, parse_address -from jasil.providers import MAIL_SEND_DEADLINE_SECONDS, ClockProvider, MailError, MailMessage +from jasil.providers import MAIL_SEND_DEADLINE_SECONDS, ClockProvider, MailError, MailFailure, MailMessage logger = logging.getLogger(__name__) @@ -46,6 +48,70 @@ _WATCH_INTERVAL_SECONDS = 0.1 _DEADLINE_EXCEEDED = "SMTP send exceeded its overall deadline" +# How far a send got: ``connecting`` until the session is up (greeting, and TLS +# when it is used), ``session`` from then on, ``data`` once the server answered +# ``DATA`` with 354 and the body may be on its way. A connection lost in ``data`` +# may have delivered the message. +_Stage = Literal["connecting", "session", "data"] + + +def _failure(error: OSError, stage: _Stage) -> MailError: + """Map an ``smtplib`` or socket failure to a :class:`MailError` that carries no server text. + + Only a lost connection is ambiguous: a reply, even to ``DATA``, says the + server did not accept the message. + """ + if isinstance(error, smtplib.SMTPAuthenticationError): + return MailError( + f"SMTP authentication failed (status {error.smtp_code})", + status=error.smtp_code, + kind=MailFailure.AUTH_FAILED, + ) + if isinstance(error, smtplib.SMTPRecipientsRefused): + # Codes only: 4xx (greylisting, retry) and 5xx (unknown user) differ. + codes = sorted({code for code, _ in error.recipients.values()}) + # ``min``: one 4xx among 5xx is worth a retry, so it is not permanent. + return MailError( + "SMTP server refused the recipients (status " + ", ".join(map(str, codes)) + ")", + status=codes[0] if codes else None, + kind=MailFailure.RECIPIENT_REFUSED, + ) + if isinstance(error, smtplib.SMTPSenderRefused): + return MailError( + f"SMTP server refused the sender (status {error.smtp_code})", + status=error.smtp_code, + kind=MailFailure.SENDER_REFUSED, + ) + if isinstance(error, smtplib.SMTPConnectError): + return MailError( + f"SMTP server refused the connection (status {error.smtp_code})", + status=error.smtp_code, + kind=MailFailure.UNREACHABLE, + ) + if isinstance(error, smtplib.SMTPDataError): + return MailError( + f"SMTP server rejected the message (status {error.smtp_code})", + status=error.smtp_code, + kind=MailFailure.MESSAGE_REJECTED, + ) + if isinstance(error, smtplib.SMTPResponseException): + return MailError(f"SMTP server refused a command (status {error.smtp_code})", status=error.smtp_code) + if isinstance(error, smtplib.SMTPNotSupportedError): + return MailError("SMTP server lacks a required feature", kind=MailFailure.UNSUPPORTED) + if stage == "connecting": + if isinstance(error, ssl.SSLError): + return MailError("TLS negotiation with the SMTP server failed", kind=MailFailure.TLS_FAILED) + return MailError("SMTP server unreachable", kind=MailFailure.UNREACHABLE) + return MailError("SMTP connection lost", kind=MailFailure.CONNECTION_LOST, delivery_unknown=stage == "data") + + +def _data_payload(body: bytes) -> bytes: + """``body`` as ``smtplib.SMTP.data`` sends it: leading dots doubled, ended by ``CRLF.CRLF``.""" + payload = re.sub(rb"(?m)^\.", b"..", body) + if not payload.endswith(b"\r\n"): + payload += b"\r\n" + return payload + b".\r\n" + def _default_local_hostname() -> str: """The EHLO name when none is configured: this host's name, if usable. @@ -182,43 +248,24 @@ def send(self, message: MailMessage) -> str: ValueError: When the message is invalid. MailError: When the server could not be reached or refused the message, or when the durable job sending it has lost its lease - (nothing is sent: another worker owns the job now). If only - *some* recipients are refused the send succeeds and a warning - records the count, since failing would make a retry re-send to - recipients that already got it. + (nothing is sent: another worker owns the job now). Its ``kind`` + names the step that failed, ``delivery_unknown`` is set when the + connection broke after the body started going out, and + ``message_id`` is set on every failure after the message was + built. If only *some* recipients are refused the send succeeds + and a warning records the count, since failing would make a + retry re-send to recipients that already got it. """ if lease_lost(): - raise MailError("Job lease lost; send abandoned") + raise MailError("Job lease lost; send abandoned", kind=MailFailure.LEASE_LOST) mime = build_mime(message, sender=self._from_address, now=self._clock.now()) message_id = str(mime["Message-ID"]) try: refused = self._deliver(mime) - except MailError: + except MailError as error: + # Every failure of a built message passes here, whichever step raised it. + error.message_id = message_id raise - except smtplib.SMTPAuthenticationError as error: - raise MailError(f"SMTP authentication failed (status {error.smtp_code})", status=error.smtp_code) from None - except smtplib.SMTPRecipientsRefused as error: - # Codes only: 4xx (greylisting, retry) and 5xx (unknown user) differ. - codes = sorted({code for code, _ in error.recipients.values()}) - # ``min``: one 4xx among 5xx is worth a retry, so it is not permanent. - raise MailError( - "SMTP server refused every recipient (status " + ", ".join(map(str, codes)) + ")", - status=codes[0] if codes else None, - ) from None - except smtplib.SMTPConnectError as error: - raise MailError( - f"SMTP server refused the connection (status {error.smtp_code})", status=error.smtp_code - ) from None - except smtplib.SMTPResponseException as error: - raise MailError( - f"SMTP server rejected the message (status {error.smtp_code})", status=error.smtp_code - ) from None - except smtplib.SMTPNotSupportedError: - raise MailError("SMTP server lacks a required feature (AUTH or SMTPUTF8)") from None - except ssl.SSLError: - raise MailError("TLS negotiation with the SMTP server failed") from None - except (smtplib.SMTPException, OSError): - raise MailError("SMTP server unreachable or connection lost") from None if refused: logger.warning( "Mail %s accepted with %d of %d recipients refused", @@ -234,7 +281,7 @@ def _arm(connection: smtplib.SMTP, deadline: float) -> None: """Cap the next step to the time left, or abort when none is.""" remaining = deadline - time.monotonic() if remaining <= 0: - raise MailError(_DEADLINE_EXCEEDED) + raise MailError(_DEADLINE_EXCEEDED, kind=MailFailure.DEADLINE) sock = getattr(connection, "sock", None) if sock is not None: sock.settimeout(min(_TIMEOUT_SECONDS, remaining)) @@ -269,7 +316,9 @@ def _connect_hosts(self) -> list[str]: return [self._host] addresses, reason = network.resolve_private_addresses(self._host) if reason is not None or not addresses: - raise MailError("SMTP relay no longer resolves to a private address; cleartext refused") + raise MailError( + "SMTP relay no longer resolves to a private address; cleartext refused", kind=MailFailure.UNREACHABLE + ) return addresses def _connect(self, connection: smtplib.SMTP, deadline: float) -> tuple[int, bytes]: @@ -279,7 +328,7 @@ def _connect(self, connection: smtplib.SMTP, deadline: float) -> tuple[int, byte # A connect is not interruptible by the watchdog, so do not start one # after the deadline. if time.monotonic() >= deadline: - raise MailError(_DEADLINE_EXCEEDED) + raise MailError(_DEADLINE_EXCEEDED, kind=MailFailure.DEADLINE) # ``connection.timeout`` bounds each address tried; cap it to the time # left, and the dial itself stops at the deadline between addresses, # so a connect begun near the deadline cannot run past it. @@ -303,7 +352,11 @@ def _connect(self, connection: smtplib.SMTP, deadline: float) -> tuple[int, byte raise AssertionError("unreachable") # pragma: no cover def _deliver(self, mime: EmailMessage) -> dict[str, tuple[int, bytes]]: - """Connect, secure, authenticate and send; returns refused recipients.""" + """Connect, secure, authenticate and send; returns refused recipients. + + Raises: + MailError: On any failure, classified by the stage it interrupted. + """ context = self._context deadline = time.monotonic() + MAIL_SEND_DEADLINE_SECONDS # Built without a host so nothing is dialed before the watchdog is armed: @@ -320,6 +373,7 @@ def _deliver(self, mime: EmailMessage) -> dict[str, tuple[int, bytes]]: finished = threading.Event() watchdog = threading.Thread(target=self._watch, args=(connection, expired, finished), daemon=True) watchdog.start() + stage: _Stage = "connecting" try: try: code, reply = self._connect(connection, deadline) @@ -329,22 +383,37 @@ def _deliver(self, mime: EmailMessage) -> dict[str, tuple[int, bytes]]: self._arm(connection, deadline) connection.ehlo() if not connection.has_extn("starttls"): - raise MailError("SMTP server does not offer STARTTLS") + raise MailError("SMTP server does not offer STARTTLS", kind=MailFailure.UNSUPPORTED) self._arm(connection, deadline) connection.starttls(context=context) connection.ehlo() + stage = "session" if self._username and self._password: self._arm(connection, deadline) self._login(connection, self._username, self._password) self._arm(connection, deadline) - # Explicit envelope sender: the bare address, never the display name. - return connection.send_message(mime, from_addr=mime["From"].addresses[0].addr_spec) - except (smtplib.SMTPException, OSError): + body = mime.as_bytes() + refused = self._envelope(connection, mime, len(body)) + self._arm(connection, deadline) + # Not ``connection.data``: it waits for the 354 too, and a connection + # lost there cannot have delivered anything. + code, reply = connection.docmd("DATA") + if code != 354: + raise smtplib.SMTPDataError(code, reply) + stage = "data" + connection.send(_data_payload(body)) + code, reply = connection.getreply() + if code != 250: + raise smtplib.SMTPDataError(code, reply) + return refused + except (smtplib.SMTPException, OSError) as error: # The watchdog's shutdown surfaces as a dropped connection, and the # last step's socket timeout is the time left, so it can fire first. if expired.is_set() or time.monotonic() >= deadline: - raise MailError(_DEADLINE_EXCEEDED) from None - raise + raise MailError( + _DEADLINE_EXCEEDED, kind=MailFailure.DEADLINE, delivery_unknown=stage == "data" + ) from None + raise _failure(error, stage) from None finally: # Not ``with``: its exit raises on a non-221 reply to QUIT, which would # report a message the server already accepted as a failure (a retry @@ -360,6 +429,41 @@ def _deliver(self, mime: EmailMessage) -> dict[str, tuple[int, bytes]]: if watched is not None: watched.close() + @staticmethod + def _envelope(connection: smtplib.SMTP, mime: EmailMessage, size: int) -> dict[str, tuple[int, bytes]]: + """Send ``MAIL FROM`` and one ``RCPT TO`` per recipient, as ``smtplib.SMTP.sendmail`` does. + + Run step by step rather than through ``sendmail`` so the caller knows when + ``DATA`` begins. ``build_mime`` emits 7-bit content and ASCII addresses, so + neither ``SMTPUTF8`` nor ``8BITMIME`` is needed. + + Returns: + The refused recipients, when some but not all were refused. + + Raises: + smtplib.SMTPSenderRefused: When ``MAIL FROM`` is refused. + smtplib.SMTPRecipientsRefused: When every recipient is refused, or the + server closes the session (421) while recipients are offered. + """ + connection.ehlo_or_helo_if_needed() + options = [f"size={size}"] if connection.does_esmtp and connection.has_extn("size") else [] + # The envelope sender is the bare address, never the display name. + sender = mime["From"].addresses[0].addr_spec + code, reply = connection.mail(sender, options) + if code != 250: + raise smtplib.SMTPSenderRefused(code, reply, sender) + refused: dict[str, tuple[int, bytes]] = {} + recipients = [address.addr_spec for address in mime["To"].addresses] + for recipient in recipients: + code, reply = connection.rcpt(recipient) + if code not in (250, 251): + refused[recipient] = (code, reply) + if code == 421: + raise smtplib.SMTPRecipientsRefused(refused) + if len(refused) == len(recipients): + raise smtplib.SMTPRecipientsRefused(refused) + return refused + @staticmethod def _login(connection: smtplib.SMTP, username: str, password: str) -> None: """Authenticate, naming the one failure ``smtplib`` raises untyped.""" @@ -369,5 +473,7 @@ def _login(connection: smtplib.SMTP, username: str, password: str) -> None: # A bare ``SMTPException`` from ``login`` means the server advertises # AUTH but no mechanism ``smtplib`` implements. if type(error) is smtplib.SMTPException: - raise MailError("SMTP server offers no supported authentication mechanism") from None + raise MailError( + "SMTP server offers no supported authentication mechanism", kind=MailFailure.UNSUPPORTED + ) from None raise diff --git a/jasil/providers.py b/jasil/providers.py index da1d834..6c70f56 100644 --- a/jasil/providers.py +++ b/jasil/providers.py @@ -11,6 +11,7 @@ from contextlib import AbstractContextManager from dataclasses import dataclass from datetime import datetime +from enum import StrEnum from pathlib import Path from typing import BinaryIO, Protocol, runtime_checkable @@ -976,6 +977,42 @@ def __hash__(self) -> int: return hash((recipients, self.subject, self.text, self.html, self.reply_to)) +class MailFailure(StrEnum): + """Which step of a send failed, so a host can react without parsing the message. + + Members may be added in a minor release; a member is never renamed or reused. + """ + + #: Not classified: the default for a :class:`MailError` raised without a kind. + OTHER = "other" + #: The server refused the credentials. + AUTH_FAILED = "auth_failed" + #: The server lacks something the send requires: STARTTLS, AUTH, or an AUTH mechanism ``smtplib`` implements. + UNSUPPORTED = "unsupported" + #: ``MAIL FROM`` was refused. Only a permanent (5xx) refusal means the sender itself is unusable; a 4xx is + #: usually rate limiting. + SENDER_REFUSED = "sender_refused" + #: Every recipient was refused, or the server closed the session (421) while recipients were offered. + RECIPIENT_REFUSED = "recipient_refused" + #: The server refused the message at ``DATA`` or at its end (content, size or policy). + MESSAGE_REJECTED = "message_rejected" + #: No session could be opened: DNS, a refused or failed connect, a greeting other than 220, or a cleartext relay + #: that no longer resolves to a private address. + UNREACHABLE = "unreachable" + #: The TLS handshake failed (certificate, hostname or protocol). + TLS_FAILED = "tls_failed" + #: The connection broke after the session was established. + CONNECTION_LOST = "connection_lost" + #: The send ran past `MAIL_SEND_DEADLINE_SECONDS`. + DEADLINE = "deadline" + #: The durable job sending the message lost its lease; nothing was sent. + LEASE_LOST = "lease_lost" + #: Runtime mail settings could not be loaded or verified; retry later. + SETTINGS_UNAVAILABLE = "settings_unavailable" + #: No mail backend is configured (:class:`MailNotConfiguredError`). + NOT_CONFIGURED = "not_configured" + + class MailError(RuntimeError): """A message could not be handed to the mail server. @@ -986,11 +1023,30 @@ class MailError(RuntimeError): Attributes: status: The SMTP status code, or ``None`` when the failure had none (unreachable server, TLS failure, deadline). + kind: The step that failed. + delivery_unknown: ``True`` when the connection was lost, or the deadline + fired, after the message body started going out: the server may have + accepted it, so a retry may deliver it twice. ``False`` means the + message was not accepted. + message_id: The ``Message-ID`` of the message that failed, angle brackets + included, when the message was built before the failure; the key to + look it up in the relay's logs. Never part of ``str(error)``. """ - def __init__(self, message: str, *, status: int | None = None) -> None: + def __init__( + self, + message: str, + *, + status: int | None = None, + kind: MailFailure = MailFailure.OTHER, + delivery_unknown: bool = False, + message_id: str | None = None, + ) -> None: super().__init__(message) self.status = status + self.kind = kind + self.delivery_unknown = delivery_unknown + self.message_id = message_id @property def permanent(self) -> bool: @@ -999,7 +1055,10 @@ def permanent(self) -> bool: class MailNotConfiguredError(MailError): - """Mail was sent while no backend is configured.""" + """Mail was sent while no backend is configured; its ``kind`` is :attr:`MailFailure.NOT_CONFIGURED`.""" + + def __init__(self, message: str, *, status: int | None = None) -> None: + super().__init__(message, status=status, kind=MailFailure.NOT_CONFIGURED) @runtime_checkable diff --git a/tests/test_mail.py b/tests/test_mail.py index 4a15558..47e1a57 100644 --- a/tests/test_mail.py +++ b/tests/test_mail.py @@ -8,15 +8,20 @@ """ import contextlib +import email +import email.policy +import functools import logging import os +import re import smtplib import socket import ssl +import sys import threading from datetime import UTC, datetime from types import SimpleNamespace -from typing import ClassVar +from typing import ClassVar, Literal import pytest @@ -25,7 +30,14 @@ from jasil._core.mail_message import build_mime from jasil.backends.mail_memory import MemoryMail, NullMail from jasil.backends.mail_smtp import SmtpMail -from jasil.providers import MAIL_SEND_DEADLINE_SECONDS, MailError, MailMessage, MailNotConfiguredError, MailProvider +from jasil.providers import ( + MAIL_SEND_DEADLINE_SECONDS, + MailError, + MailFailure, + MailMessage, + MailNotConfiguredError, + MailProvider, +) # Windows: shutdown() does not wake a thread blocked in recv(), so the watchdog cannot interrupt a read there; only the per-read timeout bounds one. CANNOT_INTERRUPT_A_READ = pytest.mark.skipif( @@ -57,6 +69,8 @@ class FakeSMTP: instances: ClassVar[list["FakeSMTP"]] = [] offers_starttls = True + offers_size = False + does_esmtp = True refused: ClassVar[dict] = {} fail_on: str | None = None quit_code = 221 @@ -69,7 +83,9 @@ def __init__(self, host="", port=0, local_hostname=None, timeout=None, context=N self.calls: list[str] = [] self.logged_in_as: str | None = None self.sent = None + self.body: bytes | None = None self.envelope_sender: str | None = None + self.mail_options: list[str] = [] self.tls_started = False FakeSMTP.instances.append(self) @@ -93,11 +109,19 @@ def close(self): def ehlo(self): self.calls.append("ehlo") + def ehlo_or_helo_if_needed(self): + if "ehlo" not in self.calls: + self.calls.append("ehlo") + def has_extn(self, name): - return FakeSMTP.offers_starttls + return {"starttls": FakeSMTP.offers_starttls, "size": FakeSMTP.offers_size}.get(name, False) def starttls(self, context=None): self.calls.append("starttls") + if FakeSMTP.fail_on == "tls": + raise ssl.SSLError("handshake") + if FakeSMTP.fail_on == "starttls-refused": + raise smtplib.SMTPResponseException(454, b"srvtext-xyz") self.tls_started = True def login(self, username, password): @@ -107,26 +131,56 @@ def login(self, username, password): raise smtplib.SMTPAuthenticationError(535, ("bad creds for " + ADDRESS).encode()) if FakeSMTP.fail_on == "no-mechanism": raise smtplib.SMTPException("No suitable authentication method found.") - - def send_message(self, mime, from_addr=None): - self.calls.append("send") - self.sent = mime - self.envelope_sender = from_addr - if FakeSMTP.fail_on == "data": - raise smtplib.SMTPDataError(554, ("srvtext-xyz " + ADDRESS).encode()) - if FakeSMTP.fail_on == "all-refused": - raise smtplib.SMTPRecipientsRefused({ADDRESS: (550, b"no")}) - if FakeSMTP.fail_on == "tls": - raise ssl.SSLError("handshake") if FakeSMTP.fail_on == "unsupported": raise smtplib.SMTPNotSupportedError("srvtext-xyz " + ADDRESS) - return FakeSMTP.refused + + def mail(self, sender, options=()): + self.calls.append("mail") + self.envelope_sender = sender + self.mail_options = list(options) + if FakeSMTP.fail_on == "sender": + return 550, ("srvtext-xyz " + ADDRESS).encode() + return 250, b"ok" + + def rcpt(self, recipient): + self.calls.append("rcpt") + if FakeSMTP.fail_on == "drop-at-rcpt": + raise smtplib.SMTPServerDisconnected("srvtext-xyz " + ADDRESS) + if FakeSMTP.fail_on == "all-refused": + return 550, ("srvtext-xyz " + ADDRESS).encode() + if FakeSMTP.fail_on == "rcpt-closing": + return 421, b"srvtext-xyz closing" + return FakeSMTP.refused.get(recipient, (250, b"ok")) + + def docmd(self, cmd): + assert cmd == "DATA" + self.calls.append("data") + if FakeSMTP.fail_on == "drop-before-354": + raise smtplib.SMTPServerDisconnected("srvtext-xyz " + ADDRESS) + if FakeSMTP.fail_on == "data-refused": + return 554, ("srvtext-xyz " + ADDRESS).encode() + return 354, b"go" + + def send(self, payload): + assert payload.endswith(b"\r\n.\r\n") + self.body = re.sub(rb"(?m)^\.\.", b".", payload.removesuffix(b".\r\n")) + self.sent = email.message_from_bytes(self.body, policy=email.policy.default) + if FakeSMTP.fail_on == "tls-in-data": + raise ssl.SSLEOFError("srvtext-xyz") + + def getreply(self): + if FakeSMTP.fail_on == "data": + return 554, ("srvtext-xyz " + ADDRESS).encode() + if FakeSMTP.fail_on == "drop-in-data": + raise smtplib.SMTPServerDisconnected("srvtext-xyz " + ADDRESS) + return 250, b"queued" @pytest.fixture(autouse=True) def fake_smtp(monkeypatch): FakeSMTP.instances = [] FakeSMTP.offers_starttls = True + FakeSMTP.offers_size = False FakeSMTP.refused = {} FakeSMTP.fail_on = None FakeSMTP.quit_code = 221 @@ -405,7 +459,7 @@ def test_starttls_is_required_before_anything_is_sent(self): _smtp().send(_message()) calls = FakeSMTP.instances[0].calls - assert calls.index("starttls") < calls.index("send") + assert calls.index("starttls") < calls.index("mail") def test_a_server_without_starttls_is_refused_not_downgraded(self): FakeSMTP.offers_starttls = False @@ -413,7 +467,7 @@ def test_a_server_without_starttls_is_refused_not_downgraded(self): with pytest.raises(MailError, match="STARTTLS"): _smtp().send(_message()) - assert "send" not in FakeSMTP.instances[0].calls + assert "mail" not in FakeSMTP.instances[0].calls def test_implicit_tls_skips_starttls(self): _smtp(implicit_tls=True).send(_message()) @@ -441,7 +495,7 @@ def test_login_happens_after_tls(self): _smtp(username="relay-user", password="pw").send(_message()) calls = FakeSMTP.instances[0].calls - assert calls.index("starttls") < calls.index("login") < calls.index("send") + assert calls.index("starttls") < calls.index("login") < calls.index("mail") def test_login_and_sender_are_independent(self): """Relays log you in as a generated identity and verify the From separately.""" @@ -633,10 +687,12 @@ def test_a_send_past_its_deadline_is_aborted(self, monkeypatch): ticks = iter([0.0, 0.0, 1000.0, 1000.0, 1000.0]) monkeypatch.setattr(mail_smtp.time, "monotonic", lambda: next(ticks)) - with pytest.raises(MailError, match="deadline"): + with pytest.raises(MailError, match="deadline") as caught: _smtp().send(_message()) - assert "send" not in FakeSMTP.instances[0].calls + assert "mail" not in FakeSMTP.instances[0].calls + assert caught.value.kind is MailFailure.DEADLINE + assert not caught.value.delivery_unknown def test_a_partial_refusal_succeeds_and_warns_with_a_count(self, caplog): FakeSMTP.refused = {"gone@example.com": (550, b"unknown")} @@ -857,30 +913,79 @@ def test_it_stops_once_the_send_ends(self): class TestFailuresAreRedacted: @pytest.mark.parametrize( - ("mode", "expect"), + ("mode", "expect", "kind", "delivery_unknown"), [ - ("connect", "unreachable"), - ("greeting", "refused the connection (status 421)"), - ("auth", "authentication failed (status 535)"), - ("no-mechanism", "no supported authentication mechanism"), - ("data", "status 554"), - ("all-refused", "refused every recipient (status 550)"), - ("tls", "TLS negotiation"), - ("unsupported", "lacks a required feature"), + ("connect", "unreachable", MailFailure.UNREACHABLE, False), + ("greeting", "refused the connection (status 421)", MailFailure.UNREACHABLE, False), + ("tls", "TLS negotiation", MailFailure.TLS_FAILED, False), + ("starttls-refused", "refused a command (status 454)", MailFailure.OTHER, False), + ("auth", "authentication failed (status 535)", MailFailure.AUTH_FAILED, False), + ("no-mechanism", "no supported authentication mechanism", MailFailure.UNSUPPORTED, False), + ("unsupported", "lacks a required feature", MailFailure.UNSUPPORTED, False), + ("sender", "refused the sender (status 550)", MailFailure.SENDER_REFUSED, False), + ("all-refused", "refused the recipients (status 550)", MailFailure.RECIPIENT_REFUSED, False), + ("rcpt-closing", "refused the recipients (status 421)", MailFailure.RECIPIENT_REFUSED, False), + ("drop-at-rcpt", "connection lost", MailFailure.CONNECTION_LOST, False), + ("data-refused", "rejected the message (status 554)", MailFailure.MESSAGE_REJECTED, False), + ("drop-before-354", "connection lost", MailFailure.CONNECTION_LOST, False), + ("data", "rejected the message (status 554)", MailFailure.MESSAGE_REJECTED, False), + ("drop-in-data", "connection lost", MailFailure.CONNECTION_LOST, True), + ("tls-in-data", "connection lost", MailFailure.CONNECTION_LOST, True), ], ) - def test_each_failure_becomes_a_mail_error_with_no_server_text(self, mode, expect, caplog): + def test_each_failure_becomes_a_mail_error_with_no_server_text(self, mode, expect, kind, delivery_unknown, caplog): FakeSMTP.fail_on = mode with caplog.at_level(logging.DEBUG), pytest.raises(MailError) as caught: _smtp(username="u", password="pw").send(_message(subject="Top secret subject", text="secret body")) assert expect in str(caught.value) + assert caught.value.kind is kind + assert caught.value.delivery_unknown is delivery_unknown assert caught.value.__cause__ is None and caught.value.__suppress_context__ for leaked in (ADDRESS, "Top secret subject", "secret body", "bad creds", "srvtext-xyz"): assert leaked not in str(caught.value) assert leaked not in caplog.text + def test_every_failure_after_the_message_was_built_carries_its_message_id(self): + FakeSMTP.fail_on = "drop-in-data" + + with pytest.raises(MailError) as caught: + _smtp().send(_message()) + + message_id = caught.value.message_id + assert message_id == FakeSMTP.instances[0].sent["Message-ID"] + assert message_id not in str(caught.value) + + def test_a_server_without_starttls_is_unsupported(self): + FakeSMTP.offers_starttls = False + + with pytest.raises(MailError) as caught: + _smtp().send(_message()) + + assert caught.value.kind is MailFailure.UNSUPPORTED + assert caught.value.message_id is not None + + def test_a_cleartext_relay_that_stops_resolving_privately_is_unreachable(self, monkeypatch): + monkeypatch.setattr(mail_smtp.network, "resolve_private_addresses", lambda host: ([], "public")) + + with pytest.raises(MailError) as caught: + _smtp(tls=False).send(_message()) + + assert caught.value.kind is MailFailure.UNREACHABLE + + @pytest.mark.parametrize("mail", [_memory, _smtp]) + def test_a_lost_lease_is_its_own_kind_and_nothing_was_built(self, monkeypatch, mail): + provider = mail() + monkeypatch.setattr(sys.modules[type(provider).__module__], "lease_lost", lambda: True) + + with pytest.raises(MailError) as caught: + provider.send(_message()) + + assert caught.value.kind is MailFailure.LEASE_LOST + assert caught.value.message_id is None + assert not caught.value.delivery_unknown + def test_success_logs_no_personal_data(self, caplog): with caplog.at_level(logging.DEBUG): _smtp().send(_message(subject="Top secret subject")) @@ -888,6 +993,124 @@ def test_success_logs_no_personal_data(self, caplog): assert ADDRESS not in caplog.text and "Top secret subject" not in caplog.text +class TestTheEnvelope: + """``SmtpMail`` runs MAIL, RCPT and DATA itself, so it knows which step a failure interrupted.""" + + def test_ehlo_comes_before_mail_even_without_starttls_or_login(self): + _smtp(implicit_tls=True).send(_message()) + + calls = FakeSMTP.instances[0].calls + assert calls.index("ehlo") < calls.index("mail") < calls.index("rcpt") < calls.index("data") + + def test_the_size_is_declared_when_the_server_offers_it(self): + FakeSMTP.offers_size = True + + _smtp().send(_message()) + + smtp = FakeSMTP.instances[0] + assert smtp.mail_options == [f"size={len(smtp.body)}"] + + def test_no_size_is_declared_otherwise(self): + _smtp().send(_message()) + + assert FakeSMTP.instances[0].mail_options == [] + + def test_a_refused_sender_stops_before_any_recipient(self): + FakeSMTP.fail_on = "sender" + + with pytest.raises(MailError): + _smtp().send(_message()) + + assert "rcpt" not in FakeSMTP.instances[0].calls + + def test_a_server_closing_during_rcpt_stops_before_data(self): + FakeSMTP.fail_on = "rcpt-closing" + + with pytest.raises(MailError) as caught: + _smtp().send(_message(to=[ADDRESS, "other@example.com"])) + + assert FakeSMTP.instances[0].calls.count("rcpt") == 1 + assert "data" not in FakeSMTP.instances[0].calls + assert not caught.value.permanent + + def test_every_recipient_is_offered(self): + _smtp().send(_message(to=[ADDRESS, "Other "])) + + assert FakeSMTP.instances[0].calls.count("rcpt") == 2 + + @pytest.mark.parametrize( + ("mode", "delivery_unknown"), + [("drop-at-rcpt", False), ("drop-before-354", False), ("drop-in-data", True)], + ) + def test_the_watchdog_firing_may_have_delivered_only_after_the_354(self, monkeypatch, mode, delivery_unknown): + monkeypatch.setattr(SmtpMail, "_watch", staticmethod(lambda connection, expired, finished: expired.set())) + FakeSMTP.fail_on = mode + + with pytest.raises(MailError) as caught: + _smtp().send(_message()) + + assert caught.value.kind is MailFailure.DEADLINE + assert caught.value.delivery_unknown is delivery_unknown + + def test_real_smtplib_declares_the_size_and_sends_the_built_message(self, monkeypatch): + monkeypatch.setattr(mail_smtp.smtplib, "SMTP", REAL_SMTP) + monkeypatch.setattr(mail_smtp.network, "resolve_private_addresses", lambda host: (["10.0.0.5"], None)) + seen: list[bytes] = [] + + peer = _Peer(monkeypatch, functools.partial(_serve_smtp, seen, drop=None)) + message_id = _smtp(tls=False).send(_message(text=".leading dot\n.")) + peer.join() + + verbs = [line.split()[0].upper() for line in seen if line[:1].isalpha()] + assert verbs[:4] == [b"EHLO", b"MAIL", b"RCPT", b"DATA"] + assert b"size=" in next(line for line in seen if line.upper().startswith(b"MAIL")) + assert any(message_id.encode() in line for line in seen) + assert b"..leading dot\r\n" in seen and b"..\r\n" in seen + + @pytest.mark.parametrize(("drop", "delivery_unknown"), [("before-354", False), ("after-body", True)]) + def test_real_smtplib_losing_the_connection_may_have_delivered_only_after_the_354( + self, monkeypatch, drop, delivery_unknown + ): + monkeypatch.setattr(mail_smtp.smtplib, "SMTP", REAL_SMTP) + monkeypatch.setattr(mail_smtp.network, "resolve_private_addresses", lambda host: (["10.0.0.5"], None)) + + peer = _Peer(monkeypatch, functools.partial(_serve_smtp, [], drop=drop)) + with pytest.raises(MailError) as caught: + _smtp(tls=False).send(_message()) + peer.join() + + assert caught.value.kind is MailFailure.CONNECTION_LOST + assert caught.value.delivery_unknown is delivery_unknown + + +def _serve_smtp(seen: list[bytes], f, *, drop: Literal["before-354", "after-body"] | None) -> None: + """A minimal SMTP peer offering SIZE; ``drop`` makes it hang up instead of answering ``DATA`` or the body.""" + f.write(b"220 hi\r\n") + f.flush() + while line := f.readline(): + seen.append(line) + verb = line.split()[0].upper() + if verb == b"EHLO": + f.write(b"250-localhost\r\n250 SIZE 100000\r\n") + elif verb == b"DATA": + if drop == "before-354": + return + f.write(b"354 go\r\n") + f.flush() + while (body := f.readline()) != b".\r\n": + seen.append(body) + if drop == "after-body": + return + f.write(b"250 queued\r\n") + elif verb == b"QUIT": + f.write(b"221 bye\r\n") + f.flush() + return + else: + f.write(b"250 ok\r\n") + f.flush() + + class TestResolvePrivateAddresses: @staticmethod def _answer(monkeypatch, *addresses): @@ -931,6 +1154,21 @@ def test_idn_domain_with_uppercase_ascii_label_is_accepted(): assert parse_address("a@Bücher.DE", role="recipient").domain == "xn--bcher-kva.DE" +def test_mail_error_defaults_describe_an_unclassified_failure(): + error = MailError("x") + + assert error.kind is MailFailure.OTHER + assert error.delivery_unknown is False + assert error.message_id is None + + +def test_not_configured_is_its_own_kind(): + assert MailNotConfiguredError("x").kind is MailFailure.NOT_CONFIGURED + with pytest.raises(MailNotConfiguredError) as caught: + NullMail().send(_message()) + assert caught.value.kind is MailFailure.NOT_CONFIGURED + + def test_mail_error_permanent_follows_the_status_code(): from jasil.providers import MailError diff --git a/tests/test_reloading.py b/tests/test_reloading.py index ff4e75e..1ddd557 100644 --- a/tests/test_reloading.py +++ b/tests/test_reloading.py @@ -21,6 +21,7 @@ GeocodedPlace, GeocodingProvider, MailError, + MailFailure, MailMessage, MailNotConfiguredError, MailProvider, @@ -346,6 +347,7 @@ def test_mail_without_a_good_backend_raises_a_transient_error(self): _mail(source).send(MESSAGE) assert not isinstance(caught.value, MailNotConfiguredError) + assert caught.value.kind is MailFailure.SETTINGS_UNAVAILABLE assert caught.value.permanent is False # The source's exception is the host's and may carry a secret, so it is not chained. assert caught.value.__cause__ is None @@ -356,8 +358,9 @@ def test_mail_after_a_rejection_raises_instead_of_reusing_old_credentials(self): provider = _mail(source) provider.send(MESSAGE) source.value = BAD_MAIL - with pytest.raises(MailNotConfiguredError): + with pytest.raises(MailNotConfiguredError) as rejected: provider.send(MESSAGE) + assert rejected.value.kind is MailFailure.NOT_CONFIGURED source.error = RuntimeError("db down") with pytest.raises(MailError, match="could not be loaded"): @@ -581,6 +584,7 @@ def test_an_unresolved_mail_relay_is_retried_after_the_delay(self, monkeypatch): with pytest.raises(MailError) as first: provider.send(MESSAGE) assert not isinstance(first.value, MailNotConfiguredError) + assert first.value.kind is MailFailure.SETTINGS_UNAVAILABLE with pytest.raises(MailError): provider.send(MESSAGE) assert calls["n"] == 1 # remembered until the delay passes