Skip to content

Commit

Permalink
Merge pull request #3075 from zowe/fix/show-config-error-2
Browse files Browse the repository at this point in the history 
Fix ZE activation and commands failing when team config contains JSON error
  • Loading branch information
@traeok
traeok authored Aug 29, 2024
2 parents bde15c6 + 843049f commit dbf9a74
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 39 deletions.
2 changes: 1 addition & 1 deletion packages/zowe-explorer/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +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)
- 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) [#3074](https://github.com/zowe/zowe-explorer-vscode/issues/3074)

## `2.17.0`

Expand Down
2 changes: 2 additions & 0 deletions packages/zowe-explorer/__mocks__/vscode.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -809,3 +809,5 @@ export enum DiagnosticSeverity {
Information = 2,
Hint = 3,
}

export class Selection {}
81 changes: 70 additions & 11 deletions packages/zowe-explorer/__tests__/__unit__/Profiles.extended.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@ async function createGlobalMocks() {
Object.defineProperty(ZoweLogger, "trace", { value: jest.fn(), configurable: true });

newMocks.mockProfileInstance = new Profiles(newMocks.log);
Object.defineProperty(Profiles, "CreateInstance", {
value: () => newMocks.mockProfileInstance,
configurable: true,
});
Object.defineProperty(Profiles, "getInstance", {
value: () => newMocks.mockProfileInstance,
configurable: true,
Expand Down Expand Up @@ -250,6 +246,17 @@ describe("Profiles Unit Test - Function createInstance", () => {
expect(mockWorkspaceFolders).toHaveBeenCalledTimes(1);
expect(profilesInstance.cwd).toBe("fakePath");
});

it("Tests that createInstance catches error and logs it", async () => {
const globalMocks = await createGlobalMocks();
jest.spyOn(Profiles.prototype, "refresh").mockRejectedValueOnce(new Error("test error"));
jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("");
const errorSpy = jest.spyOn(ZoweLogger, "error");
await expect(Profiles.createInstance(globalMocks.log)).resolves.not.toThrow();
expect(errorSpy).toBeCalledTimes(1);
expect(errorSpy).toBeCalledWith(Error("test error"));
errorSpy.mockClear();
});
});

describe("Profiles Unit Tests - Function createNewConnection for v1 Profiles", () => {
Expand Down Expand Up @@ -583,6 +590,7 @@ describe("Profiles Unit Tests - Function createZoweSession", () => {
jest.spyOn(Gui, "resolveQuickPick").mockResolvedValueOnce(new utils.FilterDescriptor("Test1"));
jest.spyOn(Gui, "resolveQuickPick").mockResolvedValueOnce(new utils.FilterDescriptor("Test2"));
jest.spyOn(Profiles.getInstance(), "getProfileInfo").mockResolvedValue({
getAllProfiles: jest.fn().mockReturnValue([]),
usingTeamConfig: false,
} as any);
jest.spyOn(Gui, "showInputBox").mockResolvedValue("test");
Expand Down Expand Up @@ -617,7 +625,7 @@ describe("Profiles Unit Tests - Function createZoweSession", () => {
spyInfo.mockClear();
});

it("Tests that createZoweSession catches error and log warning", async () => {
it("Tests that createZoweSession catches error and logs it", async () => {
const globalMocks = await createGlobalMocks();
jest.spyOn(Gui, "createQuickPick").mockReturnValue({
show: jest.fn(),
Expand All @@ -626,11 +634,12 @@ describe("Profiles Unit Tests - Function createZoweSession", () => {
} as any);
jest.spyOn(Gui, "resolveQuickPick").mockResolvedValueOnce(new utils.FilterDescriptor("Test"));
jest.spyOn(Profiles.getInstance(), "getProfileInfo").mockRejectedValueOnce(new Error("test error"));
const warnSpy = jest.spyOn(ZoweLogger, "warn");
jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("");
const errorSpy = jest.spyOn(ZoweLogger, "error");
await expect(Profiles.getInstance().createZoweSession(globalMocks.testUSSTree)).resolves.not.toThrow();
expect(warnSpy).toBeCalledTimes(1);
expect(warnSpy).toBeCalledWith(Error("test error"));
warnSpy.mockClear();
expect(errorSpy).toBeCalledTimes(1);
expect(errorSpy).toBeCalledWith(Error("test error"));
errorSpy.mockClear();
});
});

Expand Down Expand Up @@ -1099,6 +1108,17 @@ describe("Profiles Unit Tests - function promptCredentials", () => {
jest.spyOn(Profiles.getInstance(), "updateProfilesArrays").mockImplementation();
await expect(Profiles.getInstance().promptCredentials("secure_config_props")).resolves.toEqual(["test", "12345", "encodedAuth"]);
});

it("Tests that promptCredentials catches error and logs it", async () => {
const globalMocks = await createGlobalMocks();
jest.spyOn(Profiles.getInstance(), "getProfileInfo").mockRejectedValueOnce(new Error("test error"));
jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("");
const errorSpy = jest.spyOn(ZoweLogger, "error");
await expect(Profiles.getInstance().promptCredentials(globalMocks.testProfile)).resolves.not.toThrow();
expect(errorSpy).toBeCalledTimes(1);
expect(errorSpy).toBeCalledWith(Error("test error"));
errorSpy.mockClear();
});
});

describe("Profiles Unit Tests - function getDeleteProfile", () => {
Expand Down Expand Up @@ -1342,6 +1362,24 @@ describe("Profiles Unit Tests - function deleteProfile", () => {

await expect(Profiles.getInstance().deleteProfile(datasetTree, ussTree, jobsTree, testNode)).resolves.not.toThrow();
});

it("Tests that deleteProfile catches error and logs it", async () => {
const globalMocks = await createGlobalMocks();
const datasetSessionNode = createDatasetSessionNode(globalMocks.testSession, globalMocks.testProfile);
const datasetTree = createDatasetTree(datasetSessionNode, globalMocks.testProfile);
const ussSessionNode = [createUSSSessionNode(globalMocks.testSession, globalMocks.testProfile)];
const ussTree = createUSSTree([], ussSessionNode);
const jobsTree = createJobsTree(globalMocks.testSession, createIJobObject(), globalMocks.testProfile, createTreeView());

jest.spyOn(Profiles.getInstance(), "getProfileInfo").mockRejectedValueOnce(new Error("test error"));
jest.spyOn(Profiles.getInstance(), "getDeleteProfile").mockResolvedValue(globalMocks.testProfile);
jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("");
const errorSpy = jest.spyOn(ZoweLogger, "error");
await expect(Profiles.getInstance().deleteProfile(datasetTree, ussTree, jobsTree)).resolves.not.toThrow();
expect(errorSpy).toBeCalledTimes(1);
expect(errorSpy).toBeCalledWith(Error("test error"));
errorSpy.mockClear();
});
});

describe("Profiles Unit Tests - function checkCurrentProfile", () => {
Expand Down Expand Up @@ -1415,13 +1453,23 @@ describe("Profiles Unit Tests - function checkCurrentProfile", () => {
setupProfilesCheck(globalMocks);
await expect(Profiles.getInstance().checkCurrentProfile(globalMocks.testProfile)).resolves.toEqual({ name: "sestest", status: "inactive" });
});
it("should throw an error if using token auth and is logged out or has expired token", async () => {
it("should show as unverified if using token auth and is logged out or has expired token", async () => {
const globalMocks = await createGlobalMocks();
jest.spyOn(utils, "errorHandling").mockImplementation();
jest.spyOn(utils.ProfilesUtils, "isUsingTokenAuth").mockResolvedValue(true);
jest.spyOn(utils.ProfilesUtils, "isUsingTokenAuth").mockResolvedValueOnce(true);
setupProfilesCheck(globalMocks);
await expect(Profiles.getInstance().checkCurrentProfile(globalMocks.testProfile)).resolves.toEqual({ name: "sestest", status: "unverified" });
});
it("should show as unverified if profiles fail to load", async () => {
const globalMocks = await createGlobalMocks();
jest.spyOn(Profiles.getInstance(), "getProfileInfo").mockRejectedValueOnce(new Error("test error"));
jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("");
const errorSpy = jest.spyOn(ZoweLogger, "error");
await expect(Profiles.getInstance().checkCurrentProfile(globalMocks.testProfile)).resolves.toEqual({ name: "sestest", status: "unverified" });
expect(errorSpy).toBeCalledTimes(1);
expect(errorSpy).toBeCalledWith(Error("test error"));
errorSpy.mockClear();
});
});

describe("Profiles Unit Tests - function editSession", () => {
Expand Down Expand Up @@ -1491,6 +1539,17 @@ describe("Profiles Unit Tests - function editSession", () => {
base64EncodedAuth: "base64Auth",
});
});

it("Tests that editSession catches error and logs it", async () => {
const globalMocks = await createGlobalMocks();
jest.spyOn(Profiles.getInstance(), "getProfileInfo").mockRejectedValueOnce(new Error("test error"));
jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("");
const errorSpy = jest.spyOn(ZoweLogger, "error");
await expect(Profiles.getInstance().editSession(globalMocks.testProfile, globalMocks.testProfile.name)).resolves.not.toThrow();
expect(errorSpy).toBeCalledTimes(1);
expect(errorSpy).toBeCalledWith(Error("test error"));
errorSpy.mockClear();
});
});

describe("Profiles Unit Tests - function getProfileSetting", () => {
Expand Down
20 changes: 13 additions & 7 deletions packages/zowe-explorer/__tests__/__unit__/ZoweExplorerExtender.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ describe("ZoweExplorerExtender unit tests", () => {
ZoweExplorerExtender.createInstance();

Object.defineProperty(vscode.Uri, "file", { value: jest.fn(), configurable: true });
Object.defineProperty(Gui, "showTextDocument", { value: jest.fn(), configurable: true });
const showTextDocumentSpy = jest.spyOn(Gui, "showTextDocument").mockResolvedValue({} as any);

const zoweDir = getZoweDir();
const userInputs = [
Expand All @@ -167,16 +167,17 @@ describe("ZoweExplorerExtender unit tests", () => {
},
{
choice: "Show Config",
configError: `Error parsing JSON in the file '${path.join(zoweDir, "zowe.config.user.json")}'`,
configError: `Error parsing JSON in the file '${path.join(zoweDir, "zowe.config.user.json")}' at Line 4, Column 0`,
shouldFail: false,
shouldNavigate: true,
fileChecks: ["zowe.config.user.json"],
mockExistsSync: blockMocks.mockExistsSync.mockImplementationOnce,
},
{
choice: "Show Config",
configError: "Error parsing JSON",
fileChecks: ["zowe.config.user.json", "zowe.config.json"],
shouldFail: false,
shouldFail: true,
mockExistsSync: blockMocks.mockExistsSync.mockImplementationOnce,
},
{
Expand All @@ -188,18 +189,20 @@ describe("ZoweExplorerExtender unit tests", () => {
},
];
for (const userInput of userInputs) {
showTextDocumentSpy.mockClear();
blockMocks.mockErrorMessage.mockImplementationOnce((_msg, ..._items) => Promise.resolve(userInput.choice));
if (userInput.fileChecks.length > 1) {
userInput.mockExistsSync((_path) => false);
}
await ZoweExplorerExtender.showZoweConfigError(userInput.configError);
ZoweExplorerExtender.showZoweConfigError(userInput.configError);
await new Promise<void>((resolve) => process.nextTick(() => resolve()));
expect(blockMocks.mockErrorMessage).toHaveBeenCalledWith(
'Error encountered when loading your Zowe config. Click "Show Config" for more details.',
undefined,
"Show Config"
);
if (userInput.choice == null) {
expect(Gui.showTextDocument).not.toHaveBeenCalled();
expect(showTextDocumentSpy).not.toHaveBeenCalled();
} else {
if (userInput.v1) {
expect(vscode.Uri.file).toHaveBeenCalledWith(path.join(zoweDir, "profiles", "exampleType", "exampleType_meta.yaml"));
Expand All @@ -209,9 +212,12 @@ describe("ZoweExplorerExtender unit tests", () => {
}
}
if (userInput.shouldFail) {
expect(Gui.showTextDocument).not.toHaveBeenCalled();
expect(showTextDocumentSpy).not.toHaveBeenCalled();
} else {
expect(Gui.showTextDocument).toHaveBeenCalled();
expect(showTextDocumentSpy).toHaveBeenCalled();
if (userInput.shouldNavigate) {
expect((await showTextDocumentSpy.mock.results[0].value).selection).toBeDefined();
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/zowe-explorer/__tests__/__unit__/utils/ProfilesUtils.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ describe("ProfilesUtils unit tests", () => {
expect(promptAndDisableCredentialManagementSpy).toHaveBeenCalledTimes(1);
});

it("should throw new error if error is not an instance of ProfInfoErr", async () => {
it("should ignore error if it is not an instance of ProfInfoErr", async () => {
const expectedErrorMsg = "Another error unrelated to credential management";
getDirectValueSpy.mockReturnValueOnce(false);
getCredentialManagerOverrideSpy.mockReturnValue("@zowe/cli");
Expand All @@ -836,7 +836,7 @@ describe("ProfilesUtils unit tests", () => {
readProfilesFromDiskSpy.mockImplementation(() => {
throw new Error(expectedErrorMsg);
});
await expect(profUtils.ProfilesUtils.getProfileInfo(false)).rejects.toThrow(expectedErrorMsg);
await expect(profUtils.ProfilesUtils.getProfileInfo(false)).resolves.not.toThrow();
expect(promptAndDisableCredentialManagementSpy).toHaveBeenCalledTimes(0);
});
});
Expand Down
61 changes: 50 additions & 11 deletions packages/zowe-explorer/src/Profiles.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,12 @@ export class Profiles extends ProfilesCache {
// Processing stops if there are no profiles detected
public static async createInstance(log: zowe.imperative.Logger): Promise<Profiles> {
Profiles.loader = new Profiles(log, vscode.workspace.workspaceFolders?.[0]?.uri.fsPath);
await Profiles.loader.refresh(ZoweExplorerApiRegister.getInstance());
try {
await Profiles.loader.refresh(ZoweExplorerApiRegister.getInstance());
} catch (err) {
ZoweLogger.error(err);
ZoweExplorerExtender.showZoweConfigError(err.message);
}
return Profiles.loader;
}

Expand Down Expand Up @@ -99,7 +104,14 @@ export class Profiles extends ProfilesCache {
public async checkCurrentProfile(theProfile: zowe.imperative.IProfileLoaded): Promise<IProfileValidation> {
ZoweLogger.trace("Profiles.checkCurrentProfile called.");
let profileStatus: IProfileValidation;
const usingTokenAuth = await ProfilesUtils.isUsingTokenAuth(theProfile.name);
let usingTokenAuth: boolean;
try {
usingTokenAuth = await ProfilesUtils.isUsingTokenAuth(theProfile.name);
} catch (err) {
ZoweLogger.error(err);
ZoweExplorerExtender.showZoweConfigError(err.message);
return { name: theProfile.name, status: "unverified" };
}

if (usingTokenAuth && !theProfile.profile.tokenType) {
const error = new zowe.imperative.ImperativeError({
Expand Down Expand Up @@ -337,13 +349,15 @@ export class Profiles extends ProfilesCache {
let mProfileInfo: zowe.imperative.ProfileInfo;
try {
mProfileInfo = await this.getProfileInfo();
const profAllAttrs = mProfileInfo.getAllProfiles();
for (const pName of profileNamesList) {
const osLocInfo = mProfileInfo.getOsLocInfo(profAllAttrs.find((p) => p.profName === pName));
items.push(new FilterItem({ text: pName, icon: this.getProfileIcon(osLocInfo)[0] }));
}
} catch (err) {
ZoweLogger.warn(err);
ZoweLogger.error(err);
ZoweExplorerExtender.showZoweConfigError(err.message);
return;
}
const profAllAttrs = mProfileInfo.getAllProfiles();
for (const pName of profileNamesList) {
const osLocInfo = mProfileInfo.getOsLocInfo(profAllAttrs.find((p) => p.profName === pName));
items.push(new FilterItem({ text: pName, icon: this.getProfileIcon(osLocInfo)[0] }));
}

const quickpick = Gui.createQuickPick();
Expand Down Expand Up @@ -406,6 +420,7 @@ export class Profiles extends ProfilesCache {
} catch (error) {
ZoweLogger.error(error);
ZoweExplorerExtender.showZoweConfigError(error.message);
return;
}
if (config.usingTeamConfig) {
const profiles = config.getAllProfiles();
Expand Down Expand Up @@ -468,7 +483,15 @@ export class Profiles extends ProfilesCache {

public async editSession(profileLoaded: zowe.imperative.IProfileLoaded, profileName: string): Promise<any | undefined> {
ZoweLogger.trace("Profiles.editSession called.");
if ((await this.getProfileInfo()).usingTeamConfig) {
let usingTeamConfig: boolean;
try {
usingTeamConfig = (await this.getProfileInfo()).usingTeamConfig;
} catch (err) {
ZoweLogger.error(err);
ZoweExplorerExtender.showZoweConfigError(err.message);
return;
}
if (usingTeamConfig) {
const currentProfile = await this.getProfileFromConfig(profileLoaded.name);
const filePath = currentProfile.profLoc.osLoc[0];
await this.openConfigFile(filePath);
Expand Down Expand Up @@ -926,13 +949,21 @@ export class Profiles extends ProfilesCache {
placeHolder: localize("promptCredentials.passwordInputBoxOptions.placeholder", "Password"),
prompt: localize("promptCredentials.passwordInputBoxOptions.prompt", "Enter the password for the connection. Leave blank to not store."),
};
let mProfileInfo: zowe.imperative.ProfileInfo;
try {
mProfileInfo = await this.getProfileInfo();
} catch (err) {
ZoweLogger.error(err);
ZoweExplorerExtender.showZoweConfigError(err.message);
return;
}

const promptInfo = await ZoweVsCodeExtension.updateCredentials(
{
profile: typeof profile === "string" ? undefined : profile,
sessionName: typeof profile === "string" ? profile : undefined,
rePrompt,
secure: (await this.getProfileInfo()).isSecured(),
secure: mProfileInfo.isSecured(),
userInputBoxOptions,
passwordInputBoxOptions,
},
Expand Down Expand Up @@ -993,8 +1024,16 @@ export class Profiles extends ProfilesCache {
}

const deleteLabel = deletedProfile.name;
let usingTeamConfig: boolean;
try {
usingTeamConfig = (await this.getProfileInfo()).usingTeamConfig;
} catch (err) {
ZoweLogger.error(err);
ZoweExplorerExtender.showZoweConfigError(err.message);
return;
}

if ((await this.getProfileInfo()).usingTeamConfig) {
if (usingTeamConfig) {
const currentProfile = await this.getProfileFromConfig(deleteLabel);
const filePath = currentProfile.profLoc.osLoc[0];
await this.openConfigFile(filePath);
Expand Down
Loading

0 comments on commit dbf9a74

Please sign in to comment.