Merge pull request #3064 from zowe/fix/show-config-error

Fix JSON errors are ignored when config files change on disk
zowe · Aug 27, 2024 · a377226 · a377226
2 parents 2de66d0 + 464c086
commit a377226
Show file tree

Hide file tree

Showing 11 changed files with 125 additions and 121 deletions.
diff --git a/package.json b/package.json
@@ -16,7 +16,7 @@
     "vscode": "^1.53.2"
   },
   "dependencies": {
-    "@zowe/cli": "^7.28.4",
+    "@zowe/cli": "^7.29.1",
     "vscode-nls": "^4.1.2"
   },
   "devDependencies": {

diff --git a/packages/zowe-explorer-api/CHANGELOG.md b/packages/zowe-explorer-api/CHANGELOG.md
@@ -10,6 +10,7 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t
 
 - Fixed an issue where the `ZoweVsCodeExtension.updateCredentials` method could remove credentials from session when input prompt was cancelled. [#3018](https://github.com/zowe/zowe-explorer-vscode/pull/3018)
 - Updated the `@zowe/cli` dependency to fix error when using session with auth type "none". [zowe-cli#2219](https://github.com/zowe/zowe-cli/issues/2219)
+- Fixed errors being logged silently rather than thrown in `ProfilesCache.refresh` method. [#3066](https://github.com/zowe/zowe-explorer-vscode/issues/3066)
 
 ## `2.17.0`
 

diff --git a/packages/zowe-explorer-api/__tests__/__unit__/profiles/ProfilesCache.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/profiles/ProfilesCache.unit.test.ts
@@ -256,25 +256,23 @@ describe("ProfilesCache", () => {
     });
 
     describe("refresh", () => {
-        const mockLogError = jest.fn();
         const profileTypes = ["zosmf", "zftp"];
         const fakeApiRegister = {
             registeredApiTypes: jest.fn().mockReturnValue(profileTypes),
         };
 
         it("should refresh profile data for multiple profile types", async () => {
-            const profCache = new ProfilesCache({ ...fakeLogger, error: mockLogError } as unknown as zowe.imperative.Logger);
+            const profCache = new ProfilesCache(fakeLogger as unknown as zowe.imperative.Logger);
             jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(createProfInfoMock([lpar1Profile, zftpProfile]));
             await profCache.refresh(fakeApiRegister as unknown as ZoweExplorerApi.IApiRegisterClient);
             expect(profCache.allProfiles.length).toEqual(2);
             expect(profCache.allProfiles[0]).toMatchObject(lpar1Profile);
             expect(profCache.allProfiles[1]).toMatchObject(zftpProfile);
             expect(profCache.getAllTypes()).toEqual([...profileTypes, "base"]);
-            expect(mockLogError).not.toHaveBeenCalled();
         });
 
         it("should refresh profile data for and merge tokens with base profile", async () => {
-            const profCache = new ProfilesCache({ ...fakeLogger, error: mockLogError } as unknown as zowe.imperative.Logger);
+            const profCache = new ProfilesCache(fakeLogger as unknown as zowe.imperative.Logger);
             jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(
                 createProfInfoMock([lpar1ProfileWithToken, lpar2ProfileWithToken, baseProfileWithToken])
             );
@@ -284,23 +282,21 @@ describe("ProfilesCache", () => {
             expect(profCache.allProfiles[1]).toMatchObject(lpar2Profile); // without token
             expect(profCache.allProfiles[2]).toMatchObject(baseProfileWithToken);
             expect(profCache.getAllTypes()).toEqual([...profileTypes, "base"]);
-            expect(mockLogError).not.toHaveBeenCalled();
         });
 
         it("should handle error when refreshing profile data", async () => {
             const fakeError = "Profile IO Error";
-            const profCache = new ProfilesCache({ ...fakeLogger, error: mockLogError } as unknown as zowe.imperative.Logger);
+            const profCache = new ProfilesCache(fakeLogger as unknown as zowe.imperative.Logger);
             jest.spyOn(profCache, "getProfileInfo").mockImplementation(() => {
                 throw fakeError;
             });
-            await profCache.refresh(fakeApiRegister as unknown as ZoweExplorerApi.IApiRegisterClient);
+            await expect(profCache.refresh(fakeApiRegister as unknown as ZoweExplorerApi.IApiRegisterClient)).rejects.toBe(fakeError);
             expect(profCache.allProfiles.length).toEqual(0);
             expect(profCache.getAllTypes().length).toEqual(0);
-            expect(mockLogError).toHaveBeenCalledWith(fakeError);
         });
 
         it("should remove old types from profilesByType and defaultProfileByType maps when reloading profiles", async () => {
-            const profCache = new ProfilesCache({ ...fakeLogger, error: mockLogError } as unknown as zowe.imperative.Logger);
+            const profCache = new ProfilesCache(fakeLogger as unknown as zowe.imperative.Logger);
             jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(createProfInfoMock([]));
             (profCache as any).profilesByType.set("base", { name: "someProf" as any });
             (profCache as any).defaultProfileByType.set("base", { name: "someProf" as any });
@@ -312,7 +308,6 @@ describe("ProfilesCache", () => {
             expect((profCache as any).defaultProfileByType.size).toBe(0);
             expect((profCache as any).allProfiles.length).toBe(0);
             expect((profCache as any).allTypes).toEqual(["base"]);
-            expect(mockLogError).not.toHaveBeenCalled();
         });
     });
 

diff --git a/packages/zowe-explorer-api/package.json b/packages/zowe-explorer-api/package.json
@@ -22,7 +22,7 @@
   },
   "dependencies": {
     "@types/vscode": "^1.53.2",
-    "@zowe/cli": "^7.28.4",
+    "@zowe/cli": "^7.29.1",
     "@zowe/secrets-for-zowe-sdk": "^7.18.6",
     "mustache": "^4.2.0",
     "semver": "^7.5.3"

diff --git a/packages/zowe-explorer-api/src/profiles/ProfilesCache.ts b/packages/zowe-explorer-api/src/profiles/ProfilesCache.ts
@@ -193,45 +193,40 @@ export class ProfilesCache {
 
     public async refresh(apiRegister?: ZoweExplorerApi.IApiRegisterClient): Promise<void> {
         const allProfiles: zowe.imperative.IProfileLoaded[] = [];
-        let mProfileInfo: zowe.imperative.ProfileInfo;
-        try {
-            mProfileInfo = await this.getProfileInfo();
-            const allTypes = this.getAllProfileTypes(apiRegister?.registeredApiTypes() ?? []);
-            allTypes.push("base");
-            for (const type of allTypes) {
-                const tmpAllProfiles: zowe.imperative.IProfileLoaded[] = [];
-                // Step 1: Get all profiles for each registered type
-                const profilesForType = mProfileInfo.getAllProfiles(type).filter((temp) => temp.profLoc.osLoc.length !== 0);
-                if (profilesForType && profilesForType.length > 0) {
-                    for (const prof of profilesForType) {
-                        // Step 2: Merge args for each profile
-                        const profAttr = this.getMergedAttrs(mProfileInfo, prof);
-                        // Work-around. TODO: Discuss with imperative team
-                        const profileFix = this.getProfileLoaded(prof.profName, prof.profType, profAttr);
-                        // set default for type
-                        if (prof.isDefaultProfile) {
-                            this.defaultProfileByType.set(type, profileFix);
-                        }
-
-                        // Step 3: Update allProfiles list
-                        tmpAllProfiles.push(profileFix);
+        const mProfileInfo = await this.getProfileInfo();
+        const allTypes = this.getAllProfileTypes(apiRegister?.registeredApiTypes() ?? []);
+        allTypes.push("base");
+        for (const type of allTypes) {
+            const tmpAllProfiles: zowe.imperative.IProfileLoaded[] = [];
+            // Step 1: Get all profiles for each registered type
+            const profilesForType = mProfileInfo.getAllProfiles(type).filter((temp) => temp.profLoc.osLoc.length !== 0);
+            if (profilesForType && profilesForType.length > 0) {
+                for (const prof of profilesForType) {
+                    // Step 2: Merge args for each profile
+                    const profAttr = this.getMergedAttrs(mProfileInfo, prof);
+                    // Work-around. TODO: Discuss with imperative team
+                    const profileFix = this.getProfileLoaded(prof.profName, prof.profType, profAttr);
+                    // set default for type
+                    if (prof.isDefaultProfile) {
+                        this.defaultProfileByType.set(type, profileFix);
                     }
-                    allProfiles.push(...tmpAllProfiles);
-                    this.profilesByType.set(type, tmpAllProfiles);
+
+                    // Step 3: Update allProfiles list
+                    tmpAllProfiles.push(profileFix);
                 }
+                allProfiles.push(...tmpAllProfiles);
+                this.profilesByType.set(type, tmpAllProfiles);
             }
-            this.allProfiles = allProfiles;
-            this.allTypes = allTypes;
-            for (const oldType of [...this.profilesByType.keys()].filter((type) => !allProfiles.some((prof) => prof.type === type))) {
-                this.profilesByType.delete(oldType);
-                this.defaultProfileByType.delete(oldType);
-            }
-            // check for proper merging of apiml tokens
-            this.checkMergingConfigAllProfiles();
-            this.profilesForValidation = [];
-        } catch (error) {
-            this.log.error(error as string);
         }
+        this.allProfiles = allProfiles;
+        this.allTypes = allTypes;
+        for (const oldType of [...this.profilesByType.keys()].filter((type) => !allProfiles.some((prof) => prof.type === type))) {
+            this.profilesByType.delete(oldType);
+            this.defaultProfileByType.delete(oldType);
+        }
+        // check for proper merging of apiml tokens
+        this.checkMergingConfigAllProfiles();
+        this.profilesForValidation = [];
     }
 
     public validateAndParseUrl(newUrl: string): IUrlValidator {

diff --git a/packages/zowe-explorer/CHANGELOG.md b/packages/zowe-explorer/CHANGELOG.md
@@ -12,6 +12,7 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen
 
 - Fixed issue where creating a new team configuration file could cause Zowe Explorer to crash, resulting in all sessions disappearing from trees. [#2906](https://github.com/zowe/zowe-explorer-vscode/issues/2906)
 - Fixed data set not opening when the token has expired. [#3001](https://github.com/zowe/zowe-explorer-vscode/issues/3001)
+- Fixed JSON errors being ignored when `zowe.config.json` files change on disk and are reloaded. [#3066](https://github.com/zowe/zowe-explorer-vscode/issues/3066)
 
 ## `2.17.0`
 

diff --git a/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweCommandProvider.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/abstract/ZoweCommandProvider.unit.test.ts
@@ -10,7 +10,7 @@
  */
 
 import { ZoweCommandProvider } from "../../../src/abstract/ZoweCommandProvider";
-import { ZoweTreeNode } from "@zowe/zowe-explorer-api";
+import { ProfilesCache, ZoweTreeNode } from "@zowe/zowe-explorer-api";
 import * as vscode from "vscode";
 import { Profiles } from "../../../src/Profiles";
 import { ZoweDatasetNode } from "../../../src/dataset/ZoweDatasetNode";
@@ -41,7 +41,7 @@ describe("ZoweCommandProvider Unit Tests", () => {
     });
 });
 
-describe("ZoweCommandProvide Unit Tests - function checkCurrentProfile", () => {
+describe("ZoweCommandProvider Unit Tests - function checkCurrentProfile", () => {
     const testNode = new ZoweDatasetNode({
         label: "test",
         collapsibleState: vscode.TreeItemCollapsibleState.None,
@@ -51,13 +51,14 @@ describe("ZoweCommandProvide Unit Tests - function checkCurrentProfile", () => {
     testNode.contextValue = "session server";
 
     beforeEach(async () => {
-        void Profiles.createInstance(undefined);
-        Object.defineProperty(Profiles.getInstance(), "log", {
+        jest.spyOn(ProfilesCache.prototype, "refresh").mockImplementation();
+        const profilesInstance = await Profiles.createInstance(undefined);
+        Object.defineProperty(profilesInstance, "log", {
             value: {
                 error: jest.fn(),
             },
         });
-        Object.defineProperty(Profiles.getInstance(), "allProfiles", {
+        Object.defineProperty(profilesInstance, "allProfiles", {
             value: [{ name: "sestest" }, { name: "profile1" }, { name: "profile2" }],
             configurable: true,
         });

diff --git a/packages/zowe-explorer/src/Profiles.ts b/packages/zowe-explorer/src/Profiles.ts
@@ -88,7 +88,11 @@ export class Profiles extends ProfilesCache {
      */
     public async getProfileInfo(): Promise<zowe.imperative.ProfileInfo> {
         ZoweLogger.trace("Profiles.getProfileInfo called.");
-        this.mProfileInfo = await super.getProfileInfo();
+        if (this.mProfileInfo == null) {
+            this.mProfileInfo = await super.getProfileInfo();
+            // Cache profile info object until current thread is done executing
+            setImmediate(() => (this.mProfileInfo = null));
+        }
         return this.mProfileInfo;
     }
 

diff --git a/packages/zowe-explorer/src/shared/refresh.ts b/packages/zowe-explorer/src/shared/refresh.ts
@@ -19,6 +19,7 @@ import { removeSession } from "../utils/SessionUtils";
 import { ZoweLogger } from "../utils/LoggerUtils";
 import { TreeProviders } from "./TreeProviders";
 import { TreeViewUtils } from "../utils/TreeViewUtils";
+import { ZoweExplorerExtender } from "../ZoweExplorerExtender";
 
 /**
  * View (DATA SETS, JOBS, USS) refresh button
@@ -34,7 +35,13 @@ export async function refreshAll(treeProvider?: IZoweTree<IZoweTreeNode>): Promi
         }
         return;
     }
-    await Profiles.getInstance().refresh(ZoweExplorerApiRegister.getInstance());
+    try {
+        await Profiles.getInstance().refresh(ZoweExplorerApiRegister.getInstance());
+    } catch (err) {
+        ZoweLogger.error(err);
+        ZoweExplorerExtender.showZoweConfigError(err.message);
+        return;
+    }
     for (const sessNode of treeProvider.mSessionNodes) {
         const profiles = await Profiles.getInstance().fetchAllProfiles();
         const found = profiles.some((prof) => prof.name === sessNode.label.toString().trim());

diff --git a/samples/uss-profile-sample/package.json b/samples/uss-profile-sample/package.json
@@ -33,7 +33,7 @@
     "watch": "tsc -watch -p ./"
   },
   "dependencies": {
-    "@zowe/cli": "^7.28.4",
+    "@zowe/cli": "^7.29.1",
     "@zowe/zowe-explorer-api": "file:../../packages/zowe-explorer-api",
     "ssh2-sftp-client": "^9.1.0"
   },