Skip to content

Fix #611 - stop propagating user exceptions to the connection management layer#679

Merged
seratch merged 1 commit intoslackapi:masterfrom
seratch:issue-611
May 14, 2020
Merged

Fix #611 - stop propagating user exceptions to the connection management layer#679
seratch merged 1 commit intoslackapi:masterfrom
seratch:issue-611

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented May 11, 2020

Summary

#665 had a dependency on #662 but the fix can be independent. This pull request brings the only necessary changes for fixing #661

This pull request fixes #611 by stopping the propagation of user exceptions to _connect_and_read() method. Raised exceptions to _connect_and_read() results in WebSocket connection closure. That is not intended in the scenarios where developers don't handle (or forget handling) exceptions in their methods decorated by @RTMClient.run_on.

You can verify the validity of this change by running an integration test: https://github.com/slackapi/python-slackclient/blob/master/integration_tests/rtm/test_issue_611.py

Requirements (place an x in each [ ])

@seratch seratch added Version: 2x bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented rtm-client v2.6 in-progress labels May 11, 2020
@seratch seratch added this to the 2.6.0 milestone May 11, 2020
@seratch seratch requested review from aoberoi and stevengill May 11, 2020 04:14
@seratch seratch self-assigned this May 11, 2020
@codecov
Copy link

codecov bot commented May 11, 2020

Codecov Report

Merging #679 into master will decrease coverage by 0.16%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #679      +/-   ##
==========================================
- Coverage   86.92%   86.76%   -0.17%     
==========================================
  Files          16       16              
  Lines        1889     1896       +7     
  Branches      126      126              
==========================================
+ Hits         1642     1645       +3     
- Misses        210      214       +4     
  Partials       37       37              
Impacted Files Coverage Δ
slack/rtm/client.py 83.16% <50.00%> (-1.45%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6d15bb...e2565d1. Read the comment docs.

@seratch seratch merged commit b809c02 into slackapi:master May 14, 2020
@seratch seratch deleted the issue-611 branch May 14, 2020 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented rtm-client Version: 2x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RTM API exits upon callback raising exception

1 participant