Azure / Azure/azure-sdk-for-python

[Cosmos] Async refresh_task is not cleared after failed/cancelled health-check, causing sticky exception on every subsequent refresh

Open
#46,483 0 comments 2 reactions 2 assignees Claimed by @simorenoh View on GitHub
bug Client Cosmos
Dominant language
Python
Stars
5.6k
Forks
3.4k
Avg merge
1d 21h
Merged PRs (30d)
193

Description

### Description

`GlobalEndpointManager.refresh_endpoint_list` (async) only clears `self.refresh_task` on the success path. If the awaited task raised (`Exception` or `CancelledError`), the slot keeps pointing at the already-completed-with-exception task, and every subsequent call re-awaits it — re-raising the same stale exception forever (until the process restarts or another code path overwrites the slot).

This sits on the hot account-refresh path, so a single transient failure in the background health-check task can turn into recurring log spam and/or repeatedly fail legitimate refresh checks for the lifetime of the client.

Discovered while reviewing #46459 — **this bug pre-exists that PR**; #46459 does not introduce or worsen it.

### Repro / Code pointer

`sdk/cosmos/azure-cosmos/azure/cosmos/aio/_global_endpoint_manager_async.py`, around lines 148–155 on `main`:

```python
async def refresh_endpoint_list(self, database_account, **kwargs):
if self.refresh_task and self.refresh_task.done():
try:
await self.refresh_task
self.refresh_task = None # ← only cleared on success
except (Exception, asyncio.CancelledError) as exception: # pylint: disable=broad-exception-caught
logger.error("Health check task failed: %s", exception, exc_info=True)
# ← self.refresh_task is NOT cleared here → sticky reference
```

### Expected behaviour

After the awaited task is observed (success or failure), `self.refresh_task` should be cleared so the next call can either short-circuit (no pending task) or schedule a fresh one.

### Suggested fix

Move the clear into a `finally`:

```python
async def refresh_endpoint_list(self, database_account, **kwargs):
if self.refresh_task and self.refresh_task.done():
try:
await self.refresh_task
except (Exception, asyncio.CancelledError) as exception: # pylint: disable=broad-exception-caught
logger.error("Health check task failed: %s", exception, exc_info=True)
finally:
self.refresh_task = None
...
```

### Test idea

Async unit test that:
1. Assigns a pre-completed-with-exception task to `manager.refresh_task`.
2. Calls `await manager.refresh_endpoint_list(...)` twice.
3. Asserts the second call does not re-raise / re-log the first task's exception, and that `manager.refresh_task` is `None` after the first call.

### Environment

- Package: `azure-cosmos` (Python)
- Path: `sdk/cosmos/azure-cosmos`
- Branch observed: `main`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.