JohnDoee / deluge-client

A very lightweight pure-python Deluge RPC Client
MIT License
87 stars 14 forks source link

Exceptions on python 3 with decode_utf8=True #20

Closed gazpachoking closed 6 years ago

gazpachoking commented 6 years ago

Seems if the client is set to decode utf8 already, we have a problem creating an exception returned from deluge:

Traceback (most recent call last):
  File "C:/Users/Chase/PycharmProjects/deluge-client/test.py", line 7, in <module>
    print(a.call('core.get_free_space', '2', '3'))
  File "C:\Users\Chase\PycharmProjects\deluge-client\deluge_client\client.py", line 197, in call
    return self._receive_response(self.deluge_version)
  File "C:\Users\Chase\PycharmProjects\deluge-client\deluge_client\client.py", line 173, in _receive_response
    exception = type(str(exception_type.decode('utf-8', 'ignore')), (Exception, ), {})
AttributeError: 'str' object has no attribute 'decode'
gazpachoking commented 6 years ago

I guess my first question before working on a fix is: Do we need to make decode_utf8 be optional? My feeling is that text should be text, on both python 2 and 3 automatically. (i.e. py2 unicode, py3 str)

JohnDoee commented 6 years ago

I have some torrents with invalid utf-8 in them, I'll test with them and see how it's handled.

You're probably right that it doesn't make sense as an option if everything can be decoded.

... the double decoding is, of course, a bug.

JohnDoee commented 6 years ago

Although it's not the biggest of libraries, I want to do proper deprecation.

The only way I could find without breaking stuff on a release is to do it in two stages

What do you think about that ? It feels like a lot of work though, to do it in two stages.

My suggestion is in #25

gazpachoking commented 6 years ago

Yeah, seems like a plan to me. Made a PR against your PR branch with some more tests.