codingjoe / codingjoe/relay

Shape transmission rows with the Timing context manager in deliver_message

Open
#244 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.