fix: cannot cancel "Scanning CloudFormation templates..." #3987

Problem:
Cancelling the CFN registry toast would continue registering YAML files in the
background, and merely just hid the popup.

Solution:
* Exit the for-loop when the Cancel button is clicked.
* refactoring: change `registry.rebuild()` to a mandatory call--this makes it
  easier to pass a cancellation promise.

Note: Don't have a way to restart the scan, so if "Cancel" is used, the registry
template will be partial or empty until IDE restart.

Author: bryceitoc9

src/codecatalyst/devfile.ts

@@ -111,6 +111,7 @@ export function registerDevfileWatcher(devenvClient: DevEnvClient): vscode.Dispo
     const registry = new DevfileRegistry()
     const codelensProvider = new DevfileCodeLensProvider(registry, devenvClient)
     registry.addWatchPatterns([devfileGlobPattern])
+    registry.rebuild()
 
     const codelensDisposable = vscode.languages.registerCodeLensProvider(
         {

src/shared/cloudformation/activation.ts

@@ -15,6 +15,7 @@ import * as CloudFormation from './cloudformation'
 import { Commands } from '../vscode/commands2'
 import globals from '../extensionGlobals'
 import { SamCliSettings } from '../sam/cli/samCliSettings'
+import { Timeout } from '../utilities/timeoutUtils'
 
 /**
  * Creates a CloudFormationTemplateRegistry which retains the state of CloudFormation templates in a workspace.
@@ -56,12 +57,12 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
  * and slowing down the extension starting up.
  */
 function setTemplateRegistryInGlobals(registry: CloudFormationTemplateRegistry) {
-    const registrySetupFunc = async (registry: CloudFormationTemplateRegistry) => {
-        await registry.addExcludedPattern(CloudFormation.devfileExcludePattern)
-        await registry.addExcludedPattern(CloudFormation.templateFileExcludePattern)
-        await registry.addWatchPatterns([CloudFormation.templateFileGlobPattern])
-        await registry.watchUntitledFiles()
-
+    const registrySetupFunc = async (registry: CloudFormationTemplateRegistry, cancellationTimeout: Timeout) => {
+        registry.addExcludedPattern(CloudFormation.devfileExcludePattern)
+        registry.addExcludedPattern(CloudFormation.templateFileExcludePattern)
+        registry.addWatchPatterns([CloudFormation.templateFileGlobPattern])
+        registry.watchUntitledFiles()
+        await registry.rebuild(cancellationTimeout)
         return registry
     }
 

src/shared/fs/templateRegistry.ts

@@ -14,7 +14,7 @@ import { getLambdaDetails } from '../../lambda/utils'
 import { WatchedFiles, WatchedItem } from './watchedFiles'
 import { getLogger } from '../logger'
 import globals from '../extensionGlobals'
-import { sleep } from '../utilities/timeoutUtils'
+import { Timeout, sleep } from '../utilities/timeoutUtils'
 import { localize } from '../utilities/vsCodeUtils'
 import { PerfLog } from '../logger/logger'
 
@@ -71,7 +71,8 @@ export class AsyncCloudFormationTemplateRegistry {
     constructor(
         private readonly instance: CloudFormationTemplateRegistry,
         private readonly asyncSetupFunc: (
-            instance: CloudFormationTemplateRegistry
+            instance: CloudFormationTemplateRegistry,
+            cancelSetup: Timeout
         ) => Promise<CloudFormationTemplateRegistry>
     ) {}
 
@@ -86,15 +87,17 @@ export class AsyncCloudFormationTemplateRegistry {
         }
 
         let perf: PerfLog
+        const cancelSetupPromise = new Timeout(30 * 60 * 1000) // 30 min
         if (!this.setupPromise) {
             perf = new PerfLog('cfn: template registry setup')
-            this.setupPromise = this.asyncSetupFunc(this.instance)
+            this.setupPromise = this.asyncSetupFunc(this.instance, cancelSetupPromise)
         }
         this.setupPromise.then(() => {
             if (perf) {
                 perf.done()
             }
             this.isSetup = true
+            cancelSetupPromise.dispose()
         })
         // Show user a message indicating setup is in progress
         if (this.setupProgressMessage === undefined) {
@@ -111,6 +114,7 @@ export class AsyncCloudFormationTemplateRegistry {
                     token.onCancellationRequested(() => {
                         // Allows for new message to be created if templateRegistry variable attempted to be used again
                         this.setupProgressMessage = undefined
+                        cancelSetupPromise.cancel()
                     })
                     getLogger().debug('cfn: getInstance() requested, still initializing')
                     while (!this.isSetup) {

src/shared/fs/watchedFiles.ts

@@ -10,6 +10,7 @@ import * as path from 'path'
 import { globDirs, isUntitledScheme, normalizeVSCodeUri } from '../utilities/vsCodeUtils'
 import { Settings } from '../settings'
 import { once } from '../utilities/functionUtils'
+import { Timeout } from '../utilities/timeoutUtils'
 
 /**
  * Prevent `findFiles()` from recursing into these directories.
@@ -60,12 +61,18 @@ const getExcludePatternOnce = once(getExcludePattern)
  * for CFN templates among other things. WatchedFiles holds a list of pairs of
  * the absolute path to the file or "untitled:" URI along with a transform of it that is useful for
  * where it is used. For example, for templates, it parses the template and stores it.
+ *
+ * Initialize the registry by adding watch patterns and excluded patterns via
+ * `addWatchPatterns`, `watchUntitledFiles`, and `addExcludedPattern` and then
+ * calling the `rebuild` function to begin registering files.
  */
 export abstract class WatchedFiles<T> implements vscode.Disposable {
+    private isWatching: boolean = false
     private readonly disposables: vscode.Disposable[] = []
     private _isDisposed: boolean = false
     private readonly globs: vscode.GlobPattern[] = []
     private readonly excludedFilePatterns: RegExp[] = []
+    private watchingUntitledFiles: boolean = false
     private readonly registryData: Map<string, T> = new Map<string, T>()
 
     /**
@@ -93,7 +100,7 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
      * Creates watchers for each glob across all opened workspace folders (or see below to
      * watch _outside_ the workspace).
      *
-     * Fails if the watcher is disposed, or if globs have already been set;
+     * Fails if the watcher is disposed, or if the watcher has been initialized through the `rebuild` function;
      * enforce setting once to reduce rebuilds looping through all existing globs
      *
      * (since vscode 1.64):
@@ -120,12 +127,12 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
      *
      * @param globs Patterns to match against (across all opened workspace folders)
      */
-    public async addWatchPatterns(globs: vscode.GlobPattern[]): Promise<void> {
+    public addWatchPatterns(globs: vscode.GlobPattern[]) {
         if (this._isDisposed) {
             throw new Error(`${this.name}: manager has already been disposed!`)
         }
-        if (this.globs.length > 0) {
-            throw new Error(`${this.name}: watch patterns have already been established`)
+        if (this.isWatching) {
+            throw new Error(`${this.name}: cannot add watch patterns after watcher has begun watching`)
         }
         for (const glob of globs) {
             if (typeof glob === 'string' && !vscode.workspace.workspaceFolders?.[0]) {
@@ -136,15 +143,14 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
             const watcher = vscode.workspace.createFileSystemWatcher(glob)
             this.addWatcher(watcher)
         }
-
-        await this.rebuild()
     }
 
     /**
      * Create a special watcher that operates only on untitled files.
      * To "watch" the in-memory contents of an untitled:/ file we just subscribe to `onDidChangeTextDocument`
      */
-    public async watchUntitledFiles() {
+    public watchUntitledFiles() {
+        this.watchingUntitledFiles = true
         this.disposables.push(
             vscode.workspace.onDidChangeTextDocument((event: vscode.TextDocumentChangeEvent) => {
                 if (isUntitledScheme(event.document.uri)) {
@@ -162,13 +168,14 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
     /**
      * Adds a regex pattern to ignore paths containing the pattern
      */
-    public async addExcludedPattern(pattern: RegExp): Promise<void> {
+    public addExcludedPattern(pattern: RegExp) {
         if (this._isDisposed) {
             throw new Error(`${this.name}: manager has already been disposed!`)
         }
+        if (this.isWatching) {
+            throw new Error(`${this.name}: cannot add excluded patterns after watcher has begun watching`)
+        }
         this.excludedFilePatterns.push(pattern)
-
-        await this.rebuild()
     }
 
     /**
@@ -275,26 +282,37 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
     }
 
     /**
-     * Rebuilds registry using current glob and exclusion patterns.
-     * All functionality is currently internal to class, but can be made public if we want a manual "refresh" button
+     * Builds/rebuilds registry using current glob and exclusion patterns. ***Necessary to init registry***.
+     *
+     * @param cancellationTimeout Optional timeout that can trigger canceling additional loading on completion (manual or timed)
      */
-    public async rebuild(): Promise<void> {
+    public async rebuild(cancellationTimeout?: Timeout): Promise<void> {
+        let skips = 0
+        this.isWatching = true
         this.reset()
 
         const exclude = getExcludePatternOnce()
-        for (const glob of this.globs) {
+        getLogger().info(`${this.name}: Building registry with the following criteria: ${this.outputPatterns()}`)
+        for (let i = 0; i < this.globs.length && !cancellationTimeout?.completed; i++) {
+            const glob = this.globs[i]
             try {
                 const found = await vscode.workspace.findFiles(glob, exclude)
-                for (const item of found) {
-                    await this.addItem(item, true)
+                for (let j = 0; j < found.length && !cancellationTimeout?.completed; j++) {
+                    await this.addItem(found[j], true)
                 }
             } catch (e) {
+                // file not processable
+                skips++
                 const err = e as Error
-                if (err.name !== 'Canceled') {
-                    getLogger().error('watchedFiles: findFiles("%s", "%s"): %s', glob, exclude, err.message)
-                }
+                getLogger().error(`${this.name}: watchedFiles: findFiles("%s", "%s"): %s`, glob, exclude, err.message)
             }
         }
+        getLogger().info(
+            `${this.name}: Registered %s items%s%s`,
+            this.registryData.size,
+            cancellationTimeout?.completed ? ' (cancelled)' : '',
+            skips > 0 ? `, skipped ${skips} entries` : ''
+        )
     }
 
     /**
@@ -340,6 +358,18 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
             throw new Error(`FileRegistry: path is relative when it should be absolute: ${pathAsString}`)
         }
     }
+
+    private outputPatterns(): string {
+        const watch = this.globs.map(cur => cur.toString())
+        if (this.watchingUntitledFiles) {
+            watch.push('Untitled Files')
+        }
+        const exclude = this.excludedFilePatterns.map(cur => cur.toString())
+
+        return `${watch.length > 0 ? `Watching ${watch.join(', ')}` : ''}${
+            exclude.length > 0 ? `, Excluding ${exclude.join(', ')}` : ''
+        }`
+    }
 }
 
 export class NoopWatcher extends WatchedFiles<any> {

src/shared/sam/activation.ts

@@ -170,14 +170,15 @@ async function activateCodeLensRegistry(context: ExtContext) {
         // "**/…" string patterns watch recursively across _all_ workspace
         // folders (see documentation for addWatchPatterns()).
         //
-        await registry.addWatchPatterns([
+        registry.addWatchPatterns([
             pyLensProvider.pythonBasePattern,
             jsLensProvider.javascriptBasePattern,
             csLensProvider.csharpBasePattern,
             goLensProvider.goBasePattern,
             javaLensProvider.gradleBasePattern,
             javaLensProvider.mavenBasePattern,
         ])
+        await registry.rebuild()
     } catch (e) {
         vscode.window.showErrorMessage(
             localize(

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

@@ -106,7 +106,8 @@ describe('LaunchConfiguration', function () {
 
     beforeEach(async function () {
         const registry = await globals.templateRegistry
-        await registry.addWatchPatterns([CloudFormation.templateFileGlobPattern])
+        registry.addWatchPatterns([CloudFormation.templateFileGlobPattern])
+        await registry.rebuild()
 
         // TODO: remove mocks in favor of testing src/testFixtures/ data.
         mockConfigSource = mock()

src/testInteg/cloudformation/templateRegistry.test.ts

@@ -9,7 +9,7 @@ import * as fs from 'fs-extra'
 import { CloudFormationTemplateRegistry } from '../../shared/fs/templateRegistry'
 import { makeSampleSamTemplateYaml, strToYamlFile } from '../../test/shared/cloudformation/cloudformationTestUtils'
 import { getTestWorkspaceFolder } from '../integrationTestsUtilities'
-import { sleep, waitUntil } from '../../shared/utilities/timeoutUtils'
+import { Timeout, sleep, waitUntil } from '../../shared/utilities/timeoutUtils'
 import assert from 'assert'
 
 /**
@@ -44,13 +44,15 @@ describe('CloudFormation Template Registry', async function () {
         await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDir, 'test.yaml'))
         await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDirNested, 'test.yml'))
 
-        await registry.addWatchPatterns(['**/test.{yaml,yml}'])
+        registry.addWatchPatterns(['**/test.{yaml,yml}'])
+        await registry.rebuild()
 
         await registryHasTargetNumberOfFiles(registry, 2)
     })
 
     it.skip('adds dynamically-added template files with yaml and yml extensions at various nesting levels', async function () {
-        await registry.addWatchPatterns(['**/test.{yaml,yml}'])
+        registry.addWatchPatterns(['**/test.{yaml,yml}'])
+        await registry.rebuild()
 
         await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDir, 'test.yml'))
         await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDirNested, 'test.yaml'))
@@ -59,8 +61,9 @@ describe('CloudFormation Template Registry', async function () {
     })
 
     it('Ignores templates matching excluded patterns', async function () {
-        await registry.addWatchPatterns(['**/test.{yaml,yml}'])
-        await registry.addExcludedPattern(/.*nested.*/)
+        registry.addWatchPatterns(['**/test.{yaml,yml}'])
+        registry.addExcludedPattern(/.*nested.*/)
+        await registry.rebuild()
 
         await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDir, 'test.yml'))
         await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDirNested, 'test.yaml'))
@@ -72,7 +75,8 @@ describe('CloudFormation Template Registry', async function () {
         const filepath = path.join(testDir, 'changeMe.yml')
         await strToYamlFile(makeSampleSamTemplateYaml(false), filepath)
 
-        await registry.addWatchPatterns(['**/changeMe.yml'])
+        registry.addWatchPatterns(['**/changeMe.yml'])
+        await registry.rebuild()
 
         await registryHasTargetNumberOfFiles(registry, 1)
 
@@ -84,7 +88,8 @@ describe('CloudFormation Template Registry', async function () {
     })
 
     it('can handle deleted files', async function () {
-        await registry.addWatchPatterns(['**/deleteMe.yml'])
+        registry.addWatchPatterns(['**/deleteMe.yml'])
+        await registry.rebuild()
 
         // Specifically creating the file after the watcher is added
         // Otherwise, it seems the file is deleted before the file watcher realizes the file exists
@@ -99,11 +104,23 @@ describe('CloudFormation Template Registry', async function () {
         await registryHasTargetNumberOfFiles(registry, 0)
     })
 
-    it('fails if you set watch patterns multiple times', async function () {
-        await registry.addWatchPatterns(['first/set'])
+    it('fails if you set watch patterns after init', async function () {
+        registry.addWatchPatterns(['first/set'])
+        await registry.rebuild()
         await assert.rejects(async () => {
-            await registry.addWatchPatterns(['second/set'])
-        }, new Error('CloudFormationTemplateRegistry: watch patterns have already been established'))
+            registry.addWatchPatterns(['second/set'])
+        }, new Error('CloudFormationTemplateRegistry: cannot add watch patterns after watcher has begun watching'))
+        await assert.rejects(async () => {
+            registry.addExcludedPattern(/exclude me/)
+        }, new Error('CloudFormationTemplateRegistry: cannot add excluded patterns after watcher has begun watching'))
+    })
+
+    it('cancels the rebuild if the cancellation timeout is completed', async function () {
+        registry.addWatchPatterns(['**/test.{yaml,yml}'])
+        const t = new Timeout(100)
+        t.cancel()
+        await registry.rebuild(t)
+        await registryHasTargetNumberOfFiles(registry, 0)
     })
 })