Skip to content

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

Draft
Silthus wants to merge 4 commits into
feat/emails-code-formfrom
feat/emails-contract-alignment
Draft

Silthus wants to merge 4 commits into
feat/emails-code-formfrom
feat/emails-contract-alignment

Conversation

@Silthus

@Silthus Silthus commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

Moved here from PostHog#1124, closed on 2026-10-25 so this fork stack is the only copy. It opens upstream again after review.

💡 Motivation and Context

posthog.emails.send differs from the shared email SDK contract (PostHog/posthog#120921) in eleven places, in both the template form (PostHog#1093) and the code form (PostHog#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, and the code form leaves it out when absent Every send carries capture_engagement, true by default, on both forms. capture_engagement=False turns capture off for that send
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.

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 by default: per the contract's 2026-10-25 entry, it defaults to True on both forms, and every send carries it in the body: true when the caller leaves it out, false when passed. The docstrings of both forms say a project whose admin turned engagement capture off captures nothing, whatever the send asks.
    • The template route drops fields it doesn't know (DRF's default), so it would lose a false behind a 202. A template send with false goes to /messaging/send/email/ with template: {id}, which honors it. A template send with true or no value stays on /messaging_templates/{id}/send/, so the template form keeps working on today's servers.
    • Once feat(messaging): upload, send and lock code-managed email templates PostHog/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_every_send_says_whether_to_capture_engagement: both forms send capture_engagement: true when the argument is absent and false when it is passed, and a template send keeps today's route unless it passes false.
  • test_a_template_send_that_turns_capture_off_uses_the_email_send_route: the route, the template: {id} body, message_data, identifiers and the flag.
  • Local gate: pytest (4938 passed, on 08bc64b), 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 PostHog#1093 and PostHog#1101 with the contract yes, in a new PR of our own stacked on PostHog#1101, after PostHog#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
capture_engagement default True on every form, always sent; False turns it off for that send 👤 Human map conductor, contract PostHog/posthog#120921 (2026-10-25 entry)
Code for argument checks the contract doesn't name invalid_form, with the argument in field 🤖 Agent decision comment
capture_engagement route on the template form before PostHog/posthog#118924 lands False goes to /messaging/send/email/ as template: {id}; True or no value stays on the template route, which drops the flag. Replaces: any explicit value went to the new route 👤 Human Michael, 2026-10-25

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), then 1 GPT-6.1 Sol review of the capture_engagement change, proof: live run and cross-version recorded as gaps.

🤖 Generated with Claude Code

- 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 3 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>
Per the contract's 2026-10-25 entry, capture_engagement defaults to
True on both forms and the SDK always sends it. A template send that
turns capture off goes to the email send route, since the template
route drops fields it doesn't know; a template send that keeps it on
stays on the template route.

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

Silthus commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Note

🤖 Claude Opus 5.5 responding on behalf of Michael

  • 👤 capture_engagement route on the template form before the server's template-by-id route lands A template send with False goes to /messaging/send/email/ as template: {id}, the only route that honors it. True or no value stays on /messaging_templates/{id}/send/ with capture_engagement: true in the body, which that route ignores, so the template form keeps working on today's servers. This replaces the earlier line, where any explicit value went to the new route.

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