From 0cc0b765b53089fdc45fe302ced9b9227a06680b Mon Sep 17 00:00:00 2001 From: ziajka Date: Tue, 3 Jul 2018 11:34:37 +0200 Subject: [PATCH 1/3] Handle case when server is unavailable, Ref: #146 --- src/app/app.module.ts | 22 ++++-- src/app/raven-error-handler.spec.ts | 4 +- src/app/raven-error-handler.ts | 11 ++- src/app/raven-state-communicator.ts | 5 ++ src/app/services/appliance.service.spec.ts | 4 +- src/app/services/http-server.service.spec.ts | 4 +- src/app/services/http-server.service.ts | 70 ++++++++++++++++--- src/app/services/link.service.spec.ts | 4 +- src/app/services/node.service.spec.ts | 5 +- src/app/services/project.service.spec.ts | 4 +- src/app/services/snapshot.service.spec.ts | 6 +- src/app/services/symbol.service.spec.ts | 4 +- src/app/services/version.service.spec.ts | 4 +- .../testing/app-testing/app-testing.module.ts | 14 ++++ src/app/toaster-error-handler.spec.ts | 3 + src/app/toaster-error-handler.ts | 3 +- yarn.lock | 4 +- 17 files changed, 130 insertions(+), 41 deletions(-) create mode 100644 src/app/raven-state-communicator.ts create mode 100644 src/app/testing/app-testing/app-testing.module.ts diff --git a/src/app/app.module.ts b/src/app/app.module.ts index e60736e6..73f88583 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -40,7 +40,7 @@ import { ProjectService } from './services/project.service'; import { SymbolService } from "./services/symbol.service"; import { ServerService } from "./services/server.service"; import { IndexedDbService } from "./services/indexed-db.service"; -import { HttpServer } from "./services/http-server.service"; +import { HttpServer, ServerErrorHandler } from "./services/http-server.service"; import { SnapshotService } from "./services/snapshot.service"; import { ProgressDialogService } from "./common/progress-dialog/progress-dialog.service"; import { NodeService } from "./services/node.service"; @@ -80,13 +80,20 @@ import { ProgressComponent } from './common/progress/progress.component'; import { ProgressService } from "./common/progress/progress.service"; import { version } from "./version"; import { ToasterErrorHandler } from "./toaster-error-handler"; +import { environment } from "../environments/environment"; +import { RavenState } from "./raven-state-communicator"; -Raven - .config('https://b2b1cfd9b043491eb6b566fd8acee358@sentry.io/842726', { - release: version - }) - .install(); +if (environment.production) { + Raven + .config('https://b2b1cfd9b043491eb6b566fd8acee358@sentry.io/842726', { + shouldSendCallback: () => { + return RavenState.shouldSend; + }, + release: version + }) + .install(); +} @NgModule({ @@ -166,7 +173,8 @@ Raven SymbolsDataSource, SelectionManager, InRectangleHelper, - DrawingsDataSource + DrawingsDataSource, + ServerErrorHandler ], entryComponents: [ AddServerDialogComponent, diff --git a/src/app/raven-error-handler.spec.ts b/src/app/raven-error-handler.spec.ts index 6ca84027..84d46c3e 100644 --- a/src/app/raven-error-handler.spec.ts +++ b/src/app/raven-error-handler.spec.ts @@ -31,11 +31,11 @@ describe('RavenErrorHandler', () => { it('should handle error', () => { settingsService.set('crash_reports', true); environment.production = true; - expect(handler.shouldSend()()).toBeTruthy(); + expect(handler.shouldSend()).toBeTruthy(); }); it('should not handle when crash reports are disabled', () => { settingsService.set('crash_reports', false); - expect(handler.shouldSend()()).toBeFalsy(); + expect(handler.shouldSend()).toBeFalsy(); }); }); diff --git a/src/app/raven-error-handler.ts b/src/app/raven-error-handler.ts index c3288588..d9d9e353 100644 --- a/src/app/raven-error-handler.ts +++ b/src/app/raven-error-handler.ts @@ -1,24 +1,21 @@ -import * as Raven from 'raven-js'; - import { ErrorHandler, Inject, Injector } from "@angular/core"; import { SettingsService } from "./services/settings.service"; import { environment } from "../environments/environment"; +import { RavenState } from "./raven-state-communicator"; export class RavenErrorHandler implements ErrorHandler { constructor(@Inject(Injector) protected injector: Injector) {} handleError(err: any): void { - Raven.setShouldSendCallback(this.shouldSend()); + RavenState.shouldSend = this.shouldSend(); console.error(err.originalError || err); } shouldSend() { - return () => { - const settingsService: SettingsService = this.injector.get(SettingsService); - return environment.production && settingsService.get('crash_reports'); - }; + const settingsService: SettingsService = this.injector.get(SettingsService); + return environment.production && settingsService.get('crash_reports'); } } diff --git a/src/app/raven-state-communicator.ts b/src/app/raven-state-communicator.ts new file mode 100644 index 00000000..7a099177 --- /dev/null +++ b/src/app/raven-state-communicator.ts @@ -0,0 +1,5 @@ +export class RavenStateCommunicator { + public shouldSend = true; +}; + +export var RavenState = new RavenStateCommunicator(); diff --git a/src/app/services/appliance.service.spec.ts b/src/app/services/appliance.service.spec.ts index 33678df6..0f749331 100644 --- a/src/app/services/appliance.service.spec.ts +++ b/src/app/services/appliance.service.spec.ts @@ -5,6 +5,7 @@ import { HttpClient } from "@angular/common/http"; import { ApplianceService } from './appliance.service'; import { Server } from '../models/server'; import { HttpServer } from './http-server.service'; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; @@ -16,7 +17,8 @@ describe('ApplianceService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ ApplianceService, diff --git a/src/app/services/http-server.service.spec.ts b/src/app/services/http-server.service.spec.ts index 0034dd70..5ff73d16 100644 --- a/src/app/services/http-server.service.spec.ts +++ b/src/app/services/http-server.service.spec.ts @@ -5,6 +5,7 @@ import { HttpClient } from "@angular/common/http"; import { Server } from '../models/server'; import { HttpServer } from './http-server.service'; import { getTestServer } from './testing'; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; class MyType { id: number; @@ -20,7 +21,8 @@ describe('HttpServer', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ HttpServer diff --git a/src/app/services/http-server.service.ts b/src/app/services/http-server.service.ts index 56a22919..2325d1a4 100644 --- a/src/app/services/http-server.service.ts +++ b/src/app/services/http-server.service.ts @@ -1,8 +1,11 @@ import { Injectable } from '@angular/core'; -import {HttpHeaders, HttpClient, HttpParams} from '@angular/common/http'; +import { HttpHeaders, HttpClient, HttpParams, HttpErrorResponse } from '@angular/common/http'; import { Observable } from 'rxjs/Observable'; import {Server} from "../models/server"; +import { catchError } from "rxjs/operators"; + +import 'rxjs/add/observable/throw' /* tslint:disable:interface-over-type-literal */ @@ -40,57 +43,104 @@ export type HeadersOptions = { /* tslint:enable:interface-over-type-literal */ +export class ServerError extends Error { + public originalError: Error; + + constructor(message: string) { + super(message); + } + + static fromError(message: string, originalError: Error) { + const serverError = new ServerError(message); + serverError.originalError = originalError; + return serverError; + } +} + +@Injectable() +export class ServerErrorHandler { + handleError(error: HttpErrorResponse) { + let err: Error = error; + + if (error.name === 'HttpErrorResponse' && error.status === 0) { + err = ServerError.fromError("Server is unreachable", error); + } + + return Observable.throw(err); + } +} + + @Injectable() export class HttpServer { - constructor(private http: HttpClient) { } + constructor( + private http: HttpClient, + private errorHandler: ServerErrorHandler + ) { } get(server: Server, url: string, options?: JsonOptions): Observable { options = this.getJsonOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.get(intercepted.url, intercepted.options as JsonOptions); + return this.http + .get(intercepted.url, intercepted.options as JsonOptions) + .pipe(catchError(this.errorHandler.handleError)); } getText(server: Server, url: string, options?: TextOptions): Observable { options = this.getTextOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.get(intercepted.url, intercepted.options as TextOptions); + return this.http + .get(intercepted.url, intercepted.options as TextOptions) + .pipe(catchError(this.errorHandler.handleError)); } post(server: Server, url: string, body: any | null, options?: JsonOptions): Observable { options = this.getJsonOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.post(intercepted.url, body, intercepted.options); + return this.http + .post(intercepted.url, body, intercepted.options) + .pipe(catchError(this.errorHandler.handleError)); } put(server: Server, url: string, body: any, options?: JsonOptions): Observable { options = this.getJsonOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.put(intercepted.url, body, intercepted.options); + return this.http + .put(intercepted.url, body, intercepted.options) + .pipe(catchError(this.errorHandler.handleError)); } delete(server: Server, url: string, options?: JsonOptions): Observable { options = this.getJsonOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.delete(intercepted.url, intercepted.options); + return this.http + .delete(intercepted.url, intercepted.options) + .pipe(catchError(this.errorHandler.handleError)); } patch(server: Server, url: string, body: any, options?: JsonOptions): Observable { options = this.getJsonOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.patch(intercepted.url, body, intercepted.options); + return this.http + .patch(intercepted.url, body, intercepted.options) + .pipe(catchError(this.errorHandler.handleError)); } head(server: Server, url: string, options?: JsonOptions): Observable { options = this.getJsonOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.head(intercepted.url, intercepted.options); + return this.http + .head(intercepted.url, intercepted.options) + .pipe(catchError(this.errorHandler.handleError)); } options(server: Server, url: string, options?: JsonOptions): Observable { options = this.getJsonOptions(options); const intercepted = this.getOptionsForServer(server, url, options); - return this.http.options(intercepted.url, intercepted.options); + return this.http + .options(intercepted.url, intercepted.options) + .pipe(catchError(this.errorHandler.handleError)); } private getJsonOptions(options: JsonOptions): JsonOptions { diff --git a/src/app/services/link.service.spec.ts b/src/app/services/link.service.spec.ts index 9d529345..0e3c1e8f 100644 --- a/src/app/services/link.service.spec.ts +++ b/src/app/services/link.service.spec.ts @@ -8,6 +8,7 @@ import { Server } from '../models/server'; import { Node } from '../cartography/models/node'; import { Port } from '../models/port'; import { getTestServer } from './testing'; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; describe('LinkService', () => { let httpClient: HttpClient; @@ -19,7 +20,8 @@ describe('LinkService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ HttpServer, diff --git a/src/app/services/node.service.spec.ts b/src/app/services/node.service.spec.ts index 43fe5b37..565efe5b 100644 --- a/src/app/services/node.service.spec.ts +++ b/src/app/services/node.service.spec.ts @@ -5,11 +5,11 @@ import { HttpTestingController, HttpClientTestingModule } from '@angular/common/ import { HttpServer } from './http-server.service'; import { Server } from '../models/server'; import { Node } from '../cartography/models/node'; -import { Port } from '../models/port'; import { getTestServer } from './testing'; import { NodeService } from './node.service'; import { Appliance } from '../models/appliance'; import { Project } from '../models/project'; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; describe('NodeService', () => { let httpClient: HttpClient; @@ -21,7 +21,8 @@ describe('NodeService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ HttpServer, diff --git a/src/app/services/project.service.spec.ts b/src/app/services/project.service.spec.ts index 9dc0d652..ee38db72 100644 --- a/src/app/services/project.service.spec.ts +++ b/src/app/services/project.service.spec.ts @@ -10,6 +10,7 @@ import { SettingsService } from "./settings.service"; import { MockedSettingsService } from "./settings.service.spec"; import { Observable } from "rxjs/Observable"; import { Project } from "../models/project"; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; /** @@ -47,7 +48,8 @@ describe('ProjectService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ HttpServer, diff --git a/src/app/services/snapshot.service.spec.ts b/src/app/services/snapshot.service.spec.ts index 52f63ee7..32b09144 100644 --- a/src/app/services/snapshot.service.spec.ts +++ b/src/app/services/snapshot.service.spec.ts @@ -4,11 +4,10 @@ import { HttpClient } from '@angular/common/http'; import { HttpTestingController, HttpClientTestingModule } from '@angular/common/http/testing'; import { HttpServer } from './http-server.service'; import { Server } from '../models/server'; -import { Node } from '../cartography/models/node'; -import { Port } from '../models/port'; import { getTestServer } from './testing'; import { SnapshotService } from './snapshot.service'; import { Snapshot } from '../models/snapshot'; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; describe('SnapshotService', () => { @@ -21,7 +20,8 @@ describe('SnapshotService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ HttpServer, diff --git a/src/app/services/symbol.service.spec.ts b/src/app/services/symbol.service.spec.ts index ef815626..a77bf3bb 100644 --- a/src/app/services/symbol.service.spec.ts +++ b/src/app/services/symbol.service.spec.ts @@ -7,6 +7,7 @@ import { Server } from '../models/server'; import { getTestServer } from './testing'; import { SymbolService } from './symbol.service'; import { Symbol } from '../cartography/models/symbol'; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; describe('SymbolService', () => { @@ -19,7 +20,8 @@ describe('SymbolService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ HttpServer, diff --git a/src/app/services/version.service.spec.ts b/src/app/services/version.service.spec.ts index b3ca1ca2..50799f7c 100644 --- a/src/app/services/version.service.spec.ts +++ b/src/app/services/version.service.spec.ts @@ -8,6 +8,7 @@ import { Node } from '../cartography/models/node'; import { Port } from '../models/port'; import { getTestServer } from './testing'; import { VersionService } from './version.service'; +import { AppTestingModule } from "../testing/app-testing/app-testing.module"; describe('VersionService', () => { @@ -20,7 +21,8 @@ describe('VersionService', () => { beforeEach(() => { TestBed.configureTestingModule({ imports: [ - HttpClientTestingModule + HttpClientTestingModule, + AppTestingModule ], providers: [ HttpServer, diff --git a/src/app/testing/app-testing/app-testing.module.ts b/src/app/testing/app-testing/app-testing.module.ts new file mode 100644 index 00000000..d6691651 --- /dev/null +++ b/src/app/testing/app-testing/app-testing.module.ts @@ -0,0 +1,14 @@ +import { NgModule } from '@angular/core'; +import { CommonModule } from '@angular/common'; +import { ServerErrorHandler } from "../../services/http-server.service"; + +@NgModule({ + imports: [ + CommonModule + ], + declarations: [], + providers: [ + ServerErrorHandler + ] +}) +export class AppTestingModule { } diff --git a/src/app/toaster-error-handler.spec.ts b/src/app/toaster-error-handler.spec.ts index 679f8f49..84fb2a3c 100644 --- a/src/app/toaster-error-handler.spec.ts +++ b/src/app/toaster-error-handler.spec.ts @@ -3,6 +3,8 @@ import { ToasterService } from "./services/toaster.service"; import { MockedToasterService } from "./services/toaster.service.spec"; import { ToasterErrorHandler } from "./toaster-error-handler"; import { RavenErrorHandler } from "./raven-error-handler"; +import { SettingsService } from "./services/settings.service"; +import { MockedSettingsService } from "./services/settings.service.spec"; describe('ToasterErrorHandler', () => { @@ -13,6 +15,7 @@ describe('ToasterErrorHandler', () => { TestBed.configureTestingModule({ providers: [ { provide: ToasterService, useClass: MockedToasterService }, + { provide: SettingsService, useClass: MockedSettingsService }, RavenErrorHandler, ToasterErrorHandler, ] diff --git a/src/app/toaster-error-handler.ts b/src/app/toaster-error-handler.ts index eabe381c..c2c42cbe 100644 --- a/src/app/toaster-error-handler.ts +++ b/src/app/toaster-error-handler.ts @@ -8,8 +8,7 @@ export class ToasterErrorHandler extends RavenErrorHandler { super.handleError(err); const toasterService = this.injector.get(ToasterService); - const error = err.originalError || err; - toasterService.error(error.message); + toasterService.error(err.message); } } diff --git a/yarn.lock b/yarn.lock index 3099cca0..884b7bb4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6526,8 +6526,8 @@ range-parser@^1.0.3, range-parser@^1.2.0, range-parser@~1.2.0: resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.0.tgz#f49be6b487894ddc40dcc94a322f611092e00d5e" raven-js@^3.24.1: - version "3.25.1" - resolved "https://registry.yarnpkg.com/raven-js/-/raven-js-3.25.1.tgz#05df39b41af140e3b2fa34e2c522e2b4f99a98be" + version "3.26.3" + resolved "https://registry.yarnpkg.com/raven-js/-/raven-js-3.26.3.tgz#0efb49969b5b11ab965f7b0d6da4ca102b763cb0" raven@^2.6.0: version "2.6.1" From ca45804b5e2654afbce24b7b0b36f95d23b6844f Mon Sep 17 00:00:00 2001 From: ziajka Date: Tue, 3 Jul 2018 11:40:12 +0200 Subject: [PATCH 2/3] Move error handlers into one dir --- src/app/app.module.ts | 4 ++-- .../error-handlers}/raven-error-handler.spec.ts | 4 ++-- .../{ => common/error-handlers}/raven-error-handler.ts | 4 ++-- .../error-handlers}/raven-state-communicator.ts | 0 .../error-handlers}/toaster-error-handler.spec.ts | 8 ++++---- .../{ => common/error-handlers}/toaster-error-handler.ts | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) rename src/app/{ => common/error-handlers}/raven-error-handler.spec.ts (89%) rename src/app/{ => common/error-handlers}/raven-error-handler.ts (80%) rename src/app/{ => common/error-handlers}/raven-state-communicator.ts (100%) rename src/app/{ => common/error-handlers}/toaster-error-handler.spec.ts (75%) rename src/app/{ => common/error-handlers}/toaster-error-handler.ts (81%) diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 73f88583..2d08e232 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -79,9 +79,9 @@ import { LocalServerComponent } from './components/local-server/local-server.com import { ProgressComponent } from './common/progress/progress.component'; import { ProgressService } from "./common/progress/progress.service"; import { version } from "./version"; -import { ToasterErrorHandler } from "./toaster-error-handler"; +import { ToasterErrorHandler } from "./common/error-handlers/toaster-error-handler"; import { environment } from "../environments/environment"; -import { RavenState } from "./raven-state-communicator"; +import { RavenState } from "./common/error-handlers/raven-state-communicator"; if (environment.production) { diff --git a/src/app/raven-error-handler.spec.ts b/src/app/common/error-handlers/raven-error-handler.spec.ts similarity index 89% rename from src/app/raven-error-handler.spec.ts rename to src/app/common/error-handlers/raven-error-handler.spec.ts index 84d46c3e..a229b3d0 100644 --- a/src/app/raven-error-handler.spec.ts +++ b/src/app/common/error-handlers/raven-error-handler.spec.ts @@ -1,9 +1,9 @@ import { TestBed } from '@angular/core/testing'; import { PersistenceService } from "angular-persistence"; -import { SettingsService } from "./services/settings.service"; +import { SettingsService } from "../../services/settings.service"; import { RavenErrorHandler } from "./raven-error-handler"; -import { environment } from "../environments/environment"; +import { environment } from "../../../environments/environment"; describe('RavenErrorHandler', () => { diff --git a/src/app/raven-error-handler.ts b/src/app/common/error-handlers/raven-error-handler.ts similarity index 80% rename from src/app/raven-error-handler.ts rename to src/app/common/error-handlers/raven-error-handler.ts index d9d9e353..005fb537 100644 --- a/src/app/raven-error-handler.ts +++ b/src/app/common/error-handlers/raven-error-handler.ts @@ -1,7 +1,7 @@ import { ErrorHandler, Inject, Injector } from "@angular/core"; -import { SettingsService } from "./services/settings.service"; -import { environment } from "../environments/environment"; +import { SettingsService } from "../../services/settings.service"; +import { environment } from "../../../environments/environment"; import { RavenState } from "./raven-state-communicator"; diff --git a/src/app/raven-state-communicator.ts b/src/app/common/error-handlers/raven-state-communicator.ts similarity index 100% rename from src/app/raven-state-communicator.ts rename to src/app/common/error-handlers/raven-state-communicator.ts diff --git a/src/app/toaster-error-handler.spec.ts b/src/app/common/error-handlers/toaster-error-handler.spec.ts similarity index 75% rename from src/app/toaster-error-handler.spec.ts rename to src/app/common/error-handlers/toaster-error-handler.spec.ts index 84fb2a3c..10fbb578 100644 --- a/src/app/toaster-error-handler.spec.ts +++ b/src/app/common/error-handlers/toaster-error-handler.spec.ts @@ -1,10 +1,10 @@ import { TestBed } from '@angular/core/testing'; -import { ToasterService } from "./services/toaster.service"; -import { MockedToasterService } from "./services/toaster.service.spec"; +import { ToasterService } from "../../services/toaster.service"; +import { MockedToasterService } from "../../services/toaster.service.spec"; import { ToasterErrorHandler } from "./toaster-error-handler"; import { RavenErrorHandler } from "./raven-error-handler"; -import { SettingsService } from "./services/settings.service"; -import { MockedSettingsService } from "./services/settings.service.spec"; +import { SettingsService } from "../../services/settings.service"; +import { MockedSettingsService } from "../../services/settings.service.spec"; describe('ToasterErrorHandler', () => { diff --git a/src/app/toaster-error-handler.ts b/src/app/common/error-handlers/toaster-error-handler.ts similarity index 81% rename from src/app/toaster-error-handler.ts rename to src/app/common/error-handlers/toaster-error-handler.ts index c2c42cbe..a1d0d71b 100644 --- a/src/app/toaster-error-handler.ts +++ b/src/app/common/error-handlers/toaster-error-handler.ts @@ -1,5 +1,5 @@ import { RavenErrorHandler } from "./raven-error-handler"; -import { ToasterService } from "./services/toaster.service"; +import { ToasterService } from "../../services/toaster.service"; export class ToasterErrorHandler extends RavenErrorHandler { From db3e700204b201248c0ee3143206e405df8cc556 Mon Sep 17 00:00:00 2001 From: ziajka Date: Wed, 4 Jul 2018 13:44:35 +0200 Subject: [PATCH 3/3] Progress error status, Fixes: #146 --- .../common/progress/progress.component.html | 16 +++++++- .../common/progress/progress.component.scss | 12 +++++- .../progress/progress.component.spec.ts | 41 ++++++++++++++++++- src/app/common/progress/progress.component.ts | 33 +++++++++++++-- .../common/progress/progress.service.spec.ts | 35 ++++++++++++++-- src/app/common/progress/progress.service.ts | 14 ++++++- .../project-map/project-map.component.html | 5 +-- .../project-map/project-map.component.ts | 16 ++++++-- .../components/projects/projects.component.ts | 2 + 9 files changed, 153 insertions(+), 21 deletions(-) diff --git a/src/app/common/progress/progress.component.html b/src/app/common/progress/progress.component.html index c240c710..9778c6d6 100644 --- a/src/app/common/progress/progress.component.html +++ b/src/app/common/progress/progress.component.html @@ -1,8 +1,20 @@ -
-
+
+
+
+
error_outline
+
Error occurred: {{ error.message }}
+
+ + +
+
diff --git a/src/app/common/progress/progress.component.scss b/src/app/common/progress/progress.component.scss index 8bfff484..ec3a3ecf 100644 --- a/src/app/common/progress/progress.component.scss +++ b/src/app/common/progress/progress.component.scss @@ -10,9 +10,19 @@ z-index: 1000; } -.loading-spinner { +.loading-spinner, .error-state { position: fixed; top: 50%; left: 50%; transform: translate(-50%, -50%); } + +.error-state div { + text-align: center; +} + +.error-icon mat-icon { + font-size: 64px; + width: 64px; + height: 64px; +} diff --git a/src/app/common/progress/progress.component.spec.ts b/src/app/common/progress/progress.component.spec.ts index 5780e205..f0cfc8b2 100644 --- a/src/app/common/progress/progress.component.spec.ts +++ b/src/app/common/progress/progress.component.spec.ts @@ -1,25 +1,45 @@ import { async, ComponentFixture, TestBed } from '@angular/core/testing'; import { ProgressComponent } from './progress.component'; -import { MatProgressSpinnerModule } from "@angular/material"; +import { MatIconModule, MatProgressSpinnerModule } from "@angular/material"; import { ProgressService } from "./progress.service"; +import { RouterTestingModule } from "@angular/router/testing"; +import { Router } from "@angular/router"; +import { BehaviorSubject } from "rxjs/BehaviorSubject"; + + +class MockedRouter { + events: BehaviorSubject; + + constructor() { + this.events = new BehaviorSubject(true); + } +} + describe('ProgressComponent', () => { let component: ProgressComponent; let fixture: ComponentFixture; let progressService: ProgressService; + let router: MockedRouter; beforeEach(async(() => { TestBed.configureTestingModule({ imports: [ + RouterTestingModule, MatProgressSpinnerModule, + MatIconModule + ], + providers: [ + ProgressService, + { provide: Router, useClass: MockedRouter} ], - providers: [ ProgressService ], declarations: [ ProgressComponent ] }) .compileComponents(); progressService = TestBed.get(ProgressService); + router = TestBed.get(Router); })); beforeEach(() => { @@ -45,4 +65,21 @@ describe('ProgressComponent', () => { expect(component.visible).toEqual(false); }); + it( 'should set error state when error defined', () => { + const error = new Error("test"); + progressService.setError(error); + expect(component.error).toEqual(error); + }); + + it( 'should clear error when changes route', () => { + const error = new Error("test"); + component.error = error; + + spyOn(progressService, 'clear'); + + router.events.next(true); + + expect(progressService.clear).toHaveBeenCalled(); + }); + }); diff --git a/src/app/common/progress/progress.component.ts b/src/app/common/progress/progress.component.ts index 3a823ad2..cd4f2607 100644 --- a/src/app/common/progress/progress.component.ts +++ b/src/app/common/progress/progress.component.ts @@ -1,22 +1,49 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, OnDestroy, OnInit } from '@angular/core'; import { ProgressService } from "./progress.service"; +import { Router } from "@angular/router"; +import { Subscription } from "rxjs/Subscription"; @Component({ selector: 'app-progress', templateUrl: './progress.component.html', styleUrls: ['./progress.component.scss'] }) -export class ProgressComponent implements OnInit { +export class ProgressComponent implements OnInit, OnDestroy { visible = false; + error: Error; + routerSubscription: Subscription; constructor( - private progressService: ProgressService + private progressService: ProgressService, + private router: Router, ) { } ngOnInit() { this.progressService.state.subscribe((state) => { this.visible = state.visible; + + // only set error state once; ignore next "correct" states + if (state.error && !this.error) { + this.error = state.error; + } + + if (state.clear) { + this.error = null; + } + }); + + // when page changes clear error state + this.routerSubscription = this.router.events.subscribe(() => { + this.progressService.clear(); }); } + refresh() { + // unfortunately we need to use global var + location.reload(); + } + + ngOnDestroy() { + this.routerSubscription.unsubscribe(); + } } diff --git a/src/app/common/progress/progress.service.spec.ts b/src/app/common/progress/progress.service.spec.ts index dc375e57..530216ff 100644 --- a/src/app/common/progress/progress.service.spec.ts +++ b/src/app/common/progress/progress.service.spec.ts @@ -1,16 +1,43 @@ import { TestBed, inject } from '@angular/core/testing'; -import { ProgressService } from './progress.service'; +import { ProgressService, State } from './progress.service'; describe('ProgressService', () => { + let progressService: ProgressService; + beforeEach(() => { TestBed.configureTestingModule({ providers: [ProgressService] }); + + progressService = TestBed.get(ProgressService); + + spyOn(progressService.state, 'next'); }); - it('should be created', inject([ProgressService], (service: ProgressService) => { - expect(service).toBeTruthy(); - })); + it('should be created', () => { + expect(progressService).toBeTruthy(); + }); + + it('should propagate event when activated', () => { + progressService.activate(); + expect(progressService.state.next).toHaveBeenCalledWith(new State(true)); + }); + + it('should propagate event when deactivated', () => { + progressService.deactivate(); + expect(progressService.state.next).toHaveBeenCalledWith(new State(false)); + }); + + it('should propagate event on error', () => { + const error = new Error(); + progressService.setError(error); + expect(progressService.state.next).toHaveBeenCalledWith(new State(false, error)); + }); + + it('should clear an error', () => { + progressService.clear(); + expect(progressService.state.next).toHaveBeenCalledWith(new State(false, null, true)); + }); }); diff --git a/src/app/common/progress/progress.service.ts b/src/app/common/progress/progress.service.ts index 1ac037a0..4c68d3c0 100644 --- a/src/app/common/progress/progress.service.ts +++ b/src/app/common/progress/progress.service.ts @@ -5,9 +5,13 @@ import { BehaviorSubject } from "rxjs/BehaviorSubject"; export class State { public visible: boolean; + public error: Error; + public clear: boolean; - constructor(visible: boolean) { + constructor(visible: boolean, error?: Error, clear: boolean = false) { this.visible = visible; + this.error = error; + this.clear = clear; } } @@ -17,6 +21,14 @@ export class ProgressService { constructor() {} + public setError(error: Error) { + this.state.next(new State(false, error)); + } + + public clear() { + this.state.next(new State(false, null, true)); + } + public activate() { this.state.next(new State(true)); } diff --git a/src/app/components/project-map/project-map.component.html b/src/app/components/project-map/project-map.component.html index 63962683..d679d016 100644 --- a/src/app/components/project-map/project-map.component.html +++ b/src/app/components/project-map/project-map.component.html @@ -62,9 +62,6 @@
-
- - -
+ diff --git a/src/app/components/project-map/project-map.component.ts b/src/app/components/project-map/project-map.component.ts index f65b88f0..19d1537b 100644 --- a/src/app/components/project-map/project-map.component.ts +++ b/src/app/components/project-map/project-map.component.ts @@ -41,6 +41,7 @@ import { InRectangleHelper } from "../../cartography/components/map/helpers/in-r import { DrawingsDataSource } from "../../cartography/datasources/drawings-datasource"; import { Subscription } from "rxjs/Subscription"; import { SettingsService } from "../../services/settings.service"; +import { ProgressService } from "../../common/progress/progress.service"; @Component({ @@ -65,8 +66,6 @@ export class ProjectMapComponent implements OnInit, OnDestroy { protected selectionManager: SelectionManager; - public isLoading = true; - @ViewChild(MapComponent) mapChild: MapComponent; @ViewChild(NodeContextMenuComponent) nodeContextMenu: NodeContextMenuComponent; @@ -84,6 +83,7 @@ export class ProjectMapComponent implements OnInit, OnDestroy { private linkService: LinkService, private dialog: MatDialog, private progressDialogService: ProgressDialogService, + private progressService: ProgressService, private toaster: ToasterService, private projectWebServiceHandler: ProjectWebServiceHandler, private settingsService: SettingsService, @@ -98,6 +98,7 @@ export class ProjectMapComponent implements OnInit, OnDestroy { } ngOnInit() { + this.progressService.activate(); const routeSub = this.route.paramMap.subscribe((paramMap: ParamMap) => { const server_id = parseInt(paramMap.get('server_id'), 10); @@ -123,6 +124,10 @@ export class ProjectMapComponent implements OnInit, OnDestroy { }) .subscribe((project: Project) => { this.onProjectLoad(project); + }, (error) => { + this.progressService.setError(error); + }, () => { + this.progressService.deactivate(); }); }); @@ -183,7 +188,8 @@ export class ProjectMapComponent implements OnInit, OnDestroy { this.setUpMapCallbacks(project); this.setUpWS(project); - this.isLoading = false; + + this.progressService.deactivate(); }); this.subscriptions.push(subscription); } @@ -335,7 +341,9 @@ export class ProjectMapComponent implements OnInit, OnDestroy { this.nodesDataSource.clear(); this.linksDataSource.clear(); - this.ws.unsubscribe(); + if (this.ws) { + this.ws.unsubscribe(); + } this.subscriptions.forEach((subscription: Subscription) => subscription.unsubscribe()); } diff --git a/src/app/components/projects/projects.component.ts b/src/app/components/projects/projects.component.ts index 47c327c7..bb5d6a95 100644 --- a/src/app/components/projects/projects.component.ts +++ b/src/app/components/projects/projects.component.ts @@ -62,6 +62,8 @@ export class ProjectsComponent implements OnInit { .list(this.server) .subscribe((projects: Project[]) => { this.projectDatabase.addProjects(projects); + }, (error) => { + this.progressService.setError(error); }); }