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

Close notification channels on wsclient disconnect #2980

Merged
merged 2 commits into from
Apr 19, 2023

Conversation

roman-khimov
Copy link
Member

No description provided.

wsReader() closes c.done first and then goes over the list of
c.respChannels. Technically this means that any of the two can be taken in
this select.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov roman-khimov added bug Something isn't working rpc RPC server and client labels Apr 18, 2023
@roman-khimov roman-khimov added this to the v0.102.0 milestone Apr 18, 2023
@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #2980 (1a1196f) into master (a4cc6da) will decrease coverage by 0.11%.
The diff coverage is 76.92%.

❗ Current head 1a1196f differs from pull request most recent head 6eaa765. Consider uploading reports for the commit 6eaa765 to get more accurate results

@@            Coverage Diff             @@
##           master    #2980      +/-   ##
==========================================
- Coverage   84.88%   84.78%   -0.11%     
==========================================
  Files         328      328              
  Lines       43047    43059      +12     
==========================================
- Hits        36541    36506      -35     
- Misses       5023     5068      +45     
- Partials     1483     1485       +2     
Impacted Files Coverage Δ
pkg/rpcclient/wsclient.go 78.16% <76.92%> (-0.07%) ⬇️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@roman-khimov roman-khimov force-pushed the close-notification-chans-on-wsclient-disconnect branch from 3458d46 to 1a1196f Compare April 19, 2023 08:38
AnnaShaleva
AnnaShaleva previously approved these changes Apr 19, 2023
@AnnaShaleva AnnaShaleva dismissed their stale review April 19, 2023 11:53

Test is failing.

@roman-khimov roman-khimov force-pushed the close-notification-chans-on-wsclient-disconnect branch from 1a1196f to 790e657 Compare April 19, 2023 12:21
The reader is about to exit and it will close legacy c.Notifications, but it
will leave subscription channels at the same time. This is wrong since these
channels will no longer receive any new events, game over.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov roman-khimov merged commit 7b10958 into master Apr 19, 2023
@roman-khimov roman-khimov deleted the close-notification-chans-on-wsclient-disconnect branch April 19, 2023 15:11
@AnnaShaleva AnnaShaleva modified the milestones: v0.102.0, v0.101.4 Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rpc RPC server and client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants