Skip to content

feat: align email sends with the shared SDK contract - #1124

Closed
Silthus wants to merge 11 commits into
PostHog:posthog/transactional-emailsfrom
Silthus:feat/emails-contract-alignment
Closed

Silthus wants to merge 11 commits into
PostHog:posthog/transactional-emailsfrom
Silthus:feat/emails-contract-alignment

Conversation

@Silthus

@Silthus Silthus commented Oct 24, 2026 •

Copy link
Copy Markdown

💡 Motivation and Context

posthog.emails.send differs from the shared email SDK contract (PostHog/posthog#120921) in eleven places, in both the template form (#1093) and the code form (#1101). A caller who reads the contract, or moves between posthog-node and posthog-python, gets other error codes, other exceptions and missing options.

This PR makes both forms follow the contract:

Before After
A client whose host isn't PostHog Cloud raises app_host_required It raises missing_app_host
A Retry-After over 30 s fails the send at once The send waits 30 s, then retries
SendEmailResult has a replayed field No replayed: the contract has no line for it, and posthog-node doesn't expose it
A disabled client returns SendEmailResult(id=None) It raises EmailError with code client_disabled, as posthog-node does. id is always a str
A local check raises TypeError or ValueError It raises EmailValidationError, an EmailError and a ValueError, with name validation_error, the contract's code and the argument in field
No per-send retries send(..., max_retries=0) overrides the client's retries
No migrated_from migrated_from="resend" on Posthog, AsyncPosthog, Emails or AsyncEmails sends X-PostHog-Migrated-From on every email request
The template form has no capture_engagement send(template_id=..., capture_engagement=False)
An idempotency key with a trailing newline passes the local check It raises invalid_idempotency_key

client_closed stays: the contract gets a line for it.

Note

Review only the commits after a6bc0ac. This PR stacks on #1101, which stacks on #1093. GitHub can't base a pull request on a branch in a fork, and #1101's branch feat/emails-code-form lives in a fork. So this PR targets #1093's branch posthog/transactional-emails, and its diff also shows #1101's commits. It retargets to #1101's base once #1101 merges. The stack is visible on the fork, where Silthus#3 sits on Silthus#2.

Impact

Pays into Grow Workflows through email sends from code: the spec's Code-sending orgs metric. Both SDKs now behave alike, so a caller can follow one contract and one set of docs. migrated_from lets PostHog tell which sends moved from another provider once send attribution (PostHog/posthog#120628) reads the header. Expected move: small, up. Run cost: n/a.

Merge Danger

Warning

🟡 Merge risk: medium · two-way door until a release; a release makes EmailValidationError, client_disabled, max_retries, capture_engagement and migrated_from a public contract (merge-risk rubric)

Changes

  • EmailValidationError(EmailError, ValueError) for every local check of both forms.
    • Codes: invalid_template_id, invalid_template_key, invalid_idempotency_key, and invalid_form for every other argument.
    • field names the argument, such as to, cc, attachments, from_ or max_retries. It's None when the form itself is wrong, such as both template_id and template_key.
    • It is a ValueError, so except ValueError still catches what used to be a ValueError. A caller that catches TypeError doesn't catch it anymore.
  • client_disabled: the disabled check moved next to the shutdown check in _start_send, so a send runs its local checks first, then raises client_disabled or client_closed. Its name is application_error, per the contract's SDK codes.
  • Retries: the wait is max(backoff, Retry-After), capped at 30 s. max_retries on a send overrides the client's, for the send, the upload and the send after it.
  • migrated_from: a client option on Posthog and AsyncPosthog, passed to their emails, and an option of Emails and AsyncEmails. A value that isn't visible ASCII, such as an empty one or one with a line break, raises ValueError at construction, like timeout.
  • capture_engagement on the template form: the template route drops fields it doesn't know (DRF's default), so the flag would be lost behind a 202. A template send that sets it goes to /messaging/send/email/ with template: {id}, which takes it. A template send without it stays on /messaging_templates/{id}/send/. Once feat(messaging): upload, send and lock code-managed email templates posthog#118924 lands, the contract moves every template send to the new route.
  • Mechanical: the module docstring, the send and client docstrings, both changesets, the public API snapshot, and the strict types check (migrated_from, capture_engagement and max_retries on a typed send).

💚 How did you test it?

Test seams: Emails.send and AsyncEmails.send on a real client with the HTTP transport faked (FakeAdapter, httpx.MockTransport), and Posthog.emails for the client options. The tests assert the requests the client makes and what it returns or raises. Each test below failed before its change, except the two retry tests the review added for the upload and the async send, which pin behavior that already held. The upload one fails when the upload ignores a send's retries.

  • test_rejects_invalid_arguments_before_sending and test_rejects_a_code_send_that_mixes_or_misses_fields: each case asserts EmailValidationError, ValueError, name, code and field, and that no request went out. New cases: a trailing newline in the idempotency key, a negative or boolean max_retries, and a capture_engagement that isn't a bool.
  • test_rejects_an_attachment_that_is_not_a_file: invalid_form on attachments, and the content is still never repeated.
  • test_a_disabled_client_refuses_the_send and its async twin: client_disabled, application_error, no request.
  • test_a_client_with_a_custom_host_never_sends_it_the_secret_key: missing_app_host, no request.
  • test_waits_for_retry_after_up_to_30_seconds: Retry-After: 5 waits 5 s, and Retry-After: 120 waits 30 s, then the retry succeeds.
  • test_a_send_overrides_the_clients_max_retries: a send's 0 turns the client's retries off and the send fails, and a send's 1 turns them on over a client's 0 and the send succeeds. max_retries never reaches the body.
  • test_a_sends_max_retries_also_covers_its_upload: a send's 1 over a client's 0 retries a 503 on the upload, then sends. test_async_send_overrides_the_clients_max_retries covers the async send.
  • test_every_request_of_a_send_says_where_the_caller_migrated_from: the send, the upload and the second send all carry X-PostHog-Migrated-From. The async upload test and the client options test check it too, and the template send test checks a client without it sends no such header.
  • test_rejects_a_migrated_from_that_is_not_a_header_value: an empty value, a line break, or a number raises ValueError when the client is built.
  • test_a_template_send_that_sets_capture_engagement_uses_the_email_send_route: the route, the template: {id} body, message_data, identifiers and the flag.
  • Local gate: pytest (4933 passed, on 97abc5d), ruff format --check, ruff check, mypy | mypy-baseline filter, the strict types script, the public API snapshot check, and python -W error -c "import posthog".

Proofs owed:

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Decisions

Question Answer Who Where
Align #1093 and #1101 with the contract yes, in a new PR of our own stacked on #1101, after #1093 merges 👤 Human map conductor
Local check errors EmailValidationError(EmailError, ValueError) with the contract's code and field, on both forms 👤 Human map conductor
A disabled client raises client_disabled, as posthog-node does 👤 Human map conductor
replayed on the result drop it 👤 Human map conductor
Client retries option name keep email_request_max_retries, since max_retries is capture's option 👤 Human map conductor
Code for argument checks the contract doesn't name invalid_form, with the argument in field 🤖 Agent decision comment
capture_engagement on the template form before PostHog/posthog#118924 lands send it on /messaging/send/email/ as template: {id}, since the template route drops unknown fields 🤖 Agent decision comment

Open: the contract should name email_request_max_retries as Python's client option, since max_retries already configures capture. Recommendation: add it under "Language extras".

Involvement: 0 grilling rounds, 0 prototypes, reviews: a QA swarm of 8 panel lanes in round 1, 2 lanes in round 2 and 1 final read (GPT-6.1 Sol and Claude Opus 5.5), proof: live run and cross-version recorded as gaps.

🤖 Generated with Claude Code

Silthus and others added 9 commits October 23, 2026 13:48
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The code form of emails.send takes the new optional fields of the
contract. cc and bcc accept one address or a list and are always sent as
a list. Attachment bytes are Base64-encoded once, before the retry loop.
A list of byte values is refused with a TypeError that names bytes().
None of the fields reach the template upload.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The server accepts an empty address list, and the SDK checks types only,
so a computed empty list of copies no longer raises before the send.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A PostHog that doesn't take a field refuses the send with invalid_request
and names it in EmailError.field, which a test now pins. HTML with no text
goes out without a plain-text part. Template and code sends share one set
of idempotency keys, and a retry replays after its workflow is paused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Base64-encode attachment content from bytearray and memoryview too
- Refuse a template_key with a trailing newline before the send
- Take an empty html or text when the other body has content
- Raise TypeError for a cc, bcc or reply_to that is not text
- Inject the async test transport through http_client

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Base64-encode a non-contiguous memoryview through bytes()
- Raise TypeError for a subject or from_ that is not a string
- Name bytearray and memoryview in the changeset

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Raise EmailValidationError (EmailError and ValueError) for local checks,
  with the contract's code and the argument in field, on both forms
- Raise client_disabled from a disabled client, and missing_app_host
  when the client's host is not PostHog Cloud
- Wait up to 30 seconds for a longer Retry-After instead of failing
- Take max_retries per send, and capture_engagement on the template form
- Send migrated_from as X-PostHog-Migrated-From on every email request
- Refuse an idempotency key with a trailing newline
- Drop replayed from SendEmailResult

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silthus and others added 2 commits October 24, 2026 13:07
A migrated_from with a line break passed construction, then failed the
sync send with a bare ValueError. It must now be visible ASCII. The
not_found and idempotency_key docs name the cases a template send with
capture_engagement and a reused key run into. Tests now cover a send's
max_retries on the upload and the async send, assert the outcome of an
override, and read the migrated header from the prepared request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On the email send route a wrong template ID answers template_not_found,
so the not_found entry now says so for a template send with
capture_engagement. The upload test now turns retries on for one send
over a client with none, so it fails when the upload ignores them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Silthus

Silthus commented Oct 24, 2026

Copy link
Copy Markdown
Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

Decisions made while building this PR:

  • 🤖 Code for argument checks the contract doesn't name invalid_form, with the argument in field. The contract names invalid_template_id, invalid_template_key and invalid_idempotency_key, and those keep their codes.
  • 🤖 capture_engagement on the template form before feat(messaging): upload, send and lock code-managed email templates posthog#118924 lands A template send that sets it goes to /messaging/send/email/ as template: {id}, since the template route drops fields it doesn't know. Until #118924 merges such a send answers 404; a send without it stays on today's route.

@Silthus Silthus closed this Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant