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

Fix WS transport when the connection aborts #967

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

lchenut
Copy link
Contributor

@lchenut lchenut commented Oct 19, 2023

Should fix waku-org/nwaku#2140

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #967 (d804441) into unstable (60f9536) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           unstable     #967      +/-   ##
============================================
+ Coverage     83.20%   83.23%   +0.02%     
============================================
  Files            91       91              
  Lines         15308    15309       +1     
============================================
+ Hits          12737    12742       +5     
+ Misses         2571     2567       -4     
Files Coverage Δ
libp2p/transports/wstransport.nim 86.04% <100.00%> (+0.06%) ⬆️

... and 3 files with indirect coverage changes

Copy link
Contributor

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

It looks reasonable to me, thanks!

Aside from that, this is the second time we have encountered an issue with an exception not being properly captured. The last time I remember it was related to the `TransportOsError' exception.

As a further enhancement, I suggest trying to capture more generic exceptions at the end of the block.
CC: @Menduist

Copy link
Member

@vpavlin vpavlin left a comment

Choose a reason for hiding this comment

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

I was not able to test it (could not find a simple way to get this exception thrown in nwaku:)), but it for sure makes sense to handle it.

@lchenut lchenut merged commit fc4e9a8 into unstable Oct 23, 2023
10 checks passed
@lchenut lchenut deleted the fix-wstransport-enotconn branch October 23, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

bug: WSS enabled node stops accepting websocket connections after some time
3 participants