search

alanshaw / it-ws

🕸 🔌 Simple async iterators for websocket client and server connections
Other
11 stars 11 forks source link

fix: catch listen error #34

Closed mpetrunic closed 1 year ago

mpetrunic commented 1 year ago

This should fix it but for whatever reason it's not working. Please help.

Related to: https://github.com/libp2p/js-libp2p-websockets/issues/184#event-7914439705

mpetrunic commented 1 year ago

cc @achingbrain

codecov[bot] commented 1 year ago

Codecov Report

Merging #34 (b18ca2e) into master (3dea637) will increase coverage by 0.04%. The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
+ Coverage   96.02%   96.07%   +0.04%     
==========================================
  Files          22       22              
  Lines         957      967      +10     
  Branches      158      160       +2     
==========================================
+ Hits          919      929      +10     
  Misses         38       38
Flag Coverage Δ
chrome 100.00% <ø> (ø)
node 96.06% <86.66%> (+0.04%) :arrow_up:

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

Impacted Files Coverage Δ
src/server.ts 92.72% <83.78%> (+0.13%) :arrow_up:
test/server-address.spec.ts 92.00% <100.00%> (+3.76%) :arrow_up:

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

achingbrain commented 1 year ago

The reason this is failing is because the websocket server forwards any error events emitted by the underlying http(s) server when it's passed as an option.

If you stick a wsServer.on('error', () => {}) after wsServer is created the test passes (almost, the message assertion needs updating to something like await expect(server2.listen(55215)).to.eventually.be.rejected.with.property('message').that.include('EADDRINUSE')).

We probably want to be a bit smarter about error handling than that though.

We could update the internal Server class to create theWSServer instance in the listen method, call .listen on the underlying http(s) server but resolve the promise on the WSServer events?

mpetrunic commented 1 year ago

@achingbrain It works now. Hope I didn't do anything stupid 😅

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 5.0.6 :tada:

The release is available on:

Your semantic-release bot :package::rocket: