Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ export function refreshResourceListCommand(resourcesManager: ResourcesManager, e
try {
await resourcesManager.refreshResourceList(resourceTypeNode.typeName)
} catch (error) {
await handleLspError(error, 'Error refreshing resource list')
// ResourcesManager already handles the error, don't duplicate
}
})
}
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/awsService/cloudformation/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ async function startClient(context: ExtensionContext) {
},
errorHandler: {
error: (error: Error, message: Message | undefined, count: number | undefined): ErrorHandlerResult => {
void window.showErrorMessage(formatMessage(`${toString(message)} - ${toString(error)}`))
// Log but don't show popup - let callers handle user-facing errors
getLogger().error(`LSP Error: ${toString(message)} - ${toString(error)}`)
return { action: ErrorAction.Continue }
},
closed: (): CloseHandlerResult => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,11 @@ export class ResourcesManager {
if (response.resources.length > 0) {
this.resources.set(resourceType, response.resources[0])
}

this.notifyAllListeners()
} catch (error) {
await handleLspError(error, 'Error loading more resources')
await handleLspError(error, `Loading ${resourceType} resources`)
} finally {
await setContext('aws.cloudformation.loadingResources', false)
this.notifyAllListeners()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ export class StacksManager implements Disposable {
})
this.stacks = response.stacks
this.nextToken = response.nextToken
this.loaded = true
} catch (error) {
await handleLspError(error, 'Error loading stacks')
this.stacks = []
this.nextToken = undefined
} finally {
this.loaded = true
await setContext('aws.cloudformation.refreshingStacks', false)
this.notifyListeners()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
promptToSaveToFile,
addResourceTypesCommand,
removeResourceTypeCommand,
refreshResourceListCommand,
} from '../../../../awsService/cloudformation/commands/cfnCommands'
import { OptionalFlagMode } from '../../../../awsService/cloudformation/stacks/actions/stackActionRequestType'
import * as inputBox from '../../../../awsService/cloudformation/ui/inputBox'
Expand Down Expand Up @@ -418,4 +419,41 @@ describe('CfnCommands', function () {
assert.ok(mockResourcesManager.removeResourceType.calledOnceWith('AWS::S3::Bucket'))
})
})

describe('refreshResourceListCommand', function () {
it('should register refresh resource list command', function () {
const mockResourcesManager = { refreshResourceList: sinon.stub() } as any
const result = refreshResourceListCommand(mockResourcesManager, {} as any)
assert.ok(result)
assert.ok(registerCommandStub.calledOnce)
assert.strictEqual(registerCommandStub.firstCall.args[0], 'aws.cloudformation.api.refreshResourceList')
})

it('should call refreshResourceList with node typeName', async function () {
const mockResourcesManager = { refreshResourceList: sinon.stub().resolves() } as any
refreshResourceListCommand(mockResourcesManager, {} as any)

const commandHandler = registerCommandStub.firstCall.args[1]
const mockNode = { typeName: 'AWS::Lambda::Function' } as ResourceTypeNode

await commandHandler(mockNode)

assert.ok(mockResourcesManager.refreshResourceList.calledOnceWith('AWS::Lambda::Function'))
})

it('should not throw when refreshResourceList fails', async function () {
const mockResourcesManager = {
refreshResourceList: sinon.stub().rejects(new Error('Network error')),
} as any
refreshResourceListCommand(mockResourcesManager, {} as any)

const commandHandler = registerCommandStub.firstCall.args[1]
const mockNode = { typeName: 'AWS::Lambda::Function' } as ResourceTypeNode

// Should not throw - error handling is done in ResourcesManager
await assert.doesNotReject(async () => {
await commandHandler(mockNode)
})
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import * as assert from 'assert'
import * as sinon from 'sinon'
import * as vscode from 'vscode'
import { ResourcesManager } from '../../../../awsService/cloudformation/resources/resourcesManager'
import { ResourceSelector } from '../../../../awsService/cloudformation/ui/resourceSelector'
import { ResourceStateResult } from '../../../../awsService/cloudformation/resources/resourceRequestTypes'
Expand Down Expand Up @@ -277,3 +278,43 @@ describe('ResourcesManager - removeResourceType', () => {
assert.deepStrictEqual(updatedTypes, ['AWS::S3::Bucket', 'AWS::Lambda::Function'])
})
})

describe('ResourcesManager - refreshResourceList error handling', () => {
let sandbox: sinon.SinonSandbox
let mockClient: any
let mockResourceSelector: ResourceSelector
let resourcesManager: ResourcesManager
let setContextStub: sinon.SinonStub

beforeEach(() => {
sandbox = sinon.createSandbox()
mockClient = { sendRequest: sandbox.stub() }
mockResourceSelector = {} as ResourceSelector
setContextStub = sandbox.stub(vscode.commands, 'executeCommand')
sandbox.stub(getLogger(), 'error')
resourcesManager = new ResourcesManager(mockClient, mockResourceSelector)
})

afterEach(() => {
sandbox.restore()
})

it('should notify listeners even when refreshResourceList fails', async () => {
mockClient.sendRequest.rejects(new Error('Network timeout'))
const listener = sandbox.stub()
resourcesManager.addListener(listener)

await assert.rejects(() => resourcesManager.refreshResourceList('AWS::S3::Bucket'), /Network timeout/)

assert.ok(listener.calledOnce)
})

it('should clear loading context even when refreshResourceList fails', async () => {
mockClient.sendRequest.rejects(new Error('Access denied'))

await assert.rejects(() => resourcesManager.refreshResourceList('AWS::Lambda::Function'), /Access denied/)

// Should call setContext to clear loading state
assert.ok(setContextStub.calledWith('setContext', 'aws.cloudformation.refreshingResourceList', false))
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,20 @@ describe('StacksManager', () => {
assert.strictEqual(mockClient.sendRequest.calledOnce, true)
})
})

describe('error handling', () => {
it('should set loaded=true even when loadStacks fails', async () => {
// Arrange: Mock client to reject with an error
mockClient.sendRequest.rejects(new Error('Access denied'))

// Act: Try to load stacks (should fail but not throw)
await manager.ensureLoaded()

// Assert: Manager should still be marked as loaded to prevent infinite retries
assert.strictEqual(manager.isLoaded(), true)

// Assert: Stacks should be empty array after error
assert.deepStrictEqual(manager.get(), [])
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "CloudFormation: Duplicate error notifications no longer appear when operations fail"
}
Loading