TypeError: Got unexpected return type 'Future' in RedisProtocol.decr, expected <class 'int'> about asyncio-redis HOT 13 CLOSED

Hi @adamrothman,

This is clearly a bug. Thanks for reporting it!
Right now, I have no idea yet what's going on, but I'm looking into it.

from asyncio-redis.

One question. Are you using transactions?
If so, the bug could be related to that. (If not, I know I have to look into something else. -- It's not that obvious.)

from asyncio-redis.

Not in this case, no (but I have another stack trace I can open an issue for that I think is related to transactions). This is the source for the _decrement_open_connections:

async def _decrement_open_connections(self):
    decr_result = await self.redis.decr(self._open_connections_key)
    if decr_result < 0:
        # Key doesn't exist (or it expired); delete it
        await self.redis.delete([self._open_connections_key])
        return 0
    return decr_result

from asyncio-redis.

It would be great if you could also create an issue for the other trace. It could help. Thanks!

from asyncio-redis.

@jonathanslenders done (#99); thanks for looking into these!

from asyncio-redis.

Hi @adamrothman,

I could not reproduce any of the above issues, but could reproduce a very similar condition. Apparently, transactions were prone to race conditions, when they were started from different coroutines. (Which happened once in a while.)

This fix was not as easy as I thought in the first place, but I think, I have a working fix:
#100
Basically, I had to use an asyncio.Lock at the right places.

In my tests, no crashes happened anymore, and all the unit tests succeed. Do you have any possibility to test this branch?

from asyncio-redis.

@jonathanslenders it makes sense to me that your fix would address #99, but are you saying it solves this issue as well? We should be able to test out your branch; I'll deploy it tomorrow and keep you posted on the results.

from asyncio-redis.

Yes, I think that it does and they are caused by the same issue. Not 100% sure, but it definitely makes sense to me. Let me know if it works or not.

from asyncio-redis.

@jonathanslenders We have been running your patched branch since Monday – haven't seen any of this crash or the other. I'll leave it up to you whether to close the issues or seek further confirmation.

from asyncio-redis.

@jonathanslenders I am satisfied that the patch has fixed our issues. I'll close this and the other issue – feel free to merge #100. Any idea when the patch will make it into an official release?

from asyncio-redis.

Er, actually, I'm going to leave these open until the branch is merged.

from asyncio-redis.

Hi @adamrothman,
I will try to publish a new release this weekend.
Thanks again for reporting the issue and trying out the fix!

from asyncio-redis.

Hi @adamrothman,

#100 has been merged and it has been released on Pypi: https://pypi.python.org/pypi/asyncio_redis/

So closing this. Please let me know if you experience further issues.

from asyncio-redis.