Skip to content

bug(workflow): the workflow state machine dereferences a None Redis client instead of naming the failure #15012

Open

Description

@mrveiss

Found while converting autobot-backend/takeover_manager_e2e_test.py for #14979.

What happens

AsyncRedisClientMixin._get_redis() (autobot_shared/redis_mixin.py:41) returns whatever get_async_redis_client() gives it, and that is documented to be None when no client can be built. Three call sites in autobot-backend/services/workflow_automation/state_machine.py dereference the result without checking:

state_machine.py:202   raw = await redis.get(_state_key(workflow_id))
state_machine.py:213   await redis.delete(_state_key(workflow_id))
state_machine.py:219   await redis.set(...)

With no client, starting any workflow ends in:

services/workflow_automation/executor.py:270  sm_state = await self.state_machine.create(...)
services/workflow_automation/state_machine.py:150  await self._persist(state)
services/workflow_automation/state_machine.py:220  await redis.set(
AttributeError: 'NoneType' object has no attribute 'set'

Why it matters

AttributeError: 'NoneType' object has no attribute 'set' names neither the subsystem nor the cause. An operator reading it in a log has to walk three frames to learn that Redis was simply unreachable. Every other consumer of this mixin in the tree guards the result — services/temporal_invalidation_service.py:228 returns {"error": "Redis client not available"}, which is a diagnosis; state_machine.py is the outlier.

The workflow state machine is also the component that decides whether a workflow may proceed, so failing on an unclear exception mid-transition is worse here than in a cache.

Suggested fix

Either check the client and raise a named error that says the workflow state store is unavailable, or give the mixin a _require_redis() that raises that error once, so the three call sites cannot each forget. The second is preferable — the mixin has other subclasses and the same omission is available to all of them.

Evidence

Reproduced with the backend unit gate's socket-free autobot_shared.redis_client stand-in (#14932) in place; the same shape occurs for any unreachable Redis, since get_async_redis_client() returns None in both cases.

Not fixed here

#14979 is a test-collection campaign; this is a product change. The three tests that hit it now inject an in-process fakeredis so they exercise the real persistence path rather than skip.

Related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions