Skip to content

Fix TypeError in socket.error exception handling#160

Merged
vasily-v-ryabov merged 1 commit intopython-xlib:masterfrom
t-wissmann:master
Apr 1, 2020
Merged

Fix TypeError in socket.error exception handling#160
vasily-v-ryabov merged 1 commit intopython-xlib:masterfrom
t-wissmann:master

Conversation

@t-wissmann
Copy link
Contributor

@t-wissmann t-wissmann commented Mar 21, 2020

The socket.error, which is an alias to OSError since python 3.3, is not
subscriptable (and its subclasses neither are). Hence, pass the entire
exception message to close_internal().

In case of a socket.error in self.socket.send(), the former code crashed
with a TypeError.

I strongly suspect that this closes issue #127.

@t-wissmann
Fix TypeError in socket.error exception handling
1e5ff83 
The socket.error, which is an alias to OSError since python 3.3, is not
subscriptable (and its subclasses neither are). Hence, pass the entire
exception message to close_internal().

In case of a socket.error in self.socket.send(), the former crashed with
a TypeError.

I strongly suspect that this closes issue python-xlib#127.
@codecov-io
Copy link

codecov-io commented Mar 21, 2020 •
edited
Loading

Codecov Report

Merging #160 into master will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #160      +/-   ##
==========================================
- Coverage   81.96%   81.94%   -0.03%     
==========================================
  Files          41       41              
  Lines        4801     4801              
==========================================
- Hits         3935     3934       -1     
- Misses        866      867       +1

@t-wissmann
Copy link
Contributor Author

t-wissmann commented Mar 21, 2020

I'm experiencing this TypeError, because in the testsuite for my project herbstluftwm, a ConnectionResetError arises from time to time (herbstluftwm/herbstluftwm#609).

@t-wissmann t-wissmann mentioned this pull request Mar 21, 2020
Closed
t-wissmann added a commit to herbstluftwm/herbstluftwm that referenced this pull request Mar 21, 2020
@t-wissmann
Work around the type error in Xlib.display.Display
ad71eca 
While python-xlib/python-xlib#160 is not merged,
try to avoid issue #609 by attempting to open the display multiple times
as long as no TypeError occurs anymore.
@t-wissmann t-wissmann mentioned this pull request Mar 21, 2020
Merged
t-wissmann added a commit to herbstluftwm/herbstluftwm that referenced this pull request Mar 21, 2020
@t-wissmann
Work around the TypeError in Xlib.display.Display (#628)
a227538 
While python-xlib/python-xlib#160 is not merged,
try to avoid issue #609 by attempting to open the display multiple times
as long as no TypeError occurs anymore.
@vasily-v-ryabov
Copy link
Collaborator

vasily-v-ryabov commented Mar 23, 2020

How about proper handling it for Python 2.7? python-xlib still declares Py2.7 support.

@t-wissmann
Copy link
Contributor Author

t-wissmann commented Mar 23, 2020

How about proper handling it for Python 2.7? python-xlib still declares Py2.7 support.

Shouldn't this also work for Python 2.7? As long as err can be converted to a string, I think this should work:

$ python2
Python 2.7.17 (default, Oct 22 2019, 09:14:09) 
[GCC 9.2.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> print("some %s format" % 234)
some 234 format
>>>

@vasily-v-ryabov
Copy link
Collaborator

vasily-v-ryabov commented Mar 23, 2020

It's just an assumption. Could you please confirm it's tested and works under Py2.7 ?

@t-wissmann
Copy link
Contributor Author

t-wissmann commented Mar 24, 2020

I don't know how to reproduce this, since it's in the exception handler of quite a low-level exception. All I can do is looking up the documentation of socket.error and it says:

The accompanying value is either a string telling what went wrong or a pair (errno, string) representing an error returned by a system call, similar to the value accompanying os.error.

So it's even unclear to me whether err[1] worked for python2 at all.

@vasily-v-ryabov
Copy link
Collaborator

vasily-v-ryabov commented Apr 1, 2020

Well, I've checked synthetic case and similar construction works in Python 2.7. Merging...
Thanks for the fix!

@vasily-v-ryabov vasily-v-ryabov merged commit 381bc18 into python-xlib:master Apr 1, 2020
@t-wissmann
Copy link
Contributor Author

t-wissmann commented Apr 1, 2020

Thanks for taking care of it and for merging! I wasn't really sure how to properly test it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@t-wissmann @codecov-io @vasily-v-ryabov