vertica-python icon indicating copy to clipboard operation
vertica-python copied to clipboard

Various exceptions overlapped by ConnectionError in python 2.7

Open pcerny opened this issue 5 years ago • 2 comments

The issue is related to versions 0.9.1 and 0.9.2. But it is probably manifested on versions newer then 0.6.14 (not and exact version number).

On several places, there is used construct raise_from(errors.ConnectionError, e). It tries to introduce compatibility between python 2 and python 3 based on six library. Unfortunately, six library implements raise_from(a,b) as raise a in python 2, see https://github.com/benjaminp/six/blob/1.12.0/six.py#L736. In python 2, it leads to loss of error message for various system and IO errors. Simple example is connection timeout. There are even several issues (#178, #180) where stack trace shows loss of important information.

By the way, there is an issue opened for it in six, but not addressed yet, https://github.com/benjaminp/six/issues/193.

I suggest to follow http://python-future.org/compatible_idioms.html and replace raise_from with future.utils.reraise. Here is an example snippet:

import sys
from future.utils import reraise

try:
    raise IOError('inner bug')
except Exception as e:
    trackback = sys.exc_info()[2]
    reraise(ConnectionError, e, trackback)

This way, ConnectionError will report information also about initial exception e at both python 2 and python 3.

pcerny avatar Jun 05 '19 15:06 pcerny

@pcerny I agree with you.

On the other hand, as Python 2 end-of-life will be on January 1st, 2020, I wonder if it worth to make such a change or get rid of the Exception chaining completely. raise_from does not been used so many times in vertica-python, I guess raising those exceptions directly won't affect too much.

sitingren avatar Jun 06 '19 17:06 sitingren

@sitingren It depends when you plan to drop support for Python 2. If you will follow Python 2 EOL date then it is not worth to fix it.

I guess raising those exceptions directly won't affect too much

I think it is a good idea to enclose exceptions with library ones like ConnectionError or TimeoutError. It makes it a lot easier to work with library then because you can easily distinguish between library specific errors.

pcerny avatar Jun 14 '19 14:06 pcerny