[VSC-1769] Bugfixes for v1.11.0 based on Telemetry (#1675)

* rm serial port chip id error telemetry use node os arch check binary is in path before version cmd

* fix platform info test

* more fixes in child process and telemetry ignore msg

Author: brianignacio5

src/PlatformInformation.ts

@@ -13,28 +13,44 @@
 // limitations under the License.
 
 import * as os from "os";
-import * as utils from "./utils";
 
 export class PlatformInformation {
-  public static GetPlatformInformation(): Promise<PlatformInformation> {
+  public static GetPlatformInformation(): PlatformInformation {
     const platform: string = os.platform();
-    let architecturePromise: Promise<string>;
+    const arch = os.arch();
 
+    // Map os.arch() values to expected architecture strings
+    let architecture: string;
     switch (platform) {
       case "win32":
-        architecturePromise = PlatformInformation.GetWindowsArchitecture();
+        if (arch === "x64") {
+          architecture = "x86_x64";
+        } else if (arch === "ia32") {
+          architecture = "x86";
+        } else {
+          architecture = "Unknown";
+        }
         break;
       case "linux":
-        architecturePromise = PlatformInformation.GetUnixArchitecture();
-        break;
       case "darwin":
-        architecturePromise = PlatformInformation.GetUnixArchitecture();
+        if (arch === "x64") {
+          architecture = "x64";
+        } else if (arch === "ia32") {
+          architecture = "x86";
+        } else if (arch === "arm64") {
+          architecture = "arm64";
+        } else if (arch === "arm") {
+          architecture = "armhf";
+        } else {
+          architecture = arch;
+        }
+        break;
       default:
+        architecture = "Unknown";
         break;
     }
-    return Promise.all<string>([architecturePromise]).then(([architecture]) => {
-      return new PlatformInformation(platform, architecture);
-    });
+
+    return new PlatformInformation(platform, architecture);
   }
 
   public get platformToUse(): string {
@@ -75,48 +91,5 @@ export class PlatformInformation {
     }
   }
 
-  public static GetUnknownArchitecture(): string {
-    return "Unknown";
-  }
-
-  public static GetUnixArchitecture(): Promise<string> {
-    const command = "uname";
-    const args = ["-m"];
-    return utils
-      .execChildProcess(command, args, utils.extensionContext.extensionPath)
-      .then((architecture) => {
-        if (architecture) {
-          return architecture.trim();
-        }
-      });
-  }
-
-  private static GetWindowsArchitecture(): Promise<string> {
-    const command = "powershell";
-    const args = [
-      "-executionPolicy",
-      "bypass",
-      "(Get-WmiObject Win32_OperatingSystem).OSArchitecture",
-    ];
-    return utils
-      .execChildProcess(command, args, utils.extensionContext.extensionPath)
-      .then((architecture) => {
-        if (architecture) {
-          const archArray: string[] = architecture.split(os.EOL);
-          if (archArray.length > 2) {
-            const arch: string = archArray[1].trim();
-            if (arch.indexOf("64") >= 0) {
-              return "x86_x64";
-            } else if (arch.indexOf("32") >= 0) {
-              return "x86";
-            }
-          }
-        }
-        return PlatformInformation.GetUnknownArchitecture();
-      })
-      .catch((err) => {
-        return PlatformInformation.GetUnknownArchitecture();
-      });
-  }
   constructor(public platform: string, public architecture: string) {}
 }

src/espIdf/menuconfig/confServerProcess.ts

@@ -226,7 +226,9 @@ export class ConfserverProcess {
       (idfConf.readParameter("idf.sdkconfigDefaults") as string[]) || [];
 
     if (reconfigureArgs.indexOf("SDKCONFIG") === -1) {
-      reconfigureArgs.push(`-DSDKCONFIG='${ConfserverProcess.instance.configFile}'`)
+      reconfigureArgs.push(
+        `-DSDKCONFIG='${ConfserverProcess.instance.configFile}'`
+      );
     }
 
     if (
@@ -261,7 +263,11 @@ export class ConfserverProcess {
         if (code !== 0) {
           const errorMsg = `When loading default values received exit signal: ${signal}, code : ${code}`;
           OutputChannel.appendLine(errorMsg, "SDK Configuration Editor");
-          Logger.error(errorMsg, new Error(errorMsg), "ConfserverProcess setDefaultValues");
+          Logger.error(
+            errorMsg,
+            new Error(errorMsg),
+            "ConfserverProcess setDefaultValues"
+          );
         }
         ConfserverProcess.init(currWorkspace, extensionPath);
         progress.report({ increment: 70, message: "The end" });
@@ -346,7 +352,7 @@ export class ConfserverProcess {
       (idfConf.readParameter("idf.sdkconfigDefaults") as string[]) || [];
 
     if (confServerArgs.indexOf("SDKCONFIG") === -1) {
-      confServerArgs.push(`-DSDKCONFIG='${this.configFile}'`)
+      confServerArgs.push(`-DSDKCONFIG='${this.configFile}'`);
     }
 
     if (
@@ -429,6 +435,7 @@ export class ConfserverProcess {
         "Saving config to",
         "Loading config from",
         "The following config symbol(s) were not visible so were not updated",
+        "WARNING:",
       ];
 
       if (isStringNotEmpty(dataStr)) {
@@ -466,6 +473,10 @@ export class ConfserverProcess {
     OutputChannel.appendLine(
       "-----------------------END OF ERROR-----------------------"
     );
-    Logger.error(data.toString(), new Error(data.toString()), "ConfserverProcess printError");
+    Logger.error(
+      data.toString(),
+      new Error(data.toString()),
+      "ConfserverProcess printError"
+    );
   }
 }

src/espIdf/serial/serialPort.ts

@@ -114,10 +114,11 @@ export class SerialPort {
           OutputChannel.appendLine(
             `Detecting default port using esptool.py...`
           );
-          const timeout = idfConf.readParameter(
-            "idf.serialPortDetectionTimeout",
-            workspaceFolder
-          ) as number * 1000; // Convert seconds to milliseconds
+          const timeout =
+            (idfConf.readParameter(
+              "idf.serialPortDetectionTimeout",
+              workspaceFolder
+            ) as number) * 1000; // Convert seconds to milliseconds
 
           const result = await spawn(
             pythonBinPath,
@@ -126,6 +127,7 @@ export class SerialPort {
               silent: false,
               appendMode: "append",
               timeout: timeout,
+              sendToTelemetry: false,
             }
           );
 
@@ -309,7 +311,14 @@ export class SerialPort {
       const msg = error.message
         ? error.message
         : "Something went wrong while getting the serial port list";
-      Logger.errorNotify(msg, error, "SerialPort displayList");
+      const sendToTelemetry = msg.indexOf("No serial ports found") === -1;
+      Logger.errorNotify(
+        msg,
+        error,
+        "SerialPort displayList",
+        undefined,
+        sendToTelemetry
+      );
       OutputChannel.appendLine(msg, "Serial port");
       OutputChannel.appendLineAndShow(JSON.stringify(error));
     }
@@ -413,7 +422,12 @@ export class SerialPort {
             const chipIdBuffer = await spawn(
               pythonBinPath,
               [esptoolPath, "--port", serialPort.comName, "chip_id"],
-              { timeout: 2000, silent: true, appendMode: "append" }
+              {
+                timeout: 2000,
+                silent: true,
+                appendMode: "append",
+                sendToTelemetry: false,
+              }
             );
             const regexp = /Chip is(.*?)[\r]?\n/;
             const chipIdString = chipIdBuffer.toString().match(regexp);

src/idfComponentsDataProvider.ts

@@ -121,7 +121,9 @@ export class IdfTreeDataProvider implements TreeDataProvider<IdfComponent> {
       Logger.errorNotify(
         vscode.l10n.t("File project_description.json cannot be found."),
         new Error("File-Not-Found"),
-        "IDFTreeDataProvider getComponentsInProject"
+        "IDFTreeDataProvider getComponentsInProject",
+        undefined,
+        false
       );
       return null;
     }

src/idfToolsManager.ts

@@ -40,7 +40,7 @@ export interface IEspIdfTool {
 
 export class IdfToolsManager {
   public static async createIdfToolsManager(idfPath: string) {
-    const platformInfo = await PlatformInformation.GetPlatformInformation();
+    const platformInfo = PlatformInformation.GetPlatformInformation();
     const toolsJsonPath = await utils.getToolsJsonPath(idfPath);
     const toolsObj = await readJSON(toolsJsonPath);
     const idfToolsManager = new IdfToolsManager(
@@ -230,6 +230,10 @@ export class IdfToolsManager {
     const command = pkg.version_cmd[0];
     const args = pkg.version_cmd.slice(1);
     try {
+      const isBinInPath = await utils.isBinInPath(command, modifiedEnv);
+      if (!isBinInPath) {
+        return "No match";
+      }
       const binVersionResponse = await utils.execChildProcess(
         command,
         args,

src/logger/logger.ts

@@ -82,6 +82,7 @@ export class Logger {
         errorStack: error.stack,
         category,
         capturedBy: "Logger",
+        command: metadata?.command
       });
     }
     winston.log("error", message, {

src/setup/SetupPanel.ts

@@ -51,7 +51,6 @@ import {
   checkSpacesInPath,
   getEspIdfFromCMake,
   canAccessFile,
-  execChildProcess,
   compareVersion,
 } from "../utils";
 import { useIdfSetupSettings } from "./setupValidation/espIdfSetup";

src/support/checkEspIdfTools.ts

@@ -27,7 +27,7 @@ export async function checkEspIdfTools(
   reportedResult: reportObj,
   context: vscode.ExtensionContext
 ) {
-  const platformInfo = await PlatformInformation.GetPlatformInformation();
+  const platformInfo = PlatformInformation.GetPlatformInformation();
   let toolsJsonPath: string = join(
     reportedResult.configurationSettings.espIdfPath,
     "tools",

src/test/suite/PlatformInformation.test.ts

@@ -28,9 +28,8 @@ suite("PlatformInformation Tests", () => {
       extensionPath: __dirname,
     } as vscode.ExtensionContext;
     utils.setExtensionContext(mockUpContext); // Need a path to execute a child process to get info
-    return PlatformInformation.GetPlatformInformation().then((actual) => {
-      assert.equal(actual.platform, os.platform());
-      assert.equal(actual.architecture, "x86_64");
-    });
+    const actual = PlatformInformation.GetPlatformInformation();
+    assert.equal(actual.platform, os.platform());
+    assert.equal(actual.architecture, "x64");
   });
 });

src/utils.ts

@@ -75,7 +75,9 @@ export class PreCheck {
         Logger.errorNotify(
           preCheck[1],
           new Error("PRECHECK_FAILED"),
-          "utils precheck failed"
+          "utils precheck failed",
+          undefined,
+          false
         );
       }
     });
@@ -168,6 +170,8 @@ export interface ISpawnOptions extends childProcess.SpawnOptions {
   outputString?: string;
   /** Output append mode: 'appendLine', 'append', or undefined */
   appendMode?: "appendLine" | "append";
+  /** Send error to telemetry */
+  sendToTelemetry?: boolean;
 }
 
 export function spawn(
@@ -177,10 +181,11 @@ export function spawn(
     outputString: "",
     silent: false,
     appendMode: "appendLine",
+    sendToTelemetry: true,
   }
 ): Promise<Buffer> {
   let buff = Buffer.alloc(0);
-  const sendToOutputChannel = (data: Buffer) => {
+  const sendToOutputChannel = (data: any) => {
     buff = Buffer.concat([buff, data]);
     options.outputString += buff.toString();
     if (!options.silent) {
@@ -225,7 +230,13 @@ export function spawn(
         resolve(buff);
       } else {
         const err = new Error("non zero exit code " + code + EOL + EOL + buff);
-        Logger.error(err.message, err, "src utils spawn", { command });
+        Logger.error(
+          err.message,
+          err,
+          "src utils spawn",
+          { command },
+          options.sendToTelemetry
+        );
         reject(err);
       }
     });
@@ -665,7 +676,10 @@ export function execChildProcess(
           return reject(error);
         }
         if (stderr && stderr.length > 2) {
-          if (!stderr.startsWith("Open On-Chip Debugger v")) {
+          if (
+            !stderr.startsWith("Open On-Chip Debugger v") &&
+            !stderr.toLowerCase().startsWith("warning")
+          ) {
             Logger.error(
               stderr,
               new Error(stderr),
@@ -996,7 +1010,7 @@ export function validateFileSizeAndChecksum(
         const fileSize = fs.statSync(filePath).size;
         const readStream = fs.createReadStream(filePath);
         let fileChecksum: string;
-        readStream.on("data", (data) => {
+        readStream.on("data", (data: crypto.BinaryLike) => {
           shashum.update(data);
         });
         readStream.on("end", () => {