refactor: improve logging in src/proxy/processors, remove log-related stubs/spies in tests

cypress/e2e/login.cy.js

@@ -54,6 +54,6 @@ describe('Login page', () => {
 
     cy.get('.MuiSnackbarContent-message')
       .should('be.visible')
-      .and('contain', 'You entered an invalid username or password...');
+      .and('contain', 'You entered an invalid username or password.');
   });
 });

packages/git-proxy-cli/index.ts

@@ -24,7 +24,7 @@ import util from 'util';
 
 import { PushQuery } from '@finos/git-proxy/db';
 import { Action } from '@finos/git-proxy/proxy/actions';
-import { handleAndLogError } from '@finos/git-proxy/utils/errors';
+import { handleErrorAndLog } from '@finos/git-proxy/utils/errors';
 
 const GIT_PROXY_COOKIE_FILE = 'git-proxy-cookie';
 // GitProxy UI HOST and PORT (configurable via environment variable)
@@ -70,7 +70,7 @@ async function login(username: string, password: string) {
       console.error(`Error: Login '${username}': '${error.response.status}'`);
       process.exitCode = 1;
     } else {
-      handleAndLogError(error, `Error: Login '${username}'`);
+      handleErrorAndLog(error, `Error: Login '${username}'`);
       process.exitCode = 2;
     }
   }
@@ -184,7 +184,7 @@ async function getGitPushes(filters: Partial<PushQuery>) {
 
     console.log(util.inspect(records, false, null, false));
   } catch (error: unknown) {
-    handleAndLogError(error, 'Error: List');
+    handleErrorAndLog(error, 'Error: List');
     process.exitCode = 2;
   }
 }
@@ -237,7 +237,7 @@ async function authoriseGitPush(id: string) {
           process.exitCode = 4;
       }
     } else {
-      handleAndLogError(error, `Error: Authorise: '${id}'`);
+      handleErrorAndLog(error, `Error: Authorise: '${id}'`);
       process.exitCode = 2;
     }
   }
@@ -282,7 +282,7 @@ async function rejectGitPush(id: string) {
           process.exitCode = 4;
       }
     } else {
-      handleAndLogError(error, `Error: Reject: '${id}'`);
+      handleErrorAndLog(error, `Error: Reject: '${id}'`);
       process.exitCode = 2;
     }
   }
@@ -327,7 +327,7 @@ async function cancelGitPush(id: string) {
           process.exitCode = 4;
       }
     } else {
-      handleAndLogError(error, `Error: Cancel: '${id}'`);
+      handleErrorAndLog(error, `Error: Cancel: '${id}'`);
       process.exitCode = 2;
     }
   }
@@ -351,7 +351,7 @@ async function logout() {
         },
       );
     } catch (error: unknown) {
-      handleAndLogError(error, 'Warning: Logout');
+      handleErrorAndLog(error, 'Warning: Logout');
     }
   }
 
@@ -375,7 +375,7 @@ async function reloadConfig() {
 
     console.log('Configuration reloaded successfully');
   } catch (error: unknown) {
-    handleAndLogError(error, 'Error: Reload config');
+    handleErrorAndLog(error, 'Error: Reload config');
     process.exitCode = 2;
   }
 }
@@ -432,7 +432,7 @@ async function createUser(
           break;
       }
     } else {
-      handleAndLogError(error, `Error: Create User: '${username}'`);
+      handleErrorAndLog(error, `Error: Create User: '${username}'`);
       process.exitCode = 2;
     }
   }

packages/git-proxy-cli/test/testCli.test.ts

@@ -20,7 +20,7 @@ import { describe, it, beforeAll, afterAll } from 'vitest';
 
 import { setConfigFile } from '../../../src/config/file';
 import { SAMPLE_REPO } from '../../../src/proxy/processors/constants';
-import { handleAndLogError } from '../../../src/utils/errors';
+import { handleErrorAndLog } from '../../../src/utils/errors';
 
 setConfigFile(path.join(process.cwd(), 'test', 'testCli.proxy.config.json'));
 
@@ -600,7 +600,7 @@ describe('test git-proxy-cli', function () {
         try {
           await helper.removeUserFromDb(uniqueUsername);
         } catch (error: unknown) {
-          handleAndLogError(error, 'Error cleaning up user');
+          handleErrorAndLog(error, 'Error cleaning up user');
         }
       }
     });
@@ -630,7 +630,7 @@ describe('test git-proxy-cli', function () {
         try {
           await helper.removeUserFromDb(uniqueUsername);
         } catch (error: unknown) {
-          handleAndLogError(error, 'Error cleaning up user');
+          handleErrorAndLog(error, 'Error cleaning up user');
         }
       }
     });

src/config/ConfigLoader.ts

@@ -24,7 +24,7 @@ import envPaths from 'env-paths';
 import { GitProxyConfig } from './generated/config';
 import { Configuration, ConfigurationSource, FileSource, HttpSource, GitSource } from './types';
 import { loadConfig, validateConfig } from './validators';
-import { handleAndLogError, handleAndThrowError } from '../utils/errors';
+import { handleErrorAndLog, handleErrorAndThrow } from '../utils/errors';
 
 const execFileAsync = promisify(execFile);
 
@@ -98,7 +98,7 @@ export class ConfigLoader extends EventEmitter {
         console.log(`Created cache directory at ${this.cacheDir}`);
         return true;
       } catch (error: unknown) {
-        handleAndLogError(error, 'Failed to create cache directory');
+        handleErrorAndLog(error, 'Failed to create cache directory');
         return false;
       }
     }
@@ -172,7 +172,7 @@ export class ConfigLoader extends EventEmitter {
             console.log(`Loading configuration from ${source.type} source`);
             return await this.loadFromSource(source);
           } catch (error: unknown) {
-            handleAndLogError(error, `Error loading from ${source.type} source`);
+            handleErrorAndLog(error, `Error loading from ${source.type} source`);
             return null;
           }
         }),
@@ -215,7 +215,7 @@ export class ConfigLoader extends EventEmitter {
         console.log('Configuration has not changed, no update needed');
       }
     } catch (error: unknown) {
-      handleAndLogError(error, 'Error reloading configuration');
+      handleErrorAndLog(error, 'Error reloading configuration');
       this.emit('configurationError', error);
     } finally {
       this.isReloading = false;
@@ -315,15 +315,15 @@ export class ConfigLoader extends EventEmitter {
         await execFileAsync('git', ['clone', source.repository, repoDir], execOptions);
         console.log('Repository cloned successfully');
       } catch (error: unknown) {
-        handleAndThrowError(error, 'Failed to clone repository');
+        handleErrorAndThrow(error, 'Failed to clone repository');
       }
     } else {
       console.log(`Pulling latest changes from ${source.repository}`);
       try {
         await execFileAsync('git', ['pull'], { cwd: repoDir });
         console.log('Repository pulled successfully');
       } catch (error: unknown) {
-        handleAndThrowError(error, 'Failed to pull repository');
+        handleErrorAndThrow(error, 'Failed to pull repository');
       }
     }
 
@@ -334,7 +334,7 @@ export class ConfigLoader extends EventEmitter {
         await execFileAsync('git', ['checkout', source.branch], { cwd: repoDir });
         console.log(`Branch ${source.branch} checked out successfully`);
       } catch (error: unknown) {
-        handleAndThrowError(error, `Failed to checkout branch ${source.branch}`);
+        handleErrorAndThrow(error, `Failed to checkout branch ${source.branch}`);
       }
     }
 

src/config/index.ts

@@ -23,7 +23,7 @@ import { Configuration } from './types';
 import { serverConfig } from './env';
 import { getConfigFile } from './file';
 import { validateConfig } from './validators';
-import { handleAndLogError, handleAndThrowError } from '../utils/errors';
+import { handleErrorAndLog, handleErrorAndThrow } from '../utils/errors';
 
 // Cache for current configuration
 let _currentConfig: GitProxyConfig | null = null;
@@ -80,7 +80,7 @@ function loadFullConfiguration(): GitProxyConfig {
       const rawUserConfig = JSON.parse(userConfigContent);
       userSettings = cleanUndefinedValues(rawUserConfig);
     } catch (error: unknown) {
-      handleAndThrowError(error, `Error loading user config from ${userConfigFile}`);
+      handleErrorAndThrow(error, `Error loading user config from ${userConfigFile}`);
     }
   }
 
@@ -338,13 +338,13 @@ const handleConfigUpdate = async (newConfig: Configuration) => {
 
     console.log('Services restarted with new configuration');
   } catch (error: unknown) {
-    handleAndLogError(error, 'Failed to apply new configuration');
+    handleErrorAndLog(error, 'Failed to apply new configuration');
     // Attempt to restart with previous config
     try {
       const proxy = require('../proxy');
       await proxy.start();
     } catch (startError: unknown) {
-      handleAndLogError(startError, 'Failed to restart services');
+      handleErrorAndLog(startError, 'Failed to restart services');
     }
   }
 };
@@ -381,6 +381,6 @@ try {
   initializeConfigLoader();
   console.log('Configuration loaded successfully');
 } catch (error: unknown) {
-  handleAndThrowError(error, 'Failed to load configuration');
+  handleErrorAndThrow(error, 'Failed to load configuration');
   throw error;
 }

src/db/file/pushes.ts

@@ -20,7 +20,7 @@ import { Action } from '../../proxy/actions/Action';
 import { toClass } from '../helper';
 import { PushQuery } from '../types';
 import { CompletedAttestation, Rejection } from '../../proxy/processors/types';
-import { handleAndLogError } from '../../utils/errors';
+import { handleErrorAndLog } from '../../utils/errors';
 
 const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
 
@@ -34,7 +34,7 @@ if (process.env.NODE_ENV === 'test') {
 try {
   db.ensureIndex({ fieldName: 'id', unique: true });
 } catch (error: unknown) {
-  handleAndLogError(
+  handleErrorAndLog(
     error,
     'Failed to build a unique index of push id values. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ',
   );

src/db/file/repo.ts

@@ -19,7 +19,7 @@ import _ from 'lodash';
 
 import { Repo, RepoQuery } from '../types';
 import { toClass } from '../helper';
-import { handleAndLogError } from '../../utils/errors';
+import { handleErrorAndLog } from '../../utils/errors';
 
 const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
 
@@ -33,7 +33,7 @@ if (process.env.NODE_ENV === 'test') {
 try {
   db.ensureIndex({ fieldName: 'url', unique: true });
 } catch (error: unknown) {
-  handleAndLogError(
+  handleErrorAndLog(
     error,
     'Failed to build a unique index of Repository URLs. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ',
   );

src/db/file/users.ts

@@ -18,7 +18,7 @@ import fs from 'fs';
 import Datastore from '@seald-io/nedb';
 
 import { User, UserQuery } from '../types';
-import { handleAndLogError } from '../../utils/errors';
+import { handleErrorAndLog } from '../../utils/errors';
 
 const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
 
@@ -40,15 +40,15 @@ if (process.env.NODE_ENV === 'test') {
 try {
   db.ensureIndex({ fieldName: 'username', unique: true });
 } catch (error: unknown) {
-  handleAndLogError(
+  handleErrorAndLog(
     error,
     'Failed to build a unique index of usernames. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ',
   );
 }
 try {
   db.ensureIndex({ fieldName: 'email', unique: true });
 } catch (error: unknown) {
-  handleAndLogError(
+  handleErrorAndLog(
     error,
     'Failed to build a unique index of user email addresses. Please check your database file for duplicate entries or delete the duplicate through the UI and restart. ',
   );

src/plugin.ts

@@ -19,7 +19,7 @@ import { loadPlugin, resolvePlugin } from 'load-plugin';
 import Module from 'node:module';
 
 import { Action } from './proxy/actions';
-import { handleAndLogError } from './utils/errors';
+import { handleErrorAndLog } from './utils/errors';
 
 /* eslint-disable @typescript-eslint/no-unused-expressions */
 ('use strict');
@@ -122,7 +122,7 @@ class PluginLoader {
         console.log(`Loaded plugin: ${plugin.constructor.name}`);
       });
     } catch (error: unknown) {
-      handleAndLogError(error, 'Error loading plugins');
+      handleErrorAndLog(error, 'Error loading plugins');
     }
   }
 

src/proxy/actions/autoActions.ts

@@ -15,7 +15,7 @@
  */
 
 import { authorise, reject } from '../../db';
-import { handleAndLogError } from '../../utils/errors';
+import { handleErrorAndLog } from '../../utils/errors';
 import { CompletedAttestation, Rejection } from '../processors/types';
 import { Action } from './Action';
 
@@ -35,7 +35,7 @@ const attemptAutoApproval = async (action: Action) => {
 
     return true;
   } catch (error: unknown) {
-    handleAndLogError(error, 'Error during auto-approval');
+    handleErrorAndLog(error, 'Error during auto-approval');
     return false;
   }
 };
@@ -56,7 +56,7 @@ const attemptAutoRejection = async (action: Action) => {
 
     return true;
   } catch (error: unknown) {
-    handleAndLogError(error, 'Error during auto-rejection');
+    handleErrorAndLog(error, 'Error during auto-rejection');
     return false;
   }
 };

src/proxy/chain.ts

@@ -20,7 +20,7 @@ import { PluginLoader } from '../plugin';
 import { Action } from './actions';
 import * as proc from './processors';
 import { attemptAutoApproval, attemptAutoRejection } from './actions/autoActions';
-import { handleAndLogError } from '../utils/errors';
+import { handleErrorAndLog } from '../utils/errors';
 
 const pushActionChain: ((req: Request, action: Action) => Promise<Action>)[] = [
   proc.push.parsePush,
@@ -69,7 +69,7 @@ export const executeChain = async (req: Request, _res: Response): Promise<Action
       }
     }
   } catch (error: unknown) {
-    const msg = handleAndLogError(error, 'An unexpected error occurred when executing the chain');
+    const msg = handleErrorAndLog(error, 'An unexpected error occurred when executing the chain');
     action.error = true;
     action.errorMessage = msg;
   } finally {

src/proxy/processors/push-action/checkAuthorEmails.ts

@@ -57,8 +57,6 @@ const exec = async (_req: Request, action: Action): Promise<Action> => {
   const illegalEmails = uniqueAuthorEmails.filter((email) => !isEmailAllowed(email));
 
   if (illegalEmails.length > 0) {
-    console.log(`The following commit author e-mails are illegal: ${illegalEmails}`);
-
     step.error = true;
     step.log(`The following commit author e-mails are illegal: ${illegalEmails}`);
     step.setError(
@@ -69,7 +67,7 @@ const exec = async (_req: Request, action: Action): Promise<Action> => {
     return action;
   }
 
-  console.log(`The following commit author e-mails are legal: ${uniqueAuthorEmails}`);
+  step.log(`The following commit author e-mails are legal: ${uniqueAuthorEmails}`);
   action.addStep(step);
   return action;
 };