search

Grokzen / redis-py-cluster

Python cluster client for the official redis cluster. Redis 3.0+.
https://redis-py-cluster.readthedocs.io/
MIT License
1.1k stars 315 forks source link

UnboundLocalError: local variable 'connection' referenced before assignment #424

Closed kamadorueda closed 3 years ago

kamadorueda commented 3 years ago

Hello guys, I've been playing with the library a little, but it seems we have an unbound variable on edge-cases:

  File "/nix/store/fklw44xkyjdw60g7yxyq8baswc6m6p3s-integrates-back-async/site-packages/redis/client.py", line 3050, in hset
    return self.execute_command('HSET', name, *items)
  File "/nix/store/fklw44xkyjdw60g7yxyq8baswc6m6p3s-integrates-back-async/site-packages/rediscluster/client.py", line 551, in execute_command
    return self._execute_command(*args, **kwargs)
  File "/nix/store/fklw44xkyjdw60g7yxyq8baswc6m6p3s-integrates-back-async/site-packages/rediscluster/client.py", line 702, in _execute_command
    self.connection_pool.release(connection)
UnboundLocalError: local variable 'connection' referenced before assignment

Seems like on error conditions, the variable has not been declared (depending on the try-catch flows):

image

Grokzen commented 3 years ago

@kamadorueda I need to know what version of the library you are running, what redis-server version and what redis-py version you ran. And if possible the piece of code that you ran and if you can reproduce this reliably in some way?

Grokzen commented 3 years ago

@kamadorueda From a quick look you are using an older version of this lib and you should upgrade it as if you look inside master branch here https://github.com/Grokzen/redis-py-cluster/blob/master/rediscluster/client.py#L703 you will see that a fix was added that will check if connection variable is set and it might help to avoid this issue. Also if you look here https://github.com/Grokzen/redis-py-cluster/blob/master/rediscluster/client.py#L590 you will see that the variable should no longer be unbound as it should be set to None by default and avoid this case that you encounter.

kamadorueda commented 3 years ago

I hit the bug in a very extreme corner case (last version of the library on pypi)

I think it was because I had locally a redis-server instead of a redis-cluster and that is supposed to fail, and it failed, so I'm ok with that

Finally I used a local redis-cluster and it worked very well, actually this library is the one that allowed me to solve this issue: https://gitlab.com/fluidattacks/product/-/issues/3874

I don't think it's worth keeping the issue open as the behaviour on real use cases is correct

Thanks!