Skip to content

fix(rate-limit): fail open on non-RedisError connection failures - #60

Merged
arena-ai-coding-agent[bot] merged 1 commit into
mainfrom
arena/01a06064-oyster360
Sep 2, 2026
Merged

arena-ai-coding-agent[bot] merged 1 commit into
mainfrom
arena/01a06064-oyster360

Conversation

@arena-ai-coding-agent

Copy link
Copy Markdown
Contributor

Problem

The docker-integration CI job ("Docker build and integration tests") failed with:

FAILED tests/test_integration.py::test_complete_batch_lifecycle - RuntimeError: Event loop is closed
FAILED tests/test_integration.py::test_organization_isolation - RuntimeError: Event loop is closed

Root cause

Both failing tests POST /api/batches (no trailing slash). FastAPI 307-redirects that to /api/batches/, and Starlette's TestClient follows the redirect as a second internal ASGI call that runs on a fresh event loop.

The rate-limit middleware (backend/app/core/rate_limit.py) holds a single long-lived async Redis client created once at import time. When the second call's Redis operation reuses a pooled connection from the first event loop, redis-py's asyncio transport detects the mismatch and raises a bare RuntimeError("Event loop is closed") instead of a redis.RedisError subclass.

The middleware only caught redis.RedisError to implement its documented "fail open if Redis is unreachable" behavior, so this RuntimeError escaped as an unhandled exception, turning into a 500 and failing both integration tests.

Fix

  • Broadened the fail-open exception handling in create_rate_limit_middleware to also catch OSError/RuntimeError (transport-level failures and event-loop mismatches), matching the module's stated contract that Redis problems must never break a request.
  • Added a regression test (test_middleware_fails_open_when_redis_is_unreachable) covering all three failure shapes: redis.ConnectionError, RuntimeError, and OSError.

Verification

  • Reproduced the exact CI traceback locally with the pinned requirements.lock versions and a real Redis server (byte-for-byte match of the reported error).
  • Confirmed test_complete_batch_lifecycle and test_organization_isolation now pass.
  • New regression test fails on the old code and passes with the fix.
  • flake8, mypy app, and the broader pytest suite show no regressions from this change.

The 'Docker build and integration tests' job failed because
test_complete_batch_lifecycle and test_organization_isolation both
POST to /api/batches (no trailing slash), which FastAPI 307-redirects
to /api/batches/. Starlette's TestClient follows that redirect as a
second internal ASGI call that runs on a fresh event loop, while the
rate-limit middleware's Redis client is a long-lived object created
once at import time. redis-py's asyncio transport detects the pooled
connection is now attached to a different event loop and raises a bare
RuntimeError ('Event loop is closed') instead of a redis.RedisError
subclass.

create_rate_limit_middleware only caught redis.RedisError, so this
RuntimeError escaped as an unhandled 500, defeating the module's
documented 'fail open when Redis is unavailable' contract and failing
both integration tests.

Broaden the fail-open handling to also catch OSError/RuntimeError from
the Redis client, and add a regression test covering all three failure
shapes (ConnectionError, RuntimeError, OSError) against the middleware
directly.

Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
@arena-ai-coding-agent
arena-ai-coding-agent Bot merged commit 3b04f35 into main Sep 2, 2026
15 checks passed
Repository owner locked as resolved and limited conversation to collaborators Sep 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant