Skip to content

Commit

Permalink
Merge pull request #2132 from posit-dev/sagerb-eliminate-throws-for-i…
Browse files Browse the repository at this point in the history
…nternal-errors

Eliminate throws for internal errors
  • Loading branch information
sagerb authored Aug 21, 2024
2 parents c11819d + 4359e7a commit 57a9cac
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 51 deletions.
10 changes: 8 additions & 2 deletions extensions/vscode/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,15 @@
"@typescript-eslint/semi": "warn",
"curly": "warn",
"eqeqeq": "warn",
"no-throw-literal": "warn",
"no-throw-literal": "error",
"semi": "off",
"require-await": "error"
"require-await": "error",
"no-restricted-syntax": [
"error",
{
"selector": "ThrowStatement"
}
]
},
"ignorePatterns": ["out", "dist", "**/*.d.ts", "api"]
}
14 changes: 14 additions & 0 deletions extensions/vscode/src/api/types/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ export interface EventSubscriptionTargetCallbackMap {

"publish/success": OnPublishSuccessCallback;
"publish/failure": OnPublishFailureCallback;

undefined: OnUndefinedMessageCallback;
}

export const restoreMsgToStatusSuffix = (
Expand Down Expand Up @@ -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
Expand Down
17 changes: 11 additions & 6 deletions extensions/vscode/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion extensions/vscode/src/multiStepInputs/multiStepHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion extensions/vscode/src/multiStepInputs/newCredential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}).`,
Expand Down
65 changes: 49 additions & 16 deletions extensions/vscode/src/multiStepInputs/newDeployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand All @@ -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)}).`,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 &&
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 15 additions & 11 deletions extensions/vscode/src/utils/webviewConduit.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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;
}
};

Expand All @@ -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);
Expand Down
30 changes: 16 additions & 14 deletions extensions/vscode/src/views/homeView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -370,8 +371,8 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
"refreshContentRecordData::contentRecords.getAll",
error,
);
window.showInformationMessage(summary);
throw error;
window.showErrorMessage(summary);
return;
}
}

Expand All @@ -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;
}
}

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -1150,10 +1151,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.
Expand Down

0 comments on commit 57a9cac

Please sign in to comment.