Skip to content

Commit

Permalink
Error correlator and troubleshoot webview (#3243)
Browse files Browse the repository at this point in the history
* wip: ErrorCorrelator facility and work on error handling

Signed-off-by: Trae Yelovich <[email protected]>

* chore: add typedoc to ErrorCorrelation; details -> summary

Signed-off-by: Trae Yelovich <[email protected]>

* wip: Error prompts and webview PoC

Signed-off-by: Trae Yelovich <[email protected]>

* feat: Troubleshoot webview, move PersistentVSCodeAPI

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: Move TipList into component file, troubleshoot format

Signed-off-by: Trae Yelovich <[email protected]>

* wip: set up test cases and rename function

Signed-off-by: Trae Yelovich <[email protected]>

* tests: impl ErrorCorrelator.displayError test cases

Signed-off-by: Trae Yelovich <[email protected]>

* tests: ErrorCorrelator.correlateError test cases

Signed-off-by: Trae Yelovich <[email protected]>

* wip: Add more error correlations; test data set error handling

Signed-off-by: Trae Yelovich <[email protected]>

* wip(ErrorCorrelator): collapsible error section, Copy Details btn

Signed-off-by: Trae Yelovich <[email protected]>

* copy details button, fix summary toggle state

Signed-off-by: Trae Yelovich <[email protected]>

* update copied content for copy details button

Signed-off-by: Trae Yelovich <[email protected]>

* feat: support template args in error summaries

Signed-off-by: Trae Yelovich <[email protected]>

* wip: update AuthUtils.errorHandling to use correlator

Signed-off-by: Trae Yelovich <[email protected]>

* wip: update AuthUtils.errorHandling signature and update calls

Signed-off-by: Trae Yelovich <[email protected]>

* pass template args from error context, add mvs error correlation

Signed-off-by: Trae Yelovich <[email protected]>

* wip: separate function to display correlation

Signed-off-by: Trae Yelovich <[email protected]>

* wip: add params to AuthUtils.errorHandling for correlator

Signed-off-by: Trae Yelovich <[email protected]>

* tests: Resolve failing test cases

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: Use API type, then profile type for narrowing

Signed-off-by: Trae Yelovich <[email protected]>

* wip: Prompt for creds when opening DS

Signed-off-by: Trae Yelovich <[email protected]>

* fix(api): Fix profile references being lost when cache is refreshed (#3248)

* fix(api): Fix profile references being lost when cache is refreshed

Signed-off-by: Timothy Johnson <[email protected]>

* fix: Pass profile instead of profile name for updating creds

Signed-off-by: Trae Yelovich <[email protected]>

---------

Signed-off-by: Timothy Johnson <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Trae Yelovich <[email protected]>

* Revert "wip: Prompt for creds when opening DS"

This reverts commit 53e9518.

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: Add back error correlator changes

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: fix tests to handle new format

Signed-off-by: Trae Yelovich <[email protected]>

* tests: TroubleshootError webview class

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: rename TroubleshootError.setErrorData -> sendErrorData

Signed-off-by: Trae Yelovich <[email protected]>

* remaining TroubleshootError cases, add log for unknown cmd

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: NetworkError -> CorrelatedError; cleanup class, fix type guard

Signed-off-by: Trae Yelovich <[email protected]>

* impl. correlator for FSP fns; update correlator tests

Signed-off-by: Trae Yelovich <[email protected]>

* tests: resolve failing test cases

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: throw errs instead of return; update tests

Signed-off-by: Trae Yelovich <[email protected]>

* fix delete & stat error tests, run prepublish

Signed-off-by: Trae Yelovich <[email protected]>

* error handling cases for stat

Signed-off-by: Trae Yelovich <[email protected]>

* refactor _handleError, avoid use of await for errors

Signed-off-by: Trae Yelovich <[email protected]>

* wip: add coverage to DataSet FSP

Signed-off-by: Trae Yelovich <[email protected]>

* wip: more ds/uss test cases

Signed-off-by: Trae Yelovich <[email protected]>

* BaseProvider._handleError test cases

Signed-off-by: Trae Yelovich <[email protected]>

* jobs test cases for error handling

Signed-off-by: Trae Yelovich <[email protected]>

* chore: update changelogs

Signed-off-by: Trae Yelovich <[email protected]>

* expose error correlator in extender API

Signed-off-by: Trae Yelovich <[email protected]>

* chore: address changelog feedback

Signed-off-by: Trae Yelovich <[email protected]>

* fix circular dep

Signed-off-by: Trae Yelovich <[email protected]>

* allow extenders to contribute resources for errors

Signed-off-by: Trae Yelovich <[email protected]>

* handle errors when listing files in virtual workspaces

Signed-off-by: Trae Yelovich <[email protected]>

* remove check for handleError mock in listFiles test

Signed-off-by: Trae Yelovich <[email protected]>

* skip dialog if no correlation found, fix missing info in webview

Signed-off-by: Trae Yelovich <[email protected]>

* fix tests, update logic for returning selection

Signed-off-by: Trae Yelovich <[email protected]>

* offer show log opt in first dialog if correlation not found

Signed-off-by: Trae Yelovich <[email protected]>

* omit profile details from log, update failing tests

Signed-off-by: Trae Yelovich <[email protected]>

* restore changes to ZoweTreeNode

Signed-off-by: Trae Yelovich <[email protected]>

* move HandleErrorOpts to fs/types/abstract

Signed-off-by: Trae Yelovich <[email protected]>

* remove export from ErrorContext interface

Signed-off-by: Trae Yelovich <[email protected]>

* revert changes to ZoweTreeNode tests

Signed-off-by: Trae Yelovich <[email protected]>

* make IApiExplorerExtender.getErrorCorrelator optional

Signed-off-by: Trae Yelovich <[email protected]>

* update command count, run l10n prepublish

Signed-off-by: Trae Yelovich <[email protected]>

* address duplicate errors, pass profileName as template arg

Signed-off-by: Trae Yelovich <[email protected]>

* resolve failing tests from changes

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: use optional chaining to spread templateArgs

Signed-off-by: Trae Yelovich <[email protected]>

* refactor: use dsName instead of path; rm handling in autoDetectEncoding

Signed-off-by: Trae Yelovich <[email protected]>

* run package

Signed-off-by: Billie Simmons <[email protected]>

* fix: propagate USS listFiles error

Signed-off-by: Trae Yelovich <[email protected]>

---------

Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Timothy Johnson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Co-authored-by: Billie Simmons <[email protected]>
Co-authored-by: Billie Simmons <[email protected]>
  • Loading branch information
4 people authored Nov 6, 2024
1 parent 085e71d commit e07e839
Show file tree
Hide file tree
Showing 66 changed files with 1,944 additions and 465 deletions.
1 change: 1 addition & 0 deletions packages/zowe-explorer-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t

- Zowe Explorer now includes support for the [VS Code display languages](https://code.visualstudio.com/docs/getstarted/locales) French, German, Japanese, Portuguese, and Spanish. [#3239](https://github.com/zowe/zowe-explorer-vscode/pull/3239)
- Localization of strings within the webviews. [#2983](https://github.com/zowe/zowe-explorer-vscode/issues/2983)
- Leverage the new error correlation facility to provide user-friendly summaries of API and network errors. Extenders can also contribute to the correlator to provide human-readable translations of error messages, as well as tips and additional resources for how to resolve the error. [#3243](https://github.com/zowe/zowe-explorer-vscode/pull/3243)

## `3.0.1`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import * as vscode from "vscode";
import { BaseProvider, ConflictViewSelection, DirEntry, FileEntry, ZoweScheme } from "../../../src/fs";
import { Gui } from "../../../src/globals";
import { MockedProperty } from "../../../__mocks__/mockUtils";
import { ErrorCorrelator, ZoweExplorerApiType } from "../../../src";

function getGlobalMocks(): { [key: string]: any } {
return {
Expand Down Expand Up @@ -542,6 +543,35 @@ describe("_handleConflict", () => {
expect(diffOverwriteMock).toHaveBeenCalledWith(globalMocks.testFileUri);
});
});
describe("_handleError", () => {
it("calls ErrorCorrelator.displayError to display error to user - no options provided", async () => {
const displayErrorMock = jest.spyOn(ErrorCorrelator.prototype, "displayError").mockReturnValue(new Promise((res, rej) => {}));
const prov = new (BaseProvider as any)();
prov._handleError(new Error("example"));
expect(displayErrorMock).toHaveBeenCalledWith(ZoweExplorerApiType.All, new Error("example"), {
additionalContext: undefined,
allowRetry: false,
profileType: "any",
templateArgs: undefined,
});
});
it("calls ErrorCorrelator.displayError to display error to user - options provided", async () => {
const displayErrorMock = jest.spyOn(ErrorCorrelator.prototype, "displayError").mockReturnValue(new Promise((res, rej) => {}));
const prov = new (BaseProvider as any)();
prov._handleError(new Error("example"), {
additionalContext: "Failed to list data sets",
apiType: ZoweExplorerApiType.Mvs,
profileType: "zosmf",
templateArgs: {},
});
expect(displayErrorMock).toHaveBeenCalledWith(ZoweExplorerApiType.Mvs, new Error("example"), {
additionalContext: "Failed to list data sets",
allowRetry: false,
profileType: "zosmf",
templateArgs: {},
});
});
});

describe("_relocateEntry", () => {
it("returns early if the entry does not exist in the file system", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,21 @@ describe("ProfilesCache", () => {
expect(profCache.getAllTypes()).toEqual([...profileTypes, "ssh", "base"]);
});

it("should refresh profile data for existing profile and keep object reference", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
const profInfoSpy = jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(createProfInfoMock([lpar1Profile, zftpProfile]));
await profCache.refresh(fakeApiRegister as unknown as Types.IApiRegisterClient);
expect(profCache.allProfiles.length).toEqual(2);
expect(profCache.allProfiles[0]).toMatchObject(lpar1Profile);
const oldZosmfProfile = profCache.allProfiles[0];
const newZosmfProfile = { ...lpar1Profile, profile: lpar2Profile.profile };
profInfoSpy.mockResolvedValue(createProfInfoMock([newZosmfProfile, zftpProfile]));
await profCache.refresh(fakeApiRegister as unknown as Types.IApiRegisterClient);
expect(profCache.allProfiles.length).toEqual(2);
expect(profCache.allProfiles[0]).toMatchObject(newZosmfProfile);
expect(oldZosmfProfile.profile).toEqual(newZosmfProfile.profile);
});

it("should refresh profile data for and merge tokens with base profile", async () => {
const profCache = new ProfilesCache(fakeLogger as unknown as imperative.Logger);
jest.spyOn(profCache, "getProfileInfo").mockResolvedValue(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/**
* This program and the accompanying materials are made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-v20.html
*
* SPDX-License-Identifier: EPL-2.0
*
* Copyright Contributors to the Zowe Project.
*
*/

import { ErrorCorrelator, Gui, CorrelatedError, ZoweExplorerApiType } from "../../../src/";
import { commands } from "vscode";

describe("addCorrelation", () => {
it("adds a correlation for the given API and existing profile type", () => {
const fakeErrorSummary = "Example error summary for the correlator";
ErrorCorrelator.getInstance().addCorrelation(ZoweExplorerApiType.Mvs, "zosmf", {
errorCode: "403",
summary: fakeErrorSummary,
matches: ["Specific sequence 1234 encountered"],
});
expect(
(ErrorCorrelator.getInstance() as any).errorMatches.get(ZoweExplorerApiType.Mvs)["zosmf"].find((err) => err.summary === fakeErrorSummary)
).not.toBe(null);
});
it("adds a correlation for the given API and new profile type", () => {
const fakeErrorSummary = "Example error summary for the correlator";
ErrorCorrelator.getInstance().addCorrelation(ZoweExplorerApiType.Mvs, "fake-type", {
errorCode: "403",
summary: fakeErrorSummary,
matches: ["Specific sequence 5678 encountered"],
});
expect(
(ErrorCorrelator.getInstance() as any).errorMatches
.get(ZoweExplorerApiType.Mvs)
["fake-type"].find((err) => err.summary === fakeErrorSummary)
).not.toBe(null);
});
});

describe("correlateError", () => {
it("correctly correlates an error in the list of error matches", () => {
expect(
ErrorCorrelator.getInstance().correlateError(ZoweExplorerApiType.Mvs, "Client is not authorized for file access.", {
profileType: "zosmf",
})
).toStrictEqual(
new CorrelatedError({
correlation: {
errorCode: "500",
summary: "Insufficient write permissions for this data set. The data set may be read-only or locked.",
tips: [
"Check that your user or group has the appropriate permissions for this data set.",
"Ensure that the data set is not opened within a mainframe editor tool.",
],
},
errorCode: "500",
initialError: "Client is not authorized for file access.",
})
);
});
it("returns a generic CorrelatedError if no matches are available for the given profile type", () => {
expect(
ErrorCorrelator.getInstance().correlateError(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "nonsense" })
).toStrictEqual(new CorrelatedError({ initialError: "This is the full error message" }));
});
it("returns a generic CorrelatedError with the full error details if no matches are found", () => {
expect(
ErrorCorrelator.getInstance().correlateError(ZoweExplorerApiType.Mvs, "A cryptic error with no available match", { profileType: "zosmf" })
).toStrictEqual(new CorrelatedError({ initialError: "A cryptic error with no available match" }));
});
});

describe("displayError", () => {
it("calls correlateError to get an error correlation", async () => {
const correlateErrorMock = jest
.spyOn(ErrorCorrelator.prototype, "correlateError")
.mockReturnValueOnce(
new CorrelatedError({ correlation: { summary: "Summary of network error" }, initialError: "This is the full error message" })
);
const errorMessageMock = jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce(undefined);
await ErrorCorrelator.getInstance().displayError(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(correlateErrorMock).toHaveBeenCalledWith(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(errorMessageMock).toHaveBeenCalledWith("Summary of network error", { items: ["More info"] });
});
it("presents an additional dialog when the user selects 'More info'", async () => {
const correlateErrorMock = jest
.spyOn(ErrorCorrelator.prototype, "correlateError")
.mockReturnValueOnce(
new CorrelatedError({ correlation: { summary: "Summary of network error" }, initialError: "This is the full error message" })
);
const errorMessageMock = jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("More info").mockResolvedValueOnce(undefined);
await ErrorCorrelator.getInstance().displayError(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(correlateErrorMock).toHaveBeenCalledWith(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(errorMessageMock).toHaveBeenCalledWith("Summary of network error", { items: ["More info"] });
expect(errorMessageMock).toHaveBeenCalledWith("This is the full error message", { items: ["Show log", "Troubleshoot"] });
});
it("opens the Zowe Explorer output channel when the user selects 'Show log'", async () => {
const correlateErrorMock = jest
.spyOn(ErrorCorrelator.prototype, "correlateError")
.mockReturnValueOnce(
new CorrelatedError({ correlation: { summary: "Summary of network error" }, initialError: "This is the full error message" })
);
const errorMessageMock = jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("More info").mockResolvedValueOnce("Show log");
const executeCommandMock = jest.spyOn(commands, "executeCommand").mockImplementation();
await ErrorCorrelator.getInstance().displayError(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(correlateErrorMock).toHaveBeenCalledWith(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(errorMessageMock).toHaveBeenCalledWith("Summary of network error", { items: ["More info"] });
expect(errorMessageMock).toHaveBeenCalledWith("This is the full error message", { items: ["Show log", "Troubleshoot"] });
expect(executeCommandMock).toHaveBeenCalledWith("zowe.revealOutputChannel");
executeCommandMock.mockRestore();
});
it("opens the troubleshoot webview if the user selects 'Troubleshoot'", async () => {
const error = new CorrelatedError({
correlation: { summary: "Summary of network error" },
initialError: "This is the full error message",
});
const correlateErrorMock = jest.spyOn(ErrorCorrelator.getInstance(), "correlateError").mockReturnValueOnce(error);
const errorMessageMock = jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("More info").mockResolvedValueOnce("Troubleshoot");
const executeCommandMock = jest.spyOn(commands, "executeCommand").mockImplementation();
await ErrorCorrelator.getInstance().displayError(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(correlateErrorMock).toHaveBeenCalledWith(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(errorMessageMock).toHaveBeenCalledWith("Summary of network error", { items: ["More info"] });
expect(errorMessageMock).toHaveBeenCalledWith("This is the full error message", { items: ["Show log", "Troubleshoot"] });
expect(executeCommandMock).toHaveBeenCalledWith("zowe.troubleshootError", error, error.stack);
executeCommandMock.mockRestore();
});
});

describe("displayCorrelatedError", () => {
it("returns 'Retry' for the userResponse whenever the user selects 'Retry'", async () => {
const error = new CorrelatedError({
correlation: { summary: "Summary of network error" },
initialError: "This is the full error message",
});
const correlateErrorMock = jest.spyOn(ErrorCorrelator.getInstance(), "correlateError").mockReturnValueOnce(error);
const errorMessageMock = jest.spyOn(Gui, "errorMessage").mockResolvedValueOnce("Retry");
const handledErrorInfo = await ErrorCorrelator.getInstance().displayError(ZoweExplorerApiType.Mvs, "This is the full error message", {
additionalContext: "Some additional context",
allowRetry: true,
profileType: "zosmf",
});
expect(correlateErrorMock).toHaveBeenCalledWith(ZoweExplorerApiType.Mvs, "This is the full error message", { profileType: "zosmf" });
expect(errorMessageMock).toHaveBeenCalledWith("Some additional context: Summary of network error", { items: ["Retry", "More info"] });
expect(handledErrorInfo.userResponse).toBe("Retry");
});
});
8 changes: 8 additions & 0 deletions packages/zowe-explorer-api/src/extend/IApiExplorerExtender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import * as imperative from "@zowe/imperative";
import { ProfilesCache } from "../profiles/ProfilesCache";
import { ErrorCorrelator } from "../utils/ErrorCorrelator";

/**
* This interface can be used by other VS Code Extensions to access an alternative
Expand Down Expand Up @@ -45,4 +46,11 @@ export interface IApiExplorerExtender {
* or to create them automatically if it is non-existant.
*/
initForZowe(type: string, profileTypeConfigurations: imperative.ICommandProfileTypeConfiguration[]): void | Promise<void>;

/**
* Allows extenders to contribute error correlations, providing user-friendly
* summaries of API or network errors. Also gives extenders the opportunity to
* provide tips or additional resources for errors.
*/
getErrorCorrelator?(): ErrorCorrelator;
}
21 changes: 20 additions & 1 deletion packages/zowe-explorer-api/src/fs/BaseProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
*/

import * as vscode from "vscode";
import { DirEntry, FileEntry, IFileSystemEntry, FS_PROVIDER_DELAY, ConflictViewSelection, DeleteMetadata } from "./types";
import { DirEntry, FileEntry, IFileSystemEntry, FS_PROVIDER_DELAY, ConflictViewSelection, DeleteMetadata, HandleErrorOpts } from "./types";
import * as path from "path";
import { FsAbstractUtils } from "./utils";
import { Gui } from "../globals/Gui";
import { ZosEncoding } from "../tree";
import { ErrorCorrelator, ZoweExplorerApiType } from "../utils/ErrorCorrelator";

export class BaseProvider {
// eslint-disable-next-line no-magic-numbers
Expand Down Expand Up @@ -417,6 +418,24 @@ export class BaseProvider {
return entry;
}

protected _handleError(err: Error, opts?: HandleErrorOpts): void {
ErrorCorrelator.getInstance()
.displayError(opts?.apiType ?? ZoweExplorerApiType.All, err, {
additionalContext: opts?.additionalContext,
allowRetry: opts?.retry?.fn != null,
profileType: opts?.profileType ?? "any",
templateArgs: opts?.templateArgs,
})
.then(async ({ userResponse }) => {
if (userResponse === "Retry" && opts?.retry?.fn != null) {
await opts.retry.fn(...(opts?.retry.args ?? []));
}
})
.catch(() => {
throw err;
});
}

protected _lookupAsDirectory(uri: vscode.Uri, silent: boolean): DirEntry {
const entry = this.lookup(uri, silent);
if (entry != null && !FsAbstractUtils.isDirectoryEntry(entry) && !silent) {
Expand Down
12 changes: 12 additions & 0 deletions packages/zowe-explorer-api/src/fs/types/abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Duplex } from "stream";
import { IProfileLoaded } from "@zowe/imperative";
import * as vscode from "vscode";
import { ZosEncoding } from "../../tree";
import { ZoweExplorerApiType } from "../../utils/ErrorCorrelator";

export enum ZoweScheme {
DS = "zowe-ds",
Expand Down Expand Up @@ -147,3 +148,14 @@ export type UriFsInfo = {
profileName: string;
profile?: IProfileLoaded;
};

export interface HandleErrorOpts {
retry?: {
fn: (...args: any[]) => any | PromiseLike<any>;
args?: any[];
};
profileType?: string;
apiType?: ZoweExplorerApiType;
templateArgs?: Record<string, string>;
additionalContext?: string;
}
3 changes: 2 additions & 1 deletion packages/zowe-explorer-api/src/profiles/ProfilesCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ export class ProfilesCache {
}

// Step 3: Update allProfiles list
tmpAllProfiles.push(profileFix);
const existingProfile = this.allProfiles.find((tmpProf) => tmpProf.name === prof.profName && tmpProf.type === prof.profType);
tmpAllProfiles.push(existingProfile ? Object.assign(existingProfile, profileFix) : profileFix);
}
allProfiles.push(...tmpAllProfiles);
this.profilesByType.set(type, tmpAllProfiles);
Expand Down
Loading

0 comments on commit e07e839

Please sign in to comment.