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.

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

Pass websocket Close frame into ConnectionClosed #990

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

cderici
Copy link
Contributor

@cderici cderici commented Dec 4, 2023

Description

This fixes a bug in connection where we pass close code and reason directly into the websockets.exception.ConnectionClosed, where it needs a Close frame that contains those.

Fixes #989

QA Steps

This doesn't affect any normal behavior, as this is sort of a safeguard against races. The only two times we raise this explicit exception is when the MONITOR is closed but either the receiver task is running still or someone makes an rpc call, which shouldn't happen anyways. So manually checking the arguments by the doc is sufficient.

All CI tests need to pass.

Notes & Discussion

Needs to be forward ported into 3.x.

@cderici cderici added bug fix hint/2.9 going on 2.9 branch area/forward-port to be forward ported - remove label after port labels Dec 4, 2023
@cderici cderici requested review from anvial and jack-w-shaw December 4, 2023 18:21
Copy link
Member

@jack-w-shaw jack-w-shaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cderici
Copy link
Contributor Author

cderici commented Dec 5, 2023

/merge

@jujubot jujubot merged commit 5711ade into juju:2.9 Dec 5, 2023
11 of 12 checks passed
@cderici cderici mentioned this pull request Dec 5, 2023
jujubot added a commit that referenced this pull request Dec 6, 2023
#992

## What's Changed

A small release with only one change. Works with the latest `Juju 2.9.46`.

* Pass websocket Close frame into ConnectionClosed by @cderici in #990

#### Notes & Discussion

I thought we had more things landed on the 2.9 track since the `2.9.45` but apparently not :)

JUJU-5080
@cderici cderici removed the area/forward-port to be forward ported - remove label after port label Feb 7, 2024
jujubot added a commit that referenced this pull request Feb 8, 2024
#1022

#### Description

This brings onto the 3.x track some of the latest fixes from to the 2.9 track. Here're the details:

* Fix for #989 from #990
* Fix for #1001 from #1002
* Fix for #998 from #1003


#### QA Steps

No QA needed for #990. For 1002 and 1003 please refer to their QA steps. Though they are very related so I'd expect the QA for both of them can be done in one fell swoop.

* #1002 
* #1003 

All CI tests need to pass (there's still some known intermittent ones in there).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hint/2.9 going on 2.9 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants