Skip to content

Commit 9af4b24

Browse files
authored
Show cfnlint initialization failures as log message (#220)
1 parent 6ad8786 commit 9af4b24

File tree

4 files changed

+43
-8
lines changed

4 files changed

+43
-8
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
export class WorkerNotInitializedError extends Error {
2+
constructor(message: string = 'Worker not initialized') {
3+
super(message);
4+
this.name = 'WorkerNotInitializedError';
5+
}
6+
}

src/services/cfnLint/CfnLintService.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { Delayer } from '../../utils/Delayer';
1616
import { extractErrorMessage } from '../../utils/Errors';
1717
import { byteSize } from '../../utils/String';
1818
import { DiagnosticCoordinator } from '../DiagnosticCoordinator';
19+
import { WorkerNotInitializedError } from './CfnLintErrors';
1920
import { PyodideWorkerManager } from './PyodideWorkerManager';
2021

2122
export enum LintTrigger {
@@ -239,9 +240,16 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
239240
* Publish error diagnostics when linting fails
240241
*
241242
* @param uri The document URI
242-
* @param errorMessage The error message
243+
* @param error The error that occurred
243244
*/
244-
private publishErrorDiagnostics(uri: string, errorMessage: string): void {
245+
private publishErrorDiagnostics(uri: string, error: unknown): void {
246+
// Don't publish diagnostics for worker initialization errors, just log them
247+
if (error instanceof WorkerNotInitializedError) {
248+
this.log.warn('cfn-lint worker not initialized');
249+
return;
250+
}
251+
252+
const errorMessage = extractErrorMessage(error);
245253
this.publishDiagnostics(uri, [
246254
{
247255
severity: 1, // Error severity
@@ -285,9 +293,8 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
285293
}
286294
} catch (error) {
287295
this.status = STATUS.Uninitialized;
288-
const errorMessage = extractErrorMessage(error);
289296
this.logError(`linting ${fileType} by string`, error);
290-
this.publishErrorDiagnostics(uri, errorMessage);
297+
this.publishErrorDiagnostics(uri, error);
291298
} finally {
292299
this.telemetry.histogram(
293300
'lint.standaloneFile.duration',
@@ -369,9 +376,8 @@ export class CfnLintService implements SettingsConfigurable, Closeable {
369376
}
370377
} catch (error) {
371378
this.status = STATUS.Uninitialized;
372-
const errorMessage = extractErrorMessage(error);
373379
this.logError(`linting ${fileType} by file`, error);
374-
this.publishErrorDiagnostics(uri, errorMessage);
380+
this.publishErrorDiagnostics(uri, error);
375381
} finally {
376382
this.telemetry.histogram(
377383
'lint.workspaceFile.duration',

src/services/cfnLint/PyodideWorkerManager.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { CloudFormationFileType } from '../../document/Document';
55
import { CfnLintInitializationSettings } from '../../settings/Settings';
66
import { LoggerFactory } from '../../telemetry/LoggerFactory';
77
import { retryWithExponentialBackoff } from '../../utils/Retry';
8+
import { WorkerNotInitializedError } from './CfnLintErrors';
89

910
interface WorkerTask {
1011
id: string;
@@ -189,7 +190,7 @@ export class PyodideWorkerManager {
189190
}
190191

191192
if (!this.worker) {
192-
throw new Error('Worker not initialized');
193+
throw new WorkerNotInitializedError();
193194
}
194195

195196
return await new Promise<T>((resolve, reject) => {
@@ -208,7 +209,7 @@ export class PyodideWorkerManager {
208209
if (this.worker) {
209210
this.worker.postMessage({ id: taskId, action, payload });
210211
} else {
211-
reject(new Error('Worker not initialized'));
212+
reject(new WorkerNotInitializedError());
212213
}
213214
});
214215
}

tst/unit/services/cfnLint/CfnLintService.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { describe, expect, beforeEach, afterEach, vi, Mock, test } from 'vitest'
66
import { WorkspaceFolder, DiagnosticSeverity } from 'vscode-languageserver';
77
import { URI } from 'vscode-uri';
88
import { CloudFormationFileType, Document } from '../../../../src/document/Document';
9+
import { WorkerNotInitializedError } from '../../../../src/services/cfnLint/CfnLintErrors';
910
import { CfnLintService, LintTrigger, sleep } from '../../../../src/services/cfnLint/CfnLintService';
1011
import { PyodideWorkerManager } from '../../../../src/services/cfnLint/PyodideWorkerManager';
1112
import { SettingsState } from '../../../../src/settings/Settings';
@@ -501,6 +502,16 @@ describe('CfnLintService', () => {
501502
expect(diagnosticsArg[2][0].message).toContain('Python execution failed');
502503
});
503504

505+
test('should log but not publish diagnostic for Worker not initialized error in lint', async () => {
506+
mockComponents.workspace.getWorkspaceFolder.returns(undefined);
507+
mockWorkerManager.lintTemplate.rejects(new WorkerNotInitializedError());
508+
509+
await service.lint(mockTemplate, mockUri);
510+
511+
// Should not publish any diagnostics for worker initialization errors
512+
expect(mockComponents.diagnosticCoordinator.publishDiagnostics.called).toBe(false);
513+
});
514+
504515
test('should handle waitForInitialization failure', async () => {
505516
// Create a spy on waitForInitialization to make it fail
506517
const waitSpy = vi
@@ -858,6 +869,17 @@ describe('CfnLintService', () => {
858869
expect(diagnosticsArg[2][0].message).toBe('CFN Lint Error: Python execution failed');
859870
});
860871

872+
test('should log but not publish diagnostic for Worker not initialized error', async () => {
873+
const workerManager = (service as any).workerManager;
874+
// Use sinon's rejects method to simulate worker not initialized error
875+
workerManager.lintTemplate.rejects(new WorkerNotInitializedError());
876+
877+
await (service as any).lintStandaloneFile(mockTemplate, mockUri, CloudFormationFileType.Template);
878+
879+
// Should not publish any diagnostics for worker initialization errors
880+
expect(mockComponents.diagnosticCoordinator.publishDiagnostics.called).toBe(false);
881+
});
882+
861883
test('should throw if service is not initialized', async () => {
862884
// Set the main service to uninitialized state
863885
(service as any).status = 0; // STATUS.Uninitialized

0 commit comments

Comments
 (0)