fix(analytics): close _async_client on sync __exit__ and shutdown() (SDK-85) by tylerjroach · Pull Request #184 · mixpanel/mixpanel-python

tylerjroach · 2026-07-21T15:14:55Z

Summary

RemoteFeatureFlagsProvider and LocalFeatureFlagsProvider both had sync __exit__ and shutdown() methods that closed _sync_client but never _async_client. That left httpx's async connection pool + background transport dangling until gc — surfacing as ResourceWarning noise, and on long-lived processes as real file-descriptor exhaustion.

Adds a close_async_client_from_sync(httpx.AsyncClient) helper in mixpanel/flags/utils.py that handles both call sites:

No loop running (typical sync shutdown() / __exit__) → bridge via asgiref.async_to_sync and block until close completes.
Loop already running (e.g. someone calls shutdown() from inside an async function) → async_to_sync would RuntimeError here; schedule a background aclose() task and log a warning pointing at __aexit__ instead.

Also folded RemoteFeatureFlagsProvider.__exit__ into a call to shutdown() so the two sync-cleanup paths stay in sync, and made __aexit__ close both clients so the async path is symmetric.

Why the branch name lie

The three PRs that Linear was showing as attached to SDK-85 (Node #278, Java #91, Ruby #155) were all SDK-81 fixes — their branch names contained sdk-85 from an earlier naming mistake and Linear's linkback bot auto-attached them. Detached those attachments and made this the real fix.

Test plan

pytest mixpanel/flags/ — 103 tests pass (was 97; added 6)
The four new provider tests (test_sync_shutdown_closes_both_clients, test_sync_context_manager_exit_closes_both_clients in each of the two test files) fail on master and pass on this branch — verified by reverting just the lib changes locally
TestCloseAsyncClientFromSync covers both the loop-running and no-loop code paths on the helper itself

Linear: SDK-85

Both RemoteFeatureFlagsProvider and LocalFeatureFlagsProvider had sync __exit__ and shutdown() that closed only _sync_client, leaving _async_client's connection pool + background transport to leak until gc, which surfaced as ResourceWarning noise (and, on long-lived processes, real fd exhaustion). New helper `close_async_client_from_sync` in flags/utils.py handles both possible call sites: - No running event loop on the current thread (typical sync __exit__ / shutdown call site) → bridge to sync via asgiref async_to_sync and block until close completes. - A loop is already running (e.g. `shutdown()` invoked from an async function) → asgiref would raise RuntimeError here, so we schedule a background aclose() task and log a warning pointing callers at __aexit__ / awaiting aclose directly. Also folded RemoteFeatureFlagsProvider.__exit__ into a call to shutdown() to keep the two paths in sync, and made __aexit__ close _sync_client too so the async path is symmetric. New tests: - test_sync_shutdown_closes_both_clients - test_sync_context_manager_exit_closes_both_clients (one each in test_local_feature_flags.py and test_remote_feature_flags.py) - TestCloseAsyncClientFromSync covering both loop-running and no-loop code paths on the helper itself. Full flags suite: 103 pass. The 4 new provider tests fail on master.

linear-code · 2026-07-21T15:14:58Z

SDK-85

- Move `httpx` under TYPE_CHECKING (only used as a type hint). - Retain a hard reference to the fire-and-forget aclose() task in a module-level set with a done_callback that removes it once complete. Without the reference, the task can be gc'd before it runs.

greptile-apps · 2026-07-21T15:18:38Z

Confidence Score: 5/5

This looks safe to merge.

No additional blocking issues qualify for this follow-up review.

Important Files Changed

Filename	Overview
mixpanel/flags/utils.py	Adds the shared synchronous helper for closing async HTTP clients.
mixpanel/flags/local_feature_flags.py	Updates local provider shutdown and context-manager paths to close both clients.
mixpanel/flags/remote_feature_flags.py	Updates remote provider shutdown and context-manager paths to close both clients.

_{Reviews (5): Last reviewed commit: "Merge master into fix/sdk-85-close-async..." | Re-trigger Greptile}

codecov · 2026-07-21T15:35:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.25%. Comparing base (74d73f2) to head (ad048c9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
+ Coverage   96.09%   96.25%   +0.16%     
==========================================
  Files          13       13              
  Lines        2560     2616      +56     
  Branches      139      139              
==========================================
+ Hits         2460     2518      +58     
+ Misses         66       64       -2     
  Partials       34       34

Flag	Coverage Δ
openfeature-provider	`54.06% <20.00%> (-0.29%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Greptile P2 on #184: the done_callback was only discarding the task from _pending_aclose_tasks and never retrieving its exception. If aclose() raised on the executor thread, the caller saw shutdown() return successfully and the failure only surfaced as an anonymous "Task exception was never retrieved" line at gc time. Split the done_callback into _on_aclose_task_done which discards the task, ignores cancellation, and logs any exception at ERROR with type + message. Added test_logs_error_when_scheduled_aclose_raises that patches aclose to raise and asserts the error log is emitted; fails on the previous callback wiring.

tylerjroach · 2026-07-21T15:40:29Z

Pushed `f45d961` addressing the Greptile P2.

Split the task done_callback into _on_aclose_task_done which:

discards the task from _pending_aclose_tasks,
skips on cancellation,
retrieves any exception via task.exception() and logs it at ERROR ("Async HTTP client close failed: <type>: <msg>").

Otherwise the caller saw a successful shutdown() return and the failure only surfaced as an anonymous "Task exception was never retrieved" at gc time.

New test test_logs_error_when_scheduled_aclose_raises patches client.aclose() to raise RuntimeError, calls the helper from an async context, and asserts the error log fires with both the exception type and message. All 104 flag tests pass; lint + ruff format clean.

Copilot

Pull request overview

This PR fixes a resource-leak in the feature flags providers by ensuring the underlying httpx.AsyncClient is closed in the synchronous cleanup paths (__exit__ and shutdown()), preventing dangling async connection pools/transports that can trigger ResourceWarning and potentially exhaust file descriptors in long-lived processes.

Changes:

Added a close_async_client_from_sync() helper to close an httpx.AsyncClient from sync code, with special handling when an event loop is already running.
Updated RemoteFeatureFlagsProvider and LocalFeatureFlagsProvider sync cleanup (shutdown() / __exit__) to close both sync and async HTTP clients, and made async cleanup (__aexit__) symmetric.
Added unit tests covering the helper behavior and ensuring both clients are closed in sync shutdown/context-manager exit for both providers.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
mixpanel/flags/utils.py	Adds a sync-bridge helper to close `httpx.AsyncClient`, including loop-running behavior and task lifecycle handling.
mixpanel/flags/remote_feature_flags.py	Ensures `shutdown()`/`__exit__` close both clients; aligns `__exit__` with `shutdown()` and makes `__aexit__` close both clients.
mixpanel/flags/local_feature_flags.py	Ensures `shutdown()`/`__exit__` close both clients; makes `__aexit__` close both clients after stopping async polling.
mixpanel/flags/test_utils.py	Adds tests for `close_async_client_from_sync()` covering both loop/no-loop paths and error logging on task failure.
mixpanel/flags/test_remote_feature_flags.py	Adds tests verifying sync shutdown and sync context-manager exit close both HTTP clients.
mixpanel/flags/test_local_feature_flags.py	Adds tests verifying sync shutdown and sync context-manager exit close both HTTP clients.

…SDK-85) The prior running-loop branch scheduled a fire-and-forget aclose() on the current event loop. That has three problems the reviewers flagged: - Greptile P1: loop teardown can cancel the task before aclose finishes, leaving the same resource leak this change was meant to prevent. - Copilot: LocalFeatureFlagsProvider.shutdown() from an async context would schedule aclose() on _async_client while _async_polling_task was still using it (shutdown() only stops the sync polling thread). - Ketan (Option A): shutdown() returning success while cleanup is still pending is a misleading contract; better to force async callers onto __aexit__. Rewrites close_async_client_from_sync to raise RuntimeError when a loop is already running, pointing callers at __aexit__ / awaiting aclose() directly. Removes _pending_aclose_tasks, _on_aclose_task_done, the module logger, and the fire-and-forget scheduling — they only existed to make the (unsafe) running-loop path work. test_local_flags_async_with_service_account_credentials was calling provider.shutdown() from async context; the old fire-and-forget behavior masked this, the new contract surfaces it. Switched it to await provider.__aexit__(None, None, None). Replaced test_schedules_close_and_warns_when_loop_running and test_logs_error_when_scheduled_aclose_raises with test_raises_when_called_from_running_loop. 103 flag tests pass; ruff check + format clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Resolves conflicts with #181 (SDK-80 exposure_executor / dispatch_exposure) which landed on master after this branch was opened: - utils.py: union of imports (asyncio + asgiref from HEAD, logging + Executor/Future/Callable from master); both TYPE_CHECKING imports and the module-level logger kept. - local_feature_flags.py, remote_feature_flags.py: union of the .utils imports (both close_async_client_from_sync and dispatch_exposure). - test_utils.py: extended TestUtils with the dispatch_exposure / _log_tracker_future_exception tests from master, then appended the new TestCloseAsyncClientFromSync class from HEAD. 112 flag tests pass; ruff check + format clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

tylerjroach requested review from a team and tdumitrescu July 21, 2026 15:14

greptile-apps Bot reviewed Jul 21, 2026

View reviewed changes

Comment thread mixpanel/flags/utils.py Outdated

Comment thread mixpanel/flags/utils.py Outdated

style: ruff format mixpanel/flags/utils.py

913d3db

ketanmixpanel requested a review from Copilot July 22, 2026 05:05

Copilot started reviewing on behalf of ketanmixpanel July 22, 2026 05:05 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread mixpanel/flags/local_feature_flags.py

Comment thread mixpanel/flags/utils.py Outdated

ketanmixpanel reviewed Jul 22, 2026

View reviewed changes

Comment thread mixpanel/flags/utils.py

ketanmixpanel previously approved these changes Jul 23, 2026

View reviewed changes

tylerjroach dismissed ketanmixpanel’s stale review via ad048c9 July 23, 2026 18:08

ketanmixpanel approved these changes Jul 23, 2026

View reviewed changes

tylerjroach merged commit 8dece95 into master Jul 23, 2026
35 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(analytics): close _async_client on sync exit and shutdown() (SDK-85)#184

fix(analytics): close _async_client on sync exit and shutdown() (SDK-85)#184
tylerjroach merged 6 commits into
masterfrom
fix/sdk-85-close-async-client-on-sync-exit

tylerjroach commented Jul 21, 2026

Uh oh!

linear-code Bot commented Jul 21, 2026

Uh oh!

greptile-apps Bot commented Jul 21, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

tylerjroach commented Jul 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tylerjroach commented Jul 21, 2026

Summary

Why the branch name lie

Test plan

Uh oh!

linear-code Bot commented Jul 21, 2026

Uh oh!

greptile-apps Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tylerjroach commented Jul 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Jul 21, 2026 •

edited

Loading

codecov Bot commented Jul 21, 2026 •

edited

Loading