Skip to content

Commit 5eafbfa

Browse files
oss6oss6
committed
refactor: logging and error handling (closes #41)
1 parent 9c71e47 commit 5eafbfa

19 files changed

+267
-85
lines changed

‎src/cli/config.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { AppError } from '../errors';
1+
import { AppError, errorLog } from '../errors';
22
import ConfigurationService from '../services/configuration';
33
import LoggerService from '../services/logger';
44
import { AppConfig } from '../types';
@@ -28,7 +28,7 @@ export function handler(argv: Arguments): void {
2828
const loggerService = LoggerService.getInstance();
2929
const configurationService = ConfigurationService.getInstance();
3030

31-
const logger = loggerService.registerLogger(argv.verbose ? 'debug' : 'info', argv['log-file']);
31+
loggerService.registerLogger(argv.verbose ? 'debug' : 'info', argv['log-file']);
3232

3333
try {
3434
if (argv.reset) {
@@ -53,13 +53,13 @@ export function handler(argv: Arguments): void {
5353
const value = argv.value;
5454

5555
if (!argv.value) {
56-
logger.info(configurationService.config[key]);
56+
console.log(configurationService.config[key]);
5757
} else {
5858
// TODO: deserialise with validation
5959
configurationService.update(key, value);
60-
logger.info('Configuration updated successfully.');
60+
loggerService.log('info', 'Configuration updated successfully.', handler);
6161
}
6262
} catch (error) {
63-
logger.error(error.message);
63+
loggerService.log('error', errorLog(error), handler);
6464
}
6565
}

‎src/cli/run.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,10 @@ export async function handler(argv: Arguments): Promise<void> {
150150
process.on('SIGINT', async () => {
151151
console.log(`Deleting temporary web server files directory: ${temporaryWebServerFilesDirectory}`);
152152
await rimrafP(temporaryWebServerFilesDirectory);
153-
process.exit();
153+
process.exitCode = 0;
154154
});
155155
}
156156
} catch (error) {
157-
process.exit(1);
157+
process.exitCode = 1;
158158
}
159159
}

‎src/errors.ts

+15-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1+
import { ErrorLog } from './types';
2+
13
export class AppError extends Error {
2-
constructor(message: string) {
4+
details: string[] = [];
5+
6+
constructor(message: string, details: string[] = []) {
37
super(message);
48
this.name = 'AppError';
9+
this.details = details;
510
}
611
}
712

@@ -19,13 +24,6 @@ export class NoVfDependenciesFoundError extends AppError {
1924
}
2025
}
2126

22-
export class GitHubAuthenticationError extends AppError {
23-
constructor(message = '') {
24-
super(message || 'An error has occurred while authenticating to GitHub.');
25-
this.name = 'GitHubAuthenticationError';
26-
}
27-
}
28-
2927
export class MissingConfigurationError extends AppError {
3028
constructor(keys: string[]) {
3129
super(
@@ -34,3 +32,12 @@ export class MissingConfigurationError extends AppError {
3432
this.name = 'MissingConfigurationError';
3533
}
3634
}
35+
36+
export function errorLog(error: Error): ErrorLog {
37+
return {
38+
type: error.name,
39+
message: error.message,
40+
level: 'error',
41+
details: error instanceof AppError ? error.details : [],
42+
};
43+
}

‎src/index.ts

+19-17
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import OptionsService from './services/options';
55
import LoggerService from './services/logger';
66
import ApiService from './services/api';
77
import * as pipeline from './pipeline';
8-
import { AppError } from './errors';
9-
import { Logger } from 'winston';
8+
import { AppError, errorLog } from './errors';
109
import { getCacheFileName } from './helpers/misc';
1110
import DependentsService from './services/dependents';
1211

@@ -23,7 +22,6 @@ export class ServiceDiscovery {
2322
private configurationService: ConfigurationService;
2423
private apiService: ApiService;
2524
private hasBeenSetUp = false;
26-
private logger: Logger;
2725
private optionalSteps = ['getConfig', 'getChangelog', 'getDependents'];
2826

2927
// eslint-disable-next-line @typescript-eslint/no-empty-function
@@ -46,34 +44,36 @@ export class ServiceDiscovery {
4644
* @param options Options to define the service discovery behaviour.
4745
*/
4846
async setup(options: Options): Promise<void> {
49-
this.logger = this.loggerService.registerLogger(
50-
options.verbose ? 'debug' : 'info',
51-
options.logFile,
52-
!options.loggingEnabled,
53-
);
47+
this.loggerService.registerLogger(options.verbose ? 'debug' : 'info', options.logFile, !options.loggingEnabled);
5448

5549
this.optionsService = OptionsService.getInstance();
5650
this.configurationService = ConfigurationService.getInstance();
5751
this.apiService = ApiService.getInstance();
5852
this.dependentsService = DependentsService.getInstance();
5953

6054
try {
55+
this.loggerService.log('info', 'Running setup', this.setup);
56+
6157
this.optionsService.setOptions(options);
6258
await this.configurationService.setup();
6359

6460
if (options.forceRun || this.configurationService.shouldInvalidate()) {
61+
this.loggerService.log('info', 'Invalidating cache', this.setup);
62+
6563
await this.configurationService.deleteCachedComponents();
6664
this.configurationService.update('lastInvalidation', new Date());
6765
}
6866

6967
if (!this.configurationService.config.vfCoreVersion || options.forceRun) {
68+
this.loggerService.log('info', 'Retrieving vf-core version', this.setup);
69+
7070
const vfCoreVersion = await this.apiService.getVfCoreLatestReleaseVersion();
7171
this.configurationService.update('vfCoreVersion', vfCoreVersion);
7272
}
7373

7474
this.hasBeenSetUp = true;
7575
} catch (error) {
76-
this.logger.error(error.message);
76+
this.loggerService.log('error', errorLog(error), this.setup);
7777
throw error;
7878
}
7979
}
@@ -83,20 +83,21 @@ export class ServiceDiscovery {
8383
* @param reportProgress Whether to report progress in the cli.
8484
*/
8585
async run(reportProgress = false): Promise<PipelineItem[]> {
86-
if (!this.hasBeenSetUp) {
87-
throw new AppError('The ServiceDiscovery instance has not been set up.');
88-
}
89-
9086
try {
91-
this.logger.debug('Running service discovery');
87+
this.loggerService.log('info', 'Running service discovery', this.run);
88+
89+
if (!this.hasBeenSetUp) {
90+
throw new AppError('The ServiceDiscovery instance has not been set up.');
91+
}
9292

9393
this.hasBeenSetUp = false;
9494

95-
const rootDirectory = process.cwd();
9695
const options = this.optionsService.getOptions();
97-
const disabledSteps: string[] = options.disabled.filter((value) => this.optionalSteps.includes(value));
9896
const cache = await this.configurationService.getCache();
97+
9998
const projectType = (<any>ProjectType)[options.dependentsProjectType];
99+
const rootDirectory = process.cwd();
100+
100101
const potentialDependents = await this.dependentsService.getPotentialDependents(
101102
rootDirectory,
102103
projectType,
@@ -112,6 +113,7 @@ export class ServiceDiscovery {
112113

113114
const components = await pipeline.getComponents(context);
114115

116+
const disabledSteps: string[] = options.disabled.filter((value) => this.optionalSteps.includes(value));
115117
const pipelineItems = await pipeline.Pipeline.getInstance()
116118
.addStep({ fn: pipeline.getExactVersion, enabled: true })
117119
.addStep({ fn: pipeline.getPackageJson, enabled: true })
@@ -125,7 +127,7 @@ export class ServiceDiscovery {
125127
return pipelineItems;
126128
} catch (error) {
127129
this.hasBeenSetUp = false;
128-
this.logger.error(error.message);
130+
this.loggerService.log('error', errorLog(error), this.run);
129131

130132
if (error.context) {
131133
return error.context;

‎src/pipeline/index.ts

+16
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import Listr from 'listr';
2+
import LoggerService from '../services/logger';
23
import OptionsService from '../services/options';
34
import { PDiscoveryItem, PipelineContext, PipelineItem, PipelineStep } from '../types';
45
import getComponents from './steps/00-get-components';
@@ -17,6 +18,7 @@ export class Pipeline {
1718
static instance: Pipeline;
1819
private steps: PipelineStep[];
1920
private optionsService = OptionsService.getInstance();
21+
private loggerService = LoggerService.getInstance();
2022

2123
private constructor() {
2224
this.steps = [];
@@ -39,6 +41,11 @@ export class Pipeline {
3941
* @param step The pipeline step to add.
4042
*/
4143
addStep(step: PipelineStep): Pipeline {
44+
this.loggerService.log(
45+
'debug',
46+
`Adding step ${step.fn.name} (${step.enabled ? 'enabled' : 'disabled'})`,
47+
this.addStep,
48+
);
4249
this.steps.push(step);
4350
return this;
4451
}
@@ -50,6 +57,15 @@ export class Pipeline {
5057
* @param reportProgress Whether to report progress in the cli.
5158
*/
5259
async run(source: string[], context: PipelineContext, reportProgress = false): Promise<PipelineItem[]> {
60+
this.loggerService.log(
61+
'debug',
62+
{
63+
message: 'Running pipeline',
64+
details: { source },
65+
},
66+
this.run,
67+
);
68+
5369
const options = this.optionsService.getOptions();
5470
const discoveryItems: PDiscoveryItem[] = source.map((sourceItem) => ({
5571
name: sourceItem,

‎src/pipeline/steps/00-get-components.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ import { PackageJson, PipelineContext } from '../../types';
1010
*/
1111
export default async function getComponents(context: PipelineContext): Promise<string[]> {
1212
const loggerService = LoggerService.getInstance();
13-
const logger = loggerService.getLogger();
1413

15-
logger.debug('Retrieving components from package.json');
14+
loggerService.log('debug', 'Retrieving components from package.json', getComponents);
1615

1716
const packageJsonFile = path.join(context.rootDirectory, 'package.json');
1817

‎src/pipeline/steps/01-get-exact-version.ts

+20-4
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export async function parseLockFile(rootDirectory: string): Promise<LockObject>
1515
const fullLockObject = npmLockFile.dependencies as LockObject;
1616

1717
return Object.entries(fullLockObject)
18-
.filter(([pkg, _]) => pkg.includes('visual-framework'))
18+
.filter(([pkg]) => pkg.includes('visual-framework'))
1919
.reduce(
2020
(obj, [pkg, lockItem]) => ({
2121
...obj,
@@ -35,7 +35,7 @@ export async function parseLockFile(rootDirectory: string): Promise<LockObject>
3535
const yarnLockFile: LockObject = parseYarnLockFile(await fs.promises.readFile(yarnLockFileName, 'utf-8')).object;
3636

3737
return Object.entries(yarnLockFile)
38-
.filter(([pkg, _]) => pkg.includes('visual-framework'))
38+
.filter(([pkg]) => pkg.includes('visual-framework'))
3939
.reduce(
4040
(obj, [pkg, lockItem]) => ({
4141
...obj,
@@ -54,11 +54,17 @@ export async function parseLockFile(rootDirectory: string): Promise<LockObject>
5454

5555
export async function extractVersion(discoveryItem: DiscoveryItem, context: PipelineContext): Promise<string> {
5656
const loggerService = LoggerService.getInstance();
57-
const logger = loggerService.getLogger();
5857
const optionsService = OptionsService.getInstance();
5958
const options = optionsService.getOptions();
6059
const parse = async (): Promise<string> => {
61-
logger.debug(`${discoveryItem.nameWithoutPrefix} - retrieving exact version from remote`);
60+
loggerService.log(
61+
'debug',
62+
{
63+
message: 'Retrieving exact version from remote',
64+
details: { component: discoveryItem.nameWithoutPrefix },
65+
},
66+
extractVersion,
67+
);
6268

6369
const lockObject = await parseLockFile(context.rootDirectory);
6470
context.cache.lockObjects[context.rootDirectory] = lockObject;
@@ -92,6 +98,16 @@ export default async function getExactVersion(
9298
{ discoveryItem, profilingInformation }: PipelineItem,
9399
context: PipelineContext,
94100
): Promise<PipelineItem> {
101+
const loggerService = LoggerService.getInstance();
102+
loggerService.log(
103+
'debug',
104+
{
105+
message: 'Retrieving exact version',
106+
details: { component: discoveryItem.nameWithoutPrefix },
107+
},
108+
getExactVersion,
109+
);
110+
95111
if (!discoveryItem.name || !discoveryItem.nameWithoutPrefix) {
96112
throw new AppError('Package name not defined, hence could not get exact version.');
97113
}

‎src/pipeline/steps/02-get-package-json.ts

+11-4
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,25 @@ export default async function getPackageJson(
1313
{ discoveryItem, profilingInformation }: PipelineItem,
1414
context: PipelineContext,
1515
): Promise<PipelineItem> {
16+
const loggerService = LoggerService.getInstance();
17+
18+
loggerService.log(
19+
'debug',
20+
{
21+
message: 'Retrieving latest package information',
22+
details: { component: discoveryItem.nameWithoutPrefix },
23+
},
24+
getPackageJson,
25+
);
26+
1627
if (!discoveryItem.nameWithoutPrefix) {
1728
throw new AppError('Package name not defined, hence could not get package.json.');
1829
}
1930

20-
const loggerService = LoggerService.getInstance();
21-
const logger = loggerService.getLogger();
2231
const apiService = ApiService.getInstance();
2332
const optionsService = OptionsService.getInstance();
2433
const { profile } = optionsService.getOptions();
2534

26-
logger.debug(`${discoveryItem.nameWithoutPrefix} - retrieving latest package information`);
27-
2835
const { result, took } = await runAndMeasure(
2936
async () => apiService.getComponentPackageJson(discoveryItem.nameWithoutPrefix as string, context),
3037
profile,

‎src/pipeline/steps/03-get-config.ts

+11-4
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,25 @@ export default async function getConfig(
1313
{ discoveryItem, profilingInformation }: PipelineItem,
1414
context: PipelineContext,
1515
): Promise<PipelineItem> {
16+
const loggerService = LoggerService.getInstance();
17+
18+
loggerService.log(
19+
'debug',
20+
{
21+
message: 'Retrieving package configuration',
22+
details: { component: discoveryItem.nameWithoutPrefix },
23+
},
24+
getConfig,
25+
);
26+
1627
if (!discoveryItem.nameWithoutPrefix) {
1728
throw new AppError('Package name not defined, hence could not get component config.');
1829
}
1930

20-
const loggerService = LoggerService.getInstance();
21-
const logger = loggerService.getLogger();
2231
const apiService = ApiService.getInstance();
2332
const optionsService = OptionsService.getInstance();
2433
const { profile } = optionsService.getOptions();
2534

26-
logger.debug(`${discoveryItem.nameWithoutPrefix} - retrieving package configuration`);
27-
2835
const { result: yamlConfig, took: yamlConfigResponseTime } = await runAndMeasure(
2936
async () => apiService.getYamlComponentConfig(discoveryItem.nameWithoutPrefix as string, context),
3037
profile,

‎src/pipeline/steps/04-get-changelog.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,16 @@ export default async function getChangelog(
1515
context: PipelineContext,
1616
): Promise<PipelineItem> {
1717
const loggerService = LoggerService.getInstance();
18-
const logger = loggerService.getLogger();
18+
19+
loggerService.log(
20+
'debug',
21+
{
22+
message: 'Retrieving changelog if applicable',
23+
details: { component: discoveryItem.nameWithoutPrefix },
24+
},
25+
getChangelog,
26+
);
27+
1928
const apiService = ApiService.getInstance();
2029
const optionsService = OptionsService.getInstance();
2130
const { profile } = optionsService.getOptions();
@@ -24,8 +33,6 @@ export default async function getChangelog(
2433
throw new AppError('Information not complete to get changelog.');
2534
}
2635

27-
logger.debug(`${discoveryItem.nameWithoutPrefix} - retrieving changelog if applicable`);
28-
2936
if (discoveryItem.version === discoveryItem.packageJson.version) {
3037
return {
3138
discoveryItem,

0 commit comments

Comments
 (0)