From c091517b675bee4f04484b69c5f1ff50aebce820 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 16 Aug 2024 10:59:58 -0700 Subject: [PATCH 1/2] add eslint rule --- extensions/vscode/.eslintrc.json | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/extensions/vscode/.eslintrc.json b/extensions/vscode/.eslintrc.json index 151335b6b..8e2982291 100644 --- a/extensions/vscode/.eslintrc.json +++ b/extensions/vscode/.eslintrc.json @@ -25,8 +25,14 @@ "@typescript-eslint/semi": "warn", "curly": "warn", "eqeqeq": "warn", - "no-throw-literal": "warn", - "semi": "off" + "no-throw-literal": "error", + "semi": "off", + "no-restricted-syntax": [ + "error", + { + "selector": "ThrowStatement" + } + ] }, "ignorePatterns": ["out", "dist", "**/*.d.ts", "api"] } From 93a3562db754e28f9bd545cdff10bf1c9e128b88 Mon Sep 17 00:00:00 2001 From: Bill Sager Date: Fri, 16 Aug 2024 12:42:44 -0700 Subject: [PATCH 2/2] Eliminate throw statements from extension codebase --- extensions/vscode/src/api/types/events.ts | 14 ++++ extensions/vscode/src/events.ts | 17 +++-- .../src/multiStepInputs/multiStepHelper.ts | 5 +- .../src/multiStepInputs/newCredential.ts | 5 +- .../src/multiStepInputs/newDeployment.ts | 65 ++++++++++++++----- extensions/vscode/src/utils/webviewConduit.ts | 26 ++++---- extensions/vscode/src/views/homeView.ts | 30 +++++---- 7 files changed, 113 insertions(+), 49 deletions(-) diff --git a/extensions/vscode/src/api/types/events.ts b/extensions/vscode/src/api/types/events.ts index bf6e6f628..362df881b 100644 --- a/extensions/vscode/src/api/types/events.ts +++ b/extensions/vscode/src/api/types/events.ts @@ -134,6 +134,8 @@ export interface EventSubscriptionTargetCallbackMap { "publish/success": OnPublishSuccessCallback; "publish/failure": OnPublishFailureCallback; + + undefined: OnUndefinedMessageCallback; } export const restoreMsgToStatusSuffix = ( @@ -1272,6 +1274,18 @@ export function isPublishFailure(arg: Events): arg is PublishFailure { return arg.type === "publish/failure"; } +export interface UndefinedMessage extends EventStreamMessage { + type: "undefined"; + data: { + msgType: string; + }; +} + +export type OnUndefinedMessageCallback = (msg: UndefinedMessage) => void; +export function isUndefinedMessage(arg: Events): arg is PublishFailure { + return arg.type === "undefined"; +} + // Events are a union type of our base and our extended interfaces export type Events = | EventStreamMessage diff --git a/extensions/vscode/src/events.ts b/extensions/vscode/src/events.ts index 7185dce62..29afe3587 100644 --- a/extensions/vscode/src/events.ts +++ b/extensions/vscode/src/events.ts @@ -204,7 +204,7 @@ export class EventStream extends Readable implements Disposable { private messageFactory(message: EventStreamMessage): EventStreamMessage[] { // Transform restoreREnv messages into restoreEnv messages // while maintaining original message - if (message.type.includes("publish/restoreREnv")) { + if (message.type?.includes("publish/restoreREnv")) { const messages: EventStreamMessage[] = []; messages.push(message); const newMessage: EventStreamMessage = JSON.parse( @@ -231,16 +231,19 @@ export class EventStream extends Readable implements Disposable { newMessage.type = "publish/restoreEnv/progress"; break; default: - throw new Error( - `events::messageFactory: Unknown publish/restoreREnv based message: ${newMessage.type}`, + newMessage.type = "undefined"; + newMessage.data.typeStr = message.type; + console.error( + `Internal Error: events::messageFactory: Unknown publish/restoreREnv based message: ${newMessage.type}.`, ); + break; } messages.push(newMessage); return messages; } // Transform restorePythonEnv messages into restoreEnv messages // while maintaining original message - if (message.type.includes("publish/restorePythonEnv")) { + if (message.type?.includes("publish/restorePythonEnv")) { const messages: EventStreamMessage[] = []; messages.push(message); const newMessage: EventStreamMessage = JSON.parse( @@ -266,8 +269,10 @@ export class EventStream extends Readable implements Disposable { newMessage.type = "publish/restoreEnv/progress"; break; default: - throw new Error( - `events::messageFactory: Unknown publish/restorePythonEnv based message: ${newMessage.type}`, + newMessage.type = "undefined"; + newMessage.data.typeStr = message.type; + console.error( + `Internal Error: events::messageFactory: Unknown publish/restorePythonEnv based message: ${newMessage.type}.`, ); } messages.push(newMessage); diff --git a/extensions/vscode/src/multiStepInputs/multiStepHelper.ts b/extensions/vscode/src/multiStepInputs/multiStepHelper.ts index 11bcdac3e..0431ebafd 100644 --- a/extensions/vscode/src/multiStepInputs/multiStepHelper.ts +++ b/extensions/vscode/src/multiStepInputs/multiStepHelper.ts @@ -110,7 +110,10 @@ export class MultiStepInput { } else if (err === InputFlowAction.cancel) { step = undefined; } else { - throw err; + window.showErrorMessage( + `Internal Error: MultiStepInput::stepThrough, err = ${JSON.stringify(err)}.`, + ); + step = undefined; } } } diff --git a/extensions/vscode/src/multiStepInputs/newCredential.ts b/extensions/vscode/src/multiStepInputs/newCredential.ts index 5de569187..18a90c850 100644 --- a/extensions/vscode/src/multiStepInputs/newCredential.ts +++ b/extensions/vscode/src/multiStepInputs/newCredential.ts @@ -122,7 +122,10 @@ export async function newCredential( new URL(input); } catch (e) { if (!(e instanceof TypeError)) { - throw e; + return Promise.resolve({ + message: `Unexpected error within NewCredential::inputSeverUrl.finalValidation: ${JSON.stringify(e)}`, + severity: InputBoxValidationSeverity.Error, + }); } return Promise.resolve({ message: `Error: Invalid URL (${getMessageFromError(e)}).`, diff --git a/extensions/vscode/src/multiStepInputs/newDeployment.ts b/extensions/vscode/src/multiStepInputs/newDeployment.ts index 4912b5455..6da5e5151 100644 --- a/extensions/vscode/src/multiStepInputs/newDeployment.ts +++ b/extensions/vscode/src/multiStepInputs/newDeployment.ts @@ -786,9 +786,10 @@ export async function newDeployment( if (discoveredEntryPoints.length > 1) { const step = getStepInfo("inputEntryPointFileSelection", state); if (!step) { - throw new Error( - "newDeployment::inputEntryPointSelection step info not found.", + window.showErrorMessage( + "Internal Error: newDeployment::inputEntryPointFileSelection step info not found.", ); + return; } const pick = await input.showQuickPick({ @@ -844,9 +845,10 @@ export async function newDeployment( if (inspectionQuickPicks.length > 1) { const step = getStepInfo("inputEntryPointContentTypeSelection", state); if (!step) { - throw new Error( - "newDeployment::inputEntryPointSelection step info not found.", + window.showErrorMessage( + "Internal Error: newDeployment::inputEntryPointContentTypeSelection step info not found.", ); + return; } const pick = await input.showQuickPick({ @@ -881,7 +883,10 @@ export async function newDeployment( const step = getStepInfo("inputTitle", state); if (!step) { - throw new Error("newDeployment::inputTitle step info not found."); + window.showErrorMessage( + "Internal Error: newDeployment::inputTitle step info not found.", + ); + return; } let initialValue = ""; if ( @@ -927,7 +932,10 @@ export async function newDeployment( if (!newCredentialForced(state)) { const step = getStepInfo("pickCredentials", state); if (!step) { - throw new Error("newDeployment::pickCredentials step info not found."); + window.showErrorMessage( + "Internal Error: newDeployment::pickCredentials step info not found.", + ); + return; } const pick = await input.showQuickPick({ title: state.title, @@ -964,7 +972,10 @@ export async function newDeployment( const step = getStepInfo("inputServerUrl", state); if (!step) { - throw new Error("newDeployment::inputServerUrl step info not found."); + window.showErrorMessage( + "Internal Error: newDeployment::inputServerUrl step info not found.", + ); + return; } const url = await input.showInputBox({ @@ -990,7 +1001,10 @@ export async function newDeployment( new URL(input); } catch (e) { if (!(e instanceof TypeError)) { - throw e; + return Promise.resolve({ + message: `Internal Error: Unknown Error (${JSON.stringify(e)}).`, + severity: InputBoxValidationSeverity.Error, + }); } return Promise.resolve({ message: `Error: Invalid URL (${getMessageFromError(e)}).`, @@ -1053,7 +1067,10 @@ export async function newDeployment( const step = getStepInfo("inputAPIKey", state); if (!step) { - throw new Error("newDeployment::inputAPIKey step info not found."); + window.showErrorMessage( + "Internal Error: newDeployment::inputAPIKey step info not found.", + ); + return; } const apiKey = await input.showInputBox({ @@ -1135,9 +1152,10 @@ export async function newDeployment( const step = getStepInfo("inputCredentialName", state); if (!step) { - throw new Error( - "newDeployment::inputCredentialName step info not found.", + window.showErrorMessage( + "Internal Error: newDeployment::inputCredentialName step info not found.", ); + return; } const name = await input.showInputBox({ @@ -1222,7 +1240,10 @@ export async function newDeployment( state.data.apiKey === undefined || isQuickPickItem(state.data.apiKey) ) { - throw new Error("NewDeployment Unexpected type guard failure @1"); + window.showErrorMessage( + "Internal Error: NewDeployment Unexpected type guard failure @1", + ); + return; } try { // NEED an credential to be returned from this API @@ -1250,7 +1271,10 @@ export async function newDeployment( } } else { // we are not creating a credential but also do not have a required existing value - throw new Error("NewDeployment Unexpected type guard failure @2"); + window.showErrorMessage( + "Internal Error: NewDeployment Unexpected type guard failure @2", + ); + return; } // Create the Config File @@ -1294,7 +1318,10 @@ export async function newDeployment( ) { finalCredentialName = state.data.name; } else if (!state.data.credentialName) { - throw new Error("NewDeployment Unexpected type guard failure @3"); + window.showErrorMessage( + "Internal Error: NewDeployment Unexpected type guard failure @3", + ); + return; } else if ( newCredentialSelected(state) && state.data.name && @@ -1306,7 +1333,10 @@ export async function newDeployment( } if (!finalCredentialName) { // should have assigned it by now. Logic error! - throw new Error("NewDeployment Unexpected type guard failure @4"); + window.showErrorMessage( + "Internal Error: NewDeployment Unexpected type guard failure @4", + ); + return; } // Create the PreContentRecord File @@ -1336,7 +1366,10 @@ export async function newDeployment( return; } if (!newOrSelectedCredential) { - throw new Error("NewDeployment Unexpected type guard failure @5"); + window.showErrorMessage( + "Internal Error: NewDeployment Unexpected type guard failure @5", + ); + return; } return { contentRecord: newContentRecord, diff --git a/extensions/vscode/src/utils/webviewConduit.ts b/extensions/vscode/src/utils/webviewConduit.ts index fe4bb4f15..7c831635e 100644 --- a/extensions/vscode/src/utils/webviewConduit.ts +++ b/extensions/vscode/src/utils/webviewConduit.ts @@ -1,6 +1,6 @@ // Copyright (C) 2024 by Posit Software, PBC. -import { Webview, Disposable } from "vscode"; +import { Webview, Disposable, window } from "vscode"; import { WebviewToHostMessageCB } from "../types/messages/conduit"; import { isWebviewToHostMessage } from "../types/messages/webviewToHostMessages"; import { HostToWebviewMessage } from "../types/messages/hostToWebviewMessages"; @@ -22,16 +22,16 @@ export class WebviewConduit { // `\nWebviewConduit trace: ${obj.kind}: ${JSON.stringify(obj.content)}`, // ); if (!isWebviewToHostMessage(obj)) { - const msg = `\nNonConduitMessage Received: ${JSON.stringify(e)}\n`; - - throw new Error(msg); + const msg = `Internal Error: WebviewConduit::onRawMsgCB - NonConduitMessage Received: ${JSON.stringify(e)}`; + window.showErrorMessage(msg); + return; } if (this.onMsgCB) { this.onMsgCB(obj); } else { - const msg = `onMsg callback not set ahead of receiving message: ${JSON.stringify(e)}`; - console.error(msg); - throw new Error(msg); + const msg = `Internal Error: WebviewConduit::onRawMsgCB - onMsg callback not set ahead of receiving message: ${JSON.stringify(e)}`; + window.showErrorMessage(msg); + return; } }; @@ -49,14 +49,18 @@ export class WebviewConduit { this.pendingMsgs = []; }; - public onMsg = (cb: WebviewToHostMessageCB): Disposable => { + public onMsg = (cb: WebviewToHostMessageCB): Disposable | undefined => { if (!this.target) { - throw new Error( - `WebviewConduit::onMsg called before webview reference established with init().`, + window.showErrorMessage( + "Internal Error: WebviewConduit::onMsg called before webview reference established with init().", ); + return undefined; } if (this.onMsgCB) { - throw new Error(`WebviewConduit::onMsg called a second time!`); + window.showErrorMessage( + "Internal Error: WWebviewConduit::onMsg called an unexpected second time", + ); + return undefined; } this.onMsgCB = cb; return this.target.onDidReceiveMessage(this.onRawMsgCB); diff --git a/extensions/vscode/src/views/homeView.ts b/extensions/vscode/src/views/homeView.ts index 4a0d32ff3..a3cff6334 100644 --- a/extensions/vscode/src/views/homeView.ts +++ b/extensions/vscode/src/views/homeView.ts @@ -223,9 +223,10 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { case WebviewToHostMessageType.VIEW_PUBLISHING_LOG: return this.showPublishingLog(); default: - throw new Error( - `Error: onConduitMessage unhandled msg: ${JSON.stringify(msg)}`, + window.showErrorMessage( + `Internal Error: onConduitMessage unhandled msg: ${JSON.stringify(msg)}`, ); + return; } } @@ -370,8 +371,8 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { "refreshContentRecordData::contentRecords.getAll", error, ); - window.showInformationMessage(summary); - throw error; + window.showErrorMessage(summary); + return; } } @@ -382,11 +383,11 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { await refresh; } catch (error: unknown) { const summary = getSummaryStringFromError( - "refreshConfigurationData::configurations.getAll", + "Internal Error: refreshConfigurationData::configurations.getAll", error, ); - window.showInformationMessage(summary); - throw error; + window.showErrorMessage(summary); + return; } } @@ -397,11 +398,11 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { await refresh; } catch (error: unknown) { const summary = getSummaryStringFromError( - "refreshCredentialData::credentials.list", + "Internal Error: refreshCredentialData::credentials.list", error, ); - window.showInformationMessage(summary); - throw error; + window.showErrorMessage(summary); + return; } } @@ -1140,10 +1141,11 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { ); // Sets up an event listener to listen for messages passed from the webview view this.context - // and executes code based on the message that is recieved - this.disposables.push( - this.webviewConduit.onMsg(this.onConduitMessage.bind(this)), - ); + // and executes code based on the message that is received + const disp = this.webviewConduit.onMsg(this.onConduitMessage.bind(this)); + if (disp) { + this.disposables.push(disp); + } } /** * Defines and returns the HTML that should be rendered within the webview panel.