Repository navigation
Conversation
Add posthog.emails.send() on Client and AsyncClient, plus standalone Emails and AsyncEmails in posthog.emails for processes that only send email. Sends go to the app host with a project secret API key and an Idempotency-Key, and retry network errors, rate limits, an in-flight key and 503 with the same key. Errors raise EmailError with PostHog's code and a Resend-style name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c62b30cd-eb2e-496c-a73d-2a6e7a52a62e
posthog-python Compliance ReportDate: 2026-10-22T10:04:26.342980+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
|
@PostHog review |
|
@mayteio PostHog Review is reviewing this pull request. |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
| ``` | ||
| """ | ||
| if self._emails is None: | ||
| self._emails = Emails(self.secret_key, host=self.host) |
There was a problem hiding this comment.
P1 · Require an explicit app host for custom ingestion hosts
When Client.host points to an ingestion reverse proxy, emails reuses that address and sends the project secret key in its Authorization header. This exposes privileged credentials to a proxy configured only for public capture traffic; AsyncClient.emails does the same. Derive known Cloud app hosts and require an explicit app host for other addresses.
| @property | ||
| def emails(self) -> Emails: |
There was a problem hiding this comment.
P2 · Forward module-level email sends to the default client
With the SDK's module-level setup (import posthog), the advertised posthog.emails.send(...) raises AttributeError: importing the new submodule binds posthog.emails to a module without send. Adding Client.emails only supports explicitly constructed clients. Add a module-level forwarding function that uses the configured default client.
There was a problem hiding this comment.
Resolved in 275a64d by not advertising it: every example and the release note now use client.emails.send(). There is deliberately no module-level forwarder, because the default module client is usually set up without a secret_key, so posthog.emails stays the module that holds Emails, AsyncEmails and EmailError.
| if self._emails is None: | ||
| self._emails = AsyncEmails(self.secret_key, host=self.host) | ||
| self._emails.disabled = self.disabled | ||
| return self._emails |
There was a problem hiding this comment.
P2 · Close owned email transports during client shutdown
After client.emails.send(), leaving async with AsyncPosthog(...) never closes the separate email HTTP client: shutdown() closes only _http_client. Repeated client lifetimes retain email connections; the sync wrapper also omits cleanup. Wire both email transports into parent shutdown, following the lifecycle contract required by AGENTS.md:20.
| self._session: Optional[requests.Session] = None | ||
| self._session_lock = threading.Lock() |
There was a problem hiding this comment.
P2 · Reinitialize email sessions and locks after a fork
When a process forks after creating Emails, children inherit its HTTP pool and _session_lock. A lock held by another parent thread stays locked forever in the child, so _get_session() hangs before the request timeout applies. Client._reinit_after_fork() does not reset email state; replace the lock and session in a child fork hook.
| """ | ||
| if self._emails is None: | ||
| self._emails = Emails(self.secret_key, host=self.host) | ||
| self._emails.disabled = self.disabled |
There was a problem hiding this comment.
P2 · Check the parent client's disabled state when sending
When an application stores emails = client.emails and later sets client.disabled = True, emails.send(...) still sends email because this property only copies the disabled state when accessed. AsyncClient.emails has the same bypass, violating the disabled-client contract for retained helpers. Read the parent's current disabled state inside each send.
| backoff = min(_MAX_RETRY_DELAY, 2.0**attempt) * random.uniform(0.8, 1.2) | ||
| return max(backoff, error.retry_after or 0.0) |
There was a problem hiding this comment.
P3 · Apply the retry delay ceiling after jitter
With max_retries=6 or higher, a sustained network failure reaches attempt 5, where _retry_delay() caps the backoff at 30 seconds and then multiplies it by jitter. This produces waits up to 36 seconds, exceeding _MAX_RETRY_DELAY and extending blocking sends beyond the intended ceiling; cap the backoff after applying jitter.
…ports client.emails only infers PostHog Cloud app hosts. A custom host raises app_host_required before any request, so an ingestion proxy never receives the project secret key. A kept emails reference now reads the client's disabled state on every send. Shutdown closes the email transport, a forked child drops the parent's email session, and the retry ceiling applies after jitter. Docs examples use client.emails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c62b30cd-eb2e-496c-a73d-2a6e7a52a62e
marandaneto
left a comment
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review.
|
|
||
| if self._emails is not None: | ||
| try: | ||
| await self._emails.aclose() |
There was a problem hiding this comment.
blocking: Drain active email sends before closing their transport — AsyncEmails.send() is not registered in the parent's _inflight_operations, so shutdown reaches this close while an email call is still running. If an already-started request then returns a retryable 503, the send loop retries using its cached, now-closed httpx client and raises an uncaught RuntimeError instead of completing its bounded retry or returning EmailError; shutdown has already reported completion. Wait for admitted email operations before disposing their transport, as existing immediate-capture operations do. Reproduction: reproduced — python -m pytest posthog/test/test_review_candidates.py::test_async_shutdown_drains_inflight_email_before_transport_close --timeout=30 -q fails on reviewed head with RuntimeError: Cannot send a request, as the client has been closed, while the same probe passes at d2827abe107b46c1e71eb72d86e93e2ca96bda0b.
There was a problem hiding this comment.
Fixed in d20805a. A send through client.emails on AsyncClient now registers as an in-flight operation, the same way capture_immediate does, so shutdown() waits for it (retries included) before it closes the email transport. test_async_shutdown_waits_for_a_send_in_flight_and_then_refuses_sends covers the 503-then-202 case during shutdown.
| """ | ||
| if self._emails is None: | ||
| self._emails = _client_emails( | ||
| Emails, self.secret_key, self.host, lambda: self.disabled |
There was a problem hiding this comment.
blocking: Honor terminal parent shutdown before admitting email sends — The namespace checks only self.disabled; permanently shutting down the parent does not set that flag. Consequently a retained client.emails handle, or first namespace access after shutdown, can create a fresh transport and successfully send email after client.shutdown() has completed. The async namespace has the same omission at posthog/async_client.py:274. These sends also escape subsequent cleanup because repeated parent shutdown returns early; parent-owned email admission must honor the terminal lifecycle state, independently of whether standalone email clients support reopening. Reproduction: reproduced — python -m pytest posthog/test/test_review_candidates.py -k 'shutdown_is_terminal' --timeout=30 -q fails both sync and async cases on reviewed head because each sends a POST after parent shutdown, and both pass with the minimal temporary admission control.
There was a problem hiding this comment.
Fixed in d20805a. Parent shutdown is now terminal for client.emails: a retained handle or a fresh one raises EmailError with code="client_closed" and makes no request, on both Client and AsyncClient. Standalone Emails / AsyncEmails can still reopen after close(). Sync shutdown does not wait for a send already in flight; that send still completes, because a closed requests session stays usable. Covered by test_a_shut_down_client_sends_no_email.
client.emails now asks the parent client to admit each send. After shutdown, a kept or new client.emails raises EmailError with code client_closed and makes no request. On AsyncClient, an admitted send is an in-flight operation, so shutdown waits for it, retries included, before it closes the email transport. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c62b30cd-eb2e-496c-a73d-2a6e7a52a62e
marandaneto
left a comment
marandaneto
left a comment
There was a problem hiding this comment.
Public API / agent-friendliness review against the SDK guidelines. The send signature is readable and Pythonic. The only blocking concern is the advertised module-level posthog.emails.send(...) entry point, which still raises AttributeError; this is already tracked in #1093 (comment), so I have not duplicated that inline comment. All four new inline comments are optional, non-blocking suggestions; addressing them here or in a follow-up is at the author's discretion. Validation at this head: 44 email tests passed, public API snapshot check passed, and a no-network API probe reproduced the missing module entry point.
| ) -> _EmailsT: | ||
| app_host = determine_app_host(client_host) | ||
| is_cloud = app_host in (US_APP_ENDPOINT, EU_APP_ENDPOINT) | ||
| emails = factory(secret_key, host=app_host if is_cloud else None) |
There was a problem hiding this comment.
suggestion: Make email configuration explicit (non-blocking, optional) — This factory always uses the email defaults, so Posthog(..., max_retries=0, timeout=0.1).emails still has two retries and a 15-second timeout. The parent's settings are documented for event uploads, so this need not inherit them, but consider a documented email-specific configuration path for both sync and async clients, rather than requiring callers to mutate the returned helper or switch to a standalone client. This would make retry ownership easier for humans and agents to discover, in line with the API-design guidelines. Reproduction: reproduced — the focused pytest probe test_parent_configuration_is_not_email_configuration passed, confirming parent values (0.1, 0) and email values (15, 2).
There was a problem hiding this comment.
Done in 275a64d. Posthog and AsyncPosthog take email_request_timeout_seconds and email_request_max_retries, named after the existing feature_flags_request_* options and documented in the client's Args. They're validated at construction, so a bad value fails early instead of on the first send.
| ```python | ||
| from posthog.emails import Emails | ||
|
|
||
| emails = Emails("phs_...") |
There was a problem hiding this comment.
suggestion: Add a complete setup-and-verification quickstart (non-blocking, optional) — The send example is clear, but the prerequisites are scattered across docstrings. Consider a public quickstart recommending when to use client.emails versus standalone Emails, explaining the project-token/secret-key distinction, hog_flow:write, region, where to obtain a transactional template UUID, posthog[async], cleanup, and handling EmailError/the queued result. Document timeout units and scope, retry defaults, and the 30-second Retry-After policy alongside configuration. Include a way to verify success and diagnose a common configuration error, as suggested by the agent-friendly SDK guidelines. Reproduction: not reproduced — reviewed the changed documentation and examples; a clean-project integration using only published docs was not run.
There was a problem hiding this comment.
Done in 275a64d. The posthog.emails module docstring is now a quickstart: when to use client.emails vs Emails, project token vs secret key and hog_flow:write, where the template ID comes from, hosts and app_host, posthog[async], what a result means, timeout units, retry defaults and the 30 s Retry-After policy, the error codes a setup usually hits (missing_secret_key, app_host_required, 401/403, not_found, client_closed), cleanup and testing.
| def emails_with(replies: list[Reply], **options: Any) -> tuple[Emails, FakeAdapter]: | ||
| emails = Emails(SECRET_KEY, **options) | ||
| adapter = FakeAdapter(replies) | ||
| emails._get_session().mount("https://", adapter) |
There was a problem hiding this comment.
suggestion: Provide a supported no-network testing path (non-blocking, optional) — These tests need the private _get_session() transport hook, and the async cases replace _client; customers would need the same internal knowledge to assert email payloads or simulate a failure. Consider a documented fake/mock interface or supported transport injection for this feature, potentially as a follow-up. disabled=True prevents delivery but only returns id=None, so it cannot verify the attempted operation. This is the distinction made by the SDK testing guidelines. Reproduction: not reproduced — assessed the public API and test setup by inspection; no separate customer-facing mock integration was exercised.
There was a problem hiding this comment.
Done in 275a64d. Emails(session=...) takes a requests.Session and AsyncEmails(http_client=...) an httpx.AsyncClient; a transport you pass in is never closed by the SDK. The tests now use these public options instead of _get_session(), and the quickstart's testing section documents them. The remaining private access is in the client-lifecycle tests, which check the client's own transport.
|
|
||
| Needs ``secret_key``, a project secret API key with the ``hog_flow:write`` | ||
| scope. Sends go to the PostHog Cloud app host of ``host``. For any other | ||
| host, or to send email without capturing events, create |
There was a problem hiding this comment.
suggestion: Consider an app-host override on the existing client (non-blocking, optional) — Refusing to send the secret key to an ambiguous ingestion proxy is the right safety default. The ergonomic tradeoff is that a custom-host customer must create a separate Emails/AsyncEmails instance just to select the app host. Consider the explicit app_host override described in the shared email contract, or record why the standalone-client alternative is preferred. Either is an API decision rather than a security defect in the current refusal behavior; documenting the choice would help agents choose the intended integration path. Reproduction: not reproduced — compared the client property and standalone constructor with the linked contract; no custom-host network request was made.
There was a problem hiding this comment.
Done in 275a64d. Both clients take app_host. With it set, client.emails sends there; without it, a non-Cloud host still raises app_host_required before any request, and the error message now points at app_host.
marandaneto
left a comment
marandaneto
left a comment
There was a problem hiding this comment.
approving to unblock but left a few suggestions
|
@mayteio sdk-wise looks good but i guess your team should look at this as well since we dont have much context about this feature and how its gonna be used by customers, so a product-review would make sense for new stuff |
|
Thanks, its a good shout @marandaneto. I wont merge this until I've figure out what this looks like for customers, and in any case its a good reference for internal use-cases in correctly handling retries/errors. |
Posthog and AsyncPosthog take app_host, email_request_timeout_seconds and email_request_max_retries, so client.emails works behind a custom host and a job runner can turn retries off. Emails takes a session and AsyncEmails an http_client, which gives tests a supported no-network path. The posthog.emails module docs now hold a setup quickstart: keys and scope, template ID, hosts, retries, errors to expect, cleanup and testing. The release note names client.emails.send(). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c62b30cd-eb2e-496c-a73d-2a6e7a52a62e
|
Could
Both can stay retryable with the same idempotency key, so Two smaller things:
Context: PostHog's own transactional emails ( |
💡 Motivation and Context
Note
Safe retries depend on the send API's idempotency key, which landed in PostHog/posthog#120446, PostHog/posthog#120510 and PostHog/posthog#120392. Release this only once those are deployed to every region.
Changes
client.emails.send(to=, template_id=, template_data=, distinct_id=, idempotency_key=)onPosthogandAsyncPosthogqueues one email from a transactional template and returns its message ID. A result means queued, not delivered.EmailsandAsyncEmailsinposthog.emailssend without a capture client, for processes that only send email or use their own key and host.us.i.posthog.comandeu.i.posthog.commap tous.posthog.comandeu.posthog.com. The path uses@current, so no project ID is needed.client.emailsrefuses any host that is not PostHog Cloud withapp_host_required, before any request, unless the client setsapp_host. A custom host is often an ingestion proxy, which must not receive the secret key.email_request_timeout_secondsandemail_request_max_retrieson the client configure sends, next to the existingfeature_flags_request_*options.Emails(session=...)andAsyncEmails(http_client=...)accept your own transport, so tests can assert a send without private hooks.posthog.emailsmodule docs hold the setup quickstart: keys and scope, the template ID, hosts, retries, the errors a setup usually hits, cleanup and testing.Idempotency-Key. The client makes a UUID when the caller gives none.quota_limitedis never retried. ARetry-Afterover 30 seconds raises instead of waiting.max_retries=0leaves retries to a job runner.EmailErrorwith PostHog'scodeand a Resend-stylename. A disabled client sends nothing and returnsid=None.{data, error}, and v1 sends by template UUID to/messaging_templates/{id}/send/until the key-based route lands. Both are in the contract comment.determine_app_hostinrequest.py, anauthparameter on the localrequestsstub, the public API snapshot, and a changeset.💚 How did you test it?
posthog/test/test_emails.pycatches these regressions: a retriedquota_limited429 ortemplate_outdated409, an idempotency key that changes between retries, a secret key sent to an ingestion host, a disabled client that still sends, andDecimalordatetimevalues intemplate_datathat fail to encode.toand a 401 mapped to the rightname.master, a second send with the same key returned the first send's ID withreplayed=True, and MailDev received one email. The same key with another body raisedidempotency_key_reusedasinvalid_idempotent_request.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
claude-opus-5-5) in a PostHog Desktop cloud task, directed by the assignee. Skills used:writing-pr-descriptions.Emailsis a standalone class thatclient.emailswraps, becauseClient.disabledis true without a project token. An email-only process would otherwise never send.EmailErrorclass carriescodeandname. Subclasses can come later without breaking callers.template_datagoes through the sameclean()as capture properties, soDecimal,UUIDand datetimes encode the same way.Created with PostHog Desktop
🤖 Generated with Claude Code