Skip to content

Commit aa6d00b

Browse files
tylerymarkowitztylerymarkowitz
authored
improved duplicate code in error handling (#124)
Co-authored-by: Tyler Markowitz <tmarkowi@amazon.com>
1 parent 3b928e4 commit aa6d00b

File tree

3 files changed

+39
-32
lines changed

3 files changed

+39
-32
lines changed

‎src/services/CodeActionService.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ export class CodeActionService {
3636
private static readonly REMOVE_ERROR_TITLE = 'Remove validation error';
3737
private readonly log = LoggerFactory.getLogger(CodeActionService);
3838

39+
private logError(operation: string, error: unknown): void {
40+
this.log.error(`Error ${operation}: ${extractErrorMessage(error)}`);
41+
}
42+
3943
constructor(
4044
private readonly syntaxTreeManager: SyntaxTreeManager,
4145
private readonly documentManager: DocumentManager,
@@ -96,7 +100,7 @@ export class CodeActionService {
96100
fixes.push(...this.generateCfnValidationFixes(diagnostic, uri));
97101
}
98102
} catch (error) {
99-
this.log.error(`Error generating fixes for diagnostic: ${extractErrorMessage(error)}`);
103+
this.logError('generating fixes for diagnostic', error);
100104
}
101105

102106
return fixes;
@@ -222,7 +226,7 @@ export class CodeActionService {
222226
}
223227
}
224228
} catch (error) {
225-
this.log.warn(`Could not determine key-pair range from syntax tree: ${extractErrorMessage(error)}`);
229+
this.logError('determining key-pair range from syntax tree', error);
226230
}
227231

228232
// Fallback to the diagnostic range as provided by cfn-lint
@@ -268,7 +272,7 @@ export class CodeActionService {
268272
end: pointToPosition(node.endPosition),
269273
};
270274
} catch (error) {
271-
this.log.warn(`Error finding key-pair boundaries in syntax tree: ${extractErrorMessage(error)}`);
275+
this.logError('finding key-pair boundaries in syntax tree', error);
272276
return undefined;
273277
}
274278
}
@@ -303,7 +307,7 @@ export class CodeActionService {
303307
}
304308
// If we can't find a proper insertion point using syntax tree, don't generate a fix
305309
} catch (error) {
306-
this.log.warn(`Error generating add required property fix: ${extractErrorMessage(error)}`);
310+
this.logError('generating add required property fix', error);
307311
// If we can't generate a proper fix using syntax tree, don't generate a fix
308312
}
309313

@@ -336,7 +340,7 @@ export class CodeActionService {
336340

337341
return codeAction;
338342
} catch (error) {
339-
this.log.error(`Error creating code action: ${extractErrorMessage(error)}`);
343+
this.logError('creating code action', error);
340344
return undefined;
341345
}
342346
}
@@ -391,7 +395,7 @@ export class CodeActionService {
391395

392396
return undefined;
393397
} catch (error) {
394-
this.log.warn(`Error finding first child insertion point: ${extractErrorMessage(error)}`);
398+
this.logError('finding first child insertion point', error);
395399
return undefined;
396400
}
397401
}
@@ -416,7 +420,7 @@ export class CodeActionService {
416420

417421
return undefined;
418422
} catch (error) {
419-
this.log.warn(`Error finding first child position using syntax tree: ${extractErrorMessage(error)}`);
423+
this.logError('finding first child position using syntax tree', error);
420424
return undefined;
421425
}
422426
}
@@ -547,7 +551,7 @@ export class CodeActionService {
547551
}
548552
}
549553
} catch (error) {
550-
this.log.error(`Error generating refactor actions: ${extractErrorMessage(error)}`);
554+
this.logError('generating refactor actions', error);
551555
}
552556

553557
return refactorActions;
@@ -592,7 +596,7 @@ export class CodeActionService {
592596
},
593597
};
594598
} catch (error) {
595-
this.log.error(`Error generating extract to parameter action: ${extractErrorMessage(error)}`);
599+
this.logError('generating extract to parameter action', error);
596600
return undefined;
597601
}
598602
}
@@ -638,9 +642,7 @@ export class CodeActionService {
638642
},
639643
};
640644
} catch (error) {
641-
this.log.error(
642-
`Error generating extract all occurrences to parameter action: ${extractErrorMessage(error)}`,
643-
);
645+
this.logError('generating extract all occurrences to parameter action', error);
644646
return undefined;
645647
}
646648
}

‎src/services/cfnLint/CfnLintService.ts‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
5454

5555
@Telemetry() private readonly telemetry!: ScopedTelemetry;
5656

57+
private logError(operation: string, error: unknown): void {
58+
this.log.error(`Error ${operation}: ${extractErrorMessage(error)}`);
59+
}
60+
5761
// Request queue for handling requests during initialization
5862
private readonly requestQueue = new Map<
5963
string,
@@ -147,7 +151,7 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
147151
await this.workerManager.mountFolder(fsDir, mountDir);
148152
this.telemetry.count('mount.success', 1, { unit: '1' });
149153
} catch (error) {
150-
this.log.error(`Error mounting folder: ${extractErrorMessage(error)}`);
154+
this.logError('mounting folder', error);
151155
this.telemetry.count('mount.fault', 1, { unit: '1' });
152156
throw error; // Re-throw to notify caller
153157
}
@@ -205,7 +209,7 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
205209
this.diagnosticCoordinator
206210
.publishDiagnostics(CfnLintService.CFN_LINT_SOURCE, uri, diagnostics)
207211
.catch((reason) => {
208-
this.log.error(`Error publishing diagnostics: ${extractErrorMessage(reason)}`);
212+
this.logError('publishing diagnostics', reason);
209213
});
210214
}
211215

@@ -255,13 +259,13 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
255259
await this.diagnosticCoordinator
256260
.publishDiagnostics(CfnLintService.CFN_LINT_SOURCE, payload.uri, payload.diagnostics)
257261
.catch((reason) => {
258-
this.log.error(`Error publishing diagnostics: ${extractErrorMessage(reason)}`);
262+
this.logError('publishing diagnostics', reason);
259263
});
260264
}
261265
}
262266
} catch (error) {
263267
const errorMessage = extractErrorMessage(error);
264-
this.log.error(`Error linting ${fileType} by string: ${errorMessage}`);
268+
this.logError(`linting ${fileType} by string`, error);
265269
this.publishErrorDiagnostics(uri, errorMessage);
266270
}
267271
}
@@ -279,7 +283,7 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
279283
? deploymentFile['template-file-path']
280284
: undefined;
281285
} catch (error) {
282-
this.log.error(`Error parsing deployment file: ${extractErrorMessage(error)}`);
286+
this.logError('parsing deployment file', error);
283287
return undefined;
284288
}
285289
}
@@ -328,13 +332,13 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
328332
await this.diagnosticCoordinator
329333
.publishDiagnostics(CfnLintService.CFN_LINT_SOURCE, payload.uri, payload.diagnostics)
330334
.catch((reason) => {
331-
this.log.error(`Error publishing diagnostics: ${extractErrorMessage(reason)}`);
335+
this.logError('publishing diagnostics', reason);
332336
});
333337
}
334338
}
335339
} catch (error) {
336340
const errorMessage = extractErrorMessage(error);
337-
this.log.error(`Error linting ${fileType} by file: ${errorMessage}`);
341+
this.logError(`linting ${fileType} by file`, error);
338342
this.publishErrorDiagnostics(uri, errorMessage);
339343
}
340344
}
@@ -375,7 +379,7 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
375379
await this.waitForInitialization();
376380
} catch (error) {
377381
this.telemetry.count('lint.uninitialized', 1, { unit: '1' });
378-
this.log.error(`Failed to wait for CfnLintService initialization: ${extractErrorMessage(error)}`);
382+
this.logError('waiting for CfnLintService initialization', error);
379383
throw error;
380384
}
381385

@@ -391,7 +395,10 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
391395
if (fileType === CloudFormationFileType.GitSyncDeployment) {
392396
this.telemetry.count(`lint.file.${CloudFormationFileType.GitSyncDeployment}`, 1, { unit: '1' });
393397

394-
this.log.error(`GitSync deployment file ${uri} cannot be processed outside of a workspace context`);
398+
this.logError(
399+
`processing GitSync deployment file ${uri}`,
400+
new Error('cannot be processed outside of a workspace context'),
401+
);
395402
this.publishDiagnostics(uri, []);
396403
return;
397404
}
@@ -452,7 +459,7 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
452459
}
453460
this.processQueuedRequests();
454461
} catch (error) {
455-
this.log.error(`Initialization failed: ${extractErrorMessage(error)}`);
462+
this.logError('during initialization', error);
456463
// Re-throw to let callers know initialization failed
457464
throw error;
458465
}
@@ -505,7 +512,7 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
505512
request.resolve();
506513
})
507514
.catch((reason: unknown) => {
508-
this.log.error(`Error processing queued request for ${uri}: ${extractErrorMessage(reason)}`);
515+
this.logError(`processing queued request for ${uri}`, reason);
509516
request.reject(reason);
510517
});
511518
}
@@ -575,7 +582,7 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
575582

576583
// Trigger initialization if needed (but don't await it here)
577584
this.ensureInitialized().catch((error) => {
578-
this.log.error(`Failed to ensure initialization: ${extractErrorMessage(error)}`);
585+
this.logError('ensuring initialization', error);
579586
});
580587
});
581588
}

‎src/stacks/actions/StackActionOperations.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ import { CFN_VALIDATION_SOURCE } from './ValidationWorkflow';
3232

3333
const logger = LoggerFactory.getLogger('StackActionOperations');
3434

35+
function logCleanupError(error: unknown, workflowId: string, changeSetName: string, operation: string): void {
36+
logger.warn({ error, workflowId, changeSetName }, `Failed to cleanup ${operation}`);
37+
}
38+
3539
export async function processChangeSet(
3640
cfnService: CfnService,
3741
documentManager: DocumentManager,
@@ -187,10 +191,7 @@ export async function deleteStackAndChangeSet(
187191
logger,
188192
);
189193
} catch (error) {
190-
logger.warn(
191-
{ error, workflowId, changeSetName: workflow.changeSetName },
192-
'Failed to cleanup workflow resources',
193-
);
194+
logCleanupError(error, workflowId, workflow.changeSetName, 'workflow resources');
194195
}
195196
}
196197

@@ -214,10 +215,7 @@ export async function deleteChangeSet(
214215
logger,
215216
);
216217
} catch (error) {
217-
logger.warn(
218-
{ error, workflowId, changeSetName: workflow.changeSetName },
219-
'Failed to cleanup workflow resources',
220-
);
218+
logCleanupError(error, workflowId, workflow.changeSetName, 'change set');
221219
}
222220
}
223221

0 commit comments

Comments
 (0)