Handle case in which aiohttp closes the websocket due to lack of ping responses. #453
Conversation
Codecov Report
@@ Coverage Diff @@
## master #453 +/- ##
==========================================
- Coverage 57.08% 55.82% -1.26%
==========================================
Files 6 6
Lines 720 738 +18
Branches 39 41 +2
==========================================
+ Hits 411 412 +1
- Misses 301 317 +16
- Partials 8 9 +1
Continue to review full report at Codecov.
|
RodneyU215
added
Priority: High
bug
RodneyU215
left a comment
RodneyU215
left a comment
There was a problem hiding this comment.
Thanks @flyte for helping get this fixed up. I've add a few minor comments once these are addressed and we add unit tests I believe we can get this merged in.
| # True | ||
| message = await self._websocket.receive(timeout=1) | ||
| except asyncio.TimeoutError: | ||
| if not self._websocket.closed: |
There was a problem hiding this comment.
I'd love to see some unit tests added for this condition.
There was a problem hiding this comment.
I've spent some time trying to mock up enough to get this tested but I've come up short I'm afraid, and I need to get on with some work. Do you have any bright ideas about how to test this section?
|
Also just an FYI, I'll be merging PR #446 which modifies the same files so a rebase will likely be needed. |
|
I've rebased the PR onto |
|
@flyte Thanks for all of the work you've done on this. Let's merge this as-is for now and I'll work on adding some additional tests in a subsequent PR. |
|
@RodneyU215 what is the timeline to a patch release that includes this bugfix? |
|
I'm planning on releasing all of these recent changes tomorrow once #466 is
merged.
…On Sun, Jun 30, 2019, 5:44 AM Perry Naseck ***@***.***> wrote:
@RodneyU215 <https://github.com/RodneyU215> what is the timeline to a
patch release that includes this bugfix?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#453>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZM5AJZTWF5EVYILO5PEJTP5CTDNANCNFSM4H2Q5P6Q>
.
|
…at won't fail silently (slackapi/python-slack-sdk#453)
3 participants
Summary
Fixes #451
Happy to have a discussion on the right solution for this. It works as-is but I expect there's something I haven't thought of. I'm on the #sdk-python-slack channel if you want to talk in real time.
Requirements (place an
xin each[ ])