From b416fa10a83614bb47c0dba42c44a4227f7030ab Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Thu, 17 Oct 2024 10:01:41 -0400 Subject: [PATCH] refactor: throw errs instead of return; update tests Signed-off-by: Trae Yelovich --- .../utils/ErrorCorrelator.unit.test.ts | 6 ++-- .../zowe-explorer-api/src/fs/BaseProvider.ts | 4 +-- .../src/utils/ErrorCorrelator.ts | 30 +++++++++++++++---- .../dataset/DatasetFSProvider.unit.test.ts | 7 +++-- .../trees/uss/UssFSProvider.unit.test.ts | 6 ++-- .../src/trees/dataset/DatasetFSProvider.ts | 28 ++++++++--------- .../src/trees/job/JobFSProvider.ts | 9 ++++-- .../src/trees/uss/UssFSProvider.ts | 26 ++++++++-------- .../src/webviews/src/edit-history/App.tsx | 2 +- 9 files changed, 72 insertions(+), 46 deletions(-) diff --git a/packages/zowe-explorer-api/__tests__/__unit__/utils/ErrorCorrelator.unit.test.ts b/packages/zowe-explorer-api/__tests__/__unit__/utils/ErrorCorrelator.unit.test.ts index 8cf553ad1..5262fd995 100644 --- a/packages/zowe-explorer-api/__tests__/__unit__/utils/ErrorCorrelator.unit.test.ts +++ b/packages/zowe-explorer-api/__tests__/__unit__/utils/ErrorCorrelator.unit.test.ts @@ -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"); }); }); diff --git a/packages/zowe-explorer-api/src/fs/BaseProvider.ts b/packages/zowe-explorer-api/src/fs/BaseProvider.ts index a931b2f09..eb8e23250 100644 --- a/packages/zowe-explorer-api/src/fs/BaseProvider.ts +++ b/packages/zowe-explorer-api/src/fs/BaseProvider.ts @@ -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; @@ -426,7 +426,7 @@ export class BaseProvider { return entry; } - protected async _handleError(err: Error, opts?: HandleErrorOpts): Promise { + protected async _handleError(err: Error, opts?: HandleErrorOpts): Promise { return ErrorCorrelator.getInstance().displayError(opts?.apiType ?? ZoweExplorerApiType.All, err, { additionalContext: opts?.additionalContext, allowRetry: opts?.allowRetry ?? false, diff --git a/packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts b/packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts index 4b4464563..11e4658e5 100644 --- a/packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts +++ b/packages/zowe-explorer-api/src/utils/ErrorCorrelator.ts @@ -61,26 +61,31 @@ export interface DisplayErrorOpts extends CorrelateErrorOpts { export interface DisplayCorrelatedErrorOpts extends Omit {} +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; @@ -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; + } + + public toString(): string { + return this.message; + } } export enum ZoweExplorerApiType { @@ -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 { + public async displayError(api: ZoweExplorerApiType, errorDetails: string | Error, opts?: DisplayErrorOpts): Promise { 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 }), + }; } } diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/dataset/DatasetFSProvider.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/dataset/DatasetFSProvider.unit.test.ts index 002d9c7b0..65e974114 100644 --- a/packages/zowe-explorer/__tests__/__unit__/trees/dataset/DatasetFSProvider.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/trees/dataset/DatasetFSProvider.unit.test.ts @@ -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")), }; @@ -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(); @@ -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(); diff --git a/packages/zowe-explorer/__tests__/__unit__/trees/uss/UssFSProvider.unit.test.ts b/packages/zowe-explorer/__tests__/__unit__/trees/uss/UssFSProvider.unit.test.ts index 1d020ded8..95a556d4a 100644 --- a/packages/zowe-explorer/__tests__/__unit__/trees/uss/UssFSProvider.unit.test.ts +++ b/packages/zowe-explorer/__tests__/__unit__/trees/uss/UssFSProvider.unit.test.ts @@ -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); @@ -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, { diff --git a/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts b/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts index 0363e84cb..077e0e3ea 100644 --- a/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts +++ b/packages/zowe-explorer/src/trees/dataset/DatasetFSProvider.ts @@ -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, { additionalContext: vscode.l10n.t("Failed to get stats for data set {0}", uri.path), allowRetry: true, apiType: ZoweExplorerApiType.Mvs, @@ -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, { additionalContext: vscode.l10n.t("Failed to list datasets"), allowRetry: true, apiType: ZoweExplorerApiType.Mvs, @@ -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, { additionalContext: vscode.l10n.t("Failed to list dataset members"), allowRetry: true, apiType: ZoweExplorerApiType.Mvs, @@ -253,7 +253,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem throw vscode.FileSystemError.FileNotFound(uri); } } catch (err) { - const userResponse = await this._handleError(err, { + const { userResponse } = await this._handleError(err, { allowRetry: true, additionalContext: vscode.l10n.t("Failed to list data set {0}", uri.path), apiType: ZoweExplorerApiType.Mvs, @@ -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, { additionalContext: vscode.l10n.t({ message: "Failed to get contents for {0}", args: [uri.path], @@ -410,7 +410,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem if (userResponse === "Retry") { return this.fetchDatasetAtUri(uri, options); } - return; + throw correlation.asError(); } const data: Uint8Array = bufBuilder.read() ?? new Uint8Array(); @@ -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, { additionalContext: vscode.l10n.t({ message: "Failed to read {0}", args: [uri.path], @@ -459,7 +459,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem if (userResponse === "Retry") { return this.readFile(uri); } - return; + throw correlation.asError(); } // check if parent directory exists; if not, do a remote lookup @@ -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, { additionalContext: vscode.l10n.t({ message: "Failed to save {0}", args: [entry.metadata.path], @@ -610,7 +610,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem if (userResponse === "Retry") { return this.writeFile(uri, content, options); } - return; + throw correlation.asError(); } entry.data = content; @@ -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], @@ -668,7 +668,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem if (userResponse === "Retry") { return this.delete(uri, _options); } - return; + throw correlation.asError(); } parent.entries.delete(entry.name); @@ -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], @@ -714,7 +714,7 @@ export class DatasetFSProvider extends BaseProvider implements vscode.FileSystem if (userResponse === "Retry") { return this.rename(oldUri, newUri, options); } - return; + throw correlation.asError(); } parentDir.entries.delete(entry.name); diff --git a/packages/zowe-explorer/src/trees/job/JobFSProvider.ts b/packages/zowe-explorer/src/trees/job/JobFSProvider.ts index 99e81311c..5971342ed 100644 --- a/packages/zowe-explorer/src/trees/job/JobFSProvider.ts +++ b/packages/zowe-explorer/src/trees/job/JobFSProvider.ts @@ -113,7 +113,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv } } } catch (err) { - const userResponse = await this._handleError(err, { + const { correlation, userResponse } = await this._handleError(err, { allowRetry: true, apiType: ZoweExplorerApiType.Jes, profileType: uriInfo.profile?.type, @@ -121,6 +121,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.readDirectory(uri); } + throw correlation.asError(); } for (const entry of fsEntry.entries) { @@ -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, { additionalContext: vscode.l10n.t({ message: "Failed to get contents for {0}", args: [spoolEntry.name], @@ -240,6 +241,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.readFile(uri); } + throw correlation.asError(); } spoolEntry.wasAccessed = true; } @@ -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, { additionalContext: vscode.l10n.t({ message: "Failed to delete job {0}", args: [entry.job.jobname], @@ -326,6 +328,7 @@ export class JobFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.delete(uri, options); } + throw correlation.asError(); } } parent.entries.delete(entry.name); diff --git a/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts b/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts index d2fb0453f..769e52d30 100644 --- a/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts +++ b/packages/zowe-explorer/src/trees/uss/UssFSProvider.ts @@ -124,7 +124,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv try { await ussApi.move(oldInfo.path, info.path); } catch (err) { - const userResponse = await this._handleError(err, { + const { correlation, userResponse } = await this._handleError(err, { additionalContext: vscode.l10n.t({ message: "Failed to move {0}", args: [oldInfo.path], comment: "File path" }), apiType: ZoweExplorerApiType.Uss, allowRetry: true, @@ -133,7 +133,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.move(oldUri, newUri); } - return false; + throw correlation.asError(); } await this._relocateEntry(oldUri, newUri, info.path); return true; @@ -150,7 +150,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv await vscode.workspace.fs.createDirectory(uri); } } catch (err) { - const userResponse = await this._handleError(err, { + const { userResponse } = await this._handleError(err, { additionalContext: vscode.l10n.t("Failed to list {0}", err.message), apiType: ZoweExplorerApiType.Uss, allowRetry: true, @@ -294,7 +294,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv stream: bufBuilder, }); } catch (err) { - const userResponse = await this._handleError(err, { + const { correlation, userResponse } = await this._handleError(err, { additionalContext: vscode.l10n.t({ message: "Failed to get contents for {0}", args: [filePath], @@ -307,7 +307,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.fetchFileAtUri(uri, options); } - return; + throw correlation.asError(); } await this.autoDetectEncoding(file as UssFile); @@ -349,7 +349,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv entry.encoding = isBinary ? { kind: "binary" } : undefined; } } catch (err) { - const userResponse = await this._handleError(err, { + const { userResponse } = await this._handleError(err, { additionalContext: vscode.l10n.t({ message: "Failed to detect encoding for {0}", args: [entry.metadata.path], @@ -523,7 +523,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv } catch (err) { if (!err.message.includes("Rest API failure with HTTP(S) status 412")) { // Some unknown error happened, don't update the entry - const userResponse = await this._handleError(err, { + const { correlation, userResponse } = await this._handleError(err, { apiType: ZoweExplorerApiType.Uss, allowRetry: true, profileType: parentDir.metadata.profile.type, @@ -531,7 +531,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.writeFile(uri, content, options); } - return; + throw correlation.asError(); } entry.data = content; @@ -584,7 +584,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv try { await ZoweExplorerApiRegister.getUssApi(entry.metadata.profile).rename(entry.metadata.path, newPath); } 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: [entry.metadata.path], @@ -597,7 +597,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.rename(oldUri, newUri, options); } - return; + throw correlation.asError(); } parentDir.entries.delete(entry.name); @@ -626,7 +626,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv entryToDelete instanceof UssDirectory ); } 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: [entryToDelete.metadata.path], @@ -639,7 +639,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv if (userResponse === "Retry") { return this.delete(uri, _options); } - return; + throw correlation.asError(); } parent.entries.delete(entryToDelete.name); @@ -746,7 +746,7 @@ export class UssFSProvider extends BaseProvider implements vscode.FileSystemProv await api.uploadFromBuffer(Buffer.from(fileEntry.data), outputPath); } } catch (err) { - const userResponse = await this._handleError(err, { + const { userResponse } = await this._handleError(err, { additionalContext: vscode.l10n.t({ message: "Failed to copy {0} to {1}", args: [source.path, destination.path, err.message], diff --git a/packages/zowe-explorer/src/webviews/src/edit-history/App.tsx b/packages/zowe-explorer/src/webviews/src/edit-history/App.tsx index bf49e28c5..a047014c8 100644 --- a/packages/zowe-explorer/src/webviews/src/edit-history/App.tsx +++ b/packages/zowe-explorer/src/webviews/src/edit-history/App.tsx @@ -14,7 +14,7 @@ import { VSCodeDivider, VSCodePanels, VSCodePanelTab } from "@vscode/webview-ui- import { JSXInternal } from "preact/src/jsx"; import { isSecureOrigin } from "../utils"; import PersistentDataPanel from "./components/PersistentTable/PersistentDataPanel"; -import PersistentVSCodeAPI from "../PersistentVSCodeAPI" +import PersistentVSCodeAPI from "../PersistentVSCodeAPI"; import PersistentManagerHeader from "./components/PersistentManagerHeader/PersistentManagerHeader"; export function App(): JSXInternal.Element {