Skip to content

Commit d20805a

Browse files
authored
fix: refuse email sends after shutdown and drain async sends first
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
1 parent 83289f9 commit d20805a

4 files changed

Lines changed: 102 additions & 4 deletions

File tree

‎posthog/async_client.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import warnings
1111
import weakref
1212
from datetime import datetime, timezone
13-
from typing import Any, Dict, Mapping, Optional, Union
13+
from typing import Any, Callable, Dict, Mapping, Optional, Union
1414
from uuid import UUID, uuid4
1515

1616
from typing_extensions import Unpack
@@ -271,10 +271,21 @@ def emails(self) -> AsyncEmails:
271271
"""
272272
if self._emails is None:
273273
self._emails = _client_emails(
274-
AsyncEmails, self.secret_key, self.host, lambda: self.disabled
274+
AsyncEmails,
275+
self.secret_key,
276+
self.host,
277+
lambda: self.disabled,
278+
self._admit_email_send,
275279
)
276280
return self._emails
277281

282+
def _admit_email_send(self) -> Optional[Callable[[], None]]:
283+
# Shutdown waits for in-flight operations before it closes the email transport.
284+
if not self._accepting or self._closed:
285+
return None
286+
completion = self._start_inflight_operation()
287+
return lambda: self._finish_inflight_operation(completion)
288+
278289
async def __aenter__(self) -> AsyncClient:
279290
self._ensure_workers_started()
280291
return self

‎posthog/client.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
set_context_session as _context_set_context_session,
5353
tag as _context_tag,
5454
)
55-
from posthog.emails import Emails, _client_emails
55+
from posthog.emails import Emails, _client_emails, _no_op
5656
from posthog.exception_capture import ExceptionCapture
5757
from posthog._logging import _configure_posthog_logging
5858
from posthog.exception_utils import (
@@ -1155,7 +1155,11 @@ def emails(self) -> Emails:
11551155
"""
11561156
if self._emails is None:
11571157
self._emails = _client_emails(
1158-
Emails, self.secret_key, self.host, lambda: self.disabled
1158+
Emails,
1159+
self.secret_key,
1160+
self.host,
1161+
lambda: self.disabled,
1162+
lambda: None if self._shutdown_requested else _no_op,
11591163
)
11601164
return self._emails
11611165

‎posthog/emails.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,10 @@ def _retry_delay(error: EmailError, attempt: int, max_retries: int) -> Optional[
281281
return max(backoff, error.retry_after or 0.0)
282282

283283

284+
def _no_op() -> None:
285+
pass
286+
287+
284288
def _log_warnings(result: SendEmailResult) -> None:
285289
for warning in result.warnings:
286290
log.warning("PostHog email warning %s: %s", warning.code, warning.message)
@@ -323,11 +327,21 @@ def __init__(
323327
self.max_retries = max_retries
324328
self.disabled = disabled
325329
self._parent_disabled: Callable[[], bool] = lambda: False
330+
self._admit: Callable[[], Optional[Callable[[], None]]] = lambda: _no_op
326331
self._host_error: Optional[str] = None
327332

328333
def _is_disabled(self) -> bool:
329334
return self.disabled or self._parent_disabled()
330335

336+
def _start_send(self) -> Callable[[], None]:
337+
finish = self._admit()
338+
if finish is None:
339+
raise _sdk_error(
340+
"client_closed",
341+
"The PostHog client is shut down, so no email was sent.",
342+
)
343+
return finish
344+
331345
def _secret_key(self) -> str:
332346
if self._host_error:
333347
raise _sdk_error("app_host_required", self._host_error)
@@ -423,6 +437,13 @@ def send(
423437
if self._is_disabled():
424438
log.debug("PostHog client is disabled, so no email was sent.")
425439
return SendEmailResult(id=None)
440+
finish = self._start_send()
441+
try:
442+
return self._deliver(prepared)
443+
finally:
444+
finish()
445+
446+
def _deliver(self, prepared: _PreparedSend) -> SendEmailResult:
426447
auth = _BearerAuth(self._secret_key(), prepared.idempotency_key)
427448
session = self._get_session()
428449
url = self.host + prepared.path
@@ -539,6 +560,13 @@ async def send(
539560
if self._is_disabled():
540561
log.debug("PostHog client is disabled, so no email was sent.")
541562
return SendEmailResult(id=None)
563+
finish = self._start_send()
564+
try:
565+
return await self._deliver(prepared)
566+
finally:
567+
finish()
568+
569+
async def _deliver(self, prepared: _PreparedSend) -> SendEmailResult:
542570
headers = _headers(self._secret_key(), prepared.idempotency_key)
543571
httpx, client = self._get_client()
544572
url = self.host + prepared.path
@@ -610,6 +638,7 @@ def _client_emails(
610638
secret_key: Optional[str],
611639
client_host: Optional[str],
612640
is_disabled: Callable[[], bool],
641+
admit: Callable[[], Optional[Callable[[], None]]],
613642
) -> _EmailsT:
614643
app_host = determine_app_host(client_host)
615644
is_cloud = app_host in (US_APP_ENDPOINT, EU_APP_ENDPOINT)
@@ -621,4 +650,5 @@ def _client_emails(
621650
"Create posthog.emails.Emails(secret_key, host=<your PostHog app host>) instead."
622651
)
623652
emails._parent_disabled = is_disabled
653+
emails._admit = admit
624654
return emails

‎posthog/test/test_emails.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import asyncio
34
import json
45
import logging
56
from datetime import datetime, timezone
@@ -305,6 +306,21 @@ def test_a_disabled_client_sends_nothing():
305306
assert adapter.requests == []
306307

307308

309+
def test_a_shut_down_client_sends_no_email():
310+
client = Posthog("phc_test", secret_key=SECRET_KEY, sync_mode=True)
311+
emails = client.emails
312+
313+
client.shutdown()
314+
315+
with mock.patch.object(requests.Session, "request") as request:
316+
for handle in (emails, client.emails):
317+
with pytest.raises(EmailError) as raised:
318+
send(handle)
319+
assert raised.value.code == "client_closed"
320+
request.assert_not_called()
321+
assert emails._session is None
322+
323+
308324
@parameterized.expand(
309325
[
310326
("several_recipients", {"to": ["a@example.com", "b@example.com"]}, TypeError),
@@ -361,3 +377,40 @@ async def test_async_disabled_client_sends_nothing():
361377

362378
assert result.id is None
363379
assert emails._client is None
380+
381+
382+
@pytest.mark.asyncio
383+
async def test_async_shutdown_waits_for_a_send_in_flight_and_then_refuses_sends():
384+
started, release = asyncio.Event(), asyncio.Event()
385+
replies = [
386+
httpx.Response(503, json={"code": "delivery_unavailable"}),
387+
httpx.Response(202, json={"message_id": "msg_1"}),
388+
]
389+
390+
async def handler(request: httpx.Request) -> httpx.Response:
391+
started.set()
392+
await release.wait()
393+
return replies.pop(0)
394+
395+
client = AsyncPosthog("phc_test", secret_key=SECRET_KEY)
396+
emails = client.emails
397+
emails._client = httpx.AsyncClient(transport=httpx.MockTransport(handler))
398+
399+
with mock.patch("posthog.emails._retry_delay", return_value=0.0):
400+
sending = asyncio.create_task(
401+
emails.send(to="user@example.com", template_id=TEMPLATE_ID)
402+
)
403+
await started.wait()
404+
shutting_down = asyncio.create_task(client.shutdown())
405+
for _ in range(5):
406+
await asyncio.sleep(0)
407+
assert not shutting_down.done()
408+
release.set()
409+
410+
assert (await sending).id == "msg_1"
411+
await shutting_down
412+
413+
assert emails._client is None
414+
with pytest.raises(EmailError) as raised:
415+
await emails.send(to="user@example.com", template_id=TEMPLATE_ID)
416+
assert raised.value.code == "client_closed"

0 commit comments

Comments
 (0)