Skip to content
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.

Sign up for GitHub

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 reason not being set after local close #159

Merged
merged 2 commits into from Mar 19, 2023
Merged

fix reason not being set after local close #159

merged 2 commits into from Mar 19, 2023

Conversation

belm0
Copy link
Member

@belm0 belm0 commented May 23, 2021 •
edited

During a local-initiated close handshake, the following incorrect behavior was observed:

  • closed attribute would be None
  • send_message() would be silently ignored (wsproto < 0.2.0) or leak a LocalProtocolException (wsproto >= 0.2.0)

Upon local-initiated close, closed will now have the reason, and send_message() will raise ConnectionClosed.

Fixes #158.

@coveralls
Copy link

coveralls commented May 23, 2021 •
edited

Pull Request Test Coverage Report for Build 183

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 95.769%
Totals Coverage Status
Change from base Build 169: 0.008%
Covered Lines: 498
Relevant Lines: 520
💛 - Coveralls

@belm0 belm0 mentioned this pull request May 23, 2021
Closed
@belm0 belm0 changed the title Fix case of ConnectionClosed.reason not being set in the fix reason not being set after local close May 23, 2021
@belm0 belm0 force-pushed the master branch 11 times, most recently from 5b40d83 to 03c2d4b Compare January 10, 2022 14:57
@belm0 belm0 force-pushed the master branch 11 times, most recently from c4ea800 to f6cf32d Compare March 18, 2023 11:15
@belm0
Fix case of ConnectionClosed.reason not being set in the
a615ca6 
case of closing the connection on the local side.

TODO: tests
@belm0
unit test
604bd58
@belm0 belm0 merged commit 77a712a into master Mar 19, 2023
14 checks passed
@belm0 belm0 deleted the close_reason_race branch March 19, 2023 11:11
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.

get_message() sometimes missing close reason
2 participants
@belm0 @coveralls