Skip to content

Commit

Permalink
Preventing false positive alarms for connection state mode
Browse files Browse the repository at this point in the history 
Signed-off-by: Vladyslav Zhukovskyi <vzhukovs@redhat.com>
  • Loading branch information
@vzhukovs
vzhukovs committed Feb 17, 2021
1 parent f3a15db commit ad7bf94
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 36 deletions.
127 changes: 126 additions & 1 deletion packages/core/src/browser/connection-status-service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,20 @@ FrontendApplicationConfigProvider.set({
});

import { expect } from 'chai';
import { ConnectionStatus } from './connection-status-service';
import {
ConnectionStatus,
ConnectionStatusOptions,
FrontendConnectionStatusService,
PingService
} from './connection-status-service';
import { MockConnectionStatusService } from './test/mock-connection-status-service';

import * as sinon from 'sinon';

import { Container } from 'inversify';
import { WebSocketConnectionProvider } from './messaging/ws-connection-provider';
import { ILogger, Emitter, Loggable } from '../common';

disableJSDOM();

describe('connection-status', function (): void {
Expand Down Expand Up @@ -73,6 +84,120 @@ describe('connection-status', function (): void {

});

describe('frontend-connection-status', function (): void {
const OFFLINE_TIMEOUT = 10;

let testContainer: Container;

const mockSocketOpenedEmitter: Emitter<void> = new Emitter();
const mockSocketClosedEmitter: Emitter<void> = new Emitter();
const mockIncomingMessageActivityEmitter: Emitter<void> = new Emitter();

before(() => {
disableJSDOM = enableJSDOM();
});

after(() => {
disableJSDOM();
});

let timer: sinon.SinonFakeTimers;
let pingSpy: sinon.SinonSpy;
beforeEach(() => {
const mockWebSocketConnectionProvider = sinon.createStubInstance(WebSocketConnectionProvider);
const mockPingService: PingService = <PingService>{
ping(): Promise<void> {
return Promise.resolve(undefined);
}
};
const mockILogger: ILogger = <ILogger>{
trace(loggable: Loggable): Promise<void> {
return Promise.resolve(undefined);
}
};

testContainer = new Container();
testContainer.bind(FrontendConnectionStatusService).toSelf().inSingletonScope();
testContainer.bind(PingService).toConstantValue(mockPingService);
testContainer.bind(ILogger).toConstantValue(mockILogger);
testContainer.bind(ConnectionStatusOptions).toConstantValue({ offlineTimeout: OFFLINE_TIMEOUT });
testContainer.bind(WebSocketConnectionProvider).toConstantValue(mockWebSocketConnectionProvider);

sinon.stub(mockWebSocketConnectionProvider, 'onSocketOpened').value(mockSocketOpenedEmitter.event);
sinon.stub(mockWebSocketConnectionProvider, 'onSocketClosed').value(mockSocketClosedEmitter.event);
sinon.stub(mockWebSocketConnectionProvider, 'onIncomingMessageActivity').value(mockIncomingMessageActivityEmitter.event);

timer = sinon.useFakeTimers();

pingSpy = sinon.spy(mockPingService, 'ping');
});

afterEach(() => {
pingSpy.restore();
timer.restore();
testContainer.unbindAll();
});

it('should switch status to offline on websocket close', () => {
const frontendConnectionStatusService = testContainer.get<FrontendConnectionStatusService>(FrontendConnectionStatusService);
frontendConnectionStatusService['init']();
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.ONLINE);
mockSocketClosedEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.OFFLINE);
});

it('should switch status to online on websocket established', () => {
const frontendConnectionStatusService = testContainer.get<FrontendConnectionStatusService>(FrontendConnectionStatusService);
frontendConnectionStatusService['init']();
mockSocketClosedEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.OFFLINE);
mockSocketOpenedEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.ONLINE);
});

it('should switch status to online on any websocket activity', () => {
const frontendConnectionStatusService = testContainer.get<FrontendConnectionStatusService>(FrontendConnectionStatusService);
frontendConnectionStatusService['init']();
mockSocketClosedEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.OFFLINE);
mockIncomingMessageActivityEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.ONLINE);
});

it('should perform ping request after socket activity', () => {
const frontendConnectionStatusService = testContainer.get<FrontendConnectionStatusService>(FrontendConnectionStatusService);
frontendConnectionStatusService['init']();
mockIncomingMessageActivityEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.ONLINE);
sinon.assert.notCalled(pingSpy);
timer.tick(OFFLINE_TIMEOUT);
sinon.assert.calledOnce(pingSpy);
});

it('should not perform ping request before desired timeout', () => {
const frontendConnectionStatusService = testContainer.get<FrontendConnectionStatusService>(FrontendConnectionStatusService);
frontendConnectionStatusService['init']();
mockIncomingMessageActivityEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.ONLINE);
sinon.assert.notCalled(pingSpy);
timer.tick(OFFLINE_TIMEOUT - 1);
sinon.assert.notCalled(pingSpy);
});

it('should switch to offline mode if ping request was rejected', () => {
const pingService = testContainer.get<PingService>(PingService);
pingSpy.restore();
const stub = sinon.stub(pingService, 'ping').onFirstCall().throws('failed to make a ping request');
const frontendConnectionStatusService = testContainer.get<FrontendConnectionStatusService>(FrontendConnectionStatusService);
frontendConnectionStatusService['init']();
mockIncomingMessageActivityEmitter.fire(undefined);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.ONLINE);
timer.tick(OFFLINE_TIMEOUT);
sinon.assert.calledOnce(stub);
expect(frontendConnectionStatusService.currentStatus).to.be.equal(ConnectionStatus.OFFLINE);
});
});

function pause(time: number = 1): Promise<unknown> {
return new Promise(resolve => setTimeout(resolve, time));
}
71 changes: 37 additions & 34 deletions packages/core/src/browser/connection-status-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,11 @@ export abstract class AbstractConnectionStatusService implements ConnectionStatu
protected readonly statusChangeEmitter = new Emitter<ConnectionStatus>();

protected connectionStatus: ConnectionStatus = ConnectionStatus.ONLINE;
protected timer: number | undefined;

@inject(ILogger)
protected readonly logger: ILogger;

constructor(@inject(ConnectionStatusOptions) @optional() protected readonly options: ConnectionStatusOptions = ConnectionStatusOptions.DEFAULT) { }
protected constructor(@inject(ConnectionStatusOptions) @optional() protected readonly options: ConnectionStatusOptions = ConnectionStatusOptions.DEFAULT) { }

get onStatusChange(): Event<ConnectionStatus> {
return this.statusChangeEmitter.event;
Expand All @@ -98,42 +97,21 @@ export abstract class AbstractConnectionStatusService implements ConnectionStatu

dispose(): void {
this.statusChangeEmitter.dispose();
if (this.timer) {
this.clearTimeout(this.timer);
}
}

protected updateStatus(success: boolean): void {
// clear existing timer
if (this.timer) {
this.clearTimeout(this.timer);
}
const previousStatus = this.connectionStatus;
const newStatus = success ? ConnectionStatus.ONLINE : ConnectionStatus.OFFLINE;
if (previousStatus !== newStatus) {
this.connectionStatus = newStatus;
this.fireStatusChange(newStatus);
}
// schedule offline
this.timer = this.setTimeout(() => {
this.logger.trace(`No activity for ${this.options.offlineTimeout} ms. We are offline.`);
this.updateStatus(false);
}, this.options.offlineTimeout);
}

protected fireStatusChange(status: ConnectionStatus): void {
this.statusChangeEmitter.fire(status);
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
protected setTimeout(handler: (...args: any[]) => void, timeout: number): number {
return window.setTimeout(handler, timeout);
}

protected clearTimeout(handle: number): void {
window.clearTimeout(handle);
}

}

@injectable()
Expand All @@ -144,9 +122,20 @@ export class FrontendConnectionStatusService extends AbstractConnectionStatusSer
@inject(WebSocketConnectionProvider) protected readonly wsConnectionProvider: WebSocketConnectionProvider;
@inject(PingService) protected readonly pingService: PingService;

constructor(@inject(ConnectionStatusOptions) @optional() protected readonly options: ConnectionStatusOptions = ConnectionStatusOptions.DEFAULT) {
super(options);
}

@postConstruct()
protected init(): void {
this.schedulePing();
this.wsConnectionProvider.onSocketOpened(() => {
this.updateStatus(true);
this.schedulePing();
});
this.wsConnectionProvider.onSocketClosed(() => {
this.clearTimeout(this.scheduledPing);
this.updateStatus(false);
});
this.wsConnectionProvider.onIncomingMessageActivity(() => {
// natural activity
this.updateStatus(true);
Expand All @@ -155,18 +144,32 @@ export class FrontendConnectionStatusService extends AbstractConnectionStatusSer
}

protected schedulePing(): void {
if (this.scheduledPing) {
this.clearTimeout(this.scheduledPing);
}
this.clearTimeout(this.scheduledPing);
this.scheduledPing = this.setTimeout(async () => {
try {
await this.pingService.ping();
this.updateStatus(true);
} catch (e) {
this.logger.trace(e);
}
await this.performPingRequest();
this.schedulePing();
}, this.options.offlineTimeout * 0.8);
}, this.options.offlineTimeout);
}

protected async performPingRequest(): Promise<void> {
try {
await this.pingService.ping();
this.updateStatus(true);
} catch (e) {
this.updateStatus(false);
await this.logger.trace(e);
}
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
protected setTimeout(handler: (...args: any[]) => void, timeout: number): number {
return window.setTimeout(handler, timeout);
}

protected clearTimeout(handle?: number): void {
if (handle !== undefined) {
window.clearTimeout(handle);
}
}
}

Expand Down
12 changes: 11 additions & 1 deletion packages/core/src/browser/messaging/ws-connection-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/

import { injectable, interfaces, decorate, unmanaged } from 'inversify';
import { JsonRpcProxyFactory, JsonRpcProxy } from '../../common';
import { JsonRpcProxyFactory, JsonRpcProxy, Emitter, Event } from '../../common';
import { WebSocketChannel } from '../../common/messaging/web-socket-channel';
import { Endpoint } from '../endpoint';
import ReconnectingWebSocket from 'reconnecting-websocket';
Expand All @@ -34,6 +34,12 @@ export interface WebSocketOptions {
@injectable()
export class WebSocketConnectionProvider extends AbstractConnectionProvider<WebSocketOptions> {

protected readonly onSocketOpenedEmitter: Emitter<void> = new Emitter();
public onSocketOpened: Event<void> = this.onSocketOpenedEmitter.event;

protected readonly onSocketClosedEmitter: Emitter<void> = new Emitter();
public onSocketClosed: Event<void> = this.onSocketClosedEmitter.event;

static createProxy<T extends object>(container: interfaces.Container, path: string, arg?: object): JsonRpcProxy<T> {
return container.get(WebSocketConnectionProvider).createProxy<T>(path, arg);
}
Expand All @@ -45,7 +51,11 @@ export class WebSocketConnectionProvider extends AbstractConnectionProvider<WebS
const url = this.createWebSocketUrl(WebSocketChannel.wsPath);
const socket = this.createWebSocket(url);
socket.onerror = console.error;
socket.onopen = () => {
this.onSocketOpenedEmitter.fire(undefined);
};
socket.onclose = ({ code, reason }) => {
this.onSocketClosedEmitter.fire(undefined);
for (const channel of [...this.channels.values()]) {
channel.close(code, reason);
}
Expand Down

0 comments on commit ad7bf94

Please sign in to comment.