Improve suite stability with langserver errors

.azure-pipelines/common/test.yml

@@ -9,7 +9,7 @@ steps:
 
   - task: Npm@1
     displayName: "Test"
-    timeoutInMinutes: 30
+    timeoutInMinutes: 35
     inputs:
       command: custom
       customCommand: test
@@ -25,6 +25,6 @@ steps:
   - task: PublishTestResults@2
     displayName: "Publish Test Results"
     inputs:
-      testResultsFiles: "*-results.xml"
+      testResultsFiles: "test-results.xml"
       testRunTitle: "$(Agent.OS)"
     condition: succeededOrFailed()

.vscode/launch.json

@@ -52,11 +52,11 @@
                 "MOCHA_enableTimeouts": "0", // Disable time-outs
                 "DEBUGTELEMETRY": "1", // 1=quiet; verbose=see telemetry in console; 0=send telemetry
                 "NODE_DEBUG": "",
-                "DISABLE_LANGUAGE_SERVER": "0",
-                "DISABLE_SLOW_TESTS": "0",
+                "DISABLE_LANGUAGE_SERVER": "",
+                "DISABLE_SLOW_TESTS": "",
                 "ECHO_OUTPUT_CHANNEL_TO_CONSOLE": "1",
-                "BREAK_ON_ASSERT": "0",
-                "ALWAYS_ECHO_TEST_LOG": "0" // If 1 or true, always immediately echos test log to console; otherwise test log is only echoed after a failed testcase
+                "BREAK_ON_ASSERT": "",
+                "ALWAYS_ECHO_TEST_LOG": "" // If 1 or true, always immediately echos test log to console; otherwise test log is only echoed after a failed testcase
             }
         },
         // Launch extension with webpack
@@ -108,8 +108,8 @@
                 "NODE_DEBUG": "",
                 "AZCODE_ARM_IGNORE_BUNDLE": "0",
                 "ECHO_OUTPUT_CHANNEL_TO_CONSOLE": "1",
-                "BREAK_ON_ASSERT": "0",
-                "ALWAYS_ECHO_TEST_LOG": "0" // If 1 or true, always immediately echos test log to console; otherwise test log is only echoed after a failed testcase
+                "BREAK_ON_ASSERT": "",
+                "ALWAYS_ECHO_TEST_LOG": "" // If 1 or true, always immediately echos test log to console; otherwise test log is only echoed after a failed testcase
             }
         }
     ]

extension.bundle.ts

@@ -73,7 +73,7 @@ export * from "./src/language/IssueKind";
 export * from "./src/language/LineColPos";
 export * from "./src/language/ReferenceList";
 export * from "./src/language/Span";
-export * from "./src/languageclient/getAvailableResourceTypesAndVersions";
+export * from "./src/languageclient/getAvailableResourceTypesAndVersionsNoThrow";
 export * from "./src/languageclient/startArmLanguageServer";
 export * from './src/snippets/ISnippet';
 export * from './src/snippets/ISnippetManager';

gulpfile.ts

@@ -84,13 +84,18 @@ interface IExpressionMetadata {
 }
 
 function test(): cp.ChildProcess {
-    env.DEBUGTELEMETRY = 'verbose';
+    env.DEBUGTELEMETRY = '0'; // 1=quiet; verbose=see telemetry in console; 0=send telemetry
     env.CODE_TESTS_PATH = path.join(__dirname, 'dist/test');
     // This is the timeout for individual tests
     env.MOCHA_timeout = String(DEFAULT_TESTCASE_TIMEOUT_MS);
     env.MOCHA_enableTimeouts = "1";
     env.MOCHA_grep = "";
-    //env.ALWAYS_ECHO_TEST_LOG = "1";
+    env.ALWAYS_ECHO_TEST_LOG = "";
+    console.log("");
+    console.log("*******");
+    console.log("******* NOTE: After the tests run, see testlogs-<platform>/logs/testlog.txt under artifacts for full test log");
+    console.log("*******");
+    console.log("");
     return cp.spawn('node', ['./node_modules/vscode/bin/test'], { stdio: 'inherit', env });
 }
 

src/TimedMessage.ts

@@ -34,14 +34,13 @@ export class TimedMessage {
     /**
      * Called whenever the user is interacting with the extension (thus gets called a lot)
      */
-    public registerActiveUse(): void {
+    public registerActiveUseNoThrow(): void {
         if (this._alreadyCheckedThisSession) {
             return;
         }
         this._alreadyCheckedThisSession = true;
 
         // Don't wait
-        // tslint:disable-next-line: no-floating-promises
         callWithTelemetryAndErrorHandling("considerShowingMessage", async (context: IActionContext) => {
             context.errorHandling.suppressDisplay = true;
             context.telemetry.properties.message = this._message;
@@ -82,6 +81,8 @@ export class TimedMessage {
             } else {
                 assert(response === neverAskAgain);
             }
+        }).catch(err => {
+            assert.fail("Shouldn't throw");
         });
     }
 

src/documents/parameters/parameterFiles.ts

@@ -10,7 +10,7 @@ import { callWithTelemetryAndErrorHandling, DialogResponses, IActionContext, IAz
 import { armTemplateLanguageId, configKeys, configPrefix, documentSchemes, globalStateKeys } from '../../constants';
 import { ext } from '../../extensionVariables';
 import { normalizeFilePath } from '../../util/normalizedPaths';
-import { pathExists } from '../../util/pathExists';
+import { pathExistsNoThrow } from '../../util/pathExistsNoThrow';
 import { DeploymentTemplateDoc } from '../templates/DeploymentTemplateDoc';
 import { containsParametersSchema } from '../templates/schemas';
 import { DeploymentFileMapping } from './DeploymentFileMapping';
@@ -156,7 +156,7 @@ export async function openTemplateFile(mapping: DeploymentFileMapping, parameter
       throw new Error(`There is no template file currently associated with parameter file "${parameterUri.fsPath}"`);
     }
 
-    if (await pathExists(templateUri)) {
+    if (await pathExistsNoThrow(templateUri)) {
       let doc: TextDocument = await workspace.openTextDocument(templateUri);
       await window.showTextDocument(doc);
     } else {
@@ -207,7 +207,7 @@ async function createParameterFileQuickPickList(mapping: DeploymentFileMapping,
   if (currentParamUri && !currentParamFile) {
     // There is a current parameter file, but it wasn't among the list we came up with.  We must add it to the list.
     currentParamFile = { isCloseNameMatch: false, uri: currentParamUri, friendlyPath: getRelativeParameterFilePath(templateUri, currentParamUri) };
-    let exists = await pathExists(currentParamUri);
+    let exists = await pathExistsNoThrow(currentParamUri);
     currentParamFile.fileNotFound = !exists;
 
     suggestions = suggestions.concat(currentParamFile);
@@ -341,29 +341,32 @@ async function isParameterFile(filePath: string): Promise<boolean> {
 }
 
 async function doesFileContainString(filePath: string, matches: (fileSubcontents: string) => boolean, maxBytesToRead: number): Promise<boolean> {
-  // tslint:disable-next-line: typedef
-  return new Promise<boolean>((resolve, reject) => {
-    const stream = fse.createReadStream(filePath, { encoding: 'utf8' });
-
-    let content: string = '';
-    stream.on('data', (chunk: string) => {
-      content += chunk;
-      if (matches(content)) {
-        stream.close();
-        resolve(true);
-      }
+  return new Promise<boolean>((resolve, reject): void => {
+    try {
+      const stream = fse.createReadStream(filePath, { encoding: 'utf8' });
+
+      let content: string = '';
+      stream.on('data', (chunk: string) => {
+        content += chunk;
+        if (matches(content)) {
+          stream.close();
+          resolve(true);
+        }
 
-      if (content.length >= maxBytesToRead) {
-        stream.close();
+        if (content.length >= maxBytesToRead) {
+          stream.close();
+          resolve(false);
+        }
+      });
+      stream.on('end', () => {
         resolve(false);
-      }
-    });
-    stream.on('end', () => {
-      resolve(false);
-    });
-    stream.on('error', (err) => {
+      });
+      stream.on('error', (err) => {
+        reject(err);
+      });
+    } catch (err) {
       reject(err);
-    });
+    }
   });
 }
 
@@ -391,7 +394,7 @@ function removeAllExtensions(fileName: string): string {
 /**
  * Search for potential parameter file matches for the given document, and ask the user if appropriate whether to associate it
  */
-export function considerQueryingForParameterFile(mapping: DeploymentFileMapping, document: TextDocument): void {
+export function considerQueryingForParameterFileNoThrow(mapping: DeploymentFileMapping, document: TextDocument): void {
   const templateUri = document.uri;
 
   // Only deal with saved files, because we don't have an accurate
@@ -400,7 +403,7 @@ export function considerQueryingForParameterFile(mapping: DeploymentFileMapping,
     return;
   }
 
-  // tslint:disable-next-line: no-floating-promises Don't wait
+  // Don't wait
   callWithTelemetryAndErrorHandling('queryAddParameterFile', async (actionContext: IActionContext): Promise<void> => {
     const alreadyHasParamFile: boolean = !!mapping.getParameterFile(document.uri);
     actionContext.telemetry.properties.alreadyHasParamFile = String(alreadyHasParamFile);
@@ -464,6 +467,8 @@ export function considerQueryingForParameterFile(mapping: DeploymentFileMapping,
         assert("considerQueryingForParameterFile: Unexpected response");
         break;
     }
+  }).catch(err => {
+    assert.fail("callWithTelemetryAndErrorHandling in considerQueryingForParameterFile shouldn't throw");
   });
 }
 

src/documents/templates/deploymentTemplateCodeLenses.ts

@@ -7,7 +7,7 @@
 import { Range, Uri } from 'vscode';
 import { parseError } from 'vscode-azureextensionui';
 import { Span } from '../../language/Span';
-import { pathExists } from '../../util/pathExists';
+import { pathExistsNoThrow } from '../../util/pathExistsNoThrow';
 import { IGotoParameterValueArgs } from '../../vscodeIntegration/commandArguments';
 import { getVSCodeRangeFromSpan } from '../../vscodeIntegration/vscodePosition';
 import { ResolvableCodeLens } from '../DeploymentDocument';
@@ -39,7 +39,7 @@ export class ShowCurrentParameterFileCodeLens extends ResolvableCodeLens {
                 command: 'azurerm-vscode-tools.openParameterFile',
                 arguments: [this.scope.document.documentUri] // Template file uri
             };
-            if (!await pathExists(this.parameterFileUri)) {
+            if (!await pathExistsNoThrow(this.parameterFileUri)) {
                 this.command.title += " $(error) Not found";
             }
             return true;
@@ -110,7 +110,7 @@ export class ParameterDefinitionCodeLens extends ResolvableCodeLens {
             paramsSource = await this.parameterValuesSourceProvider.getValuesSource();
         } catch (err) {
             if (this.parameterValuesSourceProvider.parameterFileUri) {
-                if (!await pathExists(this.parameterValuesSourceProvider.parameterFileUri)) {
+                if (!await pathExistsNoThrow(this.parameterValuesSourceProvider.parameterFileUri)) {
                     errorMessage = `$(error) Parameter file not found`;
                 } else {
                     errorMessage = `$(error) Could not open parameter file: ${parseError(err).message}`;

src/documents/templates/linkedTemplates/linkedTemplates.ts

@@ -16,7 +16,7 @@ import { filterByType } from '../../../util/filterByType';
 import { httpGet } from '../../../util/httpGet';
 import { prependLinkedTemplateScheme, removeLinkedTemplateScheme } from '../../../util/linkedTemplateScheme';
 import { normalizeUri } from '../../../util/normalizedPaths';
-import { pathExists } from '../../../util/pathExists';
+import { pathExistsNoThrow } from '../../../util/pathExistsNoThrow';
 import { parseUri, stringifyUri } from '../../../util/uri';
 import { DeploymentTemplateDoc } from '../../templates/DeploymentTemplateDoc';
 import { LinkedTemplateScope } from '../../templates/scopes/templateScopes';
@@ -181,7 +181,7 @@ async function tryOpenLocalLinkedFile(
 ): Promise<OpenLinkedFileResult> {
     try {
         // Check first if the path exists, so we get a better error message if not
-        if (!(await pathExists(localPath))) {
+        if (!(await pathExistsNoThrow(localPath))) {
             return {
                 loadError: <Errorish>new LinkedTemplatePathNotFoundError(
                     pathType === PathType.parametersLink ?
@@ -256,7 +256,7 @@ export async function openLinkedTemplateFileCommand(linkedTemplateUri: Uri, acti
     actionContext.telemetry.properties.uriHasSas = String(!!linkedTemplateUri.query.match('[&?]sig='));
 
     if (linkedTemplateUri.scheme === documentSchemes.file) {
-        const exists = await pathExists(linkedTemplateUri);
+        const exists = await pathExistsNoThrow(linkedTemplateUri);
         actionContext.telemetry.properties.exists = String(exists);
         if (!exists) {
             const fsPath = linkedTemplateUri.fsPath;
@@ -287,7 +287,7 @@ export async function reloadLinkedTemplateFileCommand(linkedTemplateUri: Uri, ac
     actionContext.telemetry.properties.scheme = linkedTemplateUri.scheme;
 
     if (linkedTemplateUri.scheme === documentSchemes.file) {
-        const exists = await pathExists(linkedTemplateUri);
+        const exists = await pathExistsNoThrow(linkedTemplateUri);
         actionContext.telemetry.properties.exists = String(exists);
         if (!exists) {
             const fsPath = linkedTemplateUri.fsPath;

src/fixed_assert.ts

@@ -9,7 +9,7 @@
 import * as orig_assert from "assert";
 import { isWebpack } from "./constants";
 
-export const breakOnAssert: boolean = /^(true|1)?$/i.test(process.env.BREAK_ON_ASSERT ?? '');
+export const breakOnAssert: boolean = /^(true|1)$/i.test(process.env.BREAK_ON_ASSERT ?? '');
 
 function fixed_ok(value: unknown, message?: string): void {
     // tslint:disable-next-line: strict-boolean-expressions

src/languageclient/getAvailableResourceTypesAndVersions.ts → src/languageclient/getAvailableResourceTypesAndVersionsNoThrow.ts

@@ -3,10 +3,10 @@
 // Licensed under the MIT License. See License.md in the project root for license information.
 // ---------------------------------------------------------------------------------------------
 
-import { callWithTelemetryAndErrorHandlingSync } from "vscode-azureextensionui";
+import { callWithTelemetryAndErrorHandling } from "vscode-azureextensionui";
 import { ext } from "../extensionVariables";
-import { waitForLanguageServerAvailable } from "../languageclient/startArmLanguageServer";
 import { CaseInsensitiveMap } from "../util/CaseInsensitiveMap";
+import { waitForLanguageServerAvailable } from "./startArmLanguageServer";
 
 /**
  * Returns a case-insensitive map of available resource types and their apiVersions for a given ARM schema from the
@@ -15,10 +15,10 @@ import { CaseInsensitiveMap } from "../util/CaseInsensitiveMap";
  *
  * @param schema The ARM schema of the template document to query for
  */
-export async function getAvailableResourceTypesAndVersions(schema: string): Promise<CaseInsensitiveMap<string, string[]>> {
+export async function getAvailableResourceTypesAndVersionsNoThrow(schema: string): Promise<CaseInsensitiveMap<string, string[]>> {
     const map = new CaseInsensitiveMap<string, string[]>();
 
-    await callWithTelemetryAndErrorHandlingSync("getAvailableResourceTypesAndVersions", async () => {
+    await callWithTelemetryAndErrorHandling("getAvailableResourceTypesAndVersions", async () => {
         await waitForLanguageServerAvailable();
 
         const resourceTypes = <{ [key: string]: string[] }>

src/languageclient/showAvailableResourceTypesAndVersions.ts

@@ -6,11 +6,11 @@
 import { callWithTelemetryAndErrorHandlingSync } from "vscode-azureextensionui";
 import { ext } from "../extensionVariables";
 import { CaseInsensitiveMap } from "../util/CaseInsensitiveMap";
-import { getAvailableResourceTypesAndVersions } from "./getAvailableResourceTypesAndVersions";
+import { getAvailableResourceTypesAndVersionsNoThrow } from "./getAvailableResourceTypesAndVersionsNoThrow";
 
 export async function showAvailableResourceTypesAndVersions(schema: string): Promise<void> {
     await callWithTelemetryAndErrorHandlingSync("showAvailableResourceTypesAndVersions", async () => {
-        const map: CaseInsensitiveMap<string, string[]> = await getAvailableResourceTypesAndVersions(schema);
+        const map: CaseInsensitiveMap<string, string[]> = await getAvailableResourceTypesAndVersionsNoThrow(schema);
 
         for (const entry of map.entries()) {
             const key = entry[0];