feat: Add asyncio capture client#719
Draft
marandaneto wants to merge 11 commits into
Draft
Conversation
5 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
posthog-python Compliance ReportDate: 2026-07-22 11:41:28 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
marandaneto
force-pushed
the
async-sdk-01-capture
branch
2 times, most recently
from
July 2, 2026 11:25
8816f29 to
cfe5d0f
Compare
marandaneto
force-pushed
the
async-sdk-01-capture
branch
from
July 2, 2026 11:28
cfe5d0f to
3b1a554
Compare
Member
Author
|
Addressed the Greptile shutdown feedback in 43e6275 — async shutdown now offloads the inherited blocking cleanup path ( |
Member
Author
|
related PRs for early feedback @dustinbyrne |
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
Contributor
|
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
dustinbyrne
reviewed
Jul 22, 2026
Comment on lines
+187
to
+189
| async def capture( | ||
| self, event: str, **kwargs: Unpack[OptionalCaptureArgs] | ||
| ) -> Optional[str]: |
Contributor
There was a problem hiding this comment.
i think it's worth considering keeping capture synchronous regardless of the context
i like the pattern @posthog/node uses with captureImmediate/capture_immediate which allows you to wait for the event to be flushed async - otherwise, it's just a quick write to buffer
Member
Author
There was a problem hiding this comment.
The main reason capture() remains async is before_send: AsyncClient supports awaitable before_send callbacks, so capture() must be able to await the hook before queueing or dropping the event. It also preserves awaitable direct delivery when sync_mode=True. The normal buffered path is still just a quick queue write. A separate immediate/non-awaiting API could be considered independently without changing those semantics.
🦔 ReviewHog reviewed this pull requestFound 4 must fix, 9 should fix, 5 consider. Published 18 findings (view the review). |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
ReviewHog Report
Infrastructure
Issues: 6 issues
Files (3)
posthog/_async_request.pyposthog/_async_consumer.pypyproject.toml
What were the main changes
- Adds async HTTP helpers (async_post, async_get, async_flags, async_batch_post, async_remote_config) backed by lazily-imported httpx.AsyncClient with pooled/global client instances and close_async_clients() cleanup
- Adds _AsyncConsumer that drains an asyncio.Queue, batches events by size/time/count, retries with backoff, and routes to dedicated AI/events endpoints
- Declares optional
posthog[async]extra dependency on httpx in pyproject.toml - Addressed CodeQL clear-text logging findings by removing serialized request/response bodies and URLs from async debug logs
- Open review question: module-level httpx client globals could leak across event loops; consider scoping client lifecycle to AsyncClient instances instead
Feature
Issues: 12 issues
Files (2)
posthog/async_client.pyposthog/__init__.py
What were the main changes
- Adds AsyncClient(Client) with an asyncio.Queue/worker-task pipeline replacing daemon threads for capture, set, set_once, group_identify, alias, capture_exception, flush, join, and shutdown
- Supports async context manager usage (aenter/aexit) and before_send callbacks that may be awaitable
- send_feature_flags fallback now runs inherited sync get_all_flags/get_feature_variants via asyncio.to_thread instead of blocking the event loop directly (fix for reviewer P1 comment)
- Exposes AsyncClient and the public AsyncPosthog alias from posthog/init.py
- Open review discussion: whether capture() should stay fully synchronous with a captureImmediate-style async flush variant, deferred to a future PR
Comment on lines
+40
to
+45
| class AsyncClient(_Client): | ||
| """Asyncio-native PostHog client. | ||
|
|
||
| This client mirrors the synchronous ``Client`` capture lifecycle but uses an | ||
| ``asyncio.Queue`` and worker tasks instead of daemon threads. | ||
| """ |
There was a problem hiding this comment.
AsyncClient inherits from the synchronous Client, making instances silently mis-usable wherever a Client is expected
Why we think it's a valid issue
- Confirmed subclassing & isinstance:
posthog/async_client.py:40declaresclass AsyncClient(_Client)andAsyncPostHog(AsyncClient)atposthog/__init__.py:1215, soisinstance(async_client, Client)is genuinely True whilecapture/set/set_once/group_identify/alias/capture_exception/flush/join/shutdownare all re-declaredasync defkeeping the parent's sync names and return annotations. - Found — reachable silent-failure sites (not speculative): (1)
posthog/exception_capture.py:81callsself.client.capture_exception(...)synchronously;AsyncClient.__init__forwardsenable_exception_autocapturetosuper().__init__(async_client.py:115), which constructsExceptionCapture(self, ...)atclient.py:471-478, andshutdown()usesself.exception_capture(async_client.py:544). SoAsyncPostHog(..., enable_exception_autocapture=True)installs its own asynccapture_exceptionintosys.excepthook/threading.excepthook, which call it synchronously → un-awaited coroutine, exception never sent. (2)posthog/contexts.py:217— the publicnew_context(client=...)sync context manager callsnew_context.client.capture_exception(e)onexcept; handing it anAsyncClientsilently drops the coroutine. - Impact: Confirmed contract break with silent data loss via public entry points (
enable_exception_autocapture,new_context(client=...), and anyisinstance(x, Client)dispatch): the coroutine is discarded with at best aRuntimeWarning. These are two concrete triggers distinct from the separately-reportedExceptionCapturefinding, so this generalized finding names its own trigger+consequence and is not mere abstraction/taste. The finding also offers a proportionate minimal fix (runtime guard + docstrings), so it is not overengineering-only; must_fix stands given silently-dropped exception captures on a documented public option.
Issue description
class AsyncClient(_Client) means isinstance(async_client, Client) is True, yet nearly every public method (capture, set, set_once, group_identify, alias, capture_exception, flush, join, shutdown) is overridden as async def while keeping the same name and declared return-type annotation (e.g. Optional[str], None) as the synchronous parent. This is a Liskov-substitution violation with no static-typing safety net: running mypy over the full posthog package (per the repo's own mypy.ini config, warn_unused_ignores = True) reports zero override-incompatibility errors for capture/set/set_once/group_identify/alias — mypy does not flag a sync-method override becoming async when the declared return-type annotation matches. Any code, framework integration, or internal helper (see the concrete ExceptionCapture case reported separately) that is typed against Client or duck-types over it and calls these methods synchronously (without await) will silently receive an un-awaited coroutine: no event is sent, flush()/shutdown() never actually flush/join, and no error surfaces beyond a RuntimeWarning. Since AsyncClient/AsyncPostHog are fully public top-level exports (posthog/__init__.py), this is a real footgun for any consumer generically handling Client instances.
Suggested fix
Avoid subclassing the concrete synchronous Client. Extract the shared non-IO logic (_build_capture_message, _prepare_enqueue_message, _parse_send_feature_flags, _feature_flag_capture_properties, etc.) into a mixin/base that both Client and AsyncClient compose from, so AsyncClient is not an isinstance of Client and cannot be accidentally treated as a drop-in synchronous client. If full separation is impractical for this stacked PR, at minimum document this loudly in both classes' docstrings and consider a runtime guard so misuse fails noisily instead of silently.
Prompt to fix with AI (copy-paste)
## Context
@posthog/async_client.py#L40-45
<issue_description>
`class AsyncClient(_Client)` means `isinstance(async_client, Client)` is True, yet nearly every public method (`capture`, `set`, `set_once`, `group_identify`, `alias`, `capture_exception`, `flush`, `join`, `shutdown`) is overridden as `async def` while keeping the same name and declared return-type annotation (e.g. `Optional[str]`, `None`) as the synchronous parent. This is a Liskov-substitution violation with no static-typing safety net: running `mypy` over the full `posthog` package (per the repo's own mypy.ini config, `warn_unused_ignores = True`) reports zero override-incompatibility errors for `capture`/`set`/`set_once`/`group_identify`/`alias` — mypy does not flag a sync-method override becoming async when the declared return-type annotation matches. Any code, framework integration, or internal helper (see the concrete `ExceptionCapture` case reported separately) that is typed against `Client` or duck-types over it and calls these methods synchronously (without `await`) will silently receive an un-awaited coroutine: no event is sent, `flush()`/`shutdown()` never actually flush/join, and no error surfaces beyond a `RuntimeWarning`. Since `AsyncClient`/`AsyncPostHog` are fully public top-level exports (`posthog/__init__.py`), this is a real footgun for any consumer generically handling `Client` instances.
</issue_description>
<issue_validation>
- **Confirmed subclassing & isinstance:** `posthog/async_client.py:40` declares `class AsyncClient(_Client)` and `AsyncPostHog(AsyncClient)` at `posthog/__init__.py:1215`, so `isinstance(async_client, Client)` is genuinely True while `capture`/`set`/`set_once`/`group_identify`/`alias`/`capture_exception`/`flush`/`join`/`shutdown` are all re-declared `async def` keeping the parent's sync names and return annotations.
- **Found — reachable silent-failure sites (not speculative):** (1) `posthog/exception_capture.py:81` calls `self.client.capture_exception(...)` synchronously; `AsyncClient.__init__` forwards `enable_exception_autocapture` to `super().__init__` (`async_client.py:115`), which constructs `ExceptionCapture(self, ...)` at `client.py:471-478`, and `shutdown()` uses `self.exception_capture` (`async_client.py:544`). So `AsyncPostHog(..., enable_exception_autocapture=True)` installs its own *async* `capture_exception` into `sys.excepthook`/`threading.excepthook`, which call it synchronously → un-awaited coroutine, exception never sent. (2) `posthog/contexts.py:217` — the public `new_context(client=...)` sync context manager calls `new_context.client.capture_exception(e)` on `except`; handing it an `AsyncClient` silently drops the coroutine.
- **Impact:** Confirmed contract break with silent data loss via public entry points (`enable_exception_autocapture`, `new_context(client=...)`, and any `isinstance(x, Client)` dispatch): the coroutine is discarded with at best a `RuntimeWarning`. These are two concrete triggers distinct from the separately-reported `ExceptionCapture` finding, so this generalized finding names its own trigger+consequence and is not mere abstraction/taste. The finding also offers a proportionate minimal fix (runtime guard + docstrings), so it is not overengineering-only; must_fix stands given silently-dropped exception captures on a documented public option.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Avoid subclassing the concrete synchronous `Client`. Extract the shared non-IO logic (`_build_capture_message`, `_prepare_enqueue_message`, `_parse_send_feature_flags`, `_feature_flag_capture_properties`, etc.) into a mixin/base that both `Client` and `AsyncClient` compose from, so `AsyncClient` is not an `isinstance` of `Client` and cannot be accidentally treated as a drop-in synchronous client. If full separation is impractical for this stacked PR, at minimum document this loudly in both classes' docstrings and consider a runtime guard so misuse fails noisily instead of silently.
</potential_solution>
Comment on lines
+451
to
+455
| except Exception as e: | ||
| if self.debug: | ||
| raise | ||
| self.log.exception(f"Failed to capture exception: {e}") | ||
| return None |
There was a problem hiding this comment.
AsyncClient.capture_exception can raise in debug mode, breaking the parent's explicit "never throws" contract
Why we think it's a valid issue
- Checked: The sync parent
Client.capture_exception(posthog/client.py:1430), theno_throw()decorator, and the async override's except-clause. - Found:
Client.capture_exceptionis the only one of these public methods NOT decorated with@no_throw()(contrastcapture/set/set_once/group_identify/aliasatclient.py:1085,1275,1313,1350,1393). Its inlineexceptatclient.py:1570-1572unconditionally logs andreturn Nonewith noself.debugcheck, and the comment atclient.py:1464-1465documents this as intentional ('we don't unexpectedly re-raise exceptions in the user's code'). Theno_throwdecorator re-raises whenself.debug(client.py:184-185) — precisely the behaviorcapture_exceptionwas excluded from.AsyncClient.capture_exceptionatasync_client.py:451-455reintroducesif self.debug: raise, and its body (await self.capture(...),_try_attach_code_variables_to_frames,_handle_in_app) has realistic throw paths. - Impact: Confirmed divergence from an explicitly-documented parent invariant: with
debug=True,await client.capture_exception(e)called inside a user'sexcept:block can re-raise, masking the original exception or crashing exception-handling code — the exact failure the parent deliberately avoids. It is a genuine contract break directly introduced by this PR, not speculative or style. Gated todebug=True(development-time), which supportsshould_fixrather thanmust_fix.
Issue description
Client.capture_exception (posthog/client.py:1430-1572) is deliberately NOT wrapped by the no_throw() decorator used for capture/set/set_once/group_identify/alias; its own inline comment states: "this function shouldn't ever throw an error, so it logs exceptions instead of raising them. this is important to ensure we don't unexpectedly re-raise exceptions in the user's code" — and indeed its except-clause never checks self.debug. AsyncClient.capture_exception, however, wraps the same logic with except Exception as e: if self.debug: raise ..., so when debug=True this method CAN raise. Since capture_exception is designed to be called from inside a user's own except: block, raising here can mask/replace the exception the user was already handling and introduce new crashes specifically in exception-handling code paths — the exact failure mode the parent class's design intentionally avoids, regardless of debug setting.
Suggested fix
Drop the if self.debug: raise branch in AsyncClient.capture_exception's except-clause (or otherwise make it match Client.capture_exception's unconditional log-and-return-None behavior), so the async client preserves the same never-raises guarantee documented on the sync client.
Prompt to fix with AI (copy-paste)
## Context
@posthog/async_client.py#L451-455
<issue_description>
`Client.capture_exception` (posthog/client.py:1430-1572) is deliberately NOT wrapped by the `no_throw()` decorator used for `capture`/`set`/`set_once`/`group_identify`/`alias`; its own inline comment states: "this function shouldn't ever throw an error, so it logs exceptions instead of raising them. this is important to ensure we don't unexpectedly re-raise exceptions in the user's code" — and indeed its except-clause never checks `self.debug`. `AsyncClient.capture_exception`, however, wraps the same logic with `except Exception as e: if self.debug: raise ...`, so when `debug=True` this method CAN raise. Since `capture_exception` is designed to be called from inside a user's own `except:` block, raising here can mask/replace the exception the user was already handling and introduce new crashes specifically in exception-handling code paths — the exact failure mode the parent class's design intentionally avoids, regardless of debug setting.
</issue_description>
<issue_validation>
- **Checked:** The sync parent `Client.capture_exception` (`posthog/client.py:1430`), the `no_throw()` decorator, and the async override's except-clause.
- **Found:** `Client.capture_exception` is the only one of these public methods NOT decorated with `@no_throw()` (contrast `capture`/`set`/`set_once`/`group_identify`/`alias` at `client.py:1085,1275,1313,1350,1393`). Its inline `except` at `client.py:1570-1572` unconditionally logs and `return None` with no `self.debug` check, and the comment at `client.py:1464-1465` documents this as intentional ('we don't unexpectedly re-raise exceptions in the user's code'). The `no_throw` decorator re-raises when `self.debug` (`client.py:184-185`) — precisely the behavior `capture_exception` was excluded from. `AsyncClient.capture_exception` at `async_client.py:451-455` reintroduces `if self.debug: raise`, and its body (`await self.capture(...)`, `_try_attach_code_variables_to_frames`, `_handle_in_app`) has realistic throw paths.
- **Impact:** Confirmed divergence from an explicitly-documented parent invariant: with `debug=True`, `await client.capture_exception(e)` called inside a user's `except:` block can re-raise, masking the original exception or crashing exception-handling code — the exact failure the parent deliberately avoids. It is a genuine contract break directly introduced by this PR, not speculative or style. Gated to `debug=True` (development-time), which supports `should_fix` rather than `must_fix`.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Drop the `if self.debug: raise` branch in AsyncClient.capture_exception's except-clause (or otherwise make it match Client.capture_exception's unconditional log-and-return-None behavior), so the async client preserves the same never-raises guarantee documented on the sync client.
</potential_solution>
Comment on lines
+96
to
+107
| data: str | bytes = json.dumps(body, cls=DatetimeSerializer) | ||
| log.debug("making async request") | ||
| headers = {"Content-Type": "application/json", "User-Agent": USER_AGENT} | ||
| if gzip: | ||
| try: | ||
| buf = BytesIO() | ||
| with GzipFile(fileobj=buf, mode="w") as gz: | ||
| gz.write(cast(str, data).encode("utf-8")) | ||
| data = buf.getvalue() | ||
| headers["Content-Encoding"] = "gzip" | ||
| except (OSError, zlib.error) as exc: | ||
| log.warning("failed to gzip request body, sending uncompressed: %s", exc) |
There was a problem hiding this comment.
Synchronous JSON serialization and gzip compression run inline on the event loop in async_post
Why we think it's a valid issue
- Checked:
async_postbody (posthog/_async_request.py:96-110), how the consumer is scheduled (posthog/async_client.py:166-185), the batch-size cap (posthog/consumer.py:29), and the sync consumer's execution model (posthog/consumer.py:35,55). - Found: At
_async_request.py:96json.dumps(body, cls=DatetimeSerializer)runs unconditionally, and at lines 100-104GzipFilecompression runs whengzip=True, both synchronously before the only suspension point (await http_client.postat line 110) — noawait/offload between them. - Found: The consumer is started with
asyncio.create_task(consumer.run())(async_client.py:185), i.e. on the caller's event loop, not a dedicated loop/thread — so a stall blocks every other coroutine in the embedding async app, not just SDK internals. - Found:
BATCH_SIZE_LIMIT = 5 * 1024 * 1024(consumer.py:29) is a real cap hit at high event throughput;GzipFile(mode="w")uses the default compresslevel 9, so compressing a multi-MiB batch is genuine CPU work (hundreds of ms), and even ungzippedjson.dumpsof that size costs tens of ms on-loop. - Found: The sync path is
class Consumer(Thread)(consumer.py:35), so serialization/compression there runs on a background thread and never blocks a cooperative scheduler — confirming the finding's "regression in isolation" framing; the async port reintroduces on-loop blocking the threaded design avoided. - Impact: Concrete trigger (a large batch flush) and concrete consequence (event loop stalled for the serialize+compress duration, blocking unrelated coroutines in the host app) — matches the 'blocking on an async path' keep criterion, not speculative. The PR already addressed sibling feedback about "running sync flag fallback calls off the event loop," so the team treats event-loop blocking as in-scope. Fix is simple (
asyncio.to_thread).should_fixis appropriate.
Issue description
Inside the async async_post coroutine, json.dumps(body, cls=DatetimeSerializer) and, when gzip is enabled, GzipFile compression are executed synchronously with no await between them. Batches can be up to BATCH_SIZE_LIMIT (5MiB, see posthog/consumer.py), so serializing and compressing a full batch is genuine CPU-bound work performed directly on the single-threaded asyncio event loop. Any other coroutine scheduled on that same loop (other capture calls, feature-flag evaluations, or unrelated work in the host async application embedding this SDK) is blocked for the duration. The sync Consumer avoids this entirely because it runs on a dedicated background thread, so this is a regression in isolation introduced specifically by porting the transport to asyncio.
Suggested fix
Offload the serialize+compress step with await asyncio.to_thread(...) (or only compress above a size threshold) so large batch payloads don't stall the event loop while other tasks are ready to run.
Prompt to fix with AI (copy-paste)
## Context
@posthog/_async_request.py#L96-107
<issue_description>
Inside the async `async_post` coroutine, `json.dumps(body, cls=DatetimeSerializer)` and, when gzip is enabled, `GzipFile` compression are executed synchronously with no `await` between them. Batches can be up to BATCH_SIZE_LIMIT (5MiB, see posthog/consumer.py), so serializing and compressing a full batch is genuine CPU-bound work performed directly on the single-threaded asyncio event loop. Any other coroutine scheduled on that same loop (other capture calls, feature-flag evaluations, or unrelated work in the host async application embedding this SDK) is blocked for the duration. The sync Consumer avoids this entirely because it runs on a dedicated background thread, so this is a regression in isolation introduced specifically by porting the transport to asyncio.
</issue_description>
<issue_validation>
- **Checked:** `async_post` body (`posthog/_async_request.py:96-110`), how the consumer is scheduled (`posthog/async_client.py:166-185`), the batch-size cap (`posthog/consumer.py:29`), and the sync consumer's execution model (`posthog/consumer.py:35,55`).
- **Found:** At `_async_request.py:96` `json.dumps(body, cls=DatetimeSerializer)` runs unconditionally, and at lines 100-104 `GzipFile` compression runs when `gzip=True`, both synchronously before the only suspension point (`await http_client.post` at line 110) — no `await`/offload between them.
- **Found:** The consumer is started with `asyncio.create_task(consumer.run())` (`async_client.py:185`), i.e. on the caller's event loop, not a dedicated loop/thread — so a stall blocks every other coroutine in the embedding async app, not just SDK internals.
- **Found:** `BATCH_SIZE_LIMIT = 5 * 1024 * 1024` (`consumer.py:29`) is a real cap hit at high event throughput; `GzipFile(mode="w")` uses the default compresslevel 9, so compressing a multi-MiB batch is genuine CPU work (hundreds of ms), and even ungzipped `json.dumps` of that size costs tens of ms on-loop.
- **Found:** The sync path is `class Consumer(Thread)` (`consumer.py:35`), so serialization/compression there runs on a background thread and never blocks a cooperative scheduler — confirming the finding's "regression in isolation" framing; the async port reintroduces on-loop blocking the threaded design avoided.
- **Impact:** Concrete trigger (a large batch flush) and concrete consequence (event loop stalled for the serialize+compress duration, blocking unrelated coroutines in the host app) — matches the 'blocking on an async path' keep criterion, not speculative. The PR already addressed sibling feedback about "running sync flag fallback calls off the event loop," so the team treats event-loop blocking as in-scope. Fix is simple (`asyncio.to_thread`). `should_fix` is appropriate.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Offload the serialize+compress step with `await asyncio.to_thread(...)` (or only compress above a size threshold) so large batch payloads don't stall the event loop while other tasks are ready to run.
</potential_solution>
Comment on lines
+506
to
+517
| async def flush(self, timeout_seconds: Optional[float] = 10) -> None: # type: ignore[override] | ||
| if timeout_seconds is None: | ||
| await self._queue.join() | ||
| return | ||
| try: | ||
| await asyncio.wait_for(self._queue.join(), timeout=timeout_seconds) | ||
| except asyncio.TimeoutError: | ||
| self.log.warning( | ||
| "flush timed out after %s seconds with %s items pending.", | ||
| timeout_seconds, | ||
| self._queue.qsize(), | ||
| ) |
There was a problem hiding this comment.
flush() timeout warning under-reports pending items by using qsize() instead of the unfinished-task count
Why we think it's a valid issue
- Checked:
AsyncClient.flush()(async_client.py:506-517), the syncClient.flush()timeout branch (client.py:1708-1725), and the_AsyncConsumerdequeue/ack lifecycle to confirm an in-flight window actually exists. - Found: Sync
Client.flush()logsqueue.unfinished_tasksin its timeout warning (client.py:1722), counting both queued and dequeued-but-unacked items; the async version logsself._queue.qsize()(async_client.py:516). The in-flight window is real:_AsyncConsumer.next()removes items viaawait self.queue.get()(_async_consumer.py:98-99, droppingqsize), andtask_done()is only called inupload()'sfinallyafter the request completes (_async_consumer.py:82-84) — and that request can retry up toretries+1times withasyncio.sleep(min(2**attempt, 30))backoff (_async_consumer.py:161-182).queue.join()gates on unfinished tasks, notqsize, so on timeout a batch of up toflush_at(default 100) items can be mid-retry whileqsize()reports fewer, sometimes 0. - Impact: Premise verified correct — a real but low-stakes observability defect: only the count in a warning that fires on the rare flush/shutdown-timeout path is inaccurate; no data loss, wrong results, or crash. It is a genuine divergence from the sync client (not style/taste, not speculative/unreachable), which is exactly what
consideris for; the reviewer's priority is appropriate.
Issue description
On a flush() timeout, the warning logs self._queue.qsize() as the number of pending items. qsize() only reflects items still sitting in the queue's internal deque; it does not include items a consumer has already pulled via queue.get() but not yet acknowledged with task_done() (e.g. a batch currently being uploaded or retried). The synchronous Client.flush() (posthog/client.py:1694-1731) instead reports queue.unfinished_tasks, which is accurate for both queued and in-flight items. As written, the async warning can understate — sometimes report 0 — pending work while a consumer is actively retrying an in-flight batch, making the log message misleading for operators trying to diagnose a slow shutdown/flush.
Suggested fix
Report a value consistent with what actually gates queue.join() completion. asyncio.Queue doesn't expose unfinished_tasks publicly, so either track an explicit counter alongside enqueue/task_done in _enqueue/_AsyncConsumer, or clearly note in the log message that qsize() only reflects not-yet-dequeued items (not in-flight ones) to avoid misleading operators.
Prompt to fix with AI (copy-paste)
## Context
@posthog/async_client.py#L506-517
<issue_description>
On a flush() timeout, the warning logs `self._queue.qsize()` as the number of pending items. `qsize()` only reflects items still sitting in the queue's internal deque; it does not include items a consumer has already pulled via `queue.get()` but not yet acknowledged with `task_done()` (e.g. a batch currently being uploaded or retried). The synchronous `Client.flush()` (posthog/client.py:1694-1731) instead reports `queue.unfinished_tasks`, which is accurate for both queued and in-flight items. As written, the async warning can understate — sometimes report 0 — pending work while a consumer is actively retrying an in-flight batch, making the log message misleading for operators trying to diagnose a slow shutdown/flush.
</issue_description>
<issue_validation>
- **Checked:** `AsyncClient.flush()` (async_client.py:506-517), the sync `Client.flush()` timeout branch (client.py:1708-1725), and the `_AsyncConsumer` dequeue/ack lifecycle to confirm an in-flight window actually exists.
- **Found:** Sync `Client.flush()` logs `queue.unfinished_tasks` in its timeout warning (client.py:1722), counting both queued and dequeued-but-unacked items; the async version logs `self._queue.qsize()` (async_client.py:516). The in-flight window is real: `_AsyncConsumer.next()` removes items via `await self.queue.get()` (`_async_consumer.py:98-99`, dropping `qsize`), and `task_done()` is only called in `upload()`'s `finally` after the request completes (`_async_consumer.py:82-84`) — and that request can retry up to `retries+1` times with `asyncio.sleep(min(2**attempt, 30))` backoff (`_async_consumer.py:161-182`). `queue.join()` gates on unfinished tasks, not `qsize`, so on timeout a batch of up to `flush_at` (default 100) items can be mid-retry while `qsize()` reports fewer, sometimes 0.
- **Impact:** Premise verified correct — a real but low-stakes observability defect: only the count in a warning that fires on the rare flush/shutdown-timeout path is inaccurate; no data loss, wrong results, or crash. It is a genuine divergence from the sync client (not style/taste, not speculative/unreachable), which is exactly what `consider` is for; the reviewer's priority is appropriate.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Report a value consistent with what actually gates `queue.join()` completion. `asyncio.Queue` doesn't expose `unfinished_tasks` publicly, so either track an explicit counter alongside enqueue/task_done in `_enqueue`/`_AsyncConsumer`, or clearly note in the log message that `qsize()` only reflects not-yet-dequeued items (not in-flight ones) to avoid misleading operators.
</potential_solution>
Comment on lines
+42
to
+49
| def _build_client(): | ||
| httpx_module = _require_httpx() | ||
| return httpx_module.AsyncClient() | ||
|
|
||
|
|
||
| def _build_flags_client(): | ||
| httpx_module = _require_httpx() | ||
| return httpx_module.AsyncClient() |
There was a problem hiding this comment.
Async httpx clients don't set follow_redirects=True, diverging from the sync requests.Session behavior on redirecting hosts
Why we think it's a valid issue
- Checked: sync transport redirect behavior (
posthog/request.py) vs async (posthog/_async_request.py), plus the failure/retry path inposthog/_async_consumer.py. - Found (sync follows):
post()atrequest.py:246andget()atrequest.py:409callsession.post/get(...)with noallow_redirects, and_build_session/_build_flags_session(request.py:77-106) only mount retry adapters — they never disable redirects, so therequestsdefaultallow_redirects=Trueholds for POST and GET. - Found (async does not):
_build_client/_build_flags_client(_async_request.py:42-49) constructhttpx.AsyncClient()with no args (httpx defaultsfollow_redirects=False), and neitherasync_post(line 110) norasync_get(line 244) sets it per-request — confirmed nofollow_redirectsanywhere in the file. - Found (drop path): a 30x reaches
_process_async_response(_async_request.py:134), which treats any non-200 as error and raisesAPIError(30x, ...);is_retryable()(_async_consumer.py:151-158) computesnot((400<=30x<500) and ...)=True, so_sendretries through allretries(default 10) with backoff and then the batch is dropped inupload()(task_done runs infinally). Chain is as described. - Impact: Verified behavioral regression from the reference sync client on a real, supported configuration — a custom/self-hosted
hostbehind a proxy/LB doing HTTP→HTTPS upgrade or canonical-domain redirect works transparently on the sync client but silently loses all events on the async client. This is a parity/reliability break (not a speculative what-if: the trigger is external host config, not ruled out by any validation), and the fix (follow_redirects=True) is a one-liner restoring sync parity.should_fixis appropriate.
Issue description
_build_client() / _build_flags_client() construct httpx.AsyncClient() with no arguments, and none of async_post/async_get pass follow_redirects=True per-request either. httpx.AsyncClient defaults follow_redirects=False (verified directly: httpx.AsyncClient().follow_redirects is False on httpx 0.28.1, the version this PR's pyproject.toml extra allows). The sync transport in posthog/request.py uses requests.Session, whose Session.request() defaults allow_redirects=True for every HTTP method, including POST/GET — so the sync client transparently follows any 3xx response. Any deployment where the configured host is behind a proxy/load-balancer or CNAME that issues a redirect (common in self-hosted PostHog setups, e.g. HTTP->HTTPS upgrade or canonical-domain redirects) will work fine on the sync client but silently break on the async client: the 3xx response is fed straight into _process_async_response, which treats any non-200 status as a failure, attempts to JSON-parse a body that typically has no detail key, and raises APIError(30x, <raw body/text>). Because 3xx isn't in the 400 <= status < 500 range, _AsyncConsumer._send's is_retryable() treats it as retryable, so the consumer burns through all retries (default 10) with exponential backoff before finally dropping the batch — a silent, hard-to-diagnose event loss for any host configuration that the sync client handles transparently.
Suggested fix
Pass follow_redirects=True when constructing the async clients (httpx.AsyncClient(follow_redirects=True) in both _build_client() and _build_flags_client()) so async transport behavior matches the sync requests.Session default, or explicitly document/test the intentional divergence if redirects should not be followed.
Prompt to fix with AI (copy-paste)
## Context
@posthog/_async_request.py#L42-49
@posthog/_async_request.py#L109-110
@posthog/_async_request.py#L243-244
<issue_description>
`_build_client()` / `_build_flags_client()` construct `httpx.AsyncClient()` with no arguments, and none of `async_post`/`async_get` pass `follow_redirects=True` per-request either. httpx.AsyncClient defaults `follow_redirects=False` (verified directly: `httpx.AsyncClient().follow_redirects` is `False` on httpx 0.28.1, the version this PR's `pyproject.toml` extra allows). The sync transport in `posthog/request.py` uses `requests.Session`, whose `Session.request()` defaults `allow_redirects=True` for every HTTP method, including POST/GET — so the sync client transparently follows any 3xx response. Any deployment where the configured `host` is behind a proxy/load-balancer or CNAME that issues a redirect (common in self-hosted PostHog setups, e.g. HTTP->HTTPS upgrade or canonical-domain redirects) will work fine on the sync client but silently break on the async client: the 3xx response is fed straight into `_process_async_response`, which treats any non-200 status as a failure, attempts to JSON-parse a body that typically has no `detail` key, and raises `APIError(30x, <raw body/text>)`. Because 3xx isn't in the `400 <= status < 500` range, `_AsyncConsumer._send`'s `is_retryable()` treats it as retryable, so the consumer burns through all `retries` (default 10) with exponential backoff before finally dropping the batch — a silent, hard-to-diagnose event loss for any host configuration that the sync client handles transparently.
</issue_description>
<issue_validation>
- **Checked:** sync transport redirect behavior (`posthog/request.py`) vs async (`posthog/_async_request.py`), plus the failure/retry path in `posthog/_async_consumer.py`.
- **Found (sync follows):** `post()` at `request.py:246` and `get()` at `request.py:409` call `session.post/get(...)` with no `allow_redirects`, and `_build_session`/`_build_flags_session` (`request.py:77-106`) only mount retry adapters — they never disable redirects, so the `requests` default `allow_redirects=True` holds for POST and GET.
- **Found (async does not):** `_build_client`/`_build_flags_client` (`_async_request.py:42-49`) construct `httpx.AsyncClient()` with no args (httpx defaults `follow_redirects=False`), and neither `async_post` (line 110) nor `async_get` (line 244) sets it per-request — confirmed no `follow_redirects` anywhere in the file.
- **Found (drop path):** a 30x reaches `_process_async_response` (`_async_request.py:134`), which treats any non-200 as error and raises `APIError(30x, ...)`; `is_retryable()` (`_async_consumer.py:151-158`) computes `not((400<=30x<500) and ...)` = `True`, so `_send` retries through all `retries` (default 10) with backoff and then the batch is dropped in `upload()` (task_done runs in `finally`). Chain is as described.
- **Impact:** Verified behavioral regression from the reference sync client on a real, supported configuration — a custom/self-hosted `host` behind a proxy/LB doing HTTP→HTTPS upgrade or canonical-domain redirect works transparently on the sync client but silently loses all events on the async client. This is a parity/reliability break (not a speculative what-if: the trigger is external host config, not ruled out by any validation), and the fix (`follow_redirects=True`) is a one-liner restoring sync parity. `should_fix` is appropriate.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Pass `follow_redirects=True` when constructing the async clients (`httpx.AsyncClient(follow_redirects=True)` in both `_build_client()` and `_build_flags_client()`) so async transport behavior matches the sync `requests.Session` default, or explicitly document/test the intentional divergence if redirects should not be followed.
</potential_solution>
Comment on lines
+212
to
+217
| elif send_feature_flags: | ||
| self.log.warning( | ||
| "AsyncClient.capture(send_feature_flags=...) will use the synchronous " | ||
| "feature flag path until async feature flags are enabled. Prefer passing " | ||
| "flags=await client.evaluate_flags(...)." | ||
| ) |
There was a problem hiding this comment.
Deprecated send_feature_flags warning logs unconditionally on every capture() call
Why we think it's a valid issue
- Checked: The async warning site (async_client.py:212-217) and how the sync client emits its equivalent deprecation notice (client.py:1174-1182).
- Found: Confirmed the
self.log.warning(...)at async_client.py:213 fires inside theelif send_feature_flags:branch ofcapture(), i.e. once per captured event, with no dedup/rate-limit or per-instance guard. By contrast, the sync client emits its deprecation notice viawarnings.warn(..., DeprecationWarning, stacklevel=2)(client.py:1175-1182), which Python's warnings filter de-duplicates by default (roughly once per call-site). So the async version loses that natural dedup and converts a one-time deprecation notice into per-event WARNING spam. - Impact: Real but minor observability/cost issue: a high-volume integration still on the deprecated
send_feature_flags=True(exactly the audience this notice targets) gets one warning-level log line per event, inflating log volume/cost until migration completes. No functional or data impact. The suggested fix (log once per client / rate-limit) is small and standard — matching the sync client's already-deduplicated approach — so it is neither overengineering nor pure style, and the trigger is concrete rather than speculative. This is the 'real-but-minor' caseconsiderexists for; the reviewer's priority is appropriate.
Issue description
Every call to capture(..., send_feature_flags=True) emits a self.log.warning(...) about the deprecated sync fallback path. Since this fires per captured event rather than once per client/process, a legacy caller migrating an existing high-volume integration (where send_feature_flags=True was the pre-existing norm) will get one warning-level log line per event, which can meaningfully inflate log volume/cost in production before the migration to flags= is complete.
Suggested fix
Rate-limit or de-duplicate this warning (e.g. log it once per client instance, or on a periodic basis) instead of on every capture() invocation, so the deprecation notice doesn't become log spam under load.
Prompt to fix with AI (copy-paste)
## Context
@posthog/async_client.py#L212-217
<issue_description>
Every call to `capture(..., send_feature_flags=True)` emits a `self.log.warning(...)` about the deprecated sync fallback path. Since this fires per captured event rather than once per client/process, a legacy caller migrating an existing high-volume integration (where `send_feature_flags=True` was the pre-existing norm) will get one warning-level log line per event, which can meaningfully inflate log volume/cost in production before the migration to `flags=` is complete.
</issue_description>
<issue_validation>
- **Checked:** The async warning site (async_client.py:212-217) and how the sync client emits its equivalent deprecation notice (client.py:1174-1182).
- **Found:** Confirmed the `self.log.warning(...)` at async_client.py:213 fires inside the `elif send_feature_flags:` branch of `capture()`, i.e. once per captured event, with no dedup/rate-limit or per-instance guard. By contrast, the sync client emits its deprecation notice via `warnings.warn(..., DeprecationWarning, stacklevel=2)` (client.py:1175-1182), which Python's warnings filter de-duplicates by default (roughly once per call-site). So the async version loses that natural dedup and converts a one-time deprecation notice into per-event WARNING spam.
- **Impact:** Real but minor observability/cost issue: a high-volume integration still on the deprecated `send_feature_flags=True` (exactly the audience this notice targets) gets one warning-level log line per event, inflating log volume/cost until migration completes. No functional or data impact. The suggested fix (log once per client / rate-limit) is small and standard — matching the sync client's already-deduplicated approach — so it is neither overengineering nor pure style, and the trigger is concrete rather than speculative. This is the 'real-but-minor' case `consider` exists for; the reviewer's priority is appropriate.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Rate-limit or de-duplicate this warning (e.g. log it once per client instance, or on a periodic basis) instead of on every capture() invocation, so the deprecation notice doesn't become log spam under load.
</potential_solution>
Comment on lines
+187
to
+233
| async def capture( | ||
| self, event: str, **kwargs: Unpack[OptionalCaptureArgs] | ||
| ) -> Optional[str]: | ||
| try: | ||
| distinct_id = kwargs.get("distinct_id", None) | ||
| properties = kwargs.get("properties", None) | ||
| timestamp = kwargs.get("timestamp", None) | ||
| uuid = kwargs.get("uuid", None) | ||
| groups = kwargs.get("groups", None) | ||
| flags_snapshot = kwargs.get("flags", None) | ||
| send_feature_flags = kwargs.get("send_feature_flags", False) | ||
| disable_geoip = kwargs.get("disable_geoip", None) | ||
|
|
||
| msg, distinct_id = self._build_capture_message( | ||
| event, distinct_id, properties, timestamp, uuid, groups | ||
| ) | ||
|
|
||
| extra_properties: dict[str, Any] = {} | ||
| if flags_snapshot is not None: | ||
| if send_feature_flags: | ||
| self.log.warning( | ||
| "[FEATURE FLAGS] Both `flags` and `send_feature_flags` were passed to " | ||
| "capture(); using `flags` and ignoring `send_feature_flags`." | ||
| ) | ||
| extra_properties = flags_snapshot._get_event_properties() | ||
| elif send_feature_flags: | ||
| self.log.warning( | ||
| "AsyncClient.capture(send_feature_flags=...) will use the synchronous " | ||
| "feature flag path until async feature flags are enabled. Prefer passing " | ||
| "flags=await client.evaluate_flags(...)." | ||
| ) | ||
| return await self._capture_with_sync_feature_flags( | ||
| msg, | ||
| distinct_id, | ||
| groups, | ||
| send_feature_flags, | ||
| disable_geoip, | ||
| ) | ||
|
|
||
| self._apply_capture_properties(msg, extra_properties) | ||
|
|
||
| return await self._enqueue(msg, disable_geoip) | ||
| except Exception as e: | ||
| if self.debug: | ||
| raise | ||
| self.log.exception(f"Error in capture: {e}") | ||
| return None |
There was a problem hiding this comment.
AsyncClient overrides capture()/capture_exception() as coroutines while inheriting sync Client methods that call them synchronously, silently dropping events
Why we think it's a valid issue
- Checked: Both concrete call sites — the
ExceptionCapturewiring (async_client.py:91-115 forwardingenable_exception_autocapture; client.py:471-478 buildingExceptionCapture(self, ...); exception_capture.py:37-39, 81) and the inherited feature-flag path (_capture_feature_flag_called_if_needed, client.py:2618-2655). Also confirmedAsyncClientoverrides neitherget_feature_flagnor_capture_feature_flag_called_if_neededand does not guard/disable the exception_capture wiring. - Found — breakage (1):
AsyncClient.__init__forwardsenable_exception_autocapturetosuper().__init__(async_client.py:115), which constructsExceptionCapture(self, ...)(client.py:471-478) installingsys.excepthook/threading.excepthook(exception_capture.py:37-39). Those handlers callself.client.capture_exception(...)with noawait(exception_capture.py:81); sinceAsyncClient.capture_exceptionisasync def(async_client.py:353), a coroutine is built and discarded — the body never runs, so exceptions are never captured (only a GC-timeRuntimeWarning).shutdown()even callsself.exception_capture.close()(async_client.py:544), confirming the wiring is live and unguarded. - Found — breakage (2):
get_feature_flagis inherited unchanged and calls_capture_feature_flag_called_if_needed, which invokesself.capture("$feature_flag_called", ...)synchronously (client.py:2648). On anAsyncClient,self.captureisasync def(async_client.py:187), so the coroutine is discarded and the event is silently dropped — whilereported_flags.add(...)(client.py:2655) still marks it reported, so it won't retry. Reachable now: async flag evaluation is deferred to a later PR, so the inherited syncget_feature_flag()is the only flag API on the async client. - Impact: Two confirmed silent data-loss defects reachable via public/documented API (
enable_exception_autocapture=True;get_feature_flag()): exception autocapture never fires and$feature_flag_calledevents vanish, with no error beyond an easily-missedRuntimeWarning. Concrete triggers and consequences, directly caused by the PR's subclassing design. This is the concrete counterpart to the separately-reported LSP finding but stands on its own with demonstrable call sites; must_fix is warranted.
Issue description
AsyncClient(Client) overrides capture (line 187) and capture_exception (line 353) to be async def, but it inherits many Client methods unchanged that call self.capture(...) / self.client.capture_exception(...) synchronously, expecting the old fire-and-forget semantics. Two concrete, demonstrable breakages: (1) enable_exception_autocapture=True is a documented constructor parameter forwarded straight through to super().__init__() (line 115), which builds self.exception_capture = ExceptionCapture(self, ...). ExceptionCapture.__init__ installs sys.excepthook/threading.excepthook (posthog/exception_capture.py:37-39), and its handlers call self.client.capture_exception(exception, distinct_id=distinct_id) (posthog/exception_capture.py:81) with no await. Because AsyncClient.capture_exception is now a coroutine function, this line only constructs a coroutine object and discards it — the body never runs, so no exception is ever captured, and the only symptom is an easily-missed RuntimeWarning: coroutine 'AsyncClient.capture_exception' was never awaited at GC time. (2) Client.get_feature_flag() (inherited, not overridden) calls _capture_feature_flag_called_if_needed() (posthog/client.py:2618), which calls self.capture("$feature_flag_called", ...) synchronously (posthog/client.py:2648) — same silent no-op on an AsyncClient instance. This is a Liskov-substitution violation: AsyncClient is not safely usable everywhere a Client is expected, and every inherited code path that fires events synchronously breaks quietly instead of raising.
Suggested fix
Either (a) don't let AsyncClient inherit call sites that assume synchronous capture/capture_exception — override or disable get_feature_flag's auto-capture path and the ExceptionCapture wiring for AsyncClient, replacing them with async-safe scheduling (e.g. asyncio.create_task(self.capture(...)) from within the sync hook, guarded so it can't run outside a live loop), or (b) keep capture/capture_exception's outward call signature synchronous (a cheap buffer write, as suggested in the existing captureImmediate discussion on this PR) so inherited sync call sites keep working, exposing the awaitable-flush behavior via a separate method. At minimum, add a loud runtime guard/log when a sync caller invokes these coroutine methods without awaiting them, so the failure isn't silent.
Prompt to fix with AI (copy-paste)
## Context
@posthog/async_client.py#L187-233
@posthog/async_client.py#L353-357
<issue_description>
`AsyncClient(Client)` overrides `capture` (line 187) and `capture_exception` (line 353) to be `async def`, but it inherits many `Client` methods unchanged that call `self.capture(...)` / `self.client.capture_exception(...)` **synchronously**, expecting the old fire-and-forget semantics. Two concrete, demonstrable breakages: (1) `enable_exception_autocapture=True` is a documented constructor parameter forwarded straight through to `super().__init__()` (line 115), which builds `self.exception_capture = ExceptionCapture(self, ...)`. `ExceptionCapture.__init__` installs `sys.excepthook`/`threading.excepthook` (posthog/exception_capture.py:37-39), and its handlers call `self.client.capture_exception(exception, distinct_id=distinct_id)` (posthog/exception_capture.py:81) with no `await`. Because `AsyncClient.capture_exception` is now a coroutine function, this line only constructs a coroutine object and discards it — the body never runs, so no exception is ever captured, and the only symptom is an easily-missed `RuntimeWarning: coroutine 'AsyncClient.capture_exception' was never awaited` at GC time. (2) `Client.get_feature_flag()` (inherited, not overridden) calls `_capture_feature_flag_called_if_needed()` (posthog/client.py:2618), which calls `self.capture("$feature_flag_called", ...)` synchronously (posthog/client.py:2648) — same silent no-op on an `AsyncClient` instance. This is a Liskov-substitution violation: `AsyncClient` is not safely usable everywhere a `Client` is expected, and every inherited code path that fires events synchronously breaks quietly instead of raising.
</issue_description>
<issue_validation>
- **Checked:** Both concrete call sites — the `ExceptionCapture` wiring (async_client.py:91-115 forwarding `enable_exception_autocapture`; client.py:471-478 building `ExceptionCapture(self, ...)`; exception_capture.py:37-39, 81) and the inherited feature-flag path (`_capture_feature_flag_called_if_needed`, client.py:2618-2655). Also confirmed `AsyncClient` overrides neither `get_feature_flag` nor `_capture_feature_flag_called_if_needed` and does not guard/disable the exception_capture wiring.
- **Found — breakage (1):** `AsyncClient.__init__` forwards `enable_exception_autocapture` to `super().__init__` (async_client.py:115), which constructs `ExceptionCapture(self, ...)` (client.py:471-478) installing `sys.excepthook`/`threading.excepthook` (exception_capture.py:37-39). Those handlers call `self.client.capture_exception(...)` with no `await` (exception_capture.py:81); since `AsyncClient.capture_exception` is `async def` (async_client.py:353), a coroutine is built and discarded — the body never runs, so exceptions are never captured (only a GC-time `RuntimeWarning`). `shutdown()` even calls `self.exception_capture.close()` (async_client.py:544), confirming the wiring is live and unguarded.
- **Found — breakage (2):** `get_feature_flag` is inherited unchanged and calls `_capture_feature_flag_called_if_needed`, which invokes `self.capture("$feature_flag_called", ...)` synchronously (client.py:2648). On an `AsyncClient`, `self.capture` is `async def` (async_client.py:187), so the coroutine is discarded and the event is silently dropped — while `reported_flags.add(...)` (client.py:2655) still marks it reported, so it won't retry. Reachable now: async flag evaluation is deferred to a later PR, so the inherited sync `get_feature_flag()` is the only flag API on the async client.
- **Impact:** Two confirmed silent data-loss defects reachable via public/documented API (`enable_exception_autocapture=True`; `get_feature_flag()`): exception autocapture never fires and `$feature_flag_called` events vanish, with no error beyond an easily-missed `RuntimeWarning`. Concrete triggers and consequences, directly caused by the PR's subclassing design. This is the concrete counterpart to the separately-reported LSP finding but stands on its own with demonstrable call sites; must_fix is warranted.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Either (a) don't let `AsyncClient` inherit call sites that assume synchronous `capture`/`capture_exception` — override or disable `get_feature_flag`'s auto-capture path and the `ExceptionCapture` wiring for `AsyncClient`, replacing them with async-safe scheduling (e.g. `asyncio.create_task(self.capture(...))` from within the sync hook, guarded so it can't run outside a live loop), or (b) keep `capture`/`capture_exception`'s outward call signature synchronous (a cheap buffer write, as suggested in the existing `captureImmediate` discussion on this PR) so inherited sync call sites keep working, exposing the awaitable-flush behavior via a separate method. At minimum, add a loud runtime guard/log when a sync caller invokes these coroutine methods without awaiting them, so the failure isn't silent.
</potential_solution>
| pass | ||
|
|
||
|
|
||
| class AsyncPostHog(AsyncClient): |
There was a problem hiding this comment.
New public class AsyncPostHog diverges from the established Posthog naming convention (capital H vs lowercase h)
Why we think it's a valid issue
- Checked: Both public alias definitions in
posthog/__init__.py(class Posthog(Client)at line 1203;class AsyncPostHog(AsyncClient)at line 1215), whether an__all__gates exports, and the PR's own description/test command. - Found: The inconsistency is real and in the same file — the established sync alias is
Posthog(single capital, line 1203) while the new async alias isAsyncPostHog(capital P and H, line 1215). No__all__restricts exports, so both are top-level public importable classes, and the PR runscheck_public_api.py, confirming this is tracked public API surface. Concrete confusion is already demonstrated: the PR body ('introducesAsyncClient/AsyncPosthog') and its verification commandfrom posthog import AsyncPosthoguse the lowercase-h spelling, which would raiseImportErroragainst the implementedAsyncPostHog— the author's own docs disagree with the code. - Impact: Above 'pure style/taste': it is a public API identifier that becomes permanent on release (renaming later requires a deprecation cycle), it is inconsistent with its sibling alias in the same module, and the mismatch has already produced a wrong import in the PR's own description — evidence the footgun is real, not hypothetical. Naming consistency is squarely within the SDK-guidelines perspective. The fix is trivial and the window is now (draft, pre-release), which justifies should_fix (resolve before the public API ships) rather than a parked consider; it is not a functional must_fix.
Issue description
The existing public sync alias is class Posthog(Client) (posthog/init.py:1203) — only the initial "P" is capitalized, matching PostHog's own SDK convention for this class name. The new async alias is defined as class AsyncPostHog(AsyncClient) (line 1215), capitalizing both "P" and "H" ("PostHog" brand-style) — a different, inconsistent spelling from its sibling class in the very same file. This is exactly the kind of cross-SDK/naming inconsistency the SDK guidelines call out: users porting examples between the sync and async client, or typing from memory, are very likely to get the capitalization wrong. This isn't hypothetical — the PR's own description and test command (from posthog import AsyncPosthog) uses the other spelling than what was actually implemented, which would itself raise ImportError if run literally as written.
Suggested fix
Rename the class to AsyncPosthog for consistency with the existing Posthog alias's capitalization (or, if AsyncPostHog is intentional, rename Posthog too in a follow-up so the two aliases agree) before this ships as public API — once released, the class name is effectively permanent.
Prompt to fix with AI (copy-paste)
## Context
@posthog/__init__.py#L1215
<issue_description>
The existing public sync alias is `class Posthog(Client)` (posthog/__init__.py:1203) — only the initial "P" is capitalized, matching PostHog's own SDK convention for this class name. The new async alias is defined as `class AsyncPostHog(AsyncClient)` (line 1215), capitalizing both "P" and "H" ("PostHog" brand-style) — a different, inconsistent spelling from its sibling class in the very same file. This is exactly the kind of cross-SDK/naming inconsistency the SDK guidelines call out: users porting examples between the sync and async client, or typing from memory, are very likely to get the capitalization wrong. This isn't hypothetical — the PR's own description and test command (`from posthog import AsyncPosthog`) uses the *other* spelling than what was actually implemented, which would itself raise `ImportError` if run literally as written.
</issue_description>
<issue_validation>
- **Checked:** Both public alias definitions in `posthog/__init__.py` (`class Posthog(Client)` at line 1203; `class AsyncPostHog(AsyncClient)` at line 1215), whether an `__all__` gates exports, and the PR's own description/test command.
- **Found:** The inconsistency is real and in the same file — the established sync alias is `Posthog` (single capital, line 1203) while the new async alias is `AsyncPostHog` (capital P and H, line 1215). No `__all__` restricts exports, so both are top-level public importable classes, and the PR runs `check_public_api.py`, confirming this is tracked public API surface. Concrete confusion is already demonstrated: the PR body ('introduces `AsyncClient` / `AsyncPosthog`') and its verification command `from posthog import AsyncPosthog` use the lowercase-h spelling, which would raise `ImportError` against the implemented `AsyncPostHog` — the author's own docs disagree with the code.
- **Impact:** Above 'pure style/taste': it is a public API identifier that becomes permanent on release (renaming later requires a deprecation cycle), it is inconsistent with its sibling alias in the same module, and the mismatch has already produced a wrong import in the PR's own description — evidence the footgun is real, not hypothetical. Naming consistency is squarely within the SDK-guidelines perspective. The fix is trivial and the window is now (draft, pre-release), which justifies should_fix (resolve before the public API ships) rather than a parked consider; it is not a functional must_fix.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Rename the class to `AsyncPosthog` for consistency with the existing `Posthog` alias's capitalization (or, if `AsyncPostHog` is intentional, rename `Posthog` too in a follow-up so the two aliases agree) before this ships as public API — once released, the class name is effectively permanent.
</potential_solution>
Comment on lines
+212
to
+217
| elif send_feature_flags: | ||
| self.log.warning( | ||
| "AsyncClient.capture(send_feature_flags=...) will use the synchronous " | ||
| "feature flag path until async feature flags are enabled. Prefer passing " | ||
| "flags=await client.evaluate_flags(...)." | ||
| ) |
There was a problem hiding this comment.
AsyncClient's own capture() warning recommends await client.evaluate_flags(...), but evaluate_flags() is a synchronous method that doesn't exist as an async API anywhere in the codebase
Why we think it's a valid issue
- Checked: The warning text (async_client.py:213-217), the
evaluate_flagsdefinition (client.py:2844), a repo-wide search for anyasync def evaluate_flags, whetherAsyncClientoverrides it, and the sync client's equivalent deprecation wording (client.py:1175-1182). - Found: The message tells callers
Prefer passing flags=await client.evaluate_flags(...).Client.evaluate_flags(client.py:2844) is a plaindefreturningFeatureFlagEvaluationsdirectly (signature ends-> FeatureFlagEvaluations), and a repo-wide grep forasync def evaluate_flagsreturns nothing — there is no awaitable version, andAsyncClientdoes not override it. Soawait client.evaluate_flags(...)raisesTypeError: object FeatureFlagEvaluations can't be used in 'await' expression. The sync client's own deprecation message saysposthog.evaluate_flags(...)WITHOUTawait(client.py:1177-1178) — the async message introduced the incorrectawait. - Impact: Real, PR-introduced defect in user-facing guidance: the message fires on every
capture(send_feature_flags=True)call (the exact migration audience this PR targets), and following its snippet verbatim fails immediately with TypeError; dropping theawaitinstead runs a blocking sync/flagscall on the event loop (the loop-blocking issue flagged elsewhere), with noto_threadat this site. It references an async API shape that does not exist today (async FF eval is stacked in a later PR). Not speculative-unreachable (following documented guidance is expected) and not pure style — it is incorrect, actionable guidance that breaks on execution. Trivial fix; the reviewer's should_fix is appropriate.
Issue description
When send_feature_flags is passed without a flags snapshot, capture() logs: "...Prefer passing flags=await client.evaluate_flags(...)." Client.evaluate_flags() (posthog/client.py:2844) is a plain def, not async def, and there is no override anywhere in async_client.py (or elsewhere in the repo — verified there is no async def evaluate_flags at all). It returns a FeatureFlagEvaluations value directly, not a coroutine. A caller who follows this guidance literally, flags=await client.evaluate_flags(...), gets an immediate TypeError: object FeatureFlagEvaluations can't be used in 'await' expression. If they instead drop the (message-mandated) await and call it synchronously, they reintroduce exactly the "blocking sync HTTP call freezes the event loop" defect that the sibling P1 review comment on this same file asked to have fixed for the send_feature_flags fallback — except here there's no asyncio.to_thread wrapper at all, because this call site isn't actually exercised anywhere internally; it only exists as guidance text shown to every caller still using the very common send_feature_flags=True argument this PR is trying to migrate people away from.
Suggested fix
Either implement an async evaluate_flags (or wrap the sync one in asyncio.to_thread as the recommended snippet) before shipping this message, or change the log text to something that actually works today, e.g. flags=await asyncio.to_thread(client.evaluate_flags, ...), until the async feature-flag layer (stacked in the next PR) lands.
Prompt to fix with AI (copy-paste)
## Context
@posthog/async_client.py#L212-217
<issue_description>
When `send_feature_flags` is passed without a `flags` snapshot, `capture()` logs: "...Prefer passing flags=await client.evaluate_flags(...)." `Client.evaluate_flags()` (posthog/client.py:2844) is a plain `def`, not `async def`, and there is no override anywhere in `async_client.py` (or elsewhere in the repo — verified there is no `async def evaluate_flags` at all). It returns a `FeatureFlagEvaluations` value directly, not a coroutine. A caller who follows this guidance literally, `flags=await client.evaluate_flags(...)`, gets an immediate `TypeError: object FeatureFlagEvaluations can't be used in 'await' expression`. If they instead drop the (message-mandated) `await` and call it synchronously, they reintroduce exactly the "blocking sync HTTP call freezes the event loop" defect that the sibling P1 review comment on this same file asked to have fixed for the `send_feature_flags` fallback — except here there's no `asyncio.to_thread` wrapper at all, because this call site isn't actually exercised anywhere internally; it only exists as guidance text shown to every caller still using the very common `send_feature_flags=True` argument this PR is trying to migrate people away from.
</issue_description>
<issue_validation>
- **Checked:** The warning text (async_client.py:213-217), the `evaluate_flags` definition (client.py:2844), a repo-wide search for any `async def evaluate_flags`, whether `AsyncClient` overrides it, and the sync client's equivalent deprecation wording (client.py:1175-1182).
- **Found:** The message tells callers `Prefer passing flags=await client.evaluate_flags(...)`. `Client.evaluate_flags` (client.py:2844) is a plain `def` returning `FeatureFlagEvaluations` directly (signature ends `-> FeatureFlagEvaluations`), and a repo-wide grep for `async def evaluate_flags` returns nothing — there is no awaitable version, and `AsyncClient` does not override it. So `await client.evaluate_flags(...)` raises `TypeError: object FeatureFlagEvaluations can't be used in 'await' expression`. The sync client's own deprecation message says `posthog.evaluate_flags(...)` WITHOUT `await` (client.py:1177-1178) — the async message introduced the incorrect `await`.
- **Impact:** Real, PR-introduced defect in user-facing guidance: the message fires on every `capture(send_feature_flags=True)` call (the exact migration audience this PR targets), and following its snippet verbatim fails immediately with TypeError; dropping the `await` instead runs a blocking sync `/flags` call on the event loop (the loop-blocking issue flagged elsewhere), with no `to_thread` at this site. It references an async API shape that does not exist today (async FF eval is stacked in a later PR). Not speculative-unreachable (following documented guidance is expected) and not pure style — it is incorrect, actionable guidance that breaks on execution. Trivial fix; the reviewer's should_fix is appropriate.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Either implement an async `evaluate_flags` (or wrap the sync one in `asyncio.to_thread` as the recommended snippet) before shipping this message, or change the log text to something that actually works today, e.g. `flags=await asyncio.to_thread(client.evaluate_flags, ...)`, until the async feature-flag layer (stacked in the next PR) lands.
</potential_solution>
Comment on lines
+205
to
+224
| if flags_snapshot is not None: | ||
| if send_feature_flags: | ||
| self.log.warning( | ||
| "[FEATURE FLAGS] Both `flags` and `send_feature_flags` were passed to " | ||
| "capture(); using `flags` and ignoring `send_feature_flags`." | ||
| ) | ||
| extra_properties = flags_snapshot._get_event_properties() | ||
| elif send_feature_flags: | ||
| self.log.warning( | ||
| "AsyncClient.capture(send_feature_flags=...) will use the synchronous " | ||
| "feature flag path until async feature flags are enabled. Prefer passing " | ||
| "flags=await client.evaluate_flags(...)." | ||
| ) | ||
| return await self._capture_with_sync_feature_flags( | ||
| msg, | ||
| distinct_id, | ||
| groups, | ||
| send_feature_flags, | ||
| disable_geoip, | ||
| ) |
There was a problem hiding this comment.
capture() branches on raw truthiness of send_feature_flags instead of the parsed should_send flag, silently no-op'ing for an empty-dict options payload
Why we think it's a valid issue
- Checked: Sync
Client.capture()flag gating (client.py:1172-1174),_parse_send_feature_flagsreturn values (client.py:1251-1268), and the asynccapture()branch (async_client.py:196-224). - Found — premise verified: Sync gates on
flag_options["should_send"]after parsing (client.py:1174), and_parse_send_feature_flagsreturnsshould_send: Truefor any dict, including{}(client.py:1251-1260 —isinstance(send_feature_flags, dict)→should_send: True). Async instead useselif send_feature_flags:on the raw arg (async_client.py:212), parsing only later inside_capture_with_sync_feature_flags. Sincebool({})isFalse,capture(..., send_feature_flags={})skips the flag path entirely on async while sync evaluates-with-defaults. The divergence is precisely scoped: bool inputs and non-empty dicts behave identically (a truthy dict enters the branch, then gets parsed), so only the empty-dict case diverges — silently, with no warning. - Impact: Real, verified correctness/parity divergence for a valid documented input (
SendFeatureFlagsOptions == {}means 'evaluate with default options'), reachable via programmatic option construction (opts = {}then conditionally populated). Flag properties are silently omitted from the event on async only. It is an edge case on a deprecated path (low impact), but the fix is trivial and unambiguously correct — parse once up front and branch onshould_send, matching sync — so it is not a 'not worth the code' case, nor speculative/style. Fits the reviewer'sconsider: real but minor, kept on record.
Issue description
Sync Client.capture() (posthog/client.py:1161-1235) always calls flag_options = self._parse_send_feature_flags(send_feature_flags) and gates flag evaluation on flag_options["should_send"]; per _parse_send_feature_flags (client.py:1237-1273), ANY dict input — including an empty {} — unconditionally yields should_send: True. AsyncClient.capture() instead branches directly on elif send_feature_flags: (line 212), testing the raw argument's truthiness before _parse_send_feature_flags is ever called (it only runs later, inside _capture_with_sync_feature_flags). Since bool({}) is False in Python, calling capture(..., send_feature_flags={}) — a legitimate way under the documented SendFeatureFlagsOptions contract to say "evaluate with default options" — silently skips flag evaluation entirely on AsyncClient with no warning, while the identical call against a plain Client evaluates and attaches flags as expected.
Suggested fix
Call self._parse_send_feature_flags(send_feature_flags) once up front in capture() and branch on flag_options["should_send"], matching the sync implementation, instead of testing the raw parameter's truthiness.
Prompt to fix with AI (copy-paste)
## Context
@posthog/async_client.py#L205-224
<issue_description>
Sync `Client.capture()` (posthog/client.py:1161-1235) always calls `flag_options = self._parse_send_feature_flags(send_feature_flags)` and gates flag evaluation on `flag_options["should_send"]`; per `_parse_send_feature_flags` (client.py:1237-1273), ANY dict input — including an empty `{}` — unconditionally yields `should_send: True`. `AsyncClient.capture()` instead branches directly on `elif send_feature_flags:` (line 212), testing the raw argument's truthiness before `_parse_send_feature_flags` is ever called (it only runs later, inside `_capture_with_sync_feature_flags`). Since `bool({})` is `False` in Python, calling `capture(..., send_feature_flags={})` — a legitimate way under the documented `SendFeatureFlagsOptions` contract to say "evaluate with default options" — silently skips flag evaluation entirely on `AsyncClient` with no warning, while the identical call against a plain `Client` evaluates and attaches flags as expected.
</issue_description>
<issue_validation>
- **Checked:** Sync `Client.capture()` flag gating (client.py:1172-1174), `_parse_send_feature_flags` return values (client.py:1251-1268), and the async `capture()` branch (async_client.py:196-224).
- **Found — premise verified:** Sync gates on `flag_options["should_send"]` after parsing (client.py:1174), and `_parse_send_feature_flags` returns `should_send: True` for any dict, including `{}` (client.py:1251-1260 — `isinstance(send_feature_flags, dict)` → `should_send: True`). Async instead uses `elif send_feature_flags:` on the raw arg (async_client.py:212), parsing only later inside `_capture_with_sync_feature_flags`. Since `bool({})` is `False`, `capture(..., send_feature_flags={})` skips the flag path entirely on async while sync evaluates-with-defaults. The divergence is precisely scoped: bool inputs and non-empty dicts behave identically (a truthy dict enters the branch, then gets parsed), so only the empty-dict case diverges — silently, with no warning.
- **Impact:** Real, verified correctness/parity divergence for a valid documented input (`SendFeatureFlagsOptions == {}` means 'evaluate with default options'), reachable via programmatic option construction (`opts = {}` then conditionally populated). Flag properties are silently omitted from the event on async only. It is an edge case on a deprecated path (low impact), but the fix is trivial and unambiguously correct — parse once up front and branch on `should_send`, matching sync — so it is not a 'not worth the code' case, nor speculative/style. Fits the reviewer's `consider`: real but minor, kept on record.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Call `self._parse_send_feature_flags(send_feature_flags)` once up front in `capture()` and branch on `flag_options["should_send"]`, matching the sync implementation, instead of testing the raw parameter's truthiness.
</potential_solution>
# Conflicts: # posthog/client.py # references/public_api_snapshot.txt # uv.lock
| owns_client = client is None | ||
| http_client = client or _build_client() | ||
| try: | ||
| res = await http_client.post(url, data=data, headers=headers, timeout=timeout) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
Starts the stacked implementation for #103 by adding an asyncio-native client for event capture. This introduces
AsyncClient/AsyncPosthog, async HTTP helpers, and an asyncio queue/consumer pipeline so async applications can enqueue and flush events without daemon threads.This first PR intentionally focuses on the capture lifecycle and API shape. Async feature flag evaluation is stacked in the next PR. Review feedback was addressed by removing sensitive request/response payloads from async debug logs, running temporary sync flag fallback calls off the event loop, and parameterizing identify-method enqueue coverage.
💚 How did you test it?
uv run ruff check posthog/async_client.py posthog/test/test_async_client.pyuv run ruff check posthog/async_request.py posthog/test/test_async_request.pyuv run --extra test pytest posthog/test/test_async_client.py posthog/test/test_async_request.py --verbose --timeout=30uv run python -W error -c "import posthog; from posthog import AsyncPosthog"uv run --extra dev python .github/scripts/check_public_api.py📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented with pi. The stack is split so reviewers can first inspect the async capture/queue/request foundation, then the async feature flag layer, then docs.
httpxis added as an optionalposthog[async]dependency and imported lazily so normalimport posthogstill works without the extra.