WebSocketApp.run_forever(), read(): check for ssl.SSLEOFError; on exc…

[bubbleboy14]

…eption, if custom_dispatcher, pass bool(reconnect) to handleDisconnect()

[codecov]

Codecov Report

Attention: 60 lines in your changes are missing coverage. Please review.

Comparison is base (2092dfa) 83.29% compared to head (61bc3eb) 82.73%.
Report is 12 commits behind head on master.

Files	Patch %	Lines
websocket/_core.py	65.07%	15 Missing and 7 partials ⚠️
websocket/_http.py	83.33%	8 Missing and 3 partials ⚠️
websocket/_abnf.py	81.25%	4 Missing and 5 partials ⚠️
websocket/_app.py	78.94%	7 Missing and 1 partial ⚠️
websocket/_handshake.py	92.10%	1 Missing and 2 partials ⚠️
websocket/_socket.py	62.50%	2 Missing and 1 partial ⚠️
websocket/_ssl_compat.py	50.00%	2 Missing ⚠️
websocket/_utils.py	71.42%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #961      +/-   ##
==========================================
- Coverage   83.29%   82.73%   -0.57%     
==========================================
  Files          13       13              
  Lines        1389     1396       +7     
  Branches      260      264       +4     
==========================================
- Hits         1157     1155       -2     
- Misses        156      162       +6     
- Partials       76       79       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[engn33r]

Change this to from ._ssl_compat import * like the other files. _ssl_compat.py exists for the edge case where SSL can't be imported

[engn33r]

The change to specify the reconnecting bool value to handleDisconnect makes sense to me. I'd suggest a different approach for import ssl or the EOF error, though I'm not sure how exactly to replace it. It may need edits to _ssl_compat.py.

This finding shows that better test coverage of the different branches would be a good improvement to work on in the future 👍

[bubbleboy14]

Good call @engn33r I made the suggested changes.

Just realized I did "from ._ssl_compat import SSLEOFError" instead of the recommended "from ._ssl_compat import *". I kind of like the single import more, but on the other hand maybe we should be checking for the other exceptions as well.

In either case, this feels like an improvement for now, and certainly addresses the edge case I encountered.

[engn33r]

Great, thanks for the fix!