Skip to content

Commit

Permalink
fix(lambda): AppBuilder messages #6362
Browse files Browse the repository at this point in the history
## Problem
Several errors that were being shown to customers did not have enough
information for them to take action on the problem.

## Solution
Audited all errors and logs shown to customers and updated them to make
them more legible and actionable.
  • Loading branch information
seshubaws authored Jan 17, 2025
1 parent fae084b commit 9617649
Show file tree
Hide file tree
Showing 13 changed files with 59 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export async function getFiles(

return await vscode.workspace.findFiles(globPattern, excludePattern)
} catch (error) {
getLogger().error(`Failed to get files with pattern ${pattern}:`, error)
getLogger().error(`Failed to find files with pattern ${pattern}:`, error)
return []
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class AppNode implements TreeNode {
createPlaceholderItem(
localize(
'AWS.appBuilder.explorerNode.app.noResourceTree',
'[Unable to load Resource tree for this App. Update SAM template]'
'[Unable to load resource tree for this app. Ensure SAM template is correct.]'
)
),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export async function generateDeployedNode(
.Configuration as Lambda.FunctionConfiguration
newDeployedResource = new LambdaFunctionNode(lambdaNode, regionCode, configuration)
} catch (error: any) {
getLogger().error('Error getting Lambda configuration %O', error)
getLogger().error('Error getting Lambda configuration: %O', error)
throw ToolkitError.chain(error, 'Error getting Lambda configuration', {
code: 'lambdaClientError',
})
Expand All @@ -107,7 +107,7 @@ export async function generateDeployedNode(
createPlaceholderItem(
localize(
'AWS.appBuilder.explorerNode.unavailableDeployedResource',
'[Failed to retrive deployed resource.]'
'[Failed to retrive deployed resource. Ensure your AWS account is connected.]'
)
),
]
Expand All @@ -119,8 +119,8 @@ export async function generateDeployedNode(
try {
v3configuration = (await v3Client.send(v3command)).Configuration as FunctionConfiguration
logGroupName = v3configuration.LoggingConfig?.LogGroup
} catch {
getLogger().error('Error getting Lambda V3 configuration')
} catch (error: any) {
getLogger().error('Error getting Lambda V3 configuration: %O', error)
}
newDeployedResource.configuration = {
...newDeployedResource.configuration,
Expand Down Expand Up @@ -156,7 +156,10 @@ export async function generateDeployedNode(
getLogger().info('Details are missing or are incomplete for: %O', deployedResource)
return [
createPlaceholderItem(
localize('AWS.appBuilder.explorerNode.noApps', '[This resource is not yet supported.]')
localize(
'AWS.appBuilder.explorerNode.noApps',
'[This resource is not yet supported in AppBuilder.]'
)
),
]
}
Expand All @@ -166,7 +169,7 @@ export async function generateDeployedNode(
createPlaceholderItem(
localize(
'AWS.appBuilder.explorerNode.unavailableDeployedResource',
'[Failed to retrive deployed resource.]'
'[Failed to retrieve deployed resource. Ensure correct stack name and region are in the samconfig.toml, and that your account is connected.]'
)
),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,17 @@ export async function getStackName(projectRoot: vscode.Uri): Promise<any> {
} catch (error: any) {
switch (error.code) {
case SamConfigErrorCode.samNoConfigFound:
getLogger().info('No stack name or region information available in samconfig.toml: %O', error)
getLogger().info('Stack name and/or region information not found in samconfig.toml: %O', error)
break
case SamConfigErrorCode.samConfigParseError:
getLogger().error(`Error getting stack name or region information: ${error.message}`, error)
getLogger().error(
`Error parsing stack name and/or region information from samconfig.toml: ${error.message}. Ensure the information is correct.`,
error
)
void showViewLogsMessage('Encountered an issue reading samconfig.toml')
break
default:
getLogger().warn(`Error getting stack name or region information: ${error.message}`, error)
getLogger().warn(`Error parsing stack name and/or region information: ${error.message}`, error)
}
return {}
}
Expand Down
15 changes: 9 additions & 6 deletions packages/core/src/awsService/appBuilder/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ const localize = nls.loadMessageBundle()
export async function runOpenTemplate(arg?: TreeNode) {
const templateUri = arg ? (arg.resource as SamAppLocation).samTemplateUri : await promptUserForTemplate()
if (!templateUri || !(await fs.exists(templateUri))) {
throw new ToolkitError('No template provided', { code: 'NoTemplateProvided' })
throw new ToolkitError('SAM Template not found, cannot open template', { code: 'NoTemplateProvided' })
}
const document = await vscode.workspace.openTextDocument(templateUri)
await vscode.window.showTextDocument(document)
}

/**
* Find and open the lambda handler with given ResoruceNode
* Find and open the lambda handler with given ResourceNode
* If not found, a NoHandlerFound error will be raised
* @param arg ResourceNode
*/
Expand All @@ -56,9 +56,12 @@ export async function runOpenHandler(arg: ResourceNode): Promise<void> {
arg.resource.resource.Runtime
)
if (!handlerFile) {
throw new ToolkitError(`No handler file found with name "${arg.resource.resource.Handler}"`, {
code: 'NoHandlerFound',
})
throw new ToolkitError(
`No handler file found with name "${arg.resource.resource.Handler}". Ensure the file exists in the expected location."`,
{
code: 'NoHandlerFound',
}
)
}
await vscode.workspace.openTextDocument(handlerFile).then(async (doc) => await vscode.window.showTextDocument(doc))
}
Expand Down Expand Up @@ -90,7 +93,7 @@ export async function getLambdaHandlerFile(
): Promise<vscode.Uri | undefined> {
const family = getFamily(runtime)
if (!supportedRuntimeForHandler.has(family)) {
throw new ToolkitError(`Runtime ${runtime} is not supported for open handler button`, {
throw new ToolkitError(`Runtime ${runtime} is not supported for the 'Open handler' button`, {
code: 'RuntimeNotSupported',
})
}
Expand Down
12 changes: 6 additions & 6 deletions packages/core/src/awsService/appBuilder/walkthrough.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ export async function getTutorial(
const appSelected = appMap.get(project + runtime)
telemetry.record({ action: project + runtime, source: source ?? 'AppBuilderWalkthrough' })
if (!appSelected) {
throw new ToolkitError(`Tried to get template '${project}+${runtime}', but it hasn't been registered.`)
throw new ToolkitError(`Template '${project}+${runtime}' does not exist, choose another template.`)
}

try {
await getPattern(serverlessLandOwner, serverlessLandRepo, appSelected.asset, outputDir, true)
} catch (error) {
throw new ToolkitError(`Error occurred while fetching the pattern from serverlessland: ${error}`)
throw new ToolkitError(`An error occurred while fetching this pattern from Serverless Land: ${error}`)
}
}

Expand Down Expand Up @@ -190,7 +190,7 @@ export async function genWalkthroughProject(
'No'
)
if (choice !== 'Yes') {
throw new ToolkitError(`${defaultTemplateName} already exist`)
throw new ToolkitError(`A file named ${defaultTemplateName} already exists in this path.`)
}
}

Expand Down Expand Up @@ -256,9 +256,9 @@ export async function initWalkthroughProjectCommand() {
let runtimeSelected: TutorialRuntimeOptions | undefined = undefined
try {
if (!walkthroughSelected || !(typeof walkthroughSelected === 'string')) {
getLogger().info('exit on no walkthrough selected')
getLogger().info('No walkthrough selected - exiting')
void vscode.window.showErrorMessage(
localize('AWS.toolkit.lambda.walkthroughNotSelected', 'Please select a template first')
localize('AWS.toolkit.lambda.walkthroughNotSelected', 'Select a template in the walkthrough.')
)
return
}
Expand Down Expand Up @@ -322,7 +322,7 @@ export async function getOrUpdateOrInstallSAMCli(source: string) {
}
}
} catch (err) {
throw ToolkitError.chain(err, 'Failed to install or detect SAM')
throw ToolkitError.chain(err, 'Failed to install or detect SAM.')
} finally {
telemetry.record({ source: source, toolId: 'sam-cli' })
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('getFiles', () => {

const templateFiles = await getFiles(workspaceFolder, '**/template.{yml,yaml}', '**/.aws-sam/**')
assert.strictEqual(templateFiles.length, 0)
assertLogsContain('Failed to get files with pattern', false, 'error')
assertLogsContain('Failed to find files with pattern', false, 'error')
sandbox.restore()
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('samProject', () => {
assert.strictEqual(region, expectedRegion)
})

it('returns undefined give no stack name or region in samconfig file', async () => {
it('returns undefined given no stack name or region in samconfig file', async () => {
await testFolder.write(
'samconfig.toml',
generateSamconfigData({
Expand All @@ -71,24 +71,28 @@ describe('samProject', () => {

const result = await wrapperCall(undefined)
assert.deepStrictEqual(result, {})
assertLogsContain('Error getting stack name or region information: No project folder found', false, 'warn')
assertLogsContain(
'Error parsing stack name and/or region information: No project folder found',
false,
'warn'
)
})

it('returns empty object give no samconfig file found', async () => {
it('returns empty object given no samconfig file found', async () => {
// simulate error when no samconfig.toml file in directory
const result = await getStackName(projectRoot)
assert.deepStrictEqual(result, {})
assertLogsContain('No stack name or region information available in samconfig.toml', false, 'info')
assertLogsContain('Stack name and/or region information not found in samconfig.toml', false, 'info')
})

it('returns empty object give error parsing samconfig file', async () => {
it('returns empty object given error parsing samconfig file', async () => {
// simulate error when parsinf samconfig.toml: missing quote or empty value
await testFolder.write('samconfig.toml', samconfigInvalidData)

const result = await getStackName(projectRoot)
assert.deepStrictEqual(result, {})

assertLogsContain('Error getting stack name or region information:', false, 'error')
assertLogsContain('Error parsing stack name and/or region information from samconfig.toml:', false, 'error')
getTestWindow().getFirstMessage().assertError('Encountered an issue reading samconfig.toml')
})
})
Expand Down Expand Up @@ -149,17 +153,6 @@ describe('samProject', () => {
() => getApp(mockSamAppLocation),
new ToolkitError(`Template at ${mockSamAppLocation.samTemplateUri.fsPath} is not valid`)
)
// try {
// await getApp(mockSamAppLocation)
// assert.fail('Test should not reach here. Expect ToolkitError thrown')
// } catch (error) {
// assert(cloudformationTryLoadSpy.calledOnce)
// assert(error instanceof ToolkitError)
// assert.strictEqual(
// error.message,
// `Template at ${mockSamAppLocation.samTemplateUri.fsPath} is not valid`
// )
// }
})
})
})
4 changes: 2 additions & 2 deletions packages/core/src/test/awsService/appBuilder/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,9 @@ describe('AppBuilder Utils', function () {
}
try {
await runOpenTemplate(tNode as TreeNode)
assert.fail('No template provided')
assert.fail('SAM Template not found, cannot open template')
} catch (err) {
assert.strictEqual((err as Error).message, 'No template provided')
assert.strictEqual((err as Error).message, 'SAM Template not found, cannot open template')
}
// Then
assert(openCommand.neverCalledWith(sinon.match.has('fspath', sinon.match(/template.yaml/g))))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,9 @@ describe('AppBuilder Walkthrough', function () {
try {
// When
await genWalkthroughProject('Visual', workspaceUri, undefined)
assert.fail('template.yaml already exist')
assert.fail('A file named template.yaml already exists in this path.')
} catch (e) {
assert.equal((e as Error).message, 'template.yaml already exist')
assert.equal((e as Error).message, 'A file named template.yaml already exists in this path.')
}
// Then
assert.equal(await fs.readFileText(vscode.Uri.joinPath(workspaceUri, 'template.yaml')), prevInfo)
Expand Down Expand Up @@ -236,9 +236,9 @@ describe('AppBuilder Walkthrough', function () {
try {
// When
await genWalkthroughProject('S3', workspaceUri, 'python')
assert.fail('template.yaml already exist')
assert.fail('A file named template.yaml already exists in this path.')
} catch (e) {
assert.equal((e as Error).message, 'template.yaml already exist')
assert.equal((e as Error).message, 'A file named template.yaml already exists in this path.')
}
// Then no overwrite happens
assert.equal(await fs.readFileText(vscode.Uri.joinPath(workspaceUri, 'template.yaml')), prevInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe('AppNode', () => {
assert.strictEqual(resourceNode.id, 'placeholder')
assert.strictEqual(
resourceNode.resource,
'[Unable to load Resource tree for this App. Update SAM template]'
'[Unable to load resource tree for this app. Ensure SAM template is correct.]'
)
assert(getAppStub.calledOnce)
assert(getStackNameStub.notCalled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,10 @@ describe('generateDeployedNode', () => {
// Check placeholder propertries
const deployedResourceNode = deployedResourceNodes[0] as DeployedResourceNode
assert.strictEqual(deployedResourceNode.id, 'placeholder')
assert.strictEqual(deployedResourceNode.resource, '[Failed to retrive deployed resource.]')
assert.strictEqual(
deployedResourceNode.resource,
'[Failed to retrieve deployed resource. Ensure correct stack name and region are in the samconfig.toml, and that your account is connected.]'
)
})
})

Expand Down Expand Up @@ -374,7 +377,7 @@ describe('generateDeployedNode', () => {
// Check placeholder propertries
const deployedResourceNode = deployedResourceNodes[0] as DeployedResourceNode
assert.strictEqual(deployedResourceNode.id, 'placeholder')
assert.strictEqual(deployedResourceNode.resource, '[This resource is not yet supported.]')
assert.strictEqual(deployedResourceNode.resource, '[This resource is not yet supported in AppBuilder.]')
})
})
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "AppBuilder: Update error messaging to make more legible and actionable"
}

0 comments on commit 9617649

Please sign in to comment.