fix(sam): "Invalid (or missing) template file" #3946

Author: justinmk3

.changes/next-release/Bug Fix-6b798eec-0914-4eef-b384-46829bc2cde0.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "SAM debugging: \"Invalid (or missing) template file\" may occur even when a valid template.yaml is specified by `invokeTarget.templatePath` in the launch config. #2614"
+}

.changes/next-release/Bug Fix-9623dcac-8c9f-4687-a2ce-8a02b4e982be.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "`AWS: Add SAM Debug Configuration` command only works the first time it is invoked."
+}

src/lambda/vue/configEditor/samInvokeBackend.ts

@@ -351,24 +351,22 @@ async function getLaunchConfigQuickPickItems(
     const existingConfigs = launchConfig.getDebugConfigurations()
     const samValidator = new DefaultAwsSamDebugConfigurationValidator(vscode.workspace.getWorkspaceFolder(uri))
     const registry = await globals.templateRegistry
-    return existingConfigs
-        .map((val, index) => {
-            return {
-                config: val,
-                index,
-            }
-        })
-        .filter(o => {
-            const res = samValidator.validate(o.config as any as AwsSamDebuggerConfiguration, registry, true)
-            return res?.isValid
-        })
-        .map(val => {
-            return {
-                index: val.index,
-                label: val.config.name,
-                config: val.config as AwsSamDebuggerConfiguration,
-            }
-        })
+    const mapped = existingConfigs.map((val, index) => {
+        return {
+            config: val as AwsSamDebuggerConfiguration,
+            index: index,
+            label: val.name,
+        }
+    })
+    // XXX: can't use filter() with async predicate.
+    const filtered: LaunchConfigPickItem[] = []
+    for (const c of mapped) {
+        const valid = await samValidator.validate(c.config, registry, true)
+        if (valid?.isValid) {
+            filtered.push(c)
+        }
+    }
+    return filtered
 }
 
 export function finalizeConfig(config: AwsSamDebuggerConfiguration, name: string): AwsSamDebuggerConfiguration {

src/shared/debug/launchConfiguration.ts

@@ -74,7 +74,14 @@ export class LaunchConfiguration {
             isAwsSamDebugConfiguration(o)
         ) as AwsSamDebuggerConfiguration[]
         const registry = await globals.templateRegistry
-        return configs.filter(o => this.samValidator.validate(o, registry)?.isValid)
+        // XXX: can't use filter() with async predicate.
+        const validConfigs: AwsSamDebuggerConfiguration[] = []
+        for (const c of configs) {
+            if ((await this.samValidator.validate(c, registry))?.isValid) {
+                validConfigs.push(c)
+            }
+        }
+        return validConfigs
     }
 
     /**

src/shared/fs/watchedFiles.ts

@@ -172,31 +172,44 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
     }
 
     /**
-     * Adds an item to registry. Wipes any existing item in its place with new copy of the data
-     * @param uri vscode.Uri containing the item to load in
+     * Adds or updates an item in the registry, and returns the result.
+     *
+     * If the item matches an "exclude" rule, it is not added nor does it update/replace any existing item.
+     *
+     * @param uri vscode.Uri containing the item to register.
+     * @param quiet On failure, log a message instead of throwing an exception.
+     * @param contents Optional data to associate with the item, for logical (non-filesystem) URIs.
+     *
+     * @returns Item, or undefined if (1) processing fails or (2) the name matches an "exclude" rule.
      */
-    public async addItem(uri: vscode.Uri, quiet?: boolean, contents?: string): Promise<void> {
+    public async addItem(uri: vscode.Uri, quiet?: boolean, contents?: string): Promise<WatchedItem<T> | undefined> {
         const excluded = this.excludedFilePatterns.find(pattern => uri.fsPath.match(pattern))
         if (excluded) {
-            getLogger().verbose(`${this.name}: excluding path (matches "${excluded}"): ${uri.fsPath}`)
-            return
+            getLogger().verbose(`${this.name}: excluded (matches "${excluded}"): ${uri.fsPath}`)
+            return undefined
         }
         this.assertAbsolute(uri)
         const pathAsString = normalizeVSCodeUri(uri)
         try {
             const item = await this.process(uri, contents)
             if (item) {
                 this.registryData.set(pathAsString, item)
+                return {
+                    path: pathAsString,
+                    item: item,
+                }
             } else {
+                getLogger().info(`${this.name}: failed to process: ${uri}`)
                 // if value isn't valid for type, remove from registry
                 this.registryData.delete(pathAsString)
             }
         } catch (e) {
             if (!quiet) {
                 throw e
             }
-            getLogger().verbose(`${this.name}: failed to load(): ${uri}: ${(e as Error).message}`)
+            getLogger().info(`${this.name}: failed to process: ${uri}: ${(e as Error).message}`)
         }
+        return undefined
     }
 
     /**

src/shared/sam/activation.ts

@@ -72,8 +72,8 @@ export async function activate(ctx: ExtContext): Promise<void> {
     Commands.register('aws.addSamDebugConfig', async () => {
         if (!didActivateCodeLensProviders) {
             await activateSlowCodeLensesOnce()
-            await samDebugConfigCmd()
         }
+        await samDebugConfigCmd()
     })
 
     ctx.extensionContext.subscriptions.push(

src/shared/sam/debugger/awsSamDebugConfigurationValidator.ts

@@ -27,7 +27,7 @@ export interface ValidationResult {
 }
 
 export interface AwsSamDebugConfigurationValidator {
-    validate(config: AwsSamDebuggerConfiguration, registry: CloudFormationTemplateRegistry): ValidationResult
+    validate(config: AwsSamDebuggerConfiguration, registry: CloudFormationTemplateRegistry): Promise<ValidationResult>
 }
 
 export class DefaultAwsSamDebugConfigurationValidator implements AwsSamDebugConfigurationValidator {
@@ -36,11 +36,11 @@ export class DefaultAwsSamDebugConfigurationValidator implements AwsSamDebugConf
     /**
      * Validates debug configuration properties.
      */
-    public validate(
+    public async validate(
         config: AwsSamDebuggerConfiguration,
         registry: CloudFormationTemplateRegistry,
         resolveVars?: boolean
-    ): ValidationResult {
+    ): Promise<ValidationResult> {
         let rv: ValidationResult = { isValid: false, message: undefined }
         if (resolveVars) {
             config = doTraverseAndReplace(config, this.workspaceFolder?.uri.fsPath ?? '')
@@ -67,12 +67,15 @@ export class DefaultAwsSamDebugConfigurationValidator implements AwsSamDebugConf
             config.invokeTarget.target === TEMPLATE_TARGET_TYPE ||
             config.invokeTarget.target === API_TARGET_TYPE
         ) {
-            let cfnTemplate
+            let cfnTemplate: CloudFormation.Template | undefined
             if (config.invokeTarget.templatePath) {
                 const fullpath = tryGetAbsolutePath(this.workspaceFolder, config.invokeTarget.templatePath)
                 // Normalize to absolute path for use in the runner.
                 config.invokeTarget.templatePath = fullpath
-                cfnTemplate = registry.getItem(fullpath)?.item
+                // Forcefully add to the registry in case the registry scan somehow missed the file. #2614
+                // If the user (launch config) gave an explicit path we should always "find" it.
+                const uri = vscode.Uri.file(fullpath)
+                cfnTemplate = (await registry.addItem(uri, true))?.item
             }
             rv = this.validateTemplateConfig(config, config.invokeTarget.templatePath, cfnTemplate)
         } else if (config.invokeTarget.target === CODE_TARGET_TYPE) {

src/shared/sam/debugger/awsSamDebugger.ts

@@ -439,7 +439,7 @@ export class SamDebugConfigProvider implements vscode.DebugConfigurationProvider
             })
         } else {
             const registry = await globals.templateRegistry
-            const rv = configValidator.validate(config, registry)
+            const rv = await configValidator.validate(config, registry)
             if (!rv.isValid) {
                 throw new ToolkitError(`Invalid launch configuration: ${rv.message}`, { code: 'BadLaunchConfig' })
             } else if (rv.message) {

src/test/shared/debug/launchConfiguration.test.ts

@@ -112,11 +112,7 @@ describe('LaunchConfiguration', function () {
         mockConfigSource = mock()
         mockSamValidator = mock()
         when(mockConfigSource.getDebugConfigurations()).thenReturn(debugConfigurations)
-        when(mockSamValidator.validate(deepEqual(samDebugConfiguration), registry)).thenReturn(
-            await (async () => {
-                return { isValid: true }
-            })()
-        )
+        when(mockSamValidator.validate(deepEqual(samDebugConfiguration), registry)).thenResolve({ isValid: true })
     })
 
     afterEach(async function () {

src/test/shared/sam/debugger/awsSamDebugConfigurationValidator.test.ts

@@ -110,92 +110,92 @@ describe('DefaultAwsSamDebugConfigurationValidator', function () {
         validator = new DefaultAwsSamDebugConfigurationValidator(instance(mockFolder))
     })
 
-    it('returns invalid when resolving debug configurations with an invalid request type', function () {
+    it('returns invalid when resolving debug configurations with an invalid request type', async () => {
         templateConfig.request = 'not-direct-invoke'
 
-        const result = validator.validate(templateConfig, mockRegistry)
+        const result = await validator.validate(templateConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it('returns invalid when resolving debug configurations with an invalid target type', function () {
+    it('returns invalid when resolving debug configurations with an invalid target type', async () => {
         templateConfig.invokeTarget.target = 'not-valid' as any
 
-        const result = validator.validate(templateConfig as any, mockRegistry)
+        const result = await validator.validate(templateConfig as any, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it("returns invalid when resolving template debug configurations with a template that isn't in the registry", () => {
+    it("returns invalid when resolving template debug configurations with a template that isn't in the registry", async () => {
         const mockEmptyRegistry: CloudFormationTemplateRegistry = mock()
         when(mockEmptyRegistry.getItem('/')).thenReturn(undefined)
 
         validator = new DefaultAwsSamDebugConfigurationValidator(instance(mockFolder))
 
-        const result = validator.validate(templateConfig, mockRegistry)
+        const result = await validator.validate(templateConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it("returns invalid when resolving template debug configurations with a template that doesn't have the set resource", () => {
+    it("returns invalid when resolving template debug configurations with a template that doesn't have the set resource", async () => {
         const target = templateConfig.invokeTarget as TemplateTargetProperties
         target.logicalId = 'wrong'
 
-        const result = validator.validate(templateConfig, mockRegistry)
+        const result = await validator.validate(templateConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it("returns invalid when resolving template debug configurations with a template that isn't serverless", () => {
+    it("returns invalid when resolving template debug configurations with a template that isn't serverless", async () => {
         const target = templateConfig.invokeTarget as TemplateTargetProperties
         target.logicalId = 'OtherResource'
 
-        const result = validator.validate(templateConfig, mockRegistry)
+        const result = await validator.validate(templateConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it('returns undefined when resolving template debug configurations with a resource that has an invalid runtime in template', function () {
+    it('returns undefined when resolving template debug configurations with a resource that has an invalid runtime in template', async () => {
         const properties = templateData.item.Resources?.TestResource?.Properties as CloudFormation.ZipResourceProperties
         properties.Runtime = 'invalid'
 
-        const result = validator.validate(templateConfig, mockRegistry)
+        const result = await validator.validate(templateConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it("API config returns invalid when resolving with a template that isn't serverless", () => {
+    it("API config returns invalid when resolving with a template that isn't serverless", async () => {
         const target = templateConfig.invokeTarget as TemplateTargetProperties
         target.logicalId = 'OtherResource'
 
-        const result = validator.validate(apiConfig, mockRegistry)
+        const result = await validator.validate(apiConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it('API config is invalid when it does not have an API field', function () {
+    it('API config is invalid when it does not have an API field', async () => {
         const config = createApiConfig()
         config.api = undefined
 
-        const result = validator.validate(config, mockRegistry)
+        const result = await validator.validate(config, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it("API config is invalid when its path does not start with a '/'", () => {
+    it("API config is invalid when its path does not start with a '/'", async () => {
         const config = createApiConfig()
 
         config.api!.path = 'noleadingslash'
 
-        const result = validator.validate(config, mockRegistry)
+        const result = await validator.validate(config, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it('returns invalid when resolving code debug configurations with invalid runtimes', function () {
+    it('returns invalid when resolving code debug configurations with invalid runtimes', async () => {
         codeConfig.lambda = { runtime: 'asd' }
 
-        const result = validator.validate(codeConfig, mockRegistry)
+        const result = await validator.validate(codeConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 
-    it('returns invalid when Image app does not declare runtime', function () {
+    it('returns invalid when Image app does not declare runtime', async () => {
         const lambda = imageTemplateConfig.lambda
 
         delete lambda?.runtime
 
-        const result = validator.validate(templateConfig, mockRegistry)
+        const result = await validator.validate(templateConfig, mockRegistry)
         assert.strictEqual(result.isValid, false)
     })
 })