Merge pull request #5060 from FinnStutzenstein/ws

Improved WS connection failure handling

client/src/app/app.component.ts

@@ -1,8 +1,7 @@
-import { ApplicationRef, Component } from '@angular/core';
+import { Component } from '@angular/core';
 import { Router } from '@angular/router';
 
 import { TranslateService } from '@ngx-translate/core';
-import { filter, take } from 'rxjs/operators';
 
 import { ConfigService } from './core/ui-services/config.service';
 import { ConstantsService } from './core/core-services/constants.service';
@@ -65,7 +64,6 @@ export class AppComponent {
      */
     public constructor(
         translate: TranslateService,
-        appRef: ApplicationRef,
         servertimeService: ServertimeService,
         router: Router,
         operator: OperatorService,
@@ -95,15 +93,7 @@ export class AppComponent {
         this.overloadFlatMap();
         this.overloadModulo();
 
-        // Show the spinner initial
-
-        appRef.isStable
-            .pipe(
-                // take only the stable state
-                filter(s => s),
-                take(1)
-            )
-            .subscribe(() => servertimeService.startScheduler());
+        servertimeService.startScheduler();
     }
 
     /**

client/src/app/core/core-services/ping.service.ts

@@ -1,6 +1,4 @@
-import { ApplicationRef, Injectable } from '@angular/core';
-
-import { first, take } from 'rxjs/operators';
+import { Injectable } from '@angular/core';
 
 import { ConstantsService } from './constants.service';
 import { Deferred } from '../promises/deferred';
@@ -21,37 +19,25 @@ export class PingService {
      */
     private pingInterval: any;
 
-    private intervalTime: number;
+    private intervalTime = 30000;
 
-    private timeoutTime: number;
+    private timeoutTime = 5000;
 
     private lastLatency: number | null = null;
 
-    public constructor(
-        private websocketService: WebsocketService,
-        private appRef: ApplicationRef,
-        private constantsService: ConstantsService
-    ) {
+    public constructor(private websocketService: WebsocketService, private constantsService: ConstantsService) {
         this.setup();
     }
 
     private async setup(): Promise<void> {
         const gotConstants = new Deferred();
-        const isStable = new Deferred();
 
-        this.constantsService
-            .get<OpenSlidesSettings>('Settings')
-            .pipe(take(1))
-            .subscribe(settings => {
-                this.intervalTime = settings.PING_INTERVAL || 30000;
-                this.timeoutTime = settings.PING_TIMEOUT || 5000;
-                gotConstants.resolve();
-            });
-        this.appRef.isStable.pipe(first(s => s)).subscribe(() => {
-            isStable.resolve();
+        this.constantsService.get<OpenSlidesSettings>('Settings').subscribe(settings => {
+            this.intervalTime = settings.PING_INTERVAL || 30000;
+            this.timeoutTime = settings.PING_TIMEOUT || 5000;
+            gotConstants.resolve();
         });
-
-        await Promise.all([gotConstants, isStable]);
+        await gotConstants;
 
         // Connects the ping-pong mechanism to the opening and closing of the connection.
         this.websocketService.closeEvent.subscribe(() => this.stopPing());

client/src/app/core/core-services/servertime.service.ts

@@ -32,7 +32,7 @@ export class ServertimeService {
      * Starts the scheduler to sync with the server.
      */
     public startScheduler(): void {
-        this.scheduleNextRefresh(0);
+        this.scheduleNextRefresh(0.1);
     }
 
     /**

client/src/app/core/core-services/websocket.service.ts

@@ -156,7 +156,8 @@ export class WebsocketService {
     /**
      * The websocket.
      */
-    private websocket: WebSocket;
+    private websocket: WebSocket | null;
+    private websocketId: string | null;
 
     /**
      * Subjects for types of websocket messages. A subscriber can get an Observable by {@function getOberservable}.
@@ -208,8 +209,14 @@ export class WebsocketService {
      * Uses NgZone to let all callbacks run in the angular context.
      */
     public async connect(options: ConnectOptions = {}, retry: boolean = false): Promise<void> {
+        const websocketId = Math.random()
+            .toString(36)
+            .substring(7);
+        this.websocketId = websocketId;
+
         if (this.websocket) {
-            await this.close();
+            this.websocket.close();
+            this.websocket = null;
         }
 
         if (!retry) {
@@ -244,17 +251,20 @@ export class WebsocketService {
         // connection established. If this connect attept was a retry,
         // The error notice will be removed and the reconnectSubject is published.
         this.websocket.onopen = (event: Event) => {
+            if (this.websocketId !== websocketId) {
+                return;
+            }
             this.zone.run(() => {
                 this.retryCounter = 0;
 
                 if (this.shouldBeClosed) {
-                    this.dismissConnectionErrorNotice();
+                    this.offlineService.goOnline();
                     return;
                 }
 
                 this._connectionOpen = true;
                 if (retry) {
-                    this.dismissConnectionErrorNotice();
+                    this.offlineService.goOnline();
                     this._retryReconnectEvent.emit();
                 } else {
                     this._noRetryConnectEvent.emit();
@@ -268,22 +278,31 @@ export class WebsocketService {
         };
 
         this.websocket.onmessage = (event: MessageEvent) => {
+            if (this.websocketId !== websocketId) {
+                return;
+            }
             this.zone.run(() => {
                 this.handleMessage(event.data);
             });
         };
 
         this.websocket.onclose = (event: CloseEvent) => {
+            if (this.websocketId !== websocketId) {
+                return;
+            }
             this.zone.run(() => {
-                this.onclose(event.code === 1000);
+                this.onclose();
             });
         };
 
         this.websocket.onerror = (event: ErrorEvent) => {
+            if (this.websocketId !== websocketId) {
+                return;
+            }
             // place for proper error handling and debugging.
             // Required to get more information about errors
             this.zone.run(() => {
-                console.warn('Websocket is on Error state. Error: ', event);
+                console.warn('WS error event:', event);
             });
         };
     }
@@ -353,25 +372,20 @@ export class WebsocketService {
         }
     }
 
-    /**
-     * Simulates an abnormal close.
-     */
-    public simulateAbnormalClose(): void {
-        if (this.websocket) {
-            this.websocket.close();
-        }
-        this.onclose(false);
-    }
-
     /**
      * Closes the connection error notice
      */
-    private onclose(normalClose: boolean): void {
-        this.websocket = null;
+    private onclose(): void {
+        if (this.websocket) {
+            this.websocketId = null; // set to null, so now further events will be
+            // registered with the line below.
+            this.websocket.close(); // Cleanup old connection
+            this.websocket = null;
+        }
         this._connectionOpen = false;
         // 1000 is a normal close, like the close on logout
         this._closeEvent.emit();
-        if (!this.shouldBeClosed && !normalClose) {
+        if (!this.shouldBeClosed) {
             // Do not show the message snackbar on the projector
             // tests for /projector and /projector/<id>
             const onProjector = this.router.url.match(/^\/projector(\/[0-9]+\/?)?$/);
@@ -399,16 +413,12 @@ export class WebsocketService {
         }
     }
 
-    private dismissConnectionErrorNotice(): void {
-        this.offlineService.goOnline();
-    }
-
     /**
      * Closes the websocket connection.
      */
     public async close(): Promise<void> {
         this.shouldBeClosed = true;
-        this.dismissConnectionErrorNotice();
+        this.offlineService.goOnline();
         if (this.websocket) {
             this.websocket.close();
             this.websocket = null;
@@ -416,6 +426,15 @@ export class WebsocketService {
         }
     }
 
+    /**
+     * Simulates an abnormal close.
+     *
+     * Internally does not set `shouldBeClosed`, so a reconnect is forced.
+     */
+    public simulateAbnormalClose(): void {
+        this.onclose();
+    }
+
     /**
      * closes and reopens the connection. If the connection was closed before,
      * it will be just opened.