.NET: [Feature]: `RedisHistoryProvider` owns its client construction and exposes no connection options, so a host cannot bound transcript I/O

[antsok]

Description

Summary

RedisHistoryProvider.__init__ accepts source_id, redis_url, credential_provider, host, port, ssl, username, and the key/storage options — but no connection options and no **kwargs — and then constructs the client itself:

if credential_provider is not None and host is not None:
    self._redis_client = redis.Redis(host=host, port=port, ssl=ssl, username=username, credential_provider=credential_provider, decode_responses=True)
else:
    self._redis_client = redis.from_url(redis_url, decode_responses=True)

So a host cannot set socket_timeout, socket_connect_timeout, health_check_interval, or a Retry policy on the connection that reads and writes conversation transcripts. redis-py's default socket_timeout is None — wait forever — so neither form bounds a command at all.

That matters because both provider operations are on the conversation path: get_messages issues LRANGE before the model call, and save_messages issues a pipelined RPUSH/LTRIM after it. An unbounded LRANGE hangs the turn for as long as the connection stays open and idle.

A second thing surfaced while measuring it, and it is the more surprising half: the two branches above do not get the same retry policy. redis.from_url builds its pool through ConnectionPool.from_url, which never passes through Redis.__init__ and therefore never picks up its default retry=Retry(ExponentialWithJitterBackoff(...), retries=3) — connections end up with Retry(NoBackoff(), 0). redis.Redis(...) does pass through it, so those connections retry up to three times on ConnectionError and TimeoutError. Measured on redis-py 7.1.1, per connection:

constructor branch	socket_timeout	retry attempts	retries a timeout
redis.from_url(redis_url, …)	None	0	n/a
redis.Redis(host=…, credential_provider=…, …)	None	3	yes

Neither is stated anywhere, and the one that re-issues timed-out commands is the Entra/Managed-Redis branch — the production configuration. It is latent today only because a command with no timeout never raises TimeoutError: the moment a host works around the first problem, save_messages' RPUSH becomes re-issuable, and a write that timed out after the server applied it appends the same messages to the list twice. So the two halves cannot be fixed separately by a downstream host, which is part of why the ask is for the connection itself rather than for a timeout parameter.

Why the URL is not an answer

There is one channel in today's API, and it only exists for one of the two forms: because redis-py parses a URL after applying keyword arguments and lets the URL win, connection policy can be smuggled in through redis_url's query string — redis://host:6379/0?socket_timeout=5&socket_connect_timeout=3.

That is a lever rather than an interface, and it has three problems worth naming, because a reader of this issue may be about to reach for it:

• The credential_provider form has no query string at all. A host using Entra ID against Azure Managed Redis — which is the configuration the host/credential_provider/ssl parameters exist to serve, and the one a production deployment is most likely to run — has no way to reach the client. This is the half that actually motivated the report.
• Retry policy cannot be expressed that way. retry_on_timeout=true is accepted in a URL, but the useful direction is the opposite one, and there is no URL parameter for "retry connection errors but not timeouts".
• It inverts the usual precedence expectation. An application that sanitizes its own connection string (to stop an operator loosening a policy through REDIS_URL) has to make a deliberate exception for this one consumer.

What a downstream host has to do instead

Reach self._redis_client after construction and reconfigure the pool through redis-py's own accessors, before the first command opens a connection:

provider = RedisHistoryProvider(credential_provider=..., host=..., ssl=True, key_prefix=..., load_messages=False)
provider._redis_client.get_connection_kwargs().update(socket_timeout=5.0, socket_connect_timeout=3.0, health_check_interval=30)
provider._redis_client.set_retry(Retry(ExponentialWithJitterBackoff(), retries=3, supported_errors=(ConnectionError,)))

This works, and it is what we ship, guarded by a test that fails the moment _redis_client moves. It is still a private attribute on a per-instance object rather than a documented seam, and every host that cares about bounding its stores will arrive at the same line independently.

Suggestion 1 — accept a pre-built client, as RedisStore already does

The same package's vector store already takes this shape: RedisStore / RedisCollection accept redis_client: Redis | None and build one from redis_url only when none is supplied (_create_client in _vector_store.py), then validate the client's connection_pool.connection_kwargs for the invariants it needs (decode_responses=False, RESP 2). So the concept, the parameter name, and even the precedent for inspecting a caller's client are all already in this package — RedisHistoryProvider is the outlier.

A redis_client parameter would also subsume the host/port/ssl/username/credential_provider group for the Entra case rather than needing to grow alongside it: whatever redis-py supports, the caller can already build. It lets one client — one connection pool — serve several providers and stores, which today each hold their own. And it gives the provider a natural place to state its own invariant the way the vector store does: this one needs decode_responses=True, and a caller passing a bytes client should be told so at construction rather than at the first json.loads.

Suggestion 2 — failing that, pass connection options through

If owning construction is deliberate, forwarding **kwargs (or an explicit connection_options: dict[str, Any] | None) to both redis.Redis(...) and redis.from_url(...) would be enough for the timeout case and needs no change to the existing parameters. It is strictly weaker than suggestion 1 — it cannot share a pool, and it enumerates redis-py's surface rather than deferring to it — but it closes the "no bound is reachable at all" half, which is the part that can hang a request.

Suggestion 3 — either way, make the two branches agree

Independent of both: whichever way the connection is supplied, the redis_url and credential_provider branches of one constructor should not hand back connections with different retry semantics. Redis.from_url bypassing Redis.__init__'s default is redis-py's behaviour rather than this package's, which is exactly why it is worth pinning here — a caller reading this constructor has no way to see it, and the branch with the surprising policy is the one a deployment runs.

Suggestion 4 — guard save_messages' read-modify-write

save_messages reads the stored list, filters the incoming messages against it, and appends what is left:

existing_messages = await self.get_messages(session_id, state=state, **kwargs)
new_messages = filter_new_messages(existing_messages, messages)
...
async with self._redis_client.pipeline(transaction=True) as pipe:
    for serialized in serialized_messages:
        await _redis_result(pipe.rpush(key, serialized))
    await pipe.execute()

Nothing is watched, so the dedup is advisory rather than enforced, and Pipeline.execute hands the whole MULTI to the connection's Retry. A host that retries a ConnectionError at all — which includes any host following redis-py's own default, and this is the same policy suggestion 3 is about — therefore re-issues a transaction that may already have committed, because redis-py raises ConnectionError for a socket that dropped mid-reply exactly as it does for one that never connected. The result is a duplicated transcript segment, appended below the dedup that would have caught it: get_messages and filter_new_messages both ran before the pipeline was built, so they cannot see the replay of it.

WATCH on the key before the read closes it without any new API, and redis-py does the rest: it refuses to replay a watched transaction, raising WatchError from the retry's failure hook instead, so a caller that loops on WatchError re-reads and finds the committed append. The dedup then returns nothing and the retry is a no-op. Guarding it would also make the dedup mean what it reads like — today a second writer on the same session interleaves rather than losing a race — and cost one WATCH plus a bounded retry loop on a path that already does a full LRANGE.

Worth noting alongside suggestion 3: an unguarded read-modify-write is what makes the retry asymmetry a correctness problem rather than a latency one. Whichever policy the two constructor branches converge on, the write underneath it is not safe to re-issue.

Code Sample

from agent_framework.redis import RedisHistoryProvider

for provider in (
    RedisHistoryProvider(redis_url="redis://localhost:6379/0", key_format="legacy"),
    RedisHistoryProvider(credential_provider=..., host="h.redis.azure.net", port=10000, ssl=True, key_format="legacy"),
):
    connection = provider._redis_client.connection_pool.make_connection()
    print(provider._redis_client.get_connection_kwargs().get("socket_timeout"), connection.retry._retries, connection.retry._supported_errors)

# None 0 (ConnectionError, TimeoutError)
# None 3 (TimeoutError, ConnectionError)


No parameter combination changes the first column, and the second and third are not reachable at all. The URL workaround described above is available only for the first row.

Language/SDK

Python