Standardize logging formats and add more logging (#1147)

* Update logging format to use `codefile: action`

* Update logging format for ErrorHandlingService

- Use `codefile: action` format
- Fix tests

* Add more logging for AuthService

* Add additional logging

* Rename loggingService to logging

* Fix tests

src/app/app.component.ts

@@ -13,12 +13,12 @@ export class AppComponent implements AfterViewInit {
   NOT_CONNECTED_ERROR: Error = new Error('You are not connected to the internet.');
 
   constructor(public electronService: ElectronService, logger: LoggingService, public errorHandlingService: ErrorHandlingService) {
-    logger.info('AppConfig', AppConfig);
+    logger.info('AppComponent: AppConfig', AppConfig);
 
     if (electronService.isElectron()) {
-      logger.info('Mode electron');
+      logger.info('AppComponent: Mode electron');
     } else {
-      logger.info('Mode web');
+      logger.info('AppComponent: Mode web');
     }
   }
 

src/app/auth/auth.component.ts

@@ -72,7 +72,7 @@ export class AuthComponent implements OnInit, OnDestroy {
       // runs upon receiving oauthCode from the redirect
       this.authService.changeAuthState(AuthState.AwaitingAuthentication);
       this.restoreOrgDetailsFromLocalStorage();
-      this.logger.info('Obtained authorisation code from Github');
+      this.logger.info('AuthComponent: Obtained authorisation code from Github');
       this.fetchAccessToken(oauthCode, state);
     }
   }
@@ -84,11 +84,11 @@ export class AuthComponent implements OnInit, OnDestroy {
    */
   fetchAccessToken(oauthCode: string, state: string) {
     if (!this.authService.isReturnedStateSame(state)) {
-      this.logger.info(`Received incorrect state ${state}, continue waiting for correct state`);
+      this.logger.info(`AuthComponent: Received incorrect state ${state}, continue waiting for correct state`);
       return;
     }
 
-    this.logger.info('Retrieving access token from Github');
+    this.logger.info('AuthComponent: Retrieving access token from Github');
 
     const accessTokenUrl = `${AppConfig.accessTokenUrl}/${oauthCode}/client_id/${AppConfig.clientId}`;
     fetch(accessTokenUrl)
@@ -98,10 +98,10 @@ export class AuthComponent implements OnInit, OnDestroy {
           throw new Error(data.error);
         }
         this.authService.storeOAuthAccessToken(data.token);
-        this.logger.info('Sucessfully obtained access token');
+        this.logger.info('AuthComponent: Sucessfully obtained access token');
       })
       .catch((err) => {
-        this.logger.info(`Error in data fetched from access token URL: ${err}`);
+        this.logger.info(`AuthComponent: Error in data fetched from access token URL: ${err}`);
         this.errorHandlingService.handleError(err);
         this.authService.changeAuthState(AuthState.NotAuthenticated);
       });

src/app/auth/confirm-login/confirm-login.component.ts

@@ -37,7 +37,7 @@ export class ConfirmLoginComponent implements OnInit {
   }
 
   logIntoAnotherAccount() {
-    this.logger.info('Logging into another account');
+    this.logger.info('ConfirmLoginComponent: Logging into another account');
     this.electronService.clearCookies();
     this.authService.startOAuthProcess();
   }
@@ -70,7 +70,7 @@ export class ConfirmLoginComponent implements OnInit {
         (error) => {
           this.authService.changeAuthState(AuthState.NotAuthenticated);
           this.errorHandlingService.handleError(error);
-          this.logger.info(`Completion of login process failed with an error: ${error}`);
+          this.logger.info(`ConfirmLoginComponent: Completion of login process failed with an error: ${error}`);
         }
       );
   }

src/app/auth/session-selection/session-selection.component.ts

@@ -57,7 +57,7 @@ export class SessionSelectionComponent implements OnInit {
     window.localStorage.setItem('dataRepo', dataRepo);
     this.githubService.storeOrganizationDetails(org, dataRepo);
 
-    this.logger.info(`Selected Settings Repo: ${sessionInformation}`);
+    this.logger.info(`SessionSelectionComponent: Selected Settings Repo: ${sessionInformation}`);
 
     this.phaseService.storeSessionData().subscribe(
       () => {

src/app/core/services/auth.service.ts

@@ -61,6 +61,7 @@ export class AuthService {
   }
 
   reset(): void {
+    this.logger.info('AuthService: Clearing access token and setting AuthState to NotAuthenticated.');
     this.accessToken.next(undefined);
     this.changeAuthState(AuthState.NotAuthenticated);
     this.ngZone.run(() => this.router.navigate(['']));
@@ -82,12 +83,14 @@ export class AuthService {
   setTitleWithPhaseDetail(): void {
     const appSetting = require('../../../../package.json');
     const title = `${appSetting.name} ${appSetting.version} - ${this.phaseService.getPhaseDetail()}`;
+    this.logger.info(`AuthService: Setting Title as ${title}`);
     this.titleService.setTitle(title);
   }
 
   setLandingPageTitle(): void {
     const appSetting = require('../../../../package.json');
     const title = `${appSetting.name} ${appSetting.version}`;
+    this.logger.info(`AuthService: Setting LandingPageTitle as ${title}`);
     this.titleService.setTitle(title);
   }
 
@@ -99,7 +102,7 @@ export class AuthService {
     if (newAuthState === AuthState.Authenticated) {
       const sessionId = generateSessionId();
       this.issueService.setSessionId(sessionId);
-      this.logger.info(`Successfully authenticated with session: ${sessionId}`);
+      this.logger.info(`AuthService: Successfully authenticated with session: ${sessionId}`);
     }
     this.authStateSource.next(newAuthState);
   }
@@ -121,7 +124,7 @@ export class AuthService {
    * Will start the Github OAuth web flow process.
    */
   startOAuthProcess() {
-    this.logger.info('Starting authentication');
+    this.logger.info('AuthService: Starting authentication');
     const githubRepoPermission = this.phaseService.githubRepoPermissionLevel();
     this.changeAuthState(AuthState.AwaitingAuthentication);
 
@@ -134,7 +137,7 @@ export class AuthService {
           `${AppConfig.githubUrl}/login/oauth/authorize?client_id=${AppConfig.clientId}&scope=${githubRepoPermission},read:user&state=${this.state}`
         )
       );
-      this.logger.info('Redirecting for Github authentication');
+      this.logger.info('AuthService: Redirecting for Github authentication');
     }
   }
 

src/app/core/services/error-handling.service.ts

@@ -17,9 +17,9 @@ export class ErrorHandlingService implements ErrorHandler {
   constructor(private snackBar: MatSnackBar, private logger: LoggingService) {}
 
   handleError(error: HttpErrorResponse | Error | RequestError, actionCallback?: () => void) {
-    this.logger.error(error);
+    this.logger.error('ErrorHandlingService: ' + error);
     if (error instanceof Error) {
-      this.logger.debug(this.cleanStack(error.stack));
+      this.logger.debug('ErrorHandlingService: ' + this.cleanStack(error.stack));
     }
     if (error instanceof HttpErrorResponse) {
       this.handleHttpError(error, actionCallback);

src/app/core/services/github.service.ts

@@ -75,11 +75,12 @@ export class GithubService {
         return `Token ${accessToken}`;
       },
       log: {
-        debug: (message, ...otherInfo) => this.logger.debug(message, ...otherInfo),
+        debug: (message, ...otherInfo) => this.logger.debug('GithubService: ' + message, ...otherInfo),
         // Do not log info for HTTP response 304 due to repeated polling
-        info: (message, ...otherInfo) => (/304 in \d+ms$/.test(message) ? undefined : this.logger.info(message, ...otherInfo)),
-        warn: (message, ...otherInfo) => this.logger.warn(message, ...otherInfo),
-        error: (message, ...otherInfo) => this.logger.error(message, ...otherInfo)
+        info: (message, ...otherInfo) =>
+          /304 in \d+ms$/.test(message) ? undefined : this.logger.info('GithubService: ' + message, ...otherInfo),
+        warn: (message, ...otherInfo) => this.logger.warn('GithubService: ' + message, ...otherInfo),
+        error: (message, ...otherInfo) => this.logger.error('GithubService: ' + message, ...otherInfo)
       }
     });
   }
@@ -434,6 +435,7 @@ export class GithubService {
   }
 
   reset(): void {
+    this.logger.info(`GithubService: Resetting issues cache`);
     this.issuesCacheManager.clear();
     this.issuesLastModifiedManager.clear();
     this.issueQueryRefs.clear();

src/app/core/services/issue.service.ts

@@ -146,7 +146,7 @@ export class IssueService {
           return this.createIssueModel(response);
         }),
         catchError((err) => {
-          this.logger.error(err); // Log full details of error first
+          this.logger.error('IssueService: ', err); // Log full details of error first
           return throwError(err.response.data.message); // More readable error message
         })
       );
@@ -476,7 +476,7 @@ export class IssueService {
     }
 
     if (issue.parseError) {
-      this.logger.error(issue.parseError);
+      this.logger.error('IssueService: ' + issue.parseError);
     }
     return issue;
   }

src/app/core/services/mocks/mock.auth.service.ts

@@ -75,7 +75,7 @@ export class MockAuthService {
     if (newAuthState === AuthState.Authenticated) {
       const sessionId = `${Date.now()}-${uuid()}`;
       this.issueService.setSessionId(sessionId);
-      this.logger.info(`Successfully authenticated with session: ${sessionId}`);
+      this.logger.info(`MockAuthService: Successfully authenticated with session: ${sessionId}`);
     }
     this.authStateSource.next(newAuthState);
   }

src/app/core/services/user.service.ts

@@ -6,6 +6,7 @@ import { Team } from '../models/team.model';
 import { User, UserRole } from '../models/user.model';
 import { DataService } from './data.service';
 import { GithubService } from './github.service';
+import { LoggingService } from './logging.service';
 
 @Injectable({
   providedIn: 'root'
@@ -17,7 +18,7 @@ import { GithubService } from './github.service';
 export class UserService {
   public currentUser: User;
 
-  constructor(private githubService: GithubService, private dataService: DataService) {}
+  constructor(private githubService: GithubService, private dataService: DataService, private logger: LoggingService) {}
 
   /**
    * Get the authenticated user if it exist.
@@ -42,6 +43,7 @@ export class UserService {
   }
 
   reset() {
+    this.logger.info('UserService: Clearing current user');
     this.currentUser = undefined;
   }
 

src/app/shared/comment-editor/comment-editor.component.ts

@@ -134,7 +134,7 @@ export class CommentEditorComponent implements OnInit {
 
     for (let i = 0; i < files.length; i++) {
       setTimeout(() => {
-        this.logger.info(`File ${i + 1} of ${files.length}. Begin uploading ${files[i].name}.`);
+        this.logger.info(`CommentEditorComponent: File ${i + 1} of ${files.length}. Begin uploading ${files[i].name}.`);
         this.readAndUploadFile(files[i]);
       }, TIME_BETWEEN_UPLOADS_MS * i);
     }

src/app/shared/issue-tables/issue-tables.component.ts

@@ -58,7 +58,7 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
     public issueService: IssueService,
     private phaseService: PhaseService,
     private errorHandlingService: ErrorHandlingService,
-    private loggingService: LoggingService,
+    private logger: LoggingService,
     private dialogService: DialogService,
     private snackBar: MatSnackBar = null
   ) {}
@@ -100,7 +100,7 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
   }
 
   markAsResponded(issue: Issue, event: Event) {
-    this.loggingService.info(`IssueTablesComponent: Marking Issue ${issue.id} as Responded`);
+    this.logger.info(`IssueTablesComponent: Marking Issue ${issue.id} as Responded`);
     const newIssue = issue.clone(this.phaseService.currentPhase);
     newIssue.status = STATUS.Done;
     this.issueService.updateIssue(newIssue).subscribe(
@@ -119,7 +119,7 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
   }
 
   markAsPending(issue: Issue, event: Event) {
-    this.loggingService.info(`IssueTablesComponent: Marking Issue ${issue.id} as Pending`);
+    this.logger.info(`IssueTablesComponent: Marking Issue ${issue.id} as Pending`);
     const newIssue = issue.clone(this.phaseService.currentPhase);
     newIssue.status = STATUS.Incomplete;
     this.issueService.updateIssue(newIssue).subscribe(
@@ -134,11 +134,11 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
   }
 
   logIssueRespondRouting(id: number) {
-    this.loggingService.info(`IssueTablesComponent: Proceeding to Respond to Issue ${id}`);
+    this.logger.info(`IssueTablesComponent: Proceeding to Respond to Issue ${id}`);
   }
 
   logIssueEditRouting(id: number) {
-    this.loggingService.info(`IssueTablesComponent: Proceeding to Edit Issue ${id}`);
+    this.logger.info(`IssueTablesComponent: Proceeding to Edit Issue ${id}`);
   }
 
   /**
@@ -156,12 +156,12 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
   }
 
   viewIssueInBrowser(id: number, event: Event) {
-    this.loggingService.info(`IssueTablesComponent: Opening Issue ${id} on Github`);
+    this.logger.info(`IssueTablesComponent: Opening Issue ${id} on Github`);
     this.githubService.viewIssueInBrowser(id, event);
   }
 
   deleteIssue(id: number, event: Event) {
-    this.loggingService.info(`IssueTablesComponent: Deleting Issue ${id}`);
+    this.logger.info(`IssueTablesComponent: Deleting Issue ${id}`);
     this.issuesPendingDeletion = {
       ...this.issuesPendingDeletion,
       [id]: true
@@ -193,7 +193,7 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
   }
 
   undeleteIssue(id: number, event: Event) {
-    this.loggingService.info(`IssueTablesComponent: Undeleting Issue ${id}`);
+    this.logger.info(`IssueTablesComponent: Undeleting Issue ${id}`);
     this.issueService.undeleteIssue(id).subscribe(
       (reopenedIssue) => {},
       (error) => {
@@ -214,7 +214,7 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
 
     dialogRef.afterClosed().subscribe((res) => {
       if (res) {
-        this.loggingService.info(`Deleting issue ${id}`);
+        this.logger.info(`IssueTablesComponent: Deleting issue ${id}`);
         this.deleteIssue(id, event);
       }
     });

src/app/shared/layout/header.component.ts

@@ -40,7 +40,7 @@ export class HeaderComponent implements OnInit {
     public auth: AuthService,
     public phaseService: PhaseService,
     public userService: UserService,
-    public loggingService: LoggingService,
+    public logger: LoggingService,
     private location: Location,
     private githubEventService: GithubEventService,
     private issueService: IssueService,
@@ -188,13 +188,13 @@ export class HeaderComponent implements OnInit {
 
     dialogRef.afterClosed().subscribe((res) => {
       if (res) {
-        this.loggingService.info(`Logging out from ${this.userService.currentUser.loginId}`);
+        this.logger.info(`HeaderComponent: Logging out from ${this.userService.currentUser.loginId}`);
         this.logOut();
       }
     });
   }
 
   exportLogFile() {
-    this.loggingService.exportLogFile();
+    this.logger.exportLogFile();
   }
 }

tests/app/phase-team-response/issues-faulty/issues-faulty.component.spec.ts

@@ -11,7 +11,7 @@ describe('IssuesFaultyComponent', () => {
     const dummyIssue = Issue.createPhaseTeamResponseIssue(ISSUE_WITH_EMPTY_DESCRIPTION, dummyTeam);
     let issueService: IssueService;
     let issuesFaultyComponent: IssuesFaultyComponent;
-    const userService = new UserService(null, null);
+    const userService = new UserService(null, null, null);
     userService.currentUser = USER_Q;
     const DUMMY_DUPLICATE_ISSUE_ID = 1;
     const DUMMY_RESPONSE = 'dummy response';

tests/app/phase-team-response/issues-pending/issues-pending.component.spec.ts

@@ -12,7 +12,7 @@ describe('IssuesPendingComponent', () => {
     let dummyIssue: Issue;
     let issuesPendingComponent: IssuesPendingComponent;
     const issueService: IssueService = new IssueService(null, null, null, null, null, null);
-    const userService: UserService = new UserService(null, null);
+    const userService: UserService = new UserService(null, null, null);
     userService.currentUser = USER_Q;
     const DUMMY_DUPLICATE_ISSUE_ID = 1;
     const DUMMY_RESPONSE = 'dummy response';

tests/app/phase-team-response/issues-responded/issues-responded.component.spec.ts

@@ -13,7 +13,7 @@ describe('IssuesRespondedComponent', () => {
     let dummyIssue: Issue;
 
     const issueService = new IssueService(null, null, null, null, null, null);
-    const userService = new UserService(null, null);
+    const userService = new UserService(null, null, null);
     userService.currentUser = USER_Q;
     const issuesRespondedComponent = new IssuesRespondedComponent(issueService, userService);
     issuesRespondedComponent.ngOnInit();

tests/services/error-handling.service.spec.ts

@@ -27,7 +27,7 @@ describe('ErrorHandlingService', () => {
   describe('ErrorHandlingService: handleError()', () => {
     it('should log errors when handling errors', () => {
       errorHandlingService.handleError(STANDARD_ERROR);
-      expect(mockLoggingService.error).toHaveBeenCalledWith(STANDARD_ERROR);
+      expect(mockLoggingService.error).toHaveBeenCalledWith('ErrorHandlingService: ' + STANDARD_ERROR);
     });
 
     it('should use the GeneralMessageErrorComponent when handling Errors', () => {