geut / discovery-swarm-webrtc

discovery-swarm for WebRTC
MIT License
94 stars 23 forks source link

Connect Errors not Surfaced #13

Closed telackey closed 4 years ago

telackey commented 4 years ago

Errors connecting to the signal server are not logged or easily detected. Ideally, we'd do both, but at the least, surfacing the connect_error and other events would be a start.

diff --git a/lib/signal-client.js b/lib/signal-client.js
index db754c2..94c33ba 100644
--- a/lib/signal-client.js
+++ b/lib/signal-client.js
@@ -123,6 +123,10 @@ class SignalClient extends EventEmitter {

   _initialize () {
     this.socket.on('connect', () => this.emit('connect'))
+    this.socket.on('connect_error', error => this.emit('connect-error', error))
+    this.socket.on('connect_timeout', timeout => this.emit('connect-timeout', timeout))
+    this.socket.on('error', error => this.emit('error', error))
+    this.socket.on('reconnect_failed', () => this.emit('reconnect-failed'))

     this.socket.on('reconnect_error', error => {
       this.emit('reconnect-error', error)
dboreham commented 4 years ago

+1

Knowing "didn't connect" is potentially as important as knowing "did connect".

Due to the way swarm is intended to work, note these aren't necessarily fatal errors because transient network outages are expected. For that reason it would be good to have the host name percolated back to the place the event can be logged or displayed to the user because it would take a human to spot a typo in the host name, for example.

tinchoz49 commented 4 years ago

In the new version @geut/discovery-swarm-webrtc@2.2.7 each socket error would be handle by the swarm.on('error').

You can detect everything in a single point

swarm.on('error', err => {
  console.log(err.code) // socket-connect-error or socket-reconnect-error or socket-timeout ...
});