From 28d2b18f0908fc9045352b96b05dbd7b0079862f Mon Sep 17 00:00:00 2001 From: Matt Hill Date: Mon, 23 Aug 2021 15:06:24 -0600 Subject: [PATCH] fix unsubscribes on logout, better connection monitoring, and better message for lost connection --- ui/src/app/app.component.ts | 70 +++++++++--------- .../components/status/status.component.html | 4 +- .../app/components/status/status.component.ts | 1 + .../apps-routes/app-list/app-list.page.html | 4 +- .../apps-routes/app-list/app-list.page.scss | 2 +- .../apps-routes/app-show/app-show.page.html | 73 +++++++++++-------- .../apps-routes/app-show/app-show.page.ts | 21 +++--- ui/src/app/services/connection.service.ts | 17 ++--- .../app/services/patch-db/patch-db.service.ts | 15 +++- ui/src/app/services/startup-alerts.service.ts | 5 +- 10 files changed, 115 insertions(+), 97 deletions(-) diff --git a/ui/src/app/app.component.ts b/ui/src/app/app.component.ts index b14884019..623efd67e 100644 --- a/ui/src/app/app.component.ts +++ b/ui/src/app/app.component.ts @@ -3,7 +3,7 @@ import { Storage } from '@ionic/storage-angular' import { AuthService, AuthState } from './services/auth.service' import { ApiService } from './services/api/embassy-api.service' import { Router, RoutesRecognized } from '@angular/router' -import { debounceTime, distinctUntilChanged, filter, take, takeWhile } from 'rxjs/operators' +import { debounceTime, distinctUntilChanged, filter, take } from 'rxjs/operators' import { AlertController, IonicSafeString, LoadingController, ToastController } from '@ionic/angular' import { Emver } from './services/emver.service' import { SplitPaneTracker } from './services/split-pane.service' @@ -16,6 +16,7 @@ import { StartupAlertsService } from './services/startup-alerts.service' import { ConfigService } from './services/config.service' import { isEmptyObject } from './util/misc.util' import { ErrorToastService } from './services/error-toast.service' +import { Subscription } from 'rxjs' @Component({ selector: 'app-root', @@ -29,6 +30,7 @@ export class AppComponent { offlineToast: HTMLIonToastElement serverName: string unreadCount: number + subscriptions: Subscription[] = [] appPages = [ { title: 'Services', @@ -97,23 +99,28 @@ export class AppComponent { if (this.router.url.startsWith('/login')) { this.router.navigate([''], { replaceUrl: true }) } - // start the connection monitor - this.connectionService.start(auth) - // watch connection to display connectivity issues - this.watchConnection(auth) - // // watch router to highlight selected menu item - this.watchRouter(auth) - // // watch status to display/hide maintenance page - this.watchStatus(auth) - // // watch version to refresh browser window - this.watchVersion(auth) - // // watch unread notification count to display toast - this.watchNotifications(auth) - // // run startup alerts - this.startupAlertsService.runChecks() + + this.subscriptions = [ + // start the connection monitor + ...this.connectionService.start(), + // watch connection to display connectivity issues + this.watchConnection(), + // // watch router to highlight selected menu item + this.watchRouter(), + // // watch status to display/hide maintenance page + this.watchStatus(), + // // watch version to refresh browser window + this.watchVersion(), + // // watch unread notification count to display toast + this.watchNotifications(), + // // run startup alerts + this.startupAlertsService.runChecks(), + ] }) // UNVERIFIED } else if (auth === AuthState.UNVERIFIED) { + this.subscriptions.forEach(sub => sub.unsubscribe()) + this.subscriptions = [] this.showMenu = false this.patch.stop() this.storage.clear() @@ -136,12 +143,11 @@ export class AppComponent { window.open(url, '_blank') } - private watchConnection (auth: AuthState): void { - this.connectionService.watchFailure$() + private watchConnection (): Subscription { + return this.connectionService.watchFailure$() .pipe( distinctUntilChanged(), debounceTime(500), - takeWhile(() => auth === AuthState.VERIFIED), ) .subscribe(async connectionFailure => { if (connectionFailure === ConnectionFailure.None) { @@ -157,7 +163,7 @@ export class AppComponent { message = 'Phone or computer has no network connection.' break case ConnectionFailure.Diagnosing: - message = new IonicSafeString('Running network diagnostics ') + message = new IonicSafeString('Running network diagnostics ') break case ConnectionFailure.Embassy: message = 'Embassy appears to be offline.' @@ -180,11 +186,10 @@ export class AppComponent { }) } - private watchRouter (auth: AuthState): void { - this.router.events + private watchRouter (): Subscription { + return this.router.events .pipe( filter((e: RoutesRecognized) => !!e.urlAfterRedirects), - takeWhile(() => auth === AuthState.VERIFIED), ) .subscribe(e => { const appPageIndex = this.appPages.findIndex( @@ -194,11 +199,8 @@ export class AppComponent { }) } - private watchStatus (auth: AuthState): void { - this.patch.watch$('server-info', 'status') - .pipe( - takeWhile(() => auth === AuthState.VERIFIED), - ) + private watchStatus (): Subscription { + return this.patch.watch$('server-info', 'status') .subscribe(status => { const maintenance = '/maintenance' const route = this.router.url @@ -213,11 +215,8 @@ export class AppComponent { }) } - private watchVersion (auth: AuthState): void { - this.patch.watch$('server-info', 'version') - .pipe( - takeWhile(() => auth === AuthState.VERIFIED), - ) + private watchVersion (): Subscription { + return this.patch.watch$('server-info', 'version') .subscribe(version => { if (this.emver.compare(this.config.version, version) !== 0) { this.presentAlertRefreshNeeded() @@ -225,12 +224,9 @@ export class AppComponent { }) } - private watchNotifications (auth: AuthState): void { + private watchNotifications (): Subscription { let previous: number - this.patch.watch$('server-info', 'unread-notification-count') - .pipe( - takeWhile(() => auth === AuthState.VERIFIED), - ) + return this.patch.watch$('server-info', 'unread-notification-count') .subscribe(count => { this.unreadCount = count if (previous !== undefined && count > previous) this.presentToastNotifications() diff --git a/ui/src/app/components/status/status.component.html b/ui/src/app/components/status/status.component.html index 031ae01a2..db8dd6ce5 100644 --- a/ui/src/app/components/status/status.component.html +++ b/ui/src/app/components/status/status.component.html @@ -1,9 +1,9 @@

- {{ rendering.display }} + {{ disconnected ? 'Unknown' : rendering.display }}

diff --git a/ui/src/app/components/status/status.component.ts b/ui/src/app/components/status/status.component.ts index 405b0bb43..c0565bd3a 100644 --- a/ui/src/app/components/status/status.component.ts +++ b/ui/src/app/components/status/status.component.ts @@ -11,5 +11,6 @@ export class StatusComponent { @Input() size?: 'small' | 'medium' | 'large' | 'x-large' = 'large' @Input() style?: string = 'regular' @Input() weight?: string = 'normal' + @Input() disconnected?: boolean = false } diff --git a/ui/src/app/pages/apps-routes/app-list/app-list.page.html b/ui/src/app/pages/apps-routes/app-list/app-list.page.html index 192ba70cc..fd55a7c55 100644 --- a/ui/src/app/pages/apps-routes/app-list/app-list.page.html +++ b/ui/src/app/pages/apps-routes/app-list/app-list.page.html @@ -36,8 +36,8 @@ - -

{{ pkg.value.entry.state | titlecase }}...{{ (pkg.value.entry['install-progress'] | installState).totalProgress }}%

+ +

{{ pkg.value.entry.state | titlecase }}...{{ (pkg.value.entry['install-progress'] | installState).totalProgress }}%

{{ pkg.value.entry.manifest.title }} diff --git a/ui/src/app/pages/apps-routes/app-list/app-list.page.scss b/ui/src/app/pages/apps-routes/app-list/app-list.page.scss index a821184cb..fca704000 100644 --- a/ui/src/app/pages/apps-routes/app-list/app-list.page.scss +++ b/ui/src/app/pages/apps-routes/app-list/app-list.page.scss @@ -78,7 +78,7 @@ right: 0px; } -.installing-status { +.main-status { font-size: calc(8px + .4vw); font-weight: bold; margin: 0; diff --git a/ui/src/app/pages/apps-routes/app-show/app-show.page.html b/ui/src/app/pages/apps-routes/app-show/app-show.page.html index 274f5da8e..392623b6d 100644 --- a/ui/src/app/pages/apps-routes/app-show/app-show.page.html +++ b/ui/src/app/pages/apps-routes/app-show/app-show.page.html @@ -23,54 +23,69 @@ Status - + Open UI - - Configure - - - Stop - - - Fix - - - Start - + + + Configure + + + Stop + + + Fix + + + Start + + - + - + - + Health Checks - - - - - - -

{{ health.key }}

-

{{ health.value.result }}

-

{{ health.value.error }}

- - + + + + + + + + + + + + + + + + + + +

{{ health.key }}

+

{{ health.value.result }}

+

{{ health.value.error }}

+ + + - + Menu {{ button.title }} - + Dependencies diff --git a/ui/src/app/pages/apps-routes/app-show/app-show.page.ts b/ui/src/app/pages/apps-routes/app-show/app-show.page.ts index eb82b326e..1bff44580 100644 --- a/ui/src/app/pages/apps-routes/app-show/app-show.page.ts +++ b/ui/src/app/pages/apps-routes/app-show/app-show.page.ts @@ -10,7 +10,7 @@ import { ConfigService } from 'src/app/services/config.service' import { PatchDbService } from 'src/app/services/patch-db/patch-db.service' import { DependencyErrorConfigUnsatisfied, DependencyErrorNotInstalled, DependencyErrorType, MainStatus, PackageDataEntry, PackageMainStatus, PackageState } from 'src/app/services/patch-db/data-model' import { FEStatus, PkgStatusRendering, renderPkgStatus } from 'src/app/services/pkg-status-rendering.service' -import { ConnectionService } from 'src/app/services/connection.service' +import { ConnectionFailure, ConnectionService } from 'src/app/services/connection.service' import { ErrorToastService } from 'src/app/services/error-toast.service' import { AppConfigPage } from 'src/app/modals/app-config/app-config.page' @@ -24,7 +24,6 @@ export class AppShowPage { pkg: PackageDataEntry hideLAN: boolean buttons: Button[] = [] - connected: boolean FeStatus = FEStatus PackageState = PackageState DependencyErrorType = DependencyErrorType @@ -32,6 +31,7 @@ export class AppShowPage { Math = Math mainStatus: MainStatus PackageMainStatus = PackageMainStatus + connectionFailure: boolean @ViewChild(IonContent) content: IonContent subs: Subscription[] = [] @@ -53,18 +53,17 @@ export class AppShowPage { async ngOnInit () { this.pkgId = this.route.snapshot.paramMap.get('pkgId') this.subs = [ - combineLatest([ - this.patch.connected$(), - this.patch.watch$('package-data', this.pkgId), - ]) - .subscribe(([connected, pkg]) => { + // 1 + this.patch.watch$('package-data', this.pkgId) + .subscribe(pkg => { this.pkg = pkg - this.connected = connected this.rendering = renderPkgStatus(pkg.state, pkg.installed.status) + this.mainStatus = pkg.installed.status.main }), - this.patch.watch$('package-data', this.pkgId, 'installed', 'status', 'main') - .subscribe(main => { - this.mainStatus = main + // 2 + this.connectionService.watchFailure$() + .subscribe(connectionFailure => { + this.connectionFailure = connectionFailure !== ConnectionFailure.None }), ] this.setButtons() diff --git a/ui/src/app/services/connection.service.ts b/ui/src/app/services/connection.service.ts index 0a434f004..27a526627 100644 --- a/ui/src/app/services/connection.service.ts +++ b/ui/src/app/services/connection.service.ts @@ -2,9 +2,9 @@ import { Injectable } from '@angular/core' import { BehaviorSubject, combineLatest, fromEvent, merge, Subscription } from 'rxjs' import { PatchConnection, PatchDbService } from './patch-db/patch-db.service' import { HttpService, Method } from './http.service' -import { distinctUntilChanged, takeWhile } from 'rxjs/operators' +import { distinctUntilChanged } from 'rxjs/operators' import { ConfigService } from './config.service' -import { AuthState } from './auth.service' +import { pauseFor } from '../util/misc.util' @Injectable({ providedIn: 'root', @@ -23,16 +23,13 @@ export class ConnectionService { return this.connectionFailure$.asObservable() } - start (auth: AuthState) { - merge(fromEvent(window, 'online'), fromEvent(window, 'offline')) - .pipe( - takeWhile(() => auth === AuthState.VERIFIED), - ) + start (): Subscription[] { + const sub1 = merge(fromEvent(window, 'online'), fromEvent(window, 'offline')) .subscribe(event => { this.networkState$.next(event.type === 'online') - }), + }) - combineLatest([ + const sub2 = combineLatest([ // 1 this.networkState$ .pipe( @@ -46,7 +43,6 @@ export class ConnectionService { // 3 this.patch.watch$('server-info', 'connection-addresses') .pipe( - takeWhile(() => auth === AuthState.VERIFIED), distinctUntilChanged(), ), ]) @@ -76,6 +72,7 @@ export class ConnectionService { } } }) + return [sub1, sub2] } private async testAddrs (addrs: string[]): Promise { diff --git a/ui/src/app/services/patch-db/patch-db.service.ts b/ui/src/app/services/patch-db/patch-db.service.ts index e186799f0..5ddb61d80 100644 --- a/ui/src/app/services/patch-db/patch-db.service.ts +++ b/ui/src/app/services/patch-db/patch-db.service.ts @@ -2,6 +2,7 @@ import { Inject, Injectable, InjectionToken } from '@angular/core' import { Bootstrapper, PatchDB, Source, Store } from 'patch-db-client' import { BehaviorSubject, Observable, of, Subscription } from 'rxjs' import { catchError, debounceTime, finalize, map, tap } from 'rxjs/operators' +import { pauseFor } from 'src/app/util/misc.util' import { ApiService } from '../api/embassy-api.service' import { DataModel } from './data-model' @@ -40,7 +41,11 @@ export class PatchDbService { start (): void { // make sure everything is stopped before initializing - this.stop() + if (this.patchSub) { + this.patchSub.unsubscribe() + this.patchSub = undefined + } + console.log('Retrying') try { this.patchSub = this.patchDb.sync$() .pipe(debounceTime(500)) @@ -49,10 +54,12 @@ export class PatchDbService { this.patchConnection$.next(PatchConnection.Connected) this.bootstrapper.update(cache) }, - error: e => { + error: async e => { console.error('patch-db-sync sub ERROR', e) this.patchConnection$.next(PatchConnection.Disconnected) - // this.start() + console.log('Erroring out') + await pauseFor(4000) + this.start() }, complete: () => { console.warn('patch-db-sync sub COMPLETE') @@ -64,7 +71,9 @@ export class PatchDbService { } stop (): void { + console.log('STOPPING PATCH DB') this.patchConnection$.next(PatchConnection.Initializing) + this.patchDb.store.reset() if (this.patchSub) { this.patchSub.unsubscribe() this.patchSub = undefined diff --git a/ui/src/app/services/startup-alerts.service.ts b/ui/src/app/services/startup-alerts.service.ts index 49dd2fca9..493b5a573 100644 --- a/ui/src/app/services/startup-alerts.service.ts +++ b/ui/src/app/services/startup-alerts.service.ts @@ -13,6 +13,7 @@ import { PatchDbService } from './patch-db/patch-db.service' import { filter, take } from 'rxjs/operators' import { isEmptyObject } from '../util/misc.util' import { ApiService } from './api/embassy-api.service' +import { Subscription } from 'rxjs' @Injectable({ providedIn: 'root', @@ -60,8 +61,8 @@ export class StartupAlertsService { // Then, the reduce fires, quickly iterating through yielding a promise (previousDisplay) to the next element // Each promise fires more or less concurrently, so each c.check(server) is run concurrently // Then, since we await previousDisplay before c.display(res), each promise executing gets hung awaiting the display of the previous run - async runChecks (): Promise { - this.patch.watch$() + runChecks (): Subscription { + return this.patch.watch$() .pipe( filter(data => !isEmptyObject(data)), take(1),