abetaev
commented
2 years ago as a user i would prefer the following:
/* make transport instance "startable" */
class WebRTCStar implements Transport, Initializable, Startable {
/* ... */
start: () => this.sigServers.values()
.forEach(sigServer => sigServer.start()),
stop: () => this.sigServers.values()
.forEach(sigServer => sigServer.stop())
/* ... */
}
/* make SigServer instances "startable" */
class SigServer extends EventEmitter<SignalServerServerEvents> implements SignalServer, Startable {
/* ... */
start() {
this.socket = connect(this.signallingUrl, this.sioOptions);
/* ... */
}
stop() {
/* ... */
this.socket.close();
/* ... */
}
} example use case: pwa is loaded from browser's storage, at the moment app is started one of signalling servers is unavailable
- in current implementation server is excluded from the list, i need to handle it and implement retry for that server
- in suggested implementation the connection attempt will be retried by socket.io oob
i don't see any significant semantic difference between the first and other values, i think in general it would be easier to handle different scenarios without this special case.
achingbrain
github-actions[bot]
Tweaks the logic around server errors so that if we lose our connection to the sigserver, we now let socket.io's reconnection logic handle trying to reconnect instead of emitting an error that causes everything to be torn down.
Fixes: #453