Skip to content

Commit 864559e

Browse files
jescaladajescalada
authored
Merge pull request #1448 from jescalada/1281-improve-processors-logging
refactor: improve logging in `src/proxy/processors`, remove log-related stubs/spies in tests
2 parents d65c9b8 + c6c703e commit 864559e

34 files changed

+161
-196
lines changed

‎cypress/e2e/login.cy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,6 @@ describe('Login page', () => {
5454

5555
cy.get('.MuiSnackbarContent-message')
5656
.should('be.visible')
57-
.and('contain', 'You entered an invalid username or password...');
57+
.and('contain', 'You entered an invalid username or password.');
5858
});
5959
});

‎packages/git-proxy-cli/index.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import util from 'util';
2424

2525
import { PushQuery } from '@finos/git-proxy/db';
2626
import { Action } from '@finos/git-proxy/proxy/actions';
27-
import { handleAndLogError } from '@finos/git-proxy/utils/errors';
27+
import { handleErrorAndLog } from '@finos/git-proxy/utils/errors';
2828

2929
const GIT_PROXY_COOKIE_FILE = 'git-proxy-cookie';
3030
// GitProxy UI HOST and PORT (configurable via environment variable)
@@ -70,7 +70,7 @@ async function login(username: string, password: string) {
7070
console.error(`Error: Login '${username}': '${error.response.status}'`);
7171
process.exitCode = 1;
7272
} else {
73-
handleAndLogError(error, `Error: Login '${username}'`);
73+
handleErrorAndLog(error, `Error: Login '${username}'`);
7474
process.exitCode = 2;
7575
}
7676
}
@@ -184,7 +184,7 @@ async function getGitPushes(filters: Partial<PushQuery>) {
184184

185185
console.log(util.inspect(records, false, null, false));
186186
} catch (error: unknown) {
187-
handleAndLogError(error, 'Error: List');
187+
handleErrorAndLog(error, 'Error: List');
188188
process.exitCode = 2;
189189
}
190190
}
@@ -237,7 +237,7 @@ async function authoriseGitPush(id: string) {
237237
process.exitCode = 4;
238238
}
239239
} else {
240-
handleAndLogError(error, `Error: Authorise: '${id}'`);
240+
handleErrorAndLog(error, `Error: Authorise: '${id}'`);
241241
process.exitCode = 2;
242242
}
243243
}
@@ -282,7 +282,7 @@ async function rejectGitPush(id: string) {
282282
process.exitCode = 4;
283283
}
284284
} else {
285-
handleAndLogError(error, `Error: Reject: '${id}'`);
285+
handleErrorAndLog(error, `Error: Reject: '${id}'`);
286286
process.exitCode = 2;
287287
}
288288
}
@@ -327,7 +327,7 @@ async function cancelGitPush(id: string) {
327327
process.exitCode = 4;
328328
}
329329
} else {
330-
handleAndLogError(error, `Error: Cancel: '${id}'`);
330+
handleErrorAndLog(error, `Error: Cancel: '${id}'`);
331331
process.exitCode = 2;
332332
}
333333
}
@@ -351,7 +351,7 @@ async function logout() {
351351
},
352352
);
353353
} catch (error: unknown) {
354-
handleAndLogError(error, 'Warning: Logout');
354+
handleErrorAndLog(error, 'Warning: Logout');
355355
}
356356
}
357357

@@ -375,7 +375,7 @@ async function reloadConfig() {
375375

376376
console.log('Configuration reloaded successfully');
377377
} catch (error: unknown) {
378-
handleAndLogError(error, 'Error: Reload config');
378+
handleErrorAndLog(error, 'Error: Reload config');
379379
process.exitCode = 2;
380380
}
381381
}
@@ -432,7 +432,7 @@ async function createUser(
432432
break;
433433
}
434434
} else {
435-
handleAndLogError(error, `Error: Create User: '${username}'`);
435+
handleErrorAndLog(error, `Error: Create User: '${username}'`);
436436
process.exitCode = 2;
437437
}
438438
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { describe, it, beforeAll, afterAll } from 'vitest';
2020

2121
import { setConfigFile } from '../../../src/config/file';
2222
import { SAMPLE_REPO } from '../../../src/proxy/processors/constants';
23-
import { handleAndLogError } from '../../../src/utils/errors';
23+
import { handleErrorAndLog } from '../../../src/utils/errors';
2424

2525
setConfigFile(path.join(process.cwd(), 'test', 'testCli.proxy.config.json'));
2626

@@ -600,7 +600,7 @@ describe('test git-proxy-cli', function () {
600600
try {
601601
await helper.removeUserFromDb(uniqueUsername);
602602
} catch (error: unknown) {
603-
handleAndLogError(error, 'Error cleaning up user');
603+
handleErrorAndLog(error, 'Error cleaning up user');
604604
}
605605
}
606606
});
@@ -630,7 +630,7 @@ describe('test git-proxy-cli', function () {
630630
try {
631631
await helper.removeUserFromDb(uniqueUsername);
632632
} catch (error: unknown) {
633-
handleAndLogError(error, 'Error cleaning up user');
633+
handleErrorAndLog(error, 'Error cleaning up user');
634634
}
635635
}
636636
});

‎src/config/ConfigLoader.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import envPaths from 'env-paths';
2424
import { GitProxyConfig } from './generated/config';
2525
import { Configuration, ConfigurationSource, FileSource, HttpSource, GitSource } from './types';
2626
import { loadConfig, validateConfig } from './validators';
27-
import { handleAndLogError, handleAndThrowError } from '../utils/errors';
27+
import { handleErrorAndLog, handleErrorAndThrow } from '../utils/errors';
2828

2929
const execFileAsync = promisify(execFile);
3030

@@ -98,7 +98,7 @@ export class ConfigLoader extends EventEmitter {
9898
console.log(`Created cache directory at ${this.cacheDir}`);
9999
return true;
100100
} catch (error: unknown) {
101-
handleAndLogError(error, 'Failed to create cache directory');
101+
handleErrorAndLog(error, 'Failed to create cache directory');
102102
return false;
103103
}
104104
}
@@ -172,7 +172,7 @@ export class ConfigLoader extends EventEmitter {
172172
console.log(`Loading configuration from ${source.type} source`);
173173
return await this.loadFromSource(source);
174174
} catch (error: unknown) {
175-
handleAndLogError(error, `Error loading from ${source.type} source`);
175+
handleErrorAndLog(error, `Error loading from ${source.type} source`);
176176
return null;
177177
}
178178
}),
@@ -215,7 +215,7 @@ export class ConfigLoader extends EventEmitter {
215215
console.log('Configuration has not changed, no update needed');
216216
}
217217
} catch (error: unknown) {
218-
handleAndLogError(error, 'Error reloading configuration');
218+
handleErrorAndLog(error, 'Error reloading configuration');
219219
this.emit('configurationError', error);
220220
} finally {
221221
this.isReloading = false;
@@ -315,15 +315,15 @@ export class ConfigLoader extends EventEmitter {
315315
await execFileAsync('git', ['clone', source.repository, repoDir], execOptions);
316316
console.log('Repository cloned successfully');
317317
} catch (error: unknown) {
318-
handleAndThrowError(error, 'Failed to clone repository');
318+
handleErrorAndThrow(error, 'Failed to clone repository');
319319
}
320320
} else {
321321
console.log(`Pulling latest changes from ${source.repository}`);
322322
try {
323323
await execFileAsync('git', ['pull'], { cwd: repoDir });
324324
console.log('Repository pulled successfully');
325325
} catch (error: unknown) {
326-
handleAndThrowError(error, 'Failed to pull repository');
326+
handleErrorAndThrow(error, 'Failed to pull repository');
327327
}
328328
}
329329

@@ -334,7 +334,7 @@ export class ConfigLoader extends EventEmitter {
334334
await execFileAsync('git', ['checkout', source.branch], { cwd: repoDir });
335335
console.log(`Branch ${source.branch} checked out successfully`);
336336
} catch (error: unknown) {
337-
handleAndThrowError(error, `Failed to checkout branch ${source.branch}`);
337+
handleErrorAndThrow(error, `Failed to checkout branch ${source.branch}`);
338338
}
339339
}
340340

‎src/config/index.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { Configuration } from './types';
2323
import { serverConfig } from './env';
2424
import { getConfigFile } from './file';
2525
import { validateConfig } from './validators';
26-
import { handleAndLogError, handleAndThrowError } from '../utils/errors';
26+
import { handleErrorAndLog, handleErrorAndThrow } from '../utils/errors';
2727

2828
// Cache for current configuration
2929
let _currentConfig: GitProxyConfig | null = null;
@@ -80,7 +80,7 @@ function loadFullConfiguration(): GitProxyConfig {
8080
const rawUserConfig = JSON.parse(userConfigContent);
8181
userSettings = cleanUndefinedValues(rawUserConfig);
8282
} catch (error: unknown) {
83-
handleAndThrowError(error, `Error loading user config from ${userConfigFile}`);
83+
handleErrorAndThrow(error, `Error loading user config from ${userConfigFile}`);
8484
}
8585
}
8686

@@ -338,13 +338,13 @@ const handleConfigUpdate = async (newConfig: Configuration) => {
338338

339339
console.log('Services restarted with new configuration');
340340
} catch (error: unknown) {
341-
handleAndLogError(error, 'Failed to apply new configuration');
341+
handleErrorAndLog(error, 'Failed to apply new configuration');
342342
// Attempt to restart with previous config
343343
try {
344344
const proxy = require('../proxy');
345345
await proxy.start();
346346
} catch (startError: unknown) {
347-
handleAndLogError(startError, 'Failed to restart services');
347+
handleErrorAndLog(startError, 'Failed to restart services');
348348
}
349349
}
350350
};
@@ -381,6 +381,6 @@ try {
381381
initializeConfigLoader();
382382
console.log('Configuration loaded successfully');
383383
} catch (error: unknown) {
384-
handleAndThrowError(error, 'Failed to load configuration');
384+
handleErrorAndThrow(error, 'Failed to load configuration');
385385
throw error;
386386
}

‎src/db/file/pushes.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { Action } from '../../proxy/actions/Action';
2020
import { toClass } from '../helper';
2121
import { PushQuery } from '../types';
2222
import { CompletedAttestation, Rejection } from '../../proxy/processors/types';
23-
import { handleAndLogError } from '../../utils/errors';
23+
import { handleErrorAndLog } from '../../utils/errors';
2424

2525
const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
2626

@@ -34,7 +34,7 @@ if (process.env.NODE_ENV === 'test') {
3434
try {
3535
db.ensureIndex({ fieldName: 'id', unique: true });
3636
} catch (error: unknown) {
37-
handleAndLogError(
37+
handleErrorAndLog(
3838
error,
3939
'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. ',
4040
);

‎src/db/file/repo.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import _ from 'lodash';
1919

2020
import { Repo, RepoQuery } from '../types';
2121
import { toClass } from '../helper';
22-
import { handleAndLogError } from '../../utils/errors';
22+
import { handleErrorAndLog } from '../../utils/errors';
2323

2424
const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
2525

@@ -33,7 +33,7 @@ if (process.env.NODE_ENV === 'test') {
3333
try {
3434
db.ensureIndex({ fieldName: 'url', unique: true });
3535
} catch (error: unknown) {
36-
handleAndLogError(
36+
handleErrorAndLog(
3737
error,
3838
'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. ',
3939
);

‎src/db/file/users.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import fs from 'fs';
1818
import Datastore from '@seald-io/nedb';
1919

2020
import { User, UserQuery } from '../types';
21-
import { handleAndLogError } from '../../utils/errors';
21+
import { handleErrorAndLog } from '../../utils/errors';
2222

2323
const COMPACTION_INTERVAL = 1000 * 60 * 60 * 24; // once per day
2424

@@ -40,15 +40,15 @@ if (process.env.NODE_ENV === 'test') {
4040
try {
4141
db.ensureIndex({ fieldName: 'username', unique: true });
4242
} catch (error: unknown) {
43-
handleAndLogError(
43+
handleErrorAndLog(
4444
error,
4545
'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. ',
4646
);
4747
}
4848
try {
4949
db.ensureIndex({ fieldName: 'email', unique: true });
5050
} catch (error: unknown) {
51-
handleAndLogError(
51+
handleErrorAndLog(
5252
error,
5353
'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. ',
5454
);

‎src/plugin.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { loadPlugin, resolvePlugin } from 'load-plugin';
1919
import Module from 'node:module';
2020

2121
import { Action } from './proxy/actions';
22-
import { handleAndLogError } from './utils/errors';
22+
import { handleErrorAndLog } from './utils/errors';
2323

2424
/* eslint-disable @typescript-eslint/no-unused-expressions */
2525
('use strict');
@@ -122,7 +122,7 @@ class PluginLoader {
122122
console.log(`Loaded plugin: ${plugin.constructor.name}`);
123123
});
124124
} catch (error: unknown) {
125-
handleAndLogError(error, 'Error loading plugins');
125+
handleErrorAndLog(error, 'Error loading plugins');
126126
}
127127
}
128128

‎src/proxy/actions/autoActions.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616

1717
import { authorise, reject } from '../../db';
18-
import { handleAndLogError } from '../../utils/errors';
18+
import { handleErrorAndLog } from '../../utils/errors';
1919
import { CompletedAttestation, Rejection } from '../processors/types';
2020
import { Action } from './Action';
2121

@@ -35,7 +35,7 @@ const attemptAutoApproval = async (action: Action) => {
3535

3636
return true;
3737
} catch (error: unknown) {
38-
handleAndLogError(error, 'Error during auto-approval');
38+
handleErrorAndLog(error, 'Error during auto-approval');
3939
return false;
4040
}
4141
};
@@ -56,7 +56,7 @@ const attemptAutoRejection = async (action: Action) => {
5656

5757
return true;
5858
} catch (error: unknown) {
59-
handleAndLogError(error, 'Error during auto-rejection');
59+
handleErrorAndLog(error, 'Error during auto-rejection');
6060
return false;
6161
}
6262
};

0 commit comments

Comments
 (0)