Skip to content

Commit

Permalink
Allow to unbind events when closing a WebSocket connection, see #108
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
lgrahl committed Sep 26, 2018
1 parent c482f0c commit a4487ba
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 50 deletions.
2 changes: 1 addition & 1 deletion saltyrtc-client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ declare namespace saltyrtc {
decryptFromPeer(box: Box): Uint8Array;

connect(): void;
disconnect(): void;
disconnect(unbind?: boolean): void;

sendApplicationMessage(data: any): void;

Expand Down
7 changes: 4 additions & 3 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,11 @@ class SaltyRTC implements saltyrtc.SaltyRTC {
* Both the WebSocket as well as any open task connection will be closed.
*
* This method is asynchronous. To get notified when the connection is has
* been closed, subscribe to the `connection-closed` event.
* been closed, subscribe to the `connection-closed` event. To silence
* further events from the underlying connection, set `unbind` to `true`.
*/
public disconnect(): void {
this.signaling.disconnect();
public disconnect(unbind = false): void {
this.signaling.disconnect(unbind);
}

/**
Expand Down
90 changes: 47 additions & 43 deletions src/signaling/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export abstract class Signaling implements saltyrtc.Signaling {
}
this.handoverState.onBoth = () => {
this.client.emit({type: 'handover'});
this.ws.close(CloseCode.Handover);
this.closeWebsocket(CloseCode.Handover);
};
}

Expand Down Expand Up @@ -161,8 +161,10 @@ export abstract class Signaling implements saltyrtc.Signaling {

/**
* Disconnect from the signaling server.
*
* @param unbind Whether to unbind all WebSocket events.
*/
public disconnect(): void {
public disconnect(unbind = false): void {
const reason = CloseCode.ClosingNormal;

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

// Close WebSocket instance
if (this.ws !== null) {
console.debug(this.logTag, 'Disconnecting WebSocket');
this.ws.close(reason);
}
this.ws = null;
// Close WebSocket instance and unbind all events
this.closeWebsocket(reason, undefined, unbind);

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

/**
* Return connection path for websocket.
* Close the WebSocket connection.
*
* @param code The close code.
* @param reason The close reason.
* @param unbind Whether to unbind all events. This will move the Signaling
* instance into the `closed` state.
*/
private closeWebsocket(code?: number, reason?: string, unbind = false): void {
if (this.ws !== null) {
// Drop internal close codes
// see: https://github.com/saltyrtc/saltyrtc-meta/issues/110
if (code === undefined || code <= 3000) {
code = CloseCode.ClosingNormal;
}

// Disconnect
console.debug(this.logTag, `Disconnecting WebSocket, close code: ${code}`);
this.ws.close(code, reason);

// Unbind events?
if (unbind) {
this.ws.removeEventListener('open', this.onOpen);
this.ws.removeEventListener('error', this.onError);
this.ws.removeEventListener('close', this.onClose);
this.ws.removeEventListener('message', this.onMessage);
}

// Forget instance
this.ws = null;

// Move into closed state (if necessary)
if (unbind) {
this.setState('closed');
}
}
}

/**
* Return connection path for WebSocket.
*/
protected abstract getWebsocketPath(): string;

Expand Down Expand Up @@ -246,30 +282,6 @@ export abstract class Signaling implements saltyrtc.Signaling {
' (' + explainCloseCode(ev.code) + ')');
this.setState('closed');
this.client.emit({type: 'connection-closed', data: ev.code});
const logError = (reason: string) => console.error(this.logTag, 'Websocket close reason:', reason);
switch (ev.code) {
case CloseCode.GoingAway:
logError('Server is being shut down');
break;
case CloseCode.NoSharedSubprotocol:
logError('No shared sub-protocol could be found');
break;
case CloseCode.PathFull:
logError('Path full (no free responder byte)');
break;
case CloseCode.ProtocolError:
logError('Protocol error');
break;
case CloseCode.InternalError:
logError('Internal error');
break;
case CloseCode.DroppedByInitiator:
logError('Dropped by initiator');
break;
case CloseCode.InvalidKey:
logError('Invalid server key');
break;
}
}
}

Expand Down Expand Up @@ -915,19 +927,11 @@ export abstract class Signaling implements saltyrtc.Signaling {
* - Set `this.ws` to `null`
* - Set `this.status` to `new`
* - Reset the server combined sequence
* - Unbind all events
*/
public resetConnection(reason?: number): void {
// Close WebSocket instance
if (this.ws !== null) {
console.debug(this.logTag, 'Disconnecting WebSocket (close code ' + reason + ')');
if (reason === CloseCode.GoingAway) {
// See https://github.com/saltyrtc/saltyrtc-meta/pull/111/
this.ws.close();
} else {
this.ws.close(reason);
}
}
this.ws = null;
this.closeWebsocket(reason, undefined, true);

// Reset
this.server = new Server();
Expand Down
14 changes: 11 additions & 3 deletions tests/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,11 +340,19 @@ export default () => { describe('Integration Tests', function() {

spec = it('send connection-closed event only once', async (done) => {
console.info('===> TEST NAME:', spec.getFullName());
const initiator = new SaltyRTCBuilder()
.connectTo(Config.SALTYRTC_HOST, Config.SALTYRTC_PORT)
.withKeyStore(new KeyStore())
.usingTasks([new DummyTask()])
.withServerKey(Config.SALTYRTC_SERVER_PUBLIC_KEY)
.asInitiator();
let count = 0;
this.initiator.on('connection-closed', (ev) => count += 1);
this.initiator.connect();
initiator.on('connection-closed', (ev) => {
count += 1
});
initiator.connect();
await sleep(100);
this.initiator.signaling.resetConnection(3001);
(initiator as any).signaling.closeWebsocket(3001);
await sleep(100);
expect(count).toEqual(1);
done();
Expand Down

0 comments on commit a4487ba

Please sign in to comment.