Skip to content

fix: route cache-write failures through exception_handler#48

Open
waltzforvenus wants to merge 1 commit into
taylorhakes:masterfrom
waltzforvenus:fix/exception-handler-on-cache-write
Open

fix: route cache-write failures through exception_handler#48
waltzforvenus wants to merge 1 commit into
taylorhakes:masterfrom
waltzforvenus:fix/exception-handler-on-cache-write

Conversation

@waltzforvenus

Copy link
Copy Markdown

Problem

exception_handler currently only protects the cache read: client.get(key) is wrapped in try/except, but on a cache miss the write path is not. If Redis becomes unavailable between the failed get and the subsequent store (or the store itself fails — e.g. connection drop, OOM, cluster failover), the exception propagates to the caller even though an exception_handler was configured to make caching failures non-fatal.

Fix

Wrap the cache-miss path (calling the original function, serializing the result, and the Lua cache-set script) in a try/except that delegates to exception_handler when one is configured, matching the existing behaviour of the read path. Without a handler, the exception is re-raised, so behaviour is unchanged for users who don't use exception_handler.

elif not exception_handled:
    try:
        parsed_result = fn(*args, **kwargs)
        result_serialized = self.serializer(parsed_result)
        get_cache_lua_fn(self.client)(keys=[key, self.keys_key], args=[result_serialized, self.ttl, self.limit])
    except Exception as e:
        if self.exception_handler:
            parsed_result = self.exception_handler(e, self.original_fn, args, kwargs)
        else:
            raise e

The handler receives the same (exception, original_fn, args, kwargs) signature as on the read path, so existing handlers (which typically fall back to calling the function directly) work unchanged.

🤖 Generated with Claude Code

Previously exception_handler only protected the cache read (client.get).
If Redis failed while storing the result on a cache miss, the exception
propagated to the caller even when an exception_handler was configured.
Wrap the cache-miss path so storage failures are handled the same way
as lookup failures; without a handler the exception is re-raised as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@waltzforvenus

Copy link
Copy Markdown
Author

(sorry finally could be fucked to fix the formatting issues using claude, this is the version i use on prod)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant