Sitelet https://web.archive.org/web/20210106130548/https://github.com/python-xlib/python-xlib/pull/160
Skip to content

/ python-xlib

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix TypeError in socket.error exception handling #160

Merged
merged 1 commit into from Apr 1, 2020
Merged

Fix TypeError in socket.error exception handling #160

merged 1 commit into from Apr 1, 2020

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
Loading status checks…
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 #127.
@codecov-io
Copy link

@codecov-io codecov-io commented Mar 21, 2020 •
edited

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 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
Loading status checks…
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)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
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 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 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 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 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 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
1 of 2 checks passed
1 of 2 checks passed
codecov/project 81.94% (-0.03%) compared to 52960f6
Details
semaphoreci The build passed on Semaphore.
Details
@t-wissmann
Copy link
Contributor Author

@t-wissmann 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@t-wissmann @codecov-io @vasily-v-ryabov
You can’t perform that action at this time.