fix(cache): reject async key builders instead of failing every request

changelog.d/323.fixed.md

@@ -0,0 +1,8 @@
+**An `async` key builder is rejected when `@cache` is applied instead of failing every request with a 500.**
+`@cache(key_builder=...)` accepted an `async def` builder (or an object with an
+`async def __call__`, or a `functools.partial` of either) and then raised
+`TypeError` on every request, plus a "coroutine was never awaited" warning. It
+now raises `CacheXError` at decoration time, and `invalidate(request,
+key_builder=...)` raises it before touching the backend. A builder that returns
+anything but a `str` raises `CacheXError` on the request, whatever `fail_open`
+says. Key builders must be sync functions returning `str`.

docs/HTTP_CACHING.md

@@ -448,6 +448,16 @@ called for `no_store=True`, `private=True`, routes without a `ttl`, or requests
 with `Authorization` or a session on a route without `public=True` or `cache_authorized=True`. Before 0.3.8
 it was, only to feed a debug log. Keep it free of side effects.
 
+The key builder must be a sync function that returns a `str`; it is called
+without being awaited. An `async def` function, an object with an
+`async def __call__`, or a `functools.partial` of either is rejected with
+`CacheXError` when `@cache` is applied, and by `invalidate()` before it touches
+the backend. A builder that still returns something other than a `str` (for
+example a sync wrapper that returns a coroutine) raises `CacheXError` on the
+request. `fail_open` does not cover this: it is a mistake in the route, not a
+backend failure. Read anything async (a user from the database, say) in a
+dependency or middleware and put it on `request.state` for the builder.
+
 ## Clearing the cache
 
 ### By path or pattern

fastapi_cachex/cache.py

@@ -8,6 +8,7 @@
 from collections.abc import Callable
 from collections.abc import Mapping
 from collections.abc import Sequence
+from functools import partial
 from functools import update_wrapper
 from functools import wraps
 from inspect import Parameter
@@ -270,6 +271,49 @@ def default_key_builder(request: Request) -> str:
     return build_cache_key(request)
 
 
+_ASYNC_KEY_BUILDER_MSG = (
+    "key_builder must be a sync function returning str; async key builders "
+    "are not supported (the key is built without awaiting)"
+)
+
+
+def _validate_key_builder(builder: Callable[..., object]) -> None:
+    """Reject a key builder whose call returns a coroutine.
+
+    ``functools.partial`` layers are unwrapped first, so a partial of an async
+    callable object is caught as well. A builder this cannot see through
+    (say a sync wrapper returning a coroutine) is caught by ``_build_key``.
+
+    Raises:
+        CacheXError: If calling ``builder`` would return a coroutine.
+    """
+    target = builder
+    while isinstance(target, partial):
+        target = target.func
+    if _is_coroutine_callable(target):
+        raise CacheXError(_ASYNC_KEY_BUILDER_MSG)
+
+
+def _build_key(builder: CacheKeyBuilder, request: Request) -> str:
+    """Call ``builder`` and check that it returned a ``str``.
+
+    A returned coroutine is closed, so it does not also trigger a "never
+    awaited" ``RuntimeWarning``. ``fail_open`` does not cover this: it is a
+    programming error in the route, not a backend failure.
+
+    Raises:
+        CacheXError: If ``builder`` returns anything but a ``str``.
+    """
+    key: object = builder(request)
+    if isinstance(key, str):
+        return key
+    if inspect.iscoroutine(key):
+        key.close()
+        raise CacheXError(_ASYNC_KEY_BUILDER_MSG)
+    msg = f"key_builder must return a str, got {type(key).__name__}"
+    raise CacheXError(msg)
+
+
 async def invalidate(
     request: Request,
     key_builder: CacheKeyBuilder | None = None,
@@ -287,7 +331,8 @@ async def invalidate(
             a request to the same route/method as the cached one (e.g. build
             it via ``request.app.url_path_for(...)`` for a GET route).
         key_builder: Custom key builder used by the target route's ``@cache``
-            decorator, if any. If None, uses ``default_key_builder``.
+            decorator, if any. If None, uses ``default_key_builder``. Must be
+            a sync callable returning a ``str``, as for ``@cache``.
         vary: The target route's ``vary`` names, if any. Only the variant
             selected by ``request``'s own values for those headers is
             deleted; ``clear_path()`` clears every variant of a path.
@@ -299,12 +344,16 @@ async def invalidate(
         True if a cache entry existed and was deleted, False otherwise.
 
     Raises:
-        CacheXError: If ``vary`` is not a list of header names.
+        CacheXError: If ``vary`` is not a list of header names, if
+            ``key_builder`` is an ``async`` callable, or if it returns
+            something other than a ``str``. Raised before the backend is
+            touched.
     """
     builder = key_builder or default_key_builder
+    _validate_key_builder(builder)
     vary_names = _validate_vary(vary)
     cache_key = _append_key_components(
-        builder(request), _vary_components(request, vary_names)
+        _build_key(builder, request), _vary_components(request, vary_names)
     )
 
     try:
@@ -649,7 +698,7 @@ def _serialize_result(route: "APIRoute", result: object) -> object:
     )
 
 
-def _is_coroutine_callable(func: HandlerCallable) -> bool:
+def _is_coroutine_callable(func: Callable[..., object]) -> bool:
     """Report whether calling `func` returns a coroutine.
 
     `inspect.iscoroutinefunction` already sees through `functools.partial`;
@@ -791,7 +840,13 @@ def cache(
         key_builder: Custom function to build cache keys. If None, uses
             ``default_key_builder``. To add a component (user ID, tenant,
             locale) to the default key, return
-            ``build_cache_key(request, component)``.
+            ``build_cache_key(request, component)``. Must be a sync callable
+            returning a ``str``: an ``async def`` function, an object with an
+            ``async def __call__`` or a ``functools.partial`` of either is
+            rejected when the decorator is applied, and a builder that
+            returns anything but a ``str`` fails the request with
+            ``CacheXError`` (``fail_open`` does not apply; it covers backend
+            errors only).
         fail_open: When the backend raises, log a warning and answer without
             the cache: a failed read counts as a miss and a failed write
             leaves the response unstored. ``False`` lets the error propagate,
@@ -837,7 +892,9 @@ def cache(
             ``stale_ttl`` are not given together, if ``public`` and
             ``private`` are both set, if ``ttl`` is not an ``int``, is
             negative or is larger than ``MAX_TTL``, or if ``vary`` is not a
-            list of header field names (a single string is rejected).
+            list of header field names (a single string is rejected), or if
+            ``key_builder`` is an ``async`` callable. At request time, if
+            ``key_builder`` returns anything but a ``str``.
     """
 
     def decorator(func: HandlerCallable) -> AsyncResponseCallable:
@@ -863,6 +920,8 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable:
             msg = f"ttl must be at most {MAX_TTL} seconds"
             raise CacheXError(msg)
         vary_names = _validate_vary(vary)
+        if key_builder is not None:
+            _validate_key_builder(key_builder)
         if any(name.lower() == "cookie" for name in vary_names):
             # stacklevel=2: the caller applying the decorator, i.e. the line
             # of the user's @cache(...).
@@ -1039,7 +1098,7 @@ async def serve(*args: Any, **kwargs: Any) -> Response:
             # Built only here: the branches above never touch the backend, so a
             # custom key builder would run for nothing.
             cache_key = _append_key_components(
-                builder(req), _vary_components(req, vary_names)
+                _build_key(builder, req), _vary_components(req, vary_names)
             )
 
             try:

i18n/zh-TW/docs/HTTP_CACHING.md

@@ -281,6 +281,8 @@ async def my_dashboard(user: CurrentUser, response: Response):
 
 key builder 只在 `@cache` 讀取或寫入後端時執行，因此 `no_store=True`、`private=True`、沒有 `ttl` 的路由，以及路由未設定 `public=True` 或 `cache_authorized=True` 時帶有 `Authorization` 或 Session 的請求，都不會呼叫它。0.3.8 以前它仍會被呼叫，但只用於除錯日誌。請讓它不帶副作用。
 
+key builder 必須是回傳 `str` 的同步函式，呼叫時不會被 await。`async def` 函式、具有 `async def __call__` 的物件，或包裝上述兩者的 `functools.partial`，都會在套用 `@cache` 時以 `CacheXError` 拒絕；`invalidate()` 也會在存取後端之前拒絕它們。仍然回傳非 `str` 的 builder（例如回傳協程的同步包裝函式）會在請求時拋出 `CacheXError`。`fail_open` 不涵蓋這種情況：這是路由的錯誤，不是後端故障。需要非同步讀取的資料（例如從資料庫取得使用者），請在依賴項或中介軟體中讀取，放到 `request.state` 供 builder 使用。
+
 ## 清除快取 {#clearing-the-cache}
 
 ### 依路徑或模式 {#by-path-or-pattern}

tests/test_cache.py

@@ -14,6 +14,7 @@
 
 from fastapi_cachex.backends.memory import MemoryBackend
 from fastapi_cachex.cache import cache
+from fastapi_cachex.cache import invalidate
 from fastapi_cachex.exceptions import CacheXError
 from fastapi_cachex.proxy import BackendProxy
 from fastapi_cachex.types import CacheEntry
@@ -781,3 +782,110 @@ def test_public_and_private_raises():
         @cache(public=True, private=True)
         async def bad_endpoint():
             pass
+
+
+async def _async_key_builder(request: Request) -> str:
+    return request.url.path
+
+
+async def _async_key_builder_with_prefix(prefix: str, request: Request) -> str:
+    return prefix + request.url.path
+
+
+class _AsyncCallableKeyBuilder:
+    async def __call__(self, request: Request) -> str:
+        return request.url.path
+
+
+@pytest.mark.parametrize(
+    "builder",
+    [
+        _async_key_builder,
+        _AsyncCallableKeyBuilder(),
+        partial(_async_key_builder_with_prefix, "p:"),
+        partial(partial(_AsyncCallableKeyBuilder())),
+    ],
+    ids=["async-def", "async-call-object", "partial", "partial-of-async-object"],
+)
+def test_async_key_builder_raises_at_decoration(builder):
+    """An async key builder is rejected when @cache is applied (#323)."""
+    with pytest.raises(CacheXError, match="key_builder must be a sync function"):
+
+        @cache(ttl=60, key_builder=builder)
+        async def bad_endpoint():
+            pass
+
+
+def _get_request(path: str) -> Request:
+    return Request(
+        {
+            "type": "http",
+            "method": "GET",
+            "path": path,
+            "query_string": b"",
+            "headers": [(b"host", b"testserver")],
+        }
+    )
+
+
+class _RecordingBackend(MemoryBackend):
+    def __init__(self) -> None:
+        super().__init__()
+        self.touched = False
+
+    async def get_and_delete(self, key: str) -> CacheEntry | None:
+        self.touched = True
+        return await super().get_and_delete(key)
+
+
+async def test_invalidate_rejects_async_key_builder_before_the_backend():
+    """invalidate() raises for an async builder without touching the backend."""
+    backend = _RecordingBackend()
+    BackendProxy.set(backend)
+    with pytest.raises(CacheXError, match="key_builder must be a sync function"):
+        await invalidate(_get_request("/x"), key_builder=_async_key_builder)  # type: ignore[arg-type]
+    assert backend.touched is False
+
+
+def _returns_coroutine(request: Request) -> str:
+    """A sync wrapper around an async builder: undetectable when applied."""
+    return _async_key_builder(request)  # type: ignore[return-value]
+
+
+def _returns_int(request: Request) -> str:
+    return 42  # type: ignore[return-value]
+
+
+_NON_STR_BUILDERS = pytest.mark.parametrize(
+    ("builder", "match"),
+    [
+        (_returns_coroutine, "key_builder must be a sync function"),
+        (_returns_int, "key_builder must return a str, got int"),
+    ],
+    ids=["coroutine", "int"],
+)
+
+
+@_NON_STR_BUILDERS
+def test_key_builder_returning_non_str_raises_at_request_time(builder, match):
+    """A non-str key fails the request with CacheXError, not a TypeError.
+
+    fail_open does not hide it, and a returned coroutine is closed, so no
+    "never awaited" RuntimeWarning is emitted (filterwarnings=error).
+    """
+    local_app = FastAPI()
+
+    @local_app.get("/non-str-key")
+    @cache(ttl=60, key_builder=builder, fail_open=True)
+    async def endpoint():
+        return {"ok": True}
+
+    with pytest.raises(CacheXError, match=match):
+        TestClient(local_app).get("/non-str-key")
+
+
+@_NON_STR_BUILDERS
+async def test_invalidate_key_builder_returning_non_str_raises(builder, match):
+    """invalidate() raises the same CacheXError for a non-str key."""
+    with pytest.raises(CacheXError, match=match):
+        await invalidate(_get_request("/x"), key_builder=builder)