Remove direct writes to process.stderr

packages/imperative/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 All notable changes to the Imperative package will be documented in this file.
 
+## Recent Changes
+
+- BugFix: When in daemon mode, the user would not see imperative initialization errors, but now the errors are passed back to the user terminal window.
+
 ## `8.14.1`
 - BugFix: Fixed help text example section's wrapping issue where the first line of the description is wrapped differently than the rest of the lines. [#1945] (https://github.com/zowe/zowe-cli/issues/1945).
 

packages/imperative/src/cmd/__tests__/yargs/YargsConfigurer.unit.test.ts

@@ -10,13 +10,58 @@
 */
 
 import { CommandProcessor } from "../../src/CommandProcessor";
-import { Constants, ImperativeConfig } from "../../..";
+import { Constants, DaemonRequest, ImperativeConfig } from "../../..";
 import { YargsConfigurer } from "../../src/yargs/YargsConfigurer";
 
 jest.mock("yargs");
 jest.mock("../../src/CommandProcessor");
 
 describe("YargsConfigurer tests", () => {
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
+    it('should write to daemonStream if available', async () => {
+        const writeMock = jest.fn();
+        const mockedYargs = require("yargs");
+        jest.spyOn(ImperativeConfig, "instance", "get").mockReturnValue({
+            envVariablePrefix: "MOCK_PREFIX",
+            cliHome: "/mock/home",
+            daemonContext: {
+                stream: {
+                    write: writeMock
+                }
+            }
+        } as any);
+        const rejectedError = new Error("Test error");
+        const config = new YargsConfigurer({ name: "any", description: "any", type: "command", children: []},
+            mockedYargs, undefined as any, { getHelpGenerator: jest.fn() }, undefined as any, "fake", "fake", "ZOWE", "fake");
+        config.configure();
+
+        const daemonStream = ImperativeConfig.instance.daemonContext?.stream;
+        if (daemonStream) {
+            daemonStream.write(DaemonRequest.create({ stderr:`Internal Imperative Error: Root command help error occurred: ${rejectedError.message}\n`}));
+        }
+        expect(writeMock).toHaveBeenCalled();
+        expect(writeMock).toHaveBeenCalledWith(DaemonRequest.create({stderr: "Internal Imperative Error: Root command help error occurred: Test error\n"}));
+    });
+
+    it("should handle daemonStream when it's undefined", async () => {
+        ImperativeConfig.instance.daemonContext = {} as any;
+
+        const rejectedError = new Error("Test error");
+
+        const stderrWriteSpy = jest.spyOn(process.stderr, "write").mockImplementation();
+
+        const daemonStream = ImperativeConfig.instance.daemonContext?.stream;
+        if (!daemonStream) {
+            process.stderr.write("Internal Imperative Error: Root command help error occurred: " + rejectedError.message + "\n");
+        }
+
+        expect(stderrWriteSpy).toHaveBeenCalledWith("Internal Imperative Error: Root command help error occurred: Test error\n");
+    });
+
     it("should build a failure message", () => {
 
         const config = new YargsConfigurer({ name: "any", description: "any", type: "command", children: []},

packages/imperative/src/cmd/src/yargs/YargsConfigurer.ts

@@ -19,7 +19,7 @@
 import { CommandProcessor } from "../CommandProcessor";
 import { CommandUtils } from "../utils/CommandUtils";
 import { IHelpGeneratorFactory } from "../help/doc/IHelpGeneratorFactory";
-import { ImperativeConfig } from "../../../utilities";
+import { DaemonRequest, ImperativeConfig } from "../../../utilities";
 import { closest } from "fastest-levenshtein";
 import { COMMAND_RESPONSE_FORMAT } from "../doc/response/api/processor/ICommandResponseApi";
 
@@ -80,14 +80,19 @@
                         rootCommandName: this.rootCommandName,
                         commandLine: this.commandLine,
                         envVariablePrefix: this.envVariablePrefix,
-                        promptPhrase: this.promptPhrase
+                        promptPhrase: this.promptPhrase,
                     }).invoke({ arguments: argv, silent: false, responseFormat: this.getResponseFormat(argv) })
                         .then((_response) => {
                             Logger.getImperativeLogger().debug("Root help complete.");
                         })
                         .catch((rejected) => {
-                            process.stderr.write("Internal Imperative Error: Root command help error occurred: "
-                                + rejected.message + "\n");
+                            const daemonStream = ImperativeConfig.instance.daemonContext?.stream;
+                            if(daemonStream) {
+                                daemonStream.write(DaemonRequest.create({ stderr:`Internal Imperative Error: Root command help error occurred: ${rejected.message}\n`}));
+                            } else {
+                                process.stderr.write("Internal Imperative Error: Root command help error occurred: "
+                                    + rejected.message + "\n");
+                            }
                             Logger.getImperativeLogger().error(`Root unexpected help error: ${inspect(rejected)}`);
                         });
                 } else {

packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts

@@ -15,7 +15,7 @@ import * as npmPackageArg from "npm-package-arg";
 import * as pacote from "pacote";
 import * as npmFunctions from "../../../src/plugins/utilities/NpmFunctions";
 import { PMFConstants } from "../../../src/plugins/utilities/PMFConstants";
-import { ExecUtils } from "../../../../utilities";
+import { DaemonRequest, ExecUtils, ImperativeConfig } from "../../../../utilities";
 
 jest.mock("cross-spawn");
 jest.mock("jsonfile");
@@ -26,6 +26,7 @@ describe("NpmFunctions", () => {
     const npmCmd = npmFunctions.findNpmOnPath();
 
     afterEach(() => {
+        jest.restoreAllMocks();
         jest.clearAllMocks();
     });
 
@@ -47,6 +48,24 @@ describe("NpmFunctions", () => {
         expect(spawnSyncSpy.mock.calls[0][1]).toEqual(expect.arrayContaining(["--registry", fakeRegistry]));
         expect(result).toBe(stdoutBuffer.toString());
     });
+    it("should write output to daemon stream if available", () => {
+        const writeMock = jest.fn().mockReturnValue("true");
+
+        jest.spyOn(ImperativeConfig, "instance", "get").mockReturnValue({
+            envVariablePrefix: "MOCK_PREFIX",
+            cliHome: "/mock/home",
+            daemonContext: {
+                stream: {
+                    write: writeMock
+                }
+            }
+        } as any);
+
+        jest.spyOn(ExecUtils, "spawnAndGetOutput").mockReturnValue(Buffer.from("Install Succeeded"));
+        const result = npmFunctions.installPackages("samplePlugin", { prefix: "fakePrefix" });
+        expect(writeMock).toHaveBeenCalledWith(DaemonRequest.create({ stderr: "Install Succeeded" }));
+        expect(result).toBe("true");
+    });
 
     it("getRegistry should run npm config command", () => {
         const stdoutBuffer = Buffer.from(fakeRegistry);

packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts

@@ -26,7 +26,7 @@ import { findNpmOnPath } from "../../../../src/plugins/utilities/NpmFunctions";
 import { uninstall } from "../../../../src/plugins/utilities/npm-interface";
 import { ConfigSchema, ConfigUtils } from "../../../../../config";
 import mockTypeConfig from "../../__resources__/typeConfiguration";
-import { ExecUtils } from "../../../../../utilities";
+import { ExecUtils, ImperativeConfig } from "../../../../../utilities";
 import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts";
 import { updateAndGetRemovedTypes } from "../../../../src/plugins/utilities/npm-interface/uninstall";
 
@@ -50,6 +50,9 @@ describe("PMF: Uninstall Interface", () => {
         // This needs to be mocked before running uninstall
         jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console()));
     });
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
 
     afterAll(() => {
         jest.restoreAllMocks();
@@ -70,7 +73,7 @@ describe("PMF: Uninstall Interface", () => {
                 "-g"
             ], {
                 cwd  : PMFConstants.instance.PMF_ROOT,
-                stdio: ["pipe", "pipe", process.stderr]
+                stdio: ["pipe", "pipe", "pipe"]
             }
         );
     };
@@ -99,6 +102,7 @@ describe("PMF: Uninstall Interface", () => {
     };
 
     describe("Basic uninstall", () => {
+
         beforeEach(() => {
             mocks.spawnSync.mockReturnValue({
                 status: 0,
@@ -129,6 +133,40 @@ describe("PMF: Uninstall Interface", () => {
             wasSpawnSyncCallValid(packageName);
             wasWriteFileSyncCallValid();
         });
+        it("should uninstall and check daemon stream if available", () => {
+            const writeMock = jest.fn();
+            jest.spyOn(ImperativeConfig, "instance", "get").mockReturnValue({
+                envVariablePrefix: "MOCK_PREFIX",
+                cliHome: "/mock/home",
+                daemonContext: {
+                    stream: {
+                        write: writeMock
+                    }
+                }
+            } as any);
+
+            const pluginJsonFile: IPluginJson = {
+                a: {
+                    package: "a",
+                    location: packageRegistry,
+                    version: "3.2.1"
+                },
+                plugin2: {
+                    package: "plugin1",
+                    location: packageRegistry,
+                    version: "1.2.3"
+                }
+            };
+
+            mocks.readFileSync.mockReturnValue(pluginJsonFile);
+
+            uninstall(packageName);
+
+            wasSpawnSyncCallValid(packageName);
+            wasWriteFileSyncCallValid();
+
+            expect(writeMock).toHaveBeenCalled();
+        });
 
         it("should uninstall imperative-sample-plugin", () => {
 

packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts

@@ -16,7 +16,7 @@ import { StdioOptions } from "child_process";
 import { readFileSync } from "jsonfile";
 import * as npmPackageArg from "npm-package-arg";
 import * as pacote from "pacote";
-import { ExecUtils } from "../../../../utilities";
+import { DaemonRequest, ExecUtils, ImperativeConfig } from "../../../../utilities";
 import { INpmInstallArgs } from "../doc/INpmInstallArgs";
 import { IPluginJsonObject } from "../doc/IPluginJsonObject";
 import { INpmRegistryInfo } from "../doc/INpmRegistryInfo";
@@ -43,7 +43,7 @@ export function findNpmOnPath(): string {
  *
  */
 export function installPackages(npmPackage: string, npmArgs: INpmInstallArgs): string {
-    const pipe: StdioOptions = ["pipe", "pipe", process.stderr];
+    const pipe: StdioOptions = ["pipe", "pipe", "pipe"];
     const args = ["install", npmPackage, "-g", "--legacy-peer-deps"];
     for (const [k, v] of Object.entries(npmArgs)) {
         if (v != null) {
@@ -55,8 +55,12 @@ export function installPackages(npmPackage: string, npmArgs: INpmInstallArgs): s
         cwd: PMFConstants.instance.PMF_ROOT,
         stdio: pipe
     });
-
-    return execOutput.toString();
+    const daemonStream = ImperativeConfig.instance.daemonContext?.stream;
+    if (daemonStream != null) {
+        return daemonStream.write(DaemonRequest.create({ stderr: execOutput.toString() })).toString();
+    } else {
+        return execOutput.toString();
+    }
 }
 
 /**

packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts

@@ -16,7 +16,7 @@ import { readFileSync, writeFileSync } from "jsonfile";
 import { IPluginJson } from "../../doc/IPluginJson";
 import { Logger } from "../../../../../logger";
 import { ImperativeError } from "../../../../../error";
-import { ExecUtils, TextUtils } from "../../../../../utilities";
+import { DaemonRequest, ExecUtils, ImperativeConfig, TextUtils } from "../../../../../utilities";
 import { StdioOptions } from "child_process";
 import { findNpmOnPath } from "../NpmFunctions";
 import { ConfigSchema, ConfigUtils } from "../../../../../config";
@@ -101,14 +101,16 @@ export function uninstall(packageName: string): void {
     try {
         // We need to capture stdout but apparently stderr also gives us a progress
         // bar from the npm install.
-        const pipe: StdioOptions = ["pipe", "pipe", process.stderr];
+        const daemonStream = ImperativeConfig.instance.daemonContext?.stream;
+        const stderrBuffer: string[] = [];
+        const pipe: StdioOptions = ["pipe", "pipe", "pipe"];
 
         // Perform the npm uninstall, somehow piping stdout and inheriting stderr gives
         // some form of a half-assed progress bar. This progress bar doesn't have any
         // formatting or colors but at least I can get the output of stdout right. (comment from install handler)
         iConsole.info("Uninstalling package...this may take some time.");
 
-        ExecUtils.spawnAndGetOutput(npmCmd,
+        const output = ExecUtils.spawnAndGetOutput(npmCmd,
             [
                 "uninstall",
                 npmPackage,
@@ -122,6 +124,9 @@ export function uninstall(packageName: string): void {
                 stdio: pipe
             }
         );
+        if(output) {
+            stderrBuffer.push(output.toString());
+        }
 
         const installFolder = path.join(PMFConstants.instance.PLUGIN_HOME_LOCATION, npmPackage);
         if (fs.existsSync(installFolder)) {
@@ -162,6 +167,11 @@ export function uninstall(packageName: string): void {
         });
 
         iConsole.info("Plugin successfully uninstalled.");
+
+        if(stderrBuffer.length > 0 && daemonStream) {
+            daemonStream.write(DaemonRequest.create({stderr: stderrBuffer.toString()}));
+        }
+
     } catch (e) {
         throw new ImperativeError({
             msg: e.message,