Shape transmission rows with the Timing context manager in deliver_message
- Dominant language
- Python
- Stars
- 4
- Forks
- 0
- Avg merge
- 8h 24m
- Merged PRs (30d)
- 115
Description
`services/email/msa/tasks.py` builds transmission rows by hand in four places, and the `measure()` wrapper in `deliver_message()` exists only to carry a window into a row that the `Timing` context manager could stamp itself. `Transmission` already implements that protocol, and the incoming path uses it for `SpamCheck` and `WebhookDelivery`.
## Current behavior
- `deliver_message()` wraps `send_outgoing_message()` in `measure()` and creates the `FAILED` row in its `except` handler, because `Timing.__exit__` runs before the handler and would already have inserted a row.
- `send_outgoing_message()` creates the `SENT` and `BOUNCED` rows inside the MX loop, and a `FAILED` row when the recipient domain has no MX records.
- The `SENT` row's window comes from inside `send_via_mx()`, the actual SMTP connection, and is merged into the row with `**tls_details`.
## Proposed change
Let the protocol own the row:
```python
with Transmission(message=message) as transmission:
try:
send_outgoing_message(message, transmission)
except Exception as error: # storage backend raises varied exceptions
logger.exception("Transmission error for message %r", message_id)
transmission.status = Transmission.Status.FAILED
transmission.details = str(error)
message.status = OutgoingMessage.Status.FAILED
message.save(update_fields=["status"])
```
- `send_outgoing_message(message, transmission)` shapes the passed instance instead of calling `Transmission.objects.create()`.
- `record_bounce(message, transmission, ...)` and the `SENT` branch set status, `remote_host`, the TLS payload, and the SMTP answer on that instance.
- `measure()` disappears from `deliver_message()`. `started_at` is stamped on entry and `finished_at` on exit.
That turns four row-creation points into one, and it makes the outgoing path read like the incoming one.
## Notes for whoever picks this up
- Attributes set inside the block are persisted at exit. Setting them in the `except` handler is too late, because `Timing.__exit__` already inserted the row with whatever status the instance had on entry.
- Merging the `send_via_mx()` payload after `__enter__` keeps the precise connection window for `SENT` rows.
- The no-MX row's window would move from the MX lookup to the whole attempt, which is visible in the transmission gantt added in #213.
- The suspended-org rows in `deliver_message()` and `check_outgoing_spam()` have a known outcome up front, so they can enter the protocol with the fields already set.
Raised while reviewing #243.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in services/email/msa/tasks.py by reading deliver_message(), send_outgoing_message(), record_bounce(), send_via_mx(), and the Transmission/Timing protocol used by the incoming path. Trace the existing row statuses and timing payloads, then verify that one protocol-owned transmission row persists each outcome, including suspended, failed, bounced, sent, and no-MX cases, with the expected timing windows.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100