From 2d186d4804be697f2f097a6b05436728b57c97e7 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Thu, 5 Dec 2019 21:37:06 +0100 Subject: [PATCH] fix(logging): clean up and update LoggingService and its link to Sentry * update to latest sentry sdk and use LoggerService consistently * do not log debug events to remote * add Angular ErrorHandler to catch and log exceptions globally see PR #384 --- package-lock.json | 62 ++++++++++++++-- package.json | 2 +- src/app/alerts/alert.service.ts | 3 - src/app/app.module.ts | 4 +- src/app/entity/entity-mapper.service.spec.ts | 3 +- .../latest-changes/update-manager.service.ts | 10 +-- src/app/logging/logging-error-handler.ts | 14 ++++ src/app/logging/logging.service.spec.ts | 47 ++++++------ src/app/logging/logging.service.ts | 71 ++++++++++++------- 9 files changed, 148 insertions(+), 68 deletions(-) create mode 100644 src/app/logging/logging-error-handler.ts diff --git a/package-lock.json b/package-lock.json index a040b5444f..0605a241d9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2173,6 +2173,63 @@ } } }, + "@sentry/browser": { + "version": "5.9.1", + "resolved": "https://registry.npmjs.org/@sentry/browser/-/browser-5.9.1.tgz", + "integrity": "sha512-7AOabwp9yAH9h6Xe6TfDwlLxHbUSWs+SPWHI7bPlht2yDSAqkXYGSzRr5X0XQJX9oBQdx2cEPMqHyJrbNaP/og==", + "requires": { + "@sentry/core": "5.8.0", + "@sentry/types": "5.7.1", + "@sentry/utils": "5.8.0", + "tslib": "^1.9.3" + } + }, + "@sentry/core": { + "version": "5.8.0", + "resolved": "https://registry.npmjs.org/@sentry/core/-/core-5.8.0.tgz", + "integrity": "sha512-aAh2KLidIXJVGrxmHSVq2eVKbu7tZiYn5ylW6yzJXFetS5z4MA+JYaSBaG2inVYDEEqqMIkb17TyWxxziUDieg==", + "requires": { + "@sentry/hub": "5.8.0", + "@sentry/minimal": "5.8.0", + "@sentry/types": "5.7.1", + "@sentry/utils": "5.8.0", + "tslib": "^1.9.3" + } + }, + "@sentry/hub": { + "version": "5.8.0", + "resolved": "https://registry.npmjs.org/@sentry/hub/-/hub-5.8.0.tgz", + "integrity": "sha512-VdApn1ZCNwH1wwQwoO6pu53PM/qgHG+DQege0hbByluImpLBhAj9w50nXnF/8KzV4UoMIVbzCb6jXzMRmqqp9A==", + "requires": { + "@sentry/types": "5.7.1", + "@sentry/utils": "5.8.0", + "tslib": "^1.9.3" + } + }, + "@sentry/minimal": { + "version": "5.8.0", + "resolved": "https://registry.npmjs.org/@sentry/minimal/-/minimal-5.8.0.tgz", + "integrity": "sha512-MIlFOgd+JvAUrBBmq7vr9ovRH1HvckhnwzHdoUPpKRBN+rQgTyZy1o6+kA2fASCbrRqFCP+Zk7EHMACKg8DpIw==", + "requires": { + "@sentry/hub": "5.8.0", + "@sentry/types": "5.7.1", + "tslib": "^1.9.3" + } + }, + "@sentry/types": { + "version": "5.7.1", + "resolved": "https://registry.npmjs.org/@sentry/types/-/types-5.7.1.tgz", + "integrity": "sha512-tbUnTYlSliXvnou5D4C8Zr+7/wJrHLbpYX1YkLXuIJRU0NSi81bHMroAuHWILcQKWhVjaV/HZzr7Y/hhWtbXVQ==" + }, + "@sentry/utils": { + "version": "5.8.0", + "resolved": "https://registry.npmjs.org/@sentry/utils/-/utils-5.8.0.tgz", + "integrity": "sha512-KDxUvBSYi0/dHMdunbxAxD3389pcQioLtcO6CI6zt/nJXeVFolix66cRraeQvqupdLhvOk/el649W4fCPayTHw==", + "requires": { + "@sentry/types": "5.7.1", + "tslib": "^1.9.3" + } + }, "@types/crypto-js": { "version": "3.1.43", "resolved": "https://registry.npmjs.org/@types/crypto-js/-/crypto-js-3.1.43.tgz", @@ -11445,11 +11502,6 @@ "integrity": "sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==", "dev": true }, - "raven-js": { - "version": "3.27.2", - "resolved": "https://registry.npmjs.org/raven-js/-/raven-js-3.27.2.tgz", - "integrity": "sha512-mFWQcXnhRFEQe5HeFroPaEghlnqy7F5E2J3Fsab189ondqUzcjwSVi7el7F36cr6PvQYXoZ1P2F5CSF2/azeMQ==" - }, "raw-body": { "version": "2.4.0", "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.4.0.tgz", diff --git a/package.json b/package.json index ac03e45df6..84a51cb7c7 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "@angular/pwa": "^0.803.0", "@angular/router": "^8.2.2", "@angular/service-worker": "^8.2.2", + "@sentry/browser": "^5.9.1", "core-js": "^2.6.9", "crypto-js": "~3.1.9-1", "faker": "^4.1.0", @@ -37,7 +38,6 @@ "ngx-papaparse": "^3.0.3", "pouchdb-authentication": "^1.1.3", "pouchdb-browser": "^7.1.1", - "raven-js": "^3.27.2", "reflect-metadata": "^0.1.13", "rxjs": "^6.5.2", "uniqid": "^5.0.3", diff --git a/src/app/alerts/alert.service.ts b/src/app/alerts/alert.service.ts index 41a6604310..b386804c12 100644 --- a/src/app/alerts/alert.service.ts +++ b/src/app/alerts/alert.service.ts @@ -59,16 +59,13 @@ export class AlertService { switch (alert.type) { case Alert.WARNING: case Alert.DANGER: - console.warn(alert.message); this.loggingService.warn(alert.message); break; case Alert.INFO: case Alert.SUCCESS: - console.log(alert.message); this.loggingService.info(alert.message); break; case Alert.DEBUG: - console.log(alert.message); this.loggingService.debug(alert.message); break; } diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 7484a2e331..00ffc322d4 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -17,7 +17,7 @@ import { BrowserModule } from '@angular/platform-browser'; import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; -import {APP_INITIALIZER, NgModule} from '@angular/core'; +import {APP_INITIALIZER, ErrorHandler, NgModule} from '@angular/core'; import { FormsModule } from '@angular/forms'; import { HttpClientModule} from '@angular/common/http'; @@ -48,6 +48,7 @@ import {CookieService} from 'ngx-cookie-service'; import {HelpModule} from './help/help.module'; import {DemoDataModule} from './demo-data/demo-data.module'; import {MatNativeDateModule} from '@angular/material/core'; +import {LoggingErrorHandler} from './logging/logging-error-handler'; @NgModule({ declarations: [ @@ -82,6 +83,7 @@ import {MatNativeDateModule} from '@angular/material/core'; providers: [ AppConfig, { provide: APP_INITIALIZER, useFactory: initializeApp, deps: [AppConfig], multi: true }, + { provide: ErrorHandler, useClass: LoggingErrorHandler }, MatIconRegistry, CookieService, ], diff --git a/src/app/entity/entity-mapper.service.spec.ts b/src/app/entity/entity-mapper.service.spec.ts index 03b86ce7d8..d4adb8f55a 100644 --- a/src/app/entity/entity-mapper.service.spec.ts +++ b/src/app/entity/entity-mapper.service.spec.ts @@ -41,8 +41,7 @@ describe('EntityMapperService', () => { testDatabase = new MockDatabase(); entityMapper = new EntityMapperService(testDatabase, new EntitySchemaService()); - return Promise.all([testDatabase.put(existingEntity), testDatabase.put(existingEntity2)]) - .catch(err => console.log('Failed to insert entity: ' + err)); + return Promise.all([testDatabase.put(existingEntity), testDatabase.put(existingEntity2)]); }); diff --git a/src/app/latest-changes/update-manager.service.ts b/src/app/latest-changes/update-manager.service.ts index 5fce7a7f3f..784208cf71 100644 --- a/src/app/latest-changes/update-manager.service.ts +++ b/src/app/latest-changes/update-manager.service.ts @@ -20,6 +20,7 @@ import {SwUpdate} from '@angular/service-worker'; import {first} from 'rxjs/operators'; import {concat, interval} from 'rxjs'; import { MatSnackBar } from '@angular/material/snack-bar'; +import {LoggingService} from '../logging/logging.service'; @Injectable() export class UpdateManagerService { @@ -29,12 +30,13 @@ export class UpdateManagerService { constructor( private appRef: ApplicationRef, private updates: SwUpdate, - public snackBar: MatSnackBar) { - + public snackBar: MatSnackBar, + private logger: LoggingService, + ) { } public notifyUserWhenUpdateAvailable() { - this.updates.available.subscribe(event => { + this.updates.available.subscribe(() => { this.showUpdateNotification(); }); } @@ -49,7 +51,7 @@ export class UpdateManagerService { try { await this.updates.checkForUpdate(); } catch (err) { - console.log(err); + this.logger.error(err); } }); } diff --git a/src/app/logging/logging-error-handler.ts b/src/app/logging/logging-error-handler.ts new file mode 100644 index 0000000000..3261dc1b53 --- /dev/null +++ b/src/app/logging/logging-error-handler.ts @@ -0,0 +1,14 @@ +import {ErrorHandler, Injectable} from '@angular/core'; +import {LoggingService} from './logging.service'; + +@Injectable() +export class LoggingErrorHandler implements ErrorHandler { + constructor(private logger: LoggingService) {} + + handleError(error) { + this.logger.error(error); + // It is possible to show a feedback dialog to the user through Sentry: + // const eventId = Sentry.captureException(error.originalError || error); + // Sentry.showReportDialog({ eventId }); + } +} diff --git a/src/app/logging/logging.service.spec.ts b/src/app/logging/logging.service.spec.ts index 71a9e17889..a14c1041b2 100644 --- a/src/app/logging/logging.service.spec.ts +++ b/src/app/logging/logging.service.spec.ts @@ -1,34 +1,17 @@ import {LoggingService} from './logging.service'; -import * as Raven from 'raven-js'; - -class LoggingServiceRavenMock extends LoggingService { - - public latestRavenCalls: Array<[string, Raven.RavenOptions]>; - - constructor() { - super(); - this.latestRavenCalls = []; - } - - protected ravenCaptureMessage(message: string, options: Raven.RavenOptions): void { - this.latestRavenCalls.push([message, options]); - } -} +import {LogLevel} from './log-level'; describe('LoggingService', () => { const testMessage = 'FANCY_TEST_MESSAGE'; - let loggingService: LoggingServiceRavenMock; + let loggingService: LoggingService; beforeEach(() => { - loggingService = new LoggingServiceRavenMock(); + loggingService = new LoggingService(); + spyOn(loggingService, 'logToConsole'); + spyOn(loggingService, 'logToRemoteMonitoring'); }); - function checkReceivedLogMessage(level: Raven.LogLevel) { - const receivedLogRequest = loggingService.latestRavenCalls.pop(); - expect(receivedLogRequest[0]).toEqual(testMessage); - expect(receivedLogRequest[1].level).toEqual(level); - } it('should be created', () => { expect(loggingService).toBeTruthy(); @@ -37,25 +20,37 @@ describe('LoggingService', () => { it('should log a debug message', function () { loggingService.debug(testMessage); - checkReceivedLogMessage('debug'); + expect(loggingService['logToConsole']).toHaveBeenCalledWith(testMessage, LogLevel.DEBUG); + expect(loggingService['logToRemoteMonitoring']).not.toHaveBeenCalled(); }); it('should log a info message', function () { loggingService.info(testMessage); - checkReceivedLogMessage('info'); + expect(loggingService['logToConsole']).toHaveBeenCalledWith(testMessage, LogLevel.INFO); + expect(loggingService['logToRemoteMonitoring']).not.toHaveBeenCalled(); }); it('should log a warn message', function () { loggingService.warn(testMessage); - checkReceivedLogMessage('warn'); + expect(loggingService['logToConsole']).toHaveBeenCalledWith(testMessage, LogLevel.WARN); + expect(loggingService['logToRemoteMonitoring']).toHaveBeenCalledWith(testMessage, LogLevel.WARN); }); it('should log a error message', function () { loggingService.error(testMessage); - checkReceivedLogMessage('error'); + expect(loggingService['logToConsole']).toHaveBeenCalledWith(testMessage, LogLevel.ERROR); + expect(loggingService['logToRemoteMonitoring']).toHaveBeenCalledWith(testMessage, LogLevel.ERROR); + }); + + + it('should log a message through the generic log method', function () { + loggingService.log(testMessage, LogLevel.WARN); + + expect(loggingService['logToConsole']).toHaveBeenCalledWith(testMessage, LogLevel.WARN); + expect(loggingService['logToRemoteMonitoring']).toHaveBeenCalledWith(testMessage, LogLevel.WARN); }); }); diff --git a/src/app/logging/logging.service.ts b/src/app/logging/logging.service.ts index 7d77985f4f..d639913132 100644 --- a/src/app/logging/logging.service.ts +++ b/src/app/logging/logging.service.ts @@ -1,13 +1,17 @@ import {Injectable} from '@angular/core'; -import * as Raven from 'raven-js'; -import {LogLevel as RavenLogLevel, RavenOptions} from 'raven-js'; import {LogLevel} from './log-level'; +import * as Sentry from '@sentry/browser'; -Raven - .config('https://bd6aba79ca514d35bb06a4b4e0c2a21e@sentry.io/1242399') - .install(); +Sentry.init({ + dsn: 'https://bd6aba79ca514d35bb06a4b4e0c2a21e@sentry.io/1242399', + whitelistUrls: [ + /https?:\/\/(.*)\.?aam-digital\.com/ + ] +}); + +/* tslint:disable:no-console */ @Injectable({ providedIn: 'root' }) @@ -16,53 +20,68 @@ export class LoggingService { constructor() { } - public log(message: string, logLevel: LogLevel) { - let options: RavenOptions; - options = {}; - options.level = this.translateLogLevel(logLevel); - - this.ravenCaptureMessage(message, options); - } - public debug(message: string) { + public debug(message: any) { this.log(message, LogLevel.DEBUG); } - public info(message: string) { + public info(message: any) { this.log(message, LogLevel.INFO); } - public warn(message: string) { + public warn(message: any) { this.log(message, LogLevel.WARN); } - public error(message: string) { + public error(message: any) { this.log(message, LogLevel.ERROR); } - private translateLogLevel(logLevel: LogLevel): RavenLogLevel { - let retVal: RavenLogLevel; + public log(message: any, logLevel: LogLevel = LogLevel.INFO) { + this.logToConsole(message, logLevel); + + if (logLevel !== LogLevel.DEBUG && logLevel !== LogLevel.INFO) { + this.logToRemoteMonitoring(message, logLevel); + } + } + + private logToConsole(message: any, logLevel: LogLevel) { switch (+logLevel) { case LogLevel.DEBUG: - retVal = 'debug'; + console.debug(message); break; case LogLevel.INFO: - retVal = 'info'; + console.info(message); break; case LogLevel.WARN: - retVal = 'warn'; + console.warn(message); break; case LogLevel.ERROR: - retVal = 'error'; + console.error(message); + break; + default: + console.log(message); break; } + } - return retVal; + private logToRemoteMonitoring(message: any, logLevel: LogLevel) { + Sentry.captureMessage(message, this.translateLogLevel(logLevel)); } - protected ravenCaptureMessage(message: string, - options: RavenOptions) { - Raven.captureMessage(message, options); + private translateLogLevel(logLevel: LogLevel): Sentry.Severity { + switch (+logLevel) { + case LogLevel.DEBUG: + return Sentry.Severity.Debug; + case LogLevel.INFO: + return Sentry.Severity.Info; + case LogLevel.WARN: + return Sentry.Severity.Warning; + case LogLevel.ERROR: + return Sentry.Severity.Error; + default: + return Sentry.Severity.Info; + } } }