From 74d98d0b8c431aade4b12c378910df269f0e2b53 Mon Sep 17 00:00:00 2001 From: Olivia Tournois Date: Wed, 3 Oct 2018 09:57:39 +0200 Subject: [PATCH] feat(stark-core): implementation of a custom error handler --- docs/ERROR_HANDLING.md | 69 +++++++++++++++++++ .../store/stark-core-application-state.ts | 2 +- packages/stark-core/src/modules.ts | 1 + .../stark-core/src/modules/error-handling.ts | 3 + .../src/modules/error-handling/actions.ts | 1 + .../actions/error-handling.actions.ts | 28 ++++++++ .../error-handling/error-handling.module.ts | 38 ++++++++++ .../src/modules/error-handling/handlers.ts | 1 + .../error-handling/handlers/error-handler.ts | 48 +++++++++++++ .../routing/services/routing.service.spec.ts | 29 +++++--- .../routing/services/routing.service.ts | 5 +- .../src/modules/settings/settings.module.ts | 5 +- showcase/src/app/app.module.ts | 7 +- .../effects/stark-error-handling.effects.ts | 59 ++++++++++++++++ showcase/src/stark-app-config.json | 1 + starter/src/app/app.module.ts | 5 +- 16 files changed, 286 insertions(+), 16 deletions(-) create mode 100644 docs/ERROR_HANDLING.md create mode 100644 packages/stark-core/src/modules/error-handling.ts create mode 100644 packages/stark-core/src/modules/error-handling/actions.ts create mode 100644 packages/stark-core/src/modules/error-handling/actions/error-handling.actions.ts create mode 100644 packages/stark-core/src/modules/error-handling/error-handling.module.ts create mode 100644 packages/stark-core/src/modules/error-handling/handlers.ts create mode 100644 packages/stark-core/src/modules/error-handling/handlers/error-handler.ts create mode 100644 showcase/src/app/shared/effects/stark-error-handling.effects.ts diff --git a/docs/ERROR_HANDLING.md b/docs/ERROR_HANDLING.md new file mode 100644 index 0000000000..5956ed5fa9 --- /dev/null +++ b/docs/ERROR_HANDLING.md @@ -0,0 +1,69 @@ +# Stark Default Error Handling + +Stark provides its own error handler, StarkErrorHandler, that overrides [ Angular's default ErrorHandler](https://angular.io/api/core/ErrorHandler). + +This handler will dispatch a StarkUnhandledError action to the application Store which you can treat as you want. + +## StarkErrorHandlingModule + +If you want to use this handler, you simply have to import StarkErrorHandlingModule from stark-core in your `app.module.ts` file. + +```typescript +import {StarkErrorHandlingModule} from "@nationalbankbelgium/stark-core"; + +@NgModule({ + bootstrap: ... + declarations: ... + imports: [ + StarkErrorHandlingModule.forRoot(), + ... + ] + }) +``` + +## Effects definition + +To define what to do when an error happens, you can simply create a new effects file, like in the following example: + +```typescript +... +import {StarkErrorHandlingActionTypes, StarkUnhandledError} from "@nationalbankbelgium/stark-core"; + +/** + * This class is used to determine what to do with an error + */ +@Injectable() +export class StarkErrorHandlingEffects { + + public constructor( + private actions$: Actions + ) {} + + @Effect() + public doSomething(): Observable { + return this.actions$.pipe( + ofType(StarkErrorHandlingActionTypes.UNHANDLED_ERROR), + map((action: StarkUnhandledError) => { + //DO SOMETHING + }) + ); + } +} +``` + +If you do create that file, don't forget to import it in your `app.module.ts` file: + +```typescript +... +import { StarkErrorHandlingEffects } from "./shared/effects/stark-error-handler.effects"; + +@NgModule({ + bootstrap: ... + declarations: ... + imports: [ + ... + EffectsModule.forRoot([StarkErrorHandlingEffects]), + ... + ] +}) +``` diff --git a/packages/stark-core/src/common/store/stark-core-application-state.ts b/packages/stark-core/src/common/store/stark-core-application-state.ts index 159fba89a8..ec870d9c5f 100644 --- a/packages/stark-core/src/common/store/stark-core-application-state.ts +++ b/packages/stark-core/src/common/store/stark-core-application-state.ts @@ -1,10 +1,10 @@ import { StarkLoggingState } from "../../modules/logging/reducers"; import { StarkSessionState } from "../../modules/session/reducers"; import { StarkSettingsState } from "../../modules/settings/reducers"; + /** * Interface defining the shape of the application state of Stark Core (i.e., what's stored in Redux by Stark) */ export interface StarkCoreApplicationState extends StarkLoggingState, StarkSessionState, StarkSettingsState { // FIXME: still needed? - // starkApplicationMetadata: StarkApplicationMetadata; } diff --git a/packages/stark-core/src/modules.ts b/packages/stark-core/src/modules.ts index 960aed30c2..6f17060629 100644 --- a/packages/stark-core/src/modules.ts +++ b/packages/stark-core/src/modules.ts @@ -1,5 +1,6 @@ export * from "./modules/http"; export * from "./modules/logging"; +export * from "./modules/error-handling"; export * from "./modules/routing"; export * from "./modules/session"; export * from "./modules/settings"; diff --git a/packages/stark-core/src/modules/error-handling.ts b/packages/stark-core/src/modules/error-handling.ts new file mode 100644 index 0000000000..71ef3a80e1 --- /dev/null +++ b/packages/stark-core/src/modules/error-handling.ts @@ -0,0 +1,3 @@ +export * from "./error-handling/actions"; +export * from "./error-handling/handlers"; +export * from "./error-handling/error-handling.module"; diff --git a/packages/stark-core/src/modules/error-handling/actions.ts b/packages/stark-core/src/modules/error-handling/actions.ts new file mode 100644 index 0000000000..6dd9d4f2c8 --- /dev/null +++ b/packages/stark-core/src/modules/error-handling/actions.ts @@ -0,0 +1 @@ +export * from "./actions/error-handling.actions"; diff --git a/packages/stark-core/src/modules/error-handling/actions/error-handling.actions.ts b/packages/stark-core/src/modules/error-handling/actions/error-handling.actions.ts new file mode 100644 index 0000000000..ce76583665 --- /dev/null +++ b/packages/stark-core/src/modules/error-handling/actions/error-handling.actions.ts @@ -0,0 +1,28 @@ +import { Action } from "@ngrx/store"; + +/** + * All the StarkErrorHandling action types + */ +export enum StarkErrorHandlingActionTypes { + UNHANDLED_ERROR = "[StarkErrorHandling] Unhandled Error" +} + +/** + * Action that requires to display an error message as a toast notification + * @returns The created action object + */ +export class StarkUnhandledError implements Action { + /** + * The type of action + * @link StarkErrorHandlingActionTypes + */ + public readonly type: StarkErrorHandlingActionTypes.UNHANDLED_ERROR = StarkErrorHandlingActionTypes.UNHANDLED_ERROR; + + /** + * Class constructor + * @param error - the error to display + */ + public constructor(public error: any) {} +} + +export type StarkErrorHandlingActions = StarkUnhandledError; diff --git a/packages/stark-core/src/modules/error-handling/error-handling.module.ts b/packages/stark-core/src/modules/error-handling/error-handling.module.ts new file mode 100644 index 0000000000..25b9930bbb --- /dev/null +++ b/packages/stark-core/src/modules/error-handling/error-handling.module.ts @@ -0,0 +1,38 @@ +import { ErrorHandler, ModuleWithProviders, NgModule, Optional, SkipSelf } from "@angular/core"; +import { StarkErrorHandler } from "./handlers/error-handler"; + +@NgModule({}) +export class StarkErrorHandlingModule { + /** + * Instantiates the services only once since they should be singletons + * so the forRoot() should be called only by the AppModule + * @link https://angular.io/guide/singleton-services#forroot + * @returns a module with providers + */ + public static forRoot(): ModuleWithProviders { + return { + ngModule: StarkErrorHandlingModule, + providers: [ + { + provide: ErrorHandler, + useClass: StarkErrorHandler + } + ] + }; + } + + /** + * Prevents this module from being re-imported + * @link https://angular.io/guide/singleton-services#prevent-reimport-of-the-coremodule + * @param parentModule - the parent module + */ + public constructor( + @Optional() + @SkipSelf() + parentModule: StarkErrorHandlingModule + ) { + if (parentModule) { + throw new Error("StarkErrorHandlingModule is already loaded. Import it in the AppModule only"); + } + } +} diff --git a/packages/stark-core/src/modules/error-handling/handlers.ts b/packages/stark-core/src/modules/error-handling/handlers.ts new file mode 100644 index 0000000000..65c75245a1 --- /dev/null +++ b/packages/stark-core/src/modules/error-handling/handlers.ts @@ -0,0 +1 @@ +export * from "./handlers/error-handler"; diff --git a/packages/stark-core/src/modules/error-handling/handlers/error-handler.ts b/packages/stark-core/src/modules/error-handling/handlers/error-handler.ts new file mode 100644 index 0000000000..b8b908fef9 --- /dev/null +++ b/packages/stark-core/src/modules/error-handling/handlers/error-handler.ts @@ -0,0 +1,48 @@ +import { ErrorHandler, Injectable, Injector } from "@angular/core"; +import { Store } from "@ngrx/store"; +import { StarkCoreApplicationState } from "../../../common/store"; +import { StarkUnhandledError } from "../actions"; +import { STARK_LOGGING_SERVICE, StarkLoggingService } from "../../logging/services"; + +@Injectable() +export class StarkErrorHandler implements ErrorHandler { + private _starkLoggingService: StarkLoggingService; + private _applicationStore: Store; + + public constructor(private injector: Injector) {} + + /** + * Thie method will dispatch an error method, which the user can then handle + * @param error the encountered error + */ + public handleError(error: any): void { + this.starkLoggingService.error("StarkErrorHandler: an error has occurred : ", error); + this.applicationStore.dispatch(new StarkUnhandledError(error)); + } + + /** + * Gets the StarkLoggingService from the Injector. + * @throws When the service is not found (the StarkLoggingService is not provided in the app). + */ + private get starkLoggingService(): StarkLoggingService { + if (typeof this._starkLoggingService === "undefined") { + this._starkLoggingService = this.injector.get(STARK_LOGGING_SERVICE); + return this._starkLoggingService; + } + + return this._starkLoggingService; + } + + /** + * Gets the Application Store from the Injector. + * @throws When the Store is not found (the NGRX Store module is not imported in the app). + */ + private get applicationStore(): Store { + if (typeof this._applicationStore === "undefined") { + this._applicationStore = this.injector.get>(Store); + return this._applicationStore; + } + + return this._applicationStore; + } +} diff --git a/packages/stark-core/src/modules/routing/services/routing.service.spec.ts b/packages/stark-core/src/modules/routing/services/routing.service.spec.ts index e8127eb16a..ac5595f1ef 100644 --- a/packages/stark-core/src/modules/routing/services/routing.service.spec.ts +++ b/packages/stark-core/src/modules/routing/services/routing.service.spec.ts @@ -1,5 +1,5 @@ /*tslint:disable:completed-docs no-identical-functions*/ -import { Component, NgModuleFactoryLoader, NO_ERRORS_SCHEMA, SystemJsNgModuleLoader } from "@angular/core"; +import { Component, Injector, NgModuleFactoryLoader, NO_ERRORS_SCHEMA, SystemJsNgModuleLoader } from "@angular/core"; import { fakeAsync, inject, TestBed, tick } from "@angular/core/testing"; import { Ng2StateDeclaration, UIRouterModule } from "@uirouter/angular"; import { StateDeclaration, StateObject, StateService, TransitionService, UIRouter } from "@uirouter/core"; @@ -18,6 +18,9 @@ import { StarkCoreApplicationState } from "../../../common/store"; import CallInfo = jasmine.CallInfo; import Spy = jasmine.Spy; import SpyObj = jasmine.SpyObj; +import { StarkErrorHandler } from "../../error-handling"; +import { StarkXSRFService } from "../../xsrf/services"; +import { MockStarkXsrfService } from "../../xsrf/testing/xsrf.mock"; @Component({ selector: "test-home", template: "HOME" }) export class HomeComponent {} @@ -29,7 +32,9 @@ export class LogoutPageComponent {} describe("Service: StarkRoutingService", () => { let $state: StateService; let router: UIRouter; - + let mockInjectorService: SpyObj; + let mockXSRFService: StarkXSRFService; + let errorHandler: StarkErrorHandler; let routingService: StarkRoutingServiceImpl; let mockLogger: StarkLoggingService; let appConfig: StarkApplicationConfig; @@ -376,13 +381,21 @@ describe("Service: StarkRoutingService", () => { deferIntercept: true // FIXME: this option shouldn't be used but is needed for Chrome and HeadlessChrome otherwise it doesn't work. Why? }); + beforeEach(() => { + mockInjectorService = jasmine.createSpyObj("injector,", ["get"]); + mockXSRFService = new MockStarkXsrfService(); + /* tslint:disable-next-line:deprecation */ + (mockInjectorService.get).and.returnValue(mockXSRFService); + }); + const starkRoutingServiceFactory: Function = (state: StateService, transitions: TransitionService) => { appConfig = new StarkApplicationConfigImpl(); appConfig.homeStateName = "homepage"; mockLogger = new MockStarkLoggingService(mockCorrelationId); + errorHandler = new StarkErrorHandler(mockInjectorService); - return new StarkRoutingServiceImpl(mockLogger, appConfig, mockStore, state, transitions); + return new StarkRoutingServiceImpl(mockLogger, appConfig, errorHandler, mockStore, state, transitions); }; /** @@ -427,9 +440,9 @@ describe("Service: StarkRoutingService", () => { const modifiedAppConfig: StarkApplicationConfig = new StarkApplicationConfigImpl(); modifiedAppConfig.homeStateName = undefined; - expect(() => new StarkRoutingServiceImpl(mockLogger, modifiedAppConfig, mockStore, {}, {})).toThrowError( - /homeStateName/ - ); + expect( + () => new StarkRoutingServiceImpl(mockLogger, modifiedAppConfig, errorHandler, mockStore, {}, {}) + ).toThrowError(/homeStateName/); }); }); @@ -950,7 +963,7 @@ describe("Service: StarkRoutingService", () => { .navigateTo("page-01") .pipe( catchError((error: any) => { - expect(mockLogger.error).toHaveBeenCalledTimes(2); + expect(mockLogger.error).toHaveBeenCalledTimes(1); const message: string = (mockLogger.error).calls.argsFor(0)[0]; expect(message).toMatch(/Error during route transition/); return throwError(errorPrefix + error); @@ -986,7 +999,7 @@ describe("Service: StarkRoutingService", () => { .navigateTo("page-01") .pipe( catchError((error: any) => { - expect(mockLogger.error).toHaveBeenCalledTimes(2); + expect(mockLogger.error).toHaveBeenCalledTimes(1); const message: string = (mockLogger.error).calls.argsFor(0)[0]; expect(message).toMatch(/An error occurred with a resolve in the new state/); return throwError(errorPrefix + error); diff --git a/packages/stark-core/src/modules/routing/services/routing.service.ts b/packages/stark-core/src/modules/routing/services/routing.service.ts index 2c3a16407c..169f6c11bf 100644 --- a/packages/stark-core/src/modules/routing/services/routing.service.ts +++ b/packages/stark-core/src/modules/routing/services/routing.service.ts @@ -1,7 +1,7 @@ /* tslint:disable:completed-docs*/ import { Action, Store } from "@ngrx/store"; import { EMPTY, from, Observable } from "rxjs"; -import { Inject, Injectable } from "@angular/core"; +import { ErrorHandler, Inject, Injectable } from "@angular/core"; import { HookMatchCriteria, HookRegOptions, @@ -75,6 +75,7 @@ export class StarkRoutingServiceImpl implements StarkRoutingService { public constructor( @Inject(STARK_LOGGING_SERVICE) private logger: StarkLoggingService, @Inject(STARK_APP_CONFIG) private appConfig: StarkApplicationConfig, + private defaultErrorHandler: ErrorHandler, private store: Store, private $state: StateService, private $transitions: TransitionService @@ -534,7 +535,7 @@ export class StarkRoutingServiceImpl implements StarkRoutingService { } if (!this.knownRejectionCausesRegex.test(stringError)) { - this.logger.error(starkRoutingServiceName + ": defaultErrorHandler => ", new Error(stringError)); + this.defaultErrorHandler.handleError(Error(stringError)); } } ); diff --git a/packages/stark-core/src/modules/settings/settings.module.ts b/packages/stark-core/src/modules/settings/settings.module.ts index 0198aa75bd..8e4877793f 100644 --- a/packages/stark-core/src/modules/settings/settings.module.ts +++ b/packages/stark-core/src/modules/settings/settings.module.ts @@ -6,7 +6,10 @@ import { STARK_SETTINGS_SERVICE, StarkSettingsServiceImpl } from "./services"; import { StarkSettingsEffects } from "./effects"; @NgModule({ - imports: [StoreModule.forFeature("StarkSettings", starkSettingsReducers), EffectsModule.forFeature([StarkSettingsEffects])] + imports: [ + StoreModule.forFeature("StarkSettings", starkSettingsReducers), + EffectsModule.forFeature([StarkSettingsEffects]) + ] }) export class StarkSettingsModule { /** diff --git a/showcase/src/app/app.module.ts b/showcase/src/app/app.module.ts index bb694b7947..cb505d3ca8 100644 --- a/showcase/src/app/app.module.ts +++ b/showcase/src/app/app.module.ts @@ -20,7 +20,7 @@ import { MatListModule } from "@angular/material/list"; import { MatSidenavModule } from "@angular/material/sidenav"; import { MatTooltipModule } from "@angular/material/tooltip"; import { DateAdapter } from "@angular/material/core"; -import { SharedModule } from "./shared/shared.module"; +import { SharedModule } from "./shared"; import { Observable, of } from "rxjs"; import { filter, map } from "rxjs/operators"; @@ -34,6 +34,7 @@ import { StarkApplicationConfigImpl, StarkApplicationMetadata, StarkApplicationMetadataImpl, + StarkErrorHandlingModule, StarkHttpModule, StarkLoggingActionTypes, StarkLoggingModule, @@ -89,6 +90,7 @@ import { NewsModule } from "./news"; import "../styles/styles.pcss"; // load SASS styles import "../styles/styles.scss"; +import { StarkErrorHandlingEffects } from "./shared/effects/stark-error-handling.effects"; /* tslint:enable */ // TODO: where to put this factory function? @@ -195,7 +197,7 @@ export const metaReducers: MetaReducer[] = ENV !== "production" ? [logger name: "Stark Showcase - NgRx Store DevTools", // shown in the monitor page logOnly: environment.production // restrict extension to log-only mode (setting it to false enables all extension features) }), - EffectsModule.forRoot([]), // needed to set up the providers required for effects + EffectsModule.forRoot([StarkErrorHandlingEffects]), // needed to set up the providers required for effects UIRouterModule.forRoot({ states: APP_STATES, useHash: !Boolean(history.pushState), @@ -208,6 +210,7 @@ export const metaReducers: MetaReducer[] = ENV !== "production" ? [logger StarkHttpModule.forRoot(), StarkLoggingModule.forRoot(), StarkSessionModule.forRoot(), + StarkErrorHandlingModule.forRoot(), StarkSettingsModule.forRoot(), StarkRoutingModule.forRoot(), StarkUserModule.forRoot(), diff --git a/showcase/src/app/shared/effects/stark-error-handling.effects.ts b/showcase/src/app/shared/effects/stark-error-handling.effects.ts new file mode 100644 index 0000000000..717b8b2fe1 --- /dev/null +++ b/showcase/src/app/shared/effects/stark-error-handling.effects.ts @@ -0,0 +1,59 @@ +import { Injectable, Injector, NgZone } from "@angular/core"; +import { Actions, Effect, ofType } from "@ngrx/effects"; +import { map } from "rxjs/operators"; + +import { STARK_TOAST_NOTIFICATION_SERVICE, StarkMessageType, StarkToastNotificationService } from "@nationalbankbelgium/stark-ui"; +import { StarkErrorHandlingActionTypes, StarkUnhandledError } from "@nationalbankbelgium/stark-core"; +import { Observable } from "rxjs"; + +/** + * Unique Id of the displayed toaster + */ +const _uniqueId: Function = require("lodash/uniqueId"); + +/** + * This class is used to determine what to do with an error + */ +@Injectable() +export class StarkErrorHandlingEffects { + private _starkToastNotificationService: StarkToastNotificationService; + + /** + * Class constructor + * @param actions$ - the action to perform + * @param injector - the injector of the class + * @param zone - the service to execute actions inside or outside of an Angular Zone. + */ + public constructor(private actions$: Actions, private injector: Injector, private zone: NgZone) {} + + @Effect({ dispatch: false }) + public starkUnhandledError$(): Observable { + return this.actions$.pipe( + ofType(StarkErrorHandlingActionTypes.UNHANDLED_ERROR), + map((action: StarkUnhandledError) => { + this.zone.run(() => { + this.toastNotificationService + .show({ + id: _uniqueId(), + type: StarkMessageType.ERROR, + key: action.error.toString(), + code: "Unhandled error - no code" + }) + .subscribe(); + }); + }) + ); + } + + /** + * Gets the StarkToastNotificationService from the Injector. + * @throws When the service is not found (the ToastNotification module is not imported in the app) + */ + private get toastNotificationService(): StarkToastNotificationService { + if (typeof this._starkToastNotificationService === "undefined") { + this._starkToastNotificationService = this.injector.get(STARK_TOAST_NOTIFICATION_SERVICE); + return this._starkToastNotificationService; + } + return this._starkToastNotificationService; + } +} diff --git a/showcase/src/stark-app-config.json b/showcase/src/stark-app-config.json index 1e84f9b9cc..079233e98e 100644 --- a/showcase/src/stark-app-config.json +++ b/showcase/src/stark-app-config.json @@ -14,6 +14,7 @@ "angularDebugInfoEnabled": null, "debugLoggingEnabled": null, "loggingFlushPersistSize": 50, + "loggingFlushDisabled": true, "loggingFlushApplicationId": "STARK", "loggingFlushResourceName": "logging", "loggingCorrelationIdHttpHeaderName": "correlation-id", diff --git a/starter/src/app/app.module.ts b/starter/src/app/app.module.ts index bcc93d6f8b..fc932c191a 100644 --- a/starter/src/app/app.module.ts +++ b/starter/src/app/app.module.ts @@ -40,7 +40,8 @@ import { StarkSettingsModule, StarkSettingsService, StarkUser, - StarkUserModule + StarkUserModule, + StarkErrorHandlingModule } from "@nationalbankbelgium/stark-core"; import { @@ -81,7 +82,6 @@ import { NoContentComponent } from "./no-content"; // load SASS styles import "../styles/styles.scss"; /* tslint:enable */ - // TODO: where to put this factory function? export function starkAppConfigFactory(): StarkApplicationConfig { const config: any = require("../stark-app-config.json"); @@ -189,6 +189,7 @@ export const metaReducers: MetaReducer[] = ENV !== "production" ? [logger StarkAppLogoutModule, StarkAppMenuModule, StarkAppSidebarModule.forRoot(), + StarkErrorHandlingModule.forRoot(), StarkLanguageSelectorModule, StarkSvgViewBoxModule, StarkDatePickerModule,