Windows fixes (#100)

Wrong args passed, must pass shell=true due to recent security fix

Backed out the use of secure spawn() call on Windows since I couldn't
make it do anything but hang (even with shell=True and
stdio=[ignore,pipe,pipe]) and the main thing is for the linux use to be
as good as it can be

Also added a progress notificiation while running commands since
apama_project takes so long

Author: ben-c8y

CHANGELOG.md

@@ -1,5 +1,5 @@
 ## v2.2.1
-* Fixed an issue with the extension not locating the Apama installation when executed in a Docker development container. 
+* Fixed an issue with the extension not locating the Apama installation when executed in a Docker development container, and on Windows. 
 
 ## v2.2.0
 * The "Apama Projects" view now only searches the top level directory in each workspace folder for projects. It no longer searches for projects nested under the top level workspace folder.

src/apama_project/apamaProjectView.ts

@@ -282,6 +282,7 @@ export class ApamaProjectView
               }
             }
 
+            // nb: in future we might use the --json output mode added instead of parsing the text output
             apama_project
               .run(project.fsDir, ["list", "bundles"])
               .then((result) => {

src/apama_util/apamaenvironment.ts

@@ -41,7 +41,7 @@ async function getApamaExecutableCommand(command: ApamaExecutables, showError=tr
         return ok({command: `${apamaBin}/${command}`, args: []})
       }
     } else {
-      return ok({command: `${apamaBin}/apama_env.bat`, args: [command]});
+      return ok({command: `${apamaBin}\\apama_env.bat`, args: [command]});
     }
   }
 

src/apama_util/apamarunner.ts

@@ -1,116 +1,58 @@
-import { ChildProcess, spawn as spawnCallback, execFile as execFileCallback} from "child_process";
-import { Logger } from "../logger/logger";
+import { exec, execFile} from "child_process";
 import { platform } from "os";
 import { promisify } from "util";
+import { logger } from "../extension";
+import {
+  ProgressLocation,
+  window
+} from "vscode";
+
+const execPromisify = promisify(exec);
+const execFilePromisify = promisify(execFile);
 
-const spawn = promisify(spawnCallback);
-const execFile = promisify(execFileCallback);
 
 export class ApamaRunner {
   stdout = "";
   stderr = "";
 
   constructor(
+    /** Display name for this process */
     public name: string,
+    /** e.g. [XXX/apama_env, apama_project] or [XXX/apama_project] */
     public command: string[],
   ) {}
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   async run(workingDir: string, args: string[]): Promise<any> {
-    //if fails returns promise.reject including err
-    if (platform() === "win32") {
-      return await spawn(
-        this.command[0],
-        [...args.slice(1), ...args],
-        {cwd: workingDir}
-      )
-    } else {
-      return await execFile(
-        this.command[0],
-        [...this.command.slice(1), ...args],
-        {cwd: workingDir}
-      )
-    }
-  }
-}
-
-export class ApamaAsyncRunner {
-  stdout = "";
-  stderr = "";
-  child?: ChildProcess;
-
-  constructor(
-    public name: string,
-    public command: string,
-    private logger: Logger,
-  ) {}
-
-  //
-  // If you call this withShell = true it will run the command under that shell
-  // this means the process may need to be managed separately as it may get detached
-  // when you kill the parent (correlator behaves that way)
-  // I use engine_management to control the running correlator
-  //
-  // TODO: pipes configuration might be worth passing as an argument
-  //
-  public start(
-    args: string[],
-    withShell: boolean,
-    defaultHandlers: boolean,
-  ): ChildProcess {
-    //N.B. this potentially will leave the correlator running - future work required...
-    if (this.child && !this.child.killed) {
-      this.logger.appendLine(this.name + " already started, stopping...");
-      this.child.kill("SIGKILL");
-    }
-
-    this.logger.appendLine("Starting " + this.name);
-    this.child = spawnCallback(this.command[0], [...this.command.slice(1), ...args], {
-      shell: withShell,
-      stdio: ["pipe", "pipe", "pipe"],
-    });
-
-    //Running with process Id
-    this.logger.appendLine(this.name + " started, PID:" + this.child.pid);
-
-    //Notify the logger if it stopped....
-    this.child.once("exit", (exitCode) =>
-      this.logger.appendLine(this.name + " stopped, exit code: " + exitCode),
-    );
-
-    if (defaultHandlers) {
-      this.child.stdout?.setEncoding("utf8");
-      this.child.stdout?.on("data", (data: string) => {
-        if (this.logger) {
-          this.logger.appendLine(data);
-        }
-      });
-    }
-
-    return this.child;
-  }
-
-  public stop(): Promise<void> {
-    return new Promise((resolve) => {
-      if (this.child && !this.child.killed) {
-        this.child.once("exit", () => {
-          resolve();
-        });
-
-        this.logger.appendLine("Process " + this.name + " stopping...");
-        this.child.kill("SIGINT");
-        const attemptedToKill = this.child;
-        setTimeout(() => {
-          if (!attemptedToKill.killed) {
-            this.logger.appendLine(
-              "Failed to stop shell in 5 seconds, killing...",
+    return await window.withProgress(
+      {
+        // since sometimes (e.g. apama_project) the command takes a long time, giving a progress notification is helpful
+        location: ProgressLocation.Notification,
+        title: `Running ${this.name}...`,
+        cancellable: false,
+      },
+      async () => 
+      {
+        try {
+          if (platform() === "win32") {
+            logger.debug("Running command: " + this.command[0] + " " + [...this.command.slice(1), ...args].join(" "));
+            return await execPromisify([...this.command, ...args].join(" "), { cwd: workingDir });
+          } else {
+            return await execFilePromisify(
+              this.command[0],
+              [...this.command.slice(1), ...args],
+              { cwd: workingDir }
             );
-            attemptedToKill.kill("SIGKILL");
           }
-        }, 5000);
-      } else {
-        resolve();
+        } catch (error) {
+          const enhancedError = new Error(
+            `Error running command '${this.command[0]} ${[...this.command.slice(1), ...args].join(" ")}': ${error}`
+          );
+          if (error instanceof Error) enhancedError.stack = error.stack;
+          throw enhancedError;
+        }
       }
-    });
+    );
   }
 }
+

src/extension.ts

@@ -1,6 +1,7 @@
 "use_strict";
 
 import * as path from "path";
+import { platform } from "os";
 
 import {
   ExtensionContext,
@@ -36,7 +37,7 @@ import { ExecutableResolver } from "./settings/ExecutableResolver";
 /** VSCode clients for talking to each language server, keyed by workspace folder URI */
 const servers = new Map<string, LanguageClient>();
 
-const logger = new Logger("Apama Extension Client");
+export const logger = new Logger("Apama Extension Client");
 
 /** The ExtensionContext.globalState */
 let globalState: Memento;
@@ -190,6 +191,7 @@ async function startLanguageServers(
     transport: TransportKind.stdio, // passes the "--stdio" option implicitly
     command: eplBuddyCommand.command,
     args: [...eplBuddyCommand.args, "-s"], // "-s" is just for backwards compat with pre-10.15.6.2 eplbuddy
+    options: {shell: platform() === "win32"}
   };
 
   const initializationOptions = {
@@ -218,15 +220,19 @@ async function startLanguageServers(
         workspaceFolder: folder,
       };
 
-      // TODO: we should really reload this if the APAMA_HOME config changes
       const languageClient = new LanguageClient(
         "apamaLanguageClient",
         `Apama Lang Server [${folder.name}]`,
         serverOptions,
         clientOptions,
       );
       servers.set(folder.uri.toString(), languageClient);
-      await languageClient.start();
+      try {
+        await languageClient.start();
+      } catch (e) {
+        logger.error(`Failed to start language server using "${serverOptions.command}" with args [${serverOptions.args}] : ${e}`);
+        throw e;
+      }
       serverVersion = languageClient.initializeResult?.serverInfo?.version;
     }