refactor: improve error handling

johannesjo · Nov 1, 2019 · 754beb2 · 754beb2
1 parent 393682e
commit 754beb2
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 41 deletions.
diff --git a/src/app/app.constants.ts b/src/app/app.constants.ts
@@ -22,7 +22,6 @@ export const ALL_THEMES = [
   'deep-orange',
 ];
 
-export const HANDLED_ERROR = '--HANDLED_ERROR--';
 
 export enum LanguageCode {
   en = 'en',
@@ -104,3 +103,5 @@ export enum THEME_COLOR_MAP {
   'grey' = '#9e9e9e',
   'blue-grey' = '#607d8b',
 }
+
+export const HANDLED_ERROR_PROP_STR = 'HANDLED_ERROR_PROP';
diff --git a/src/app/core/error-handler/global-error-handler.class.ts b/src/app/core/error-handler/global-error-handler.class.ts
@@ -1,15 +1,13 @@
 import {ErrorHandler, Injectable} from '@angular/core';
 import {isObject} from '../../util/is-object';
 import {getJiraResponseErrorTxt} from '../../util/get-jira-response-error-text';
-import {HANDLED_ERROR, IS_ELECTRON} from '../../app.constants';
+import {HANDLED_ERROR_PROP_STR, IS_ELECTRON} from '../../app.constants';
 import {ElectronService} from 'ngx-electron';
 import {BannerService} from '../banner/banner.service';
 
 let isWasErrorAlertCreated = false;
 
 const _createErrorAlert = (eSvc: ElectronService, err: string, stackTrace: string) => {
-  console.log(stackTrace);
-
   if (isWasErrorAlertCreated) {
     return;
   }
@@ -42,6 +40,10 @@ const _createErrorAlert = (eSvc: ElectronService, err: string, stackTrace: strin
 };
 
 
+const isHandledError = (err): boolean => {
+  return (err && err.hasOwnProperty(HANDLED_ERROR_PROP_STR));
+};
+
 @Injectable()
 export class GlobalErrorHandler implements ErrorHandler {
   private _electronLogger: any;
@@ -61,9 +63,8 @@ export class GlobalErrorHandler implements ErrorHandler {
     // tslint:disable-next-line
     const stack = err && err.stack;
 
-
     // if not our custom error handler we have a critical error on our hands
-    if (!err || (!err.handledError && (errStr && !errStr.match(HANDLED_ERROR)))) {
+    if (!isHandledError(err)) {
       const errorStr = this._getErrorStr(err) || errStr;
 
       // NOTE: dom exceptions will break all rendering that's why

diff --git a/src/app/features/google/google-api.service.ts b/src/app/features/google/google-api.service.ts
@@ -1,7 +1,7 @@
 import {Injectable} from '@angular/core';
 import {GOOGLE_DEFAULT_FIELDS_FOR_DRIVE, GOOGLE_DISCOVERY_DOCS, GOOGLE_SCOPES, GOOGLE_SETTINGS} from './google.const';
 import * as moment from 'moment';
-import {IS_ELECTRON} from '../../app.constants';
+import {HANDLED_ERROR_PROP_STR, IS_ELECTRON} from '../../app.constants';
 import {MultiPartBuilder} from './util/multi-part-builder';
 import {HttpClient, HttpHeaders, HttpParams, HttpRequest} from '@angular/common/http';
 import {SnackService} from '../../core/snack/snack.service';
@@ -192,15 +192,15 @@ export class GoogleApiService {
           };
         } else {
           this._handleError('No data found');
-          return throwError({handledError: 'No data found'});
+          return throwError({[HANDLED_ERROR_PROP_STR]: 'No data found'});
         }
       }));
   }
 
   getFileInfo$(fileId): Observable<any> {
     if (!fileId) {
       this._snackIt('ERROR', T.F.GOOGLE.S_API.ERR_NO_FILE_ID);
-      throwError({handledError: 'No file id given'});
+      throwError({[HANDLED_ERROR_PROP_STR]: 'No file id given'});
     }
 
     return this._mapHttp$({
@@ -217,7 +217,7 @@ export class GoogleApiService {
   findFile$(fileName): Observable<any> {
     if (!fileName) {
       this._snackIt('ERROR', T.F.GOOGLE.S_API.ERR_NO_FILE_NAME);
-      return throwError({handledError: 'No file name given'});
+      return throwError({[HANDLED_ERROR_PROP_STR]: 'No file name given'});
     }
 
     return this._mapHttp$({
@@ -235,7 +235,7 @@ export class GoogleApiService {
   loadFile$(fileId): Observable<any> {
     if (!fileId) {
       this._snackIt('ERROR', T.F.GOOGLE.S_API.ERR_NO_FILE_ID);
-      throwError({handledError: 'No file id given'});
+      throwError({[HANDLED_ERROR_PROP_STR]: 'No file id given'});
     }
 
     const loadFile = this._mapHttp$({
@@ -447,7 +447,7 @@ export class GoogleApiService {
           } else if (res && (res.status >= 0)) {
             this._handleError('Could not connect to google. Check your internet connection.');
           }
-          return throwError({handledError: res});
+          return throwError({[HANDLED_ERROR_PROP_STR]: res});
         }),
       );
   }

diff --git a/src/app/features/google/store/google-drive-sync.effects.ts b/src/app/features/google/store/google-drive-sync.effects.ts
@@ -2,19 +2,7 @@ import {Injectable} from '@angular/core';
 import {Actions, Effect, ofType} from '@ngrx/effects';
 import {Store} from '@ngrx/store';
 import {GlobalConfigActionTypes, UpdateGlobalConfigSection} from '../../config/store/global-config.actions';
-import {
-  catchError,
-  concatMap,
-  distinctUntilChanged,
-  exhaustMap,
-  filter,
-  map,
-  mapTo,
-  switchMap,
-  take,
-  tap,
-  withLatestFrom
-} from 'rxjs/operators';
+import {catchError, concatMap, distinctUntilChanged, exhaustMap, filter, map, mapTo, switchMap, take, tap, withLatestFrom} from 'rxjs/operators';
 import {combineLatest, EMPTY, from, interval, Observable, of, throwError, zip} from 'rxjs';
 import {GoogleDriveSyncService} from '../google-drive-sync.service';
 import {GoogleApiService} from '../google-api.service';
@@ -49,6 +37,7 @@ import {TranslateService} from '@ngx-translate/core';
 import {T} from '../../../t.const';
 import {GlobalSyncService} from '../../../core/global-sync/global-sync.service';
 import {SyncProvider} from '../../../core/global-sync/sync-provider';
+import {HANDLED_ERROR_PROP_STR} from '../../../app.constants';
 
 @Injectable()
 export class GoogleDriveSyncEffects {
@@ -494,7 +483,7 @@ export class GoogleDriveSyncEffects {
   // DATE HELPER
   private _loadFile(): Observable<any> {
     if (!this._config.syncFileName) {
-      return throwError({handledError: 'No file name specified'});
+      return throwError({[HANDLED_ERROR_PROP_STR]: 'No file name specified'});
     }
     return this._googleApiService.loadFile$(this._config._backupDocId);
   }
@@ -540,7 +529,7 @@ export class GoogleDriveSyncEffects {
   }
 
   // SIMPLE HELPER
-  private _isNewerThan(strDate1: string|number, strDate2: string|number) {
+  private _isNewerThan(strDate1: string | number, strDate2: string | number) {
     const d1 = new Date(strDate1);
     const d2 = new Date(strDate2);
     return (d1.getTime() > d2.getTime());

diff --git a/src/app/features/issue/github/github-api.service.ts b/src/app/features/issue/github/github-api.service.ts
@@ -12,7 +12,7 @@ import {GithubComment, GithubIssue} from './github-issue/github-issue.model';
 import {SearchResultItem} from '../issue';
 import {loadFromLs, saveToLs} from '../../../core/persistence/local-storage';
 import {LS_GITHUB_ISSUE_CACHE_PREFIX} from '../../../core/persistence/ls-keys.const';
-import {HANDLED_ERROR} from '../../../app.constants';
+import {HANDLED_ERROR_PROP_STR} from '../../../app.constants';
 import {T} from '../../../t.const';
 
 const BASE = GITHUB_API_BASE_URL;
@@ -174,7 +174,9 @@ export class GithubApiService {
         type: 'ERROR',
         msg: T.F.GITHUB.S.ERR_NOT_CONFIGURED
       });
-      throw new Error(`${HANDLED_ERROR} Not enough settings`);
+      const e = new Error(`Not enough settings`);
+      e[HANDLED_ERROR_PROP_STR] = 'Not enough settings';
+      throw e;
     }
   }
 
@@ -198,10 +200,10 @@ export class GithubApiService {
       });
     }
     if (error && error.message) {
-      return throwError({handledError: 'Github: ' + error.message});
+      return throwError({[HANDLED_ERROR_PROP_STR]: 'Github: ' + error.message});
     }
 
-    return throwError({handledError: 'Github: Api request failed.'});
+    return throwError({[HANDLED_ERROR_PROP_STR]: 'Github: Api request failed.'});
   }
 
   private _mergeIssuesAndComments(issues: GithubIssue[], comments: GithubComment[]): GithubIssue[] {

diff --git a/src/app/features/issue/jira/jira-api.service.ts b/src/app/features/issue/jira/jira-api.service.ts
@@ -21,7 +21,7 @@ import {JiraCfg} from './jira';
 import {ElectronService} from 'ngx-electron';
 import {IPC} from '../../../../../electron/ipc-events.const';
 import {SnackService} from '../../../core/snack/snack.service';
-import {IS_ELECTRON} from '../../../app.constants';
+import {HANDLED_ERROR_PROP_STR, IS_ELECTRON} from '../../../app.constants';
 import {loadFromSessionStorage, saveToSessionStorage} from '../../../core/persistence/local-storage';
 import {combineLatest, Observable, throwError} from 'rxjs';
 import {SearchResultItem} from '../issue';
@@ -88,7 +88,7 @@ export class JiraApiService {
           .pipe(catchError((err) => {
             this._blockAccess();
             checkConnectionSub.unsubscribe();
-            return throwError({handledError: err});
+            return throwError({[HANDLED_ERROR_PROP_STR]: err});
           }))
           .subscribe(() => {
             this.unblockAccess();
@@ -144,7 +144,7 @@ export class JiraApiService {
     const searchQuery = this._cfg.autoAddBacklogJqlQuery;
 
     if (!searchQuery) {
-      return throwError({handledError: 'JiraApi: No search query for auto import'});
+      return throwError({[HANDLED_ERROR_PROP_STR]: 'JiraApi: No search query for auto import'});
     }
 
     return this._sendRequest$({
@@ -243,7 +243,7 @@ export class JiraApiService {
           ? T.F.JIRA.S.EXTENSION_NOT_LOADED
           : T.F.JIRA.S.INSUFFICIENT_SETTINGS,
       });
-      return throwError({handledError: 'Insufficient Settings for Jira'});
+      return throwError({[HANDLED_ERROR_PROP_STR]: 'Insufficient Settings for Jira'});
     }
 
     if (this._isBlockAccess && !isForce) {
@@ -257,7 +257,7 @@ export class JiraApiService {
           fn: () => this.unblockAccess()
         }
       });
-      return throwError({handledError: 'Blocked access to prevent being shut out'});
+      return throwError({[HANDLED_ERROR_PROP_STR]: 'Blocked access to prevent being shut out'});
     }
 
     // assign uuid to request to know which responsive belongs to which promise
@@ -316,7 +316,7 @@ export class JiraApiService {
         catchError((err) => {
           const errTxt = getJiraResponseErrorTxt(err);
           this._snackService.open({type: 'ERROR', msg: `Jira: ${errTxt}`});
-          return throwError({handledError: errTxt});
+          return throwError({[HANDLED_ERROR_PROP_STR]: errTxt});
         }),
         first(),
       );

diff --git a/src/app/features/issue/jira/jira-cfg-stepper/jira-cfg-stepper.component.ts b/src/app/features/issue/jira/jira-cfg-stepper/jira-cfg-stepper.component.ts
@@ -17,7 +17,7 @@ import {expandAnimation} from '../../../../ui/animations/expand.ani';
 import {catchError} from 'rxjs/operators';
 import {Subscription, throwError} from 'rxjs';
 import {T} from '../../../../t.const';
-import {HelperClasses} from '../../../../app.constants';
+import {HANDLED_ERROR_PROP_STR, HelperClasses} from '../../../../app.constants';
 
 @Component({
   selector: 'jira-cfg-stepper',
@@ -76,7 +76,7 @@ export class JiraCfgStepperComponent implements OnDestroy {
           this.isTestCredentialsSuccess = false;
           this.user = null;
           this._changeDetectorRef.detectChanges();
-          return throwError({handledError: err});
+          return throwError({[HANDLED_ERROR_PROP_STR]: err});
         }))
         .subscribe((user: JiraOriginalUser) => {
           this.user = user;

diff --git a/src/app/features/issue/jira/jira-issue/store/jira-issue.effects.ts b/src/app/features/issue/jira/jira-issue/store/jira-issue.effects.ts
@@ -34,6 +34,7 @@ import {isEmail} from '../../../../../util/is-email';
 import {T} from '../../../../../t.const';
 import {truncate} from '../../../../../util/truncate';
 import {ProjectService} from '../../../../project/project.service';
+import {HANDLED_ERROR_PROP_STR} from '../../../../../app.constants';
 
 @Injectable()
 export class JiraIssueEffects {
@@ -180,7 +181,7 @@ export class JiraIssueEffects {
           });
           return EMPTY;
         } else if (!issue) {
-          return throwError({handledError: 'Jira: Issue Data not found'});
+          return throwError({[HANDLED_ERROR_PROP_STR]: 'Jira: Issue Data not found'});
         } else if (!issue.assignee || issue.assignee.name !== currentUserName) {
           return this._matDialog.open(DialogConfirmComponent, {
             restoreFocus: true,
@@ -353,7 +354,7 @@ export class JiraIssueEffects {
             type: 'ERROR',
           });
           // NOTE: we would kill the whole effect chain if we do this
-          // return throwError({handledError: 'Jira: No valid transition configured'});
+          // return throwError({[HANDLED_ERROR_PROP_STR]: 'Jira: No valid transition configured'});
           return timer(2000).pipe(concatMap(() => this._openTransitionDialog(issue, localState)));
         }