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

Try to fix errors of using WebSockets closed from client side #737

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

serhiy-storchaka
Copy link
Contributor

@serhiy-storchaka serhiy-storchaka commented Jul 15, 2022

  • The "top" handler now listen the web socket and handles pings and closing the connection from client side. It allows to simple close the connection from client side (Cannot properly close ws connection in one-sided way. aio-libs/aiohttp#3443).
  • The "top" and the "log_ws" handlers now check whether the web socket was closed before reading from or writing to it. It saves us from numerous error records in the service logs. Similar issues may be fixed in other handlers (port forward, attach), although I did not see such log records for them (they should be more rare).
  • The "log_ws" handler no longer sets WS protocol "v2.channels.neu.ro", as it is not compatible with it.
  • Some refactoring.
  • Fixed some minor errors and typos.

@codecov
Copy link

codecov bot commented Jul 15, 2022

Codecov Report

Merging #737 (1a963cf) into master (1a963cf) will not change coverage.
The diff coverage is n/a.

❗ Current head 1a963cf differs from pull request most recent head 328606e. Consider uploading reports for the commit 328606e to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #737   +/-   ##
=======================================
  Coverage   85.86%   85.86%           
=======================================
  Files          12       12           
  Lines        2080     2080           
  Branches      318      318           
=======================================
  Hits         1786     1786           
  Misses        187      187           
  Partials      107      107           
Flag Coverage Δ
integration 81.00% <0.00%> (ø)
unit 46.05% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@serhiy-storchaka serhiy-storchaka merged commit 71cfa0e into master Jul 19, 2022
@serhiy-storchaka serhiy-storchaka deleted the ss/closed-ws branch July 19, 2022 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants