Exceptions are cached?

[RouquinBlanc]

Hello,

I'm seeing some weird thing with Exception... The lru_cache decorator would not cache anything in case an exception is raised, so something like this:

# not very useful but shows well the difference
foo = 0
@lru_cache(maxsize=10)
def return_something_sync(a):
    global foo
    foo += 1
    raise Exception(foo, time.time())

Would raise something like this:

return_something_sync(1)
>>> Exception: (1, 1564754537.844053)
return_something_sync(1)
>>> Exception: (2, 1564754539.949204)

So a new exception each time. But with alru_cache, the exception itself is cached and raised back!

foo = 0
@alru_cache(maxsize=10)
async def return_something(a):
    global foo
    foo += 1
    raise Exception(foo, time.time())

Will raise:

await return_something(1)
>>> Exception: (1, 1564754484.767096)
await return_something(1)
>>> Exception: (1, 1564754484.767096)

I would expect the exception not to be kept in cache, like for the sync equivalent; otherwise we have basically no way to call again the same function in case of transient issue (which can frequently happen if involving network access...).

Or am I missing something?

Thanks

[RouquinBlanc]

OK... and then after I read the code, and see that there is a cache_exceptions option to turn that off!

Anyway, shouldn't that option be False by default to be consistent with the sync version?

[GrazingScientist]

OK... and then after I read the code, and see that there is a cache_exceptions option to turn that off!

Anyway, shouldn't that option be False by default to be consistent with the sync version?

Totally agree! Thank you for mentioning this parameter! :)

This parameter (or at least this behaviour of caching exceptions) should be mentioned in the README!

[spolloni]

Anyway, shouldn't that option be False by default to be consistent with the sync version?

+1

[RouquinBlanc]

Even the base example is kind of wrong:

@alru_cache(maxsize=32)
async def get_pep(num):
    resource = 'http://www.python.org/dev/peps/pep-%04d/' % num
    async with aiohttp.ClientSession() as session:
        try:
            async with session.get(resource) as s:
                return await s.read()
        except aiohttp.ClientError:
            return 'Not Found'

If you don't manage to get an answer, you may want the exception to pop, and not a "not found" to be cached; and if you retry, you don't want the cached exception to pop in your face without even trying 😅

I could really understand that changing the default behavior is risky, because it may break some clients (although I don't really see a lot of scenarios where you would want exceptions to be cached). But at least a fair warning should be added in the documentation, and maybe a nuance that 100% port of Python built-in function functools.lru_cache for asyncio is a bit overstated...

[pbelskiy]

Why default is True? lru_cache don't cache any exceptions

[asvetlov]

Agree. The cache_exceptions was changed to False by default in ce90ae7 The fix will be available as a part of async-lru 2.0 release