Skip to content

Commit

Permalink
refactor: throw errs instead of return; update tests
Browse files Browse the repository at this point in the history
Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>
  • Loading branch information
traeok committed Oct 17, 2024
1 parent a3167c9 commit b416fa1
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,20 +127,20 @@ describe("displayError", () => {
});

describe("displayCorrelatedError", () => {
it("returns 'Retry' whenever the user selects 'Retry'", async () => {
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 userResponse = await ErrorCorrelator.getInstance().displayError(ZoweExplorerApiType.Mvs, "This is the full error message", {
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(userResponse).toBe("Retry");
expect(handledErrorInfo.userResponse).toBe("Retry");
});
});
4 changes: 2 additions & 2 deletions packages/zowe-explorer-api/src/fs/BaseProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import * as path from "path";
import { FsAbstractUtils } from "./utils";
import { Gui } from "../globals/Gui";
import { ZosEncoding } from "../tree";
import { ErrorCorrelator, ZoweExplorerApiType } from "../utils/ErrorCorrelator";
import { ErrorCorrelator, HandledErrorInfo, ZoweExplorerApiType } from "../utils/ErrorCorrelator";

export interface HandleErrorOpts {
allowRetry?: boolean;
Expand Down Expand Up @@ -426,7 +426,7 @@ export class BaseProvider {
return entry;
}

protected async _handleError(err: Error, opts?: HandleErrorOpts): Promise<string> {
protected async _handleError(err: Error, opts?: HandleErrorOpts): Promise<HandledErrorInfo> {

Check warning on line 429 in packages/zowe-explorer-api/src/fs/BaseProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer-api/src/fs/BaseProvider.ts#L429

Added line #L429 was not covered by tests
return ErrorCorrelator.getInstance().displayError(opts?.apiType ?? ZoweExplorerApiType.All, err, {
additionalContext: opts?.additionalContext,
allowRetry: opts?.allowRetry ?? false,
Expand Down
30 changes: 24 additions & 6 deletions packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,31 @@ export interface DisplayErrorOpts extends CorrelateErrorOpts {

export interface DisplayCorrelatedErrorOpts extends Omit<DisplayErrorOpts, "profileType"> {}

export interface HandledErrorInfo {
correlation: CorrelatedError;
userResponse: string | undefined;
}

/**
* Representation of the given error as a correlated error (wrapper around the `Error` class).
*
* Used to cache the error info such as tips, the match that was encountered and the full error message.
*/
export class CorrelatedError {
public errorCode?: string;

public constructor(public properties: CorrelatedErrorProps) {
if (properties.initialError instanceof Error) {
// preserve stack from initial error
}

this.errorCode = this.initial instanceof ImperativeError ? this.initial.errorCode : this.properties.errorCode;
}

public get stack(): string | undefined {
return this.initial instanceof Error ? this.initial.stack : undefined;
}

public get errorCode(): string | undefined {
return this.initial instanceof ImperativeError ? this.initial.errorCode : this.properties.errorCode ?? undefined;
}

public get message(): string {
if (this.properties.correlation) {
return this.properties.correlation.summary;
Expand All @@ -92,6 +97,16 @@ export class CorrelatedError {
public get initial(): Error | string {
return this.properties.initialError;
}

public asError(): Error {
const err = new Error(this.message);
err.stack = this.stack;
return err;

Check warning on line 104 in packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts#L101-L104

Added lines #L101 - L104 were not covered by tests
}

public toString(): string {
return this.message;

Check warning on line 108 in packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts#L107-L108

Added lines #L107 - L108 were not covered by tests
}
}

export enum ZoweExplorerApiType {
Expand Down Expand Up @@ -317,8 +332,11 @@ export class ErrorCorrelator extends Singleton {
* @param allowRetry Whether to allow retrying the action
* @returns The user selection ("Retry" [if enabled] or "Troubleshoot")
*/
public async displayError(api: ZoweExplorerApiType, errorDetails: string | Error, opts?: DisplayErrorOpts): Promise<string | undefined> {
public async displayError(api: ZoweExplorerApiType, errorDetails: string | Error, opts?: DisplayErrorOpts): Promise<HandledErrorInfo> {
const error = this.correlateError(api, errorDetails, { profileType: opts?.profileType, templateArgs: opts?.templateArgs });
return this.displayCorrelatedError(error, { additionalContext: opts?.additionalContext, allowRetry: opts?.allowRetry });
return {
correlation: error,
userResponse: await this.displayCorrelatedError(error, { additionalContext: opts?.additionalContext, allowRetry: opts?.allowRetry }),
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,7 @@ describe("delete", () => {
const fakePs = { ...testEntries.ps };
const fakeSession = { ...testEntries.session, entries: new Map() };
fakeSession.entries.set("USER.DATA.PS", fakePs);

const mockMvsApi = {
deleteDataSet: jest.fn().mockRejectedValueOnce(new Error("Data set does not exist on remote")),
};
Expand All @@ -836,7 +837,7 @@ describe("delete", () => {
const errorMsgMock = jest.spyOn(Gui, "errorMessage").mockImplementation();
jest.spyOn(DatasetFSProvider.instance as any, "_lookupParentDirectory").mockReturnValueOnce(fakeSession);

await DatasetFSProvider.instance.delete(testUris.ps, { recursive: false });
await expect(DatasetFSProvider.instance.delete(testUris.ps, { recursive: false })).rejects.toThrow();
expect(mockMvsApi.deleteDataSet).toHaveBeenCalledWith(fakePs.name, { responseTimeout: undefined });
expect(_lookupMock).toHaveBeenCalledWith(testUris.ps, false);
expect(_fireSoonMock).toHaveBeenCalled();
Expand Down Expand Up @@ -922,7 +923,9 @@ describe("rename", () => {
const _lookupParentDirectoryMock = jest
.spyOn(DatasetFSProvider.instance as any, "_lookupParentDirectory")
.mockReturnValueOnce({ ...testEntries.session });
await DatasetFSProvider.instance.rename(testUris.pds, testUris.pds.with({ path: "/USER.DATA.PDS2" }), { overwrite: true });
await expect(
DatasetFSProvider.instance.rename(testUris.pds, testUris.pds.with({ path: "/USER.DATA.PDS2" }), { overwrite: true })
).rejects.toThrow();
expect(mockMvsApi.renameDataSet).toHaveBeenCalledWith("USER.DATA.PDS", "USER.DATA.PDS2");
expect(errMsgSpy).toHaveBeenCalledWith("Failed to rename USER.DATA.PDS: could not upload data set", { items: ["Retry", "More info"] });
_lookupMock.mockRestore();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,9 @@ describe("rename", () => {
};
(UssFSProvider.instance as any).root.entries.set("sestest", sessionEntry);

await UssFSProvider.instance.rename(testUris.folder, testUris.folder.with({ path: "/sestest/aFolder2" }), { overwrite: true });
await expect(
UssFSProvider.instance.rename(testUris.folder, testUris.folder.with({ path: "/sestest/aFolder2" }), { overwrite: true })
).rejects.toThrow();
expect(mockUssApi.rename).toHaveBeenCalledWith("/aFolder", "/aFolder2");
expect(folderEntry.metadata.path).toBe("/aFolder");
expect(sessionEntry.entries.has("aFolder2")).toBe(false);
Expand Down Expand Up @@ -882,7 +884,7 @@ describe("delete", () => {
delete: deleteMock,
} as any);
const handleErrorMock = jest.spyOn((BaseProvider as any).prototype, "_handleError");
await UssFSProvider.instance.delete(testUris.file, { recursive: false });
await expect(UssFSProvider.instance.delete(testUris.file, { recursive: false })).rejects.toThrow();
expect(getDelInfoMock).toHaveBeenCalledWith(testUris.file);
expect(deleteMock).toHaveBeenCalledWith(testEntries.file.metadata.path, false);
expect(handleErrorMock).toHaveBeenCalledWith(exampleError, {
Expand Down
28 changes: 14 additions & 14 deletions packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
resp = await ZoweExplorerApiRegister.getMvsApi(uriInfo.profile).dataSet(path.parse(uri.path).name, { attributes: true });
}
} catch (err) {
const userResponse = await this._handleError(err, {
const { userResponse } = await this._handleError(err, {

Check warning on line 104 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L104

Added line #L104 was not covered by tests
additionalContext: vscode.l10n.t("Failed to get stats for data set {0}", uri.path),
allowRetry: true,
apiType: ZoweExplorerApiType.Mvs,
Expand Down Expand Up @@ -154,7 +154,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
}
}
} catch (err) {
const userResponse = await this._handleError(err, {
const { userResponse } = await this._handleError(err, {

Check warning on line 157 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L157

Added line #L157 was not covered by tests
additionalContext: vscode.l10n.t("Failed to list datasets"),
allowRetry: true,
apiType: ZoweExplorerApiType.Mvs,
Expand Down Expand Up @@ -198,7 +198,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
try {
members = await ZoweExplorerApiRegister.getMvsApi(uriInfo.profile).allMembers(path.posix.basename(uri.path));
} catch (err) {
const userResponse = await this._handleError(err, {
const { userResponse } = await this._handleError(err, {

Check warning on line 201 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L201

Added line #L201 was not covered by tests
additionalContext: vscode.l10n.t("Failed to list dataset members"),
allowRetry: true,
apiType: ZoweExplorerApiType.Mvs,
Expand Down Expand Up @@ -253,7 +253,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
throw vscode.FileSystemError.FileNotFound(uri);

Check warning on line 253 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L252-L253

Added lines #L252 - L253 were not covered by tests
}
} catch (err) {
const userResponse = await this._handleError(err, {
const { userResponse } = await this._handleError(err, {

Check warning on line 256 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L256

Added line #L256 was not covered by tests
allowRetry: true,
additionalContext: vscode.l10n.t("Failed to list data set {0}", uri.path),
apiType: ZoweExplorerApiType.Mvs,
Expand Down Expand Up @@ -397,7 +397,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
stream: bufBuilder,
});
} catch (err) {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {

Check warning on line 400 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L400

Added line #L400 was not covered by tests
additionalContext: vscode.l10n.t({
message: "Failed to get contents for {0}",
args: [uri.path],
Expand All @@ -410,7 +410,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
if (userResponse === "Retry") {
return this.fetchDatasetAtUri(uri, options);

Check warning on line 411 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L411

Added line #L411 was not covered by tests
}
return;
throw correlation.asError();

Check warning on line 413 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L413

Added line #L413 was not covered by tests
}
const data: Uint8Array = bufBuilder.read() ?? new Uint8Array();

Expand Down Expand Up @@ -446,7 +446,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
ds = this._lookupAsFile(uri) as DsEntry;
} catch (err) {
if (!(err instanceof vscode.FileSystemError) || err.code !== "FileNotFound") {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {

Check warning on line 449 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L449

Added line #L449 was not covered by tests
additionalContext: vscode.l10n.t({
message: "Failed to read {0}",
args: [uri.path],
Expand All @@ -459,7 +459,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
if (userResponse === "Retry") {
return this.readFile(uri);

Check warning on line 460 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L460

Added line #L460 was not covered by tests
}
return;
throw correlation.asError();

Check warning on line 462 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L462

Added line #L462 was not covered by tests
}

// check if parent directory exists; if not, do a remote lookup
Expand Down Expand Up @@ -597,7 +597,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
}
} catch (err) {
if (!err.message.includes("Rest API failure with HTTP(S) status 412")) {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {

Check warning on line 600 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L600

Added line #L600 was not covered by tests
additionalContext: vscode.l10n.t({
message: "Failed to save {0}",
args: [entry.metadata.path],
Expand All @@ -610,7 +610,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
if (userResponse === "Retry") {
return this.writeFile(uri, content, options);

Check warning on line 611 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L611

Added line #L611 was not covered by tests
}
return;
throw correlation.asError();

Check warning on line 613 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L613

Added line #L613 was not covered by tests
}

entry.data = content;
Expand Down Expand Up @@ -655,7 +655,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
responseTimeout: entry.metadata.profile.profile?.responseTimeout,
});
} catch (err) {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {
additionalContext: vscode.l10n.t({
message: "Failed to delete {0}",
args: [entry.metadata.path],
Expand All @@ -668,7 +668,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
if (userResponse === "Retry") {
return this.delete(uri, _options);

Check warning on line 669 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L669

Added line #L669 was not covered by tests
}
return;
throw correlation.asError();
}

parent.entries.delete(entry.name);
Expand Down Expand Up @@ -701,7 +701,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
);
}
} catch (err) {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {
additionalContext: vscode.l10n.t({
message: "Failed to rename {0}",
args: [oldName],
Expand All @@ -714,7 +714,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem
if (userResponse === "Retry") {
return this.rename(oldUri, newUri, options);

Check warning on line 715 in packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts#L715

Added line #L715 was not covered by tests
}
return;
throw correlation.asError();
}

parentDir.entries.delete(entry.name);
Expand Down
9 changes: 6 additions & 3 deletions packages/zowe-explorer/src/trees/job/JobFSProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,15 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv
}
}
} catch (err) {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {

Check warning on line 116 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L116

Added line #L116 was not covered by tests
allowRetry: true,
apiType: ZoweExplorerApiType.Jes,
profileType: uriInfo.profile?.type,
});
if (userResponse === "Retry") {
return this.readDirectory(uri);

Check warning on line 122 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L122

Added line #L122 was not covered by tests
}
throw correlation.asError();

Check warning on line 124 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L124

Added line #L124 was not covered by tests
}

for (const entry of fsEntry.entries) {
Expand Down Expand Up @@ -227,7 +228,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv
try {
await this.fetchSpoolAtUri(uri);
} catch (err) {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {

Check warning on line 231 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L231

Added line #L231 was not covered by tests
additionalContext: vscode.l10n.t({
message: "Failed to get contents for {0}",
args: [spoolEntry.name],
Expand All @@ -240,6 +241,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv
if (userResponse === "Retry") {
return this.readFile(uri);

Check warning on line 242 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L242

Added line #L242 was not covered by tests
}
throw correlation.asError();

Check warning on line 244 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L244

Added line #L244 was not covered by tests
}
spoolEntry.wasAccessed = true;
}
Expand Down Expand Up @@ -313,7 +315,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv
try {
await ZoweExplorerApiRegister.getJesApi(profInfo.profile).deleteJob(entry.job.jobname, entry.job.jobid);
} catch (err) {
const userResponse = await this._handleError(err, {
const { correlation, userResponse } = await this._handleError(err, {

Check warning on line 318 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L318

Added line #L318 was not covered by tests
additionalContext: vscode.l10n.t({
message: "Failed to delete job {0}",
args: [entry.job.jobname],
Expand All @@ -326,6 +328,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv
if (userResponse === "Retry") {
return this.delete(uri, options);

Check warning on line 329 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L329

Added line #L329 was not covered by tests
}
throw correlation.asError();

Check warning on line 331 in packages/zowe-explorer/src/trees/job/JobFSProvider.ts

View check run for this annotation

Codecov / codecov/patch

packages/zowe-explorer/src/trees/job/JobFSProvider.ts#L331

Added line #L331 was not covered by tests
}
}
parent.entries.delete(entry.name);
Expand Down
Loading

0 comments on commit b416fa1

Please sign in to comment.