Skip to content

Commit a4487ba

Browse files
committed
Allow to unbind events when closing a WebSocket connection, see #108
Main use case is suppressing further log messages from old WebSocket connections when the application has already discarded it. The main problem of #108 remains unresolved (race condition).
1 parent c482f0c commit a4487ba

File tree

4 files changed

+63
-50
lines changed

4 files changed

+63
-50
lines changed

saltyrtc-client.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ declare namespace saltyrtc {
236236
decryptFromPeer(box: Box): Uint8Array;
237237

238238
connect(): void;
239-
disconnect(): void;
239+
disconnect(unbind?: boolean): void;
240240

241241
sendApplicationMessage(data: any): void;
242242

src/client.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,10 +456,11 @@ class SaltyRTC implements saltyrtc.SaltyRTC {
456456
* Both the WebSocket as well as any open task connection will be closed.
457457
*
458458
* This method is asynchronous. To get notified when the connection is has
459-
* been closed, subscribe to the `connection-closed` event.
459+
* been closed, subscribe to the `connection-closed` event. To silence
460+
* further events from the underlying connection, set `unbind` to `true`.
460461
*/
461-
public disconnect(): void {
462-
this.signaling.disconnect();
462+
public disconnect(unbind = false): void {
463+
this.signaling.disconnect(unbind);
463464
}
464465

465466
/**

src/signaling/common.ts

Lines changed: 47 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export abstract class Signaling implements saltyrtc.Signaling {
9191
}
9292
this.handoverState.onBoth = () => {
9393
this.client.emit({type: 'handover'});
94-
this.ws.close(CloseCode.Handover);
94+
this.closeWebsocket(CloseCode.Handover);
9595
};
9696
}
9797

@@ -161,8 +161,10 @@ export abstract class Signaling implements saltyrtc.Signaling {
161161

162162
/**
163163
* Disconnect from the signaling server.
164+
*
165+
* @param unbind Whether to unbind all WebSocket events.
164166
*/
165-
public disconnect(): void {
167+
public disconnect(unbind = false): void {
166168
const reason = CloseCode.ClosingNormal;
167169

168170
// Update state
@@ -173,12 +175,8 @@ export abstract class Signaling implements saltyrtc.Signaling {
173175
this.sendClose(reason);
174176
}
175177

176-
// Close WebSocket instance
177-
if (this.ws !== null) {
178-
console.debug(this.logTag, 'Disconnecting WebSocket');
179-
this.ws.close(reason);
180-
}
181-
this.ws = null;
178+
// Close WebSocket instance and unbind all events
179+
this.closeWebsocket(reason, undefined, unbind);
182180

183181
// Close task connections
184182
if (this.task !== null) {
@@ -191,7 +189,45 @@ export abstract class Signaling implements saltyrtc.Signaling {
191189
}
192190

193191
/**
194-
* Return connection path for websocket.
192+
* Close the WebSocket connection.
193+
*
194+
* @param code The close code.
195+
* @param reason The close reason.
196+
* @param unbind Whether to unbind all events. This will move the Signaling
197+
* instance into the `closed` state.
198+
*/
199+
private closeWebsocket(code?: number, reason?: string, unbind = false): void {
200+
if (this.ws !== null) {
201+
// Drop internal close codes
202+
// see: https://github.com/saltyrtc/saltyrtc-meta/issues/110
203+
if (code === undefined || code <= 3000) {
204+
code = CloseCode.ClosingNormal;
205+
}
206+
207+
// Disconnect
208+
console.debug(this.logTag, `Disconnecting WebSocket, close code: ${code}`);
209+
this.ws.close(code, reason);
210+
211+
// Unbind events?
212+
if (unbind) {
213+
this.ws.removeEventListener('open', this.onOpen);
214+
this.ws.removeEventListener('error', this.onError);
215+
this.ws.removeEventListener('close', this.onClose);
216+
this.ws.removeEventListener('message', this.onMessage);
217+
}
218+
219+
// Forget instance
220+
this.ws = null;
221+
222+
// Move into closed state (if necessary)
223+
if (unbind) {
224+
this.setState('closed');
225+
}
226+
}
227+
}
228+
229+
/**
230+
* Return connection path for WebSocket.
195231
*/
196232
protected abstract getWebsocketPath(): string;
197233

@@ -246,30 +282,6 @@ export abstract class Signaling implements saltyrtc.Signaling {
246282
' (' + explainCloseCode(ev.code) + ')');
247283
this.setState('closed');
248284
this.client.emit({type: 'connection-closed', data: ev.code});
249-
const logError = (reason: string) => console.error(this.logTag, 'Websocket close reason:', reason);
250-
switch (ev.code) {
251-
case CloseCode.GoingAway:
252-
logError('Server is being shut down');
253-
break;
254-
case CloseCode.NoSharedSubprotocol:
255-
logError('No shared sub-protocol could be found');
256-
break;
257-
case CloseCode.PathFull:
258-
logError('Path full (no free responder byte)');
259-
break;
260-
case CloseCode.ProtocolError:
261-
logError('Protocol error');
262-
break;
263-
case CloseCode.InternalError:
264-
logError('Internal error');
265-
break;
266-
case CloseCode.DroppedByInitiator:
267-
logError('Dropped by initiator');
268-
break;
269-
case CloseCode.InvalidKey:
270-
logError('Invalid server key');
271-
break;
272-
}
273285
}
274286
}
275287

@@ -915,19 +927,11 @@ export abstract class Signaling implements saltyrtc.Signaling {
915927
* - Set `this.ws` to `null`
916928
* - Set `this.status` to `new`
917929
* - Reset the server combined sequence
930+
* - Unbind all events
918931
*/
919932
public resetConnection(reason?: number): void {
920933
// Close WebSocket instance
921-
if (this.ws !== null) {
922-
console.debug(this.logTag, 'Disconnecting WebSocket (close code ' + reason + ')');
923-
if (reason === CloseCode.GoingAway) {
924-
// See https://github.com/saltyrtc/saltyrtc-meta/pull/111/
925-
this.ws.close();
926-
} else {
927-
this.ws.close(reason);
928-
}
929-
}
930-
this.ws = null;
934+
this.closeWebsocket(reason, undefined, true);
931935

932936
// Reset
933937
this.server = new Server();

tests/integration.spec.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,19 @@ export default () => { describe('Integration Tests', function() {
340340

341341
spec = it('send connection-closed event only once', async (done) => {
342342
console.info('===> TEST NAME:', spec.getFullName());
343+
const initiator = new SaltyRTCBuilder()
344+
.connectTo(Config.SALTYRTC_HOST, Config.SALTYRTC_PORT)
345+
.withKeyStore(new KeyStore())
346+
.usingTasks([new DummyTask()])
347+
.withServerKey(Config.SALTYRTC_SERVER_PUBLIC_KEY)
348+
.asInitiator();
343349
let count = 0;
344-
this.initiator.on('connection-closed', (ev) => count += 1);
345-
this.initiator.connect();
350+
initiator.on('connection-closed', (ev) => {
351+
count += 1
352+
});
353+
initiator.connect();
346354
await sleep(100);
347-
this.initiator.signaling.resetConnection(3001);
355+
(initiator as any).signaling.closeWebsocket(3001);
348356
await sleep(100);
349357
expect(count).toEqual(1);
350358
done();

0 commit comments

Comments
 (0)