Report cli command execution error with user's data redacted

This PR fixes #2078.

Signed-off-by: Denis Golovin dgolovin@redhat.com

Author: dgolovin

src/odo.ts

@@ -754,7 +754,7 @@ export class OdoImpl implements Odo {
             cwd ? {cwd} : { }
         );
         if (result.error && fail) {
-            throw new VsCommandError(`${result.error}`, `Error when running command: ${commandPrivacy}`, result.error);
+            throw new VsCommandError(`${result.error.message}`, `Error when running command: ${commandPrivacy}`, result.error);
         };
         return result;
     }
@@ -859,7 +859,7 @@ export class OdoImpl implements Odo {
 
     public async createComponentFromFolder(application: OpenShiftObject, type: string, version: string, name: string, location: Uri, starter: string = undefined, useExistingDevfile = false): Promise<OpenShiftObject> {
         await this.execute(Command.createLocalComponent(application.getParent().getName(), application.getName(), type, version, name, location.fsPath, starter, useExistingDevfile), location.fsPath);
-        if (workspace.workspaceFolders && application.getParent().getParent()) { // if there are workspace folders and cluster is acvessible
+        if (workspace.workspaceFolders && application.getParent().getParent()) { // if there are workspace folders and cluster is accessible
             const targetApplication = (await this.getApplications(application.getParent())).find((value) => value === application);
             if (!targetApplication) {
                 await this.insertAndReveal(application);

src/openshift/application.ts

@@ -17,7 +17,16 @@ export class Application extends OpenShiftItem {
     static async describe(treeItem: OpenShiftObject): Promise<void> {
         const application = await Application.getOpenShiftCmdData(treeItem,
             'Select Application you want to describe');
-        if (application) Application.odo.executeInTerminal(Command.describeApplication(application.getParent().getName(), application.getName()), undefined, `OpenShift: Describe '${application.getName()}' Application`);
+        if (application) {
+            Application.odo.executeInTerminal(
+                Command.describeApplication(
+                    application.getParent().getName(),
+                    application.getName()
+                ),
+                undefined,
+                `OpenShift: Describe '${application.getName()}' Application`
+            );
+        }
     }
 
     @vsCommand('openshift.app.delete', true)
@@ -29,9 +38,14 @@ export class Application extends OpenShiftItem {
             const appName = application.getName();
             const value = await window.showWarningMessage(`Do you want to delete Application '${appName}'?`, 'Yes', 'Cancel');
             if (value === 'Yes') {
-                return Progress.execFunctionWithProgress(`Deleting the Application '${appName}'`, () => Application.odo.deleteApplication(application))
-                    .then(() => `Application '${appName}' successfully deleted`)
-                    .catch((err) => Promise.reject(new VsCommandError(`Failed to delete Application with error '${err}'`, 'Failed to delete Application')));
+                try {
+                    await Progress.execFunctionWithProgress(`Deleting the Application '${appName}'`, () => Application.odo.deleteApplication(application));
+                    return `Application '${appName}' successfully deleted`
+                } catch (err) {
+                    const telemetryMessage = err instanceof VsCommandError ? err.telemetryMessage : err.message;
+                    throw new VsCommandError(`Failed to delete Application with error '${err.message}'`,
+                            `Failed to delete Application with error ${telemetryMessage}`, err);
+                }
             }
         }
         return null;

src/openshift/component.ts

@@ -27,12 +27,12 @@ import { SourceType } from '../odo/config';
 import { ComponentKind, ComponentTypeAdapter, DevfileComponentType, ImageStreamTag, isDevfileComponent, isImageStreamTag } from '../odo/componentType';
 import { Url } from '../odo/url';
 import { ComponentDescription, StarterProjectDescription } from '../odo/catalog';
+import { isStarterProject, StarterProject } from '../odo/componentTypeDescription';
 import path = require('path');
-
 import globby = require('globby');
 import treeKill = require('tree-kill');
 import fs = require('fs-extra');
-import { isStarterProject, StarterProject } from '../odo/componentTypeDescription';
+import { NewComponentCommandProps } from '../telemetry';
 
 const waitPort = require('wait-port');
 
@@ -111,7 +111,7 @@ export class Component extends OpenShiftItem {
     static async create(application: OpenShiftApplication): Promise<string> {
         if (!application) return null;
 
-        return Component.createFromLocal(application).catch((err) => Promise.reject(new VsCommandError(`Failed to create Component with error '${err}'`, 'Failed to create Component with error')));
+        return Component.createFromLocal(application);
     }
 
     @vsCommand('openshift.component.delete', true)
@@ -684,34 +684,45 @@ export class Component extends OpenShiftItem {
         }
 
         const refreshComponentsView = workspace.getWorkspaceFolder(folder);
-
-        await Progress.execFunctionWithProgress(
-            `Creating new Component '${componentName}'`,
-            () => Component.odo.createComponentFromFolder(
-                application,
-                componentType? componentType.name : undefined, // in case of using existing devfile
-                componentType? componentType.version : undefined,
-                componentName,
-                folder,
-                createStarter,
-                useExistingDevfile
-            )
-        );
-
-        // when creating component based on existing workspace folder refresh components view
-        if (refreshComponentsView) {
-            commands.executeCommand('openshift.componentsView.refresh');
-        }
-
-        const result:any = new String(`Component '${componentName}' successfully created. To deploy it on cluster, perform 'Push' action.`);
-        result.properties = {
+        const creatComponentProperties: NewComponentCommandProps  = {
             'component_kind': componentType?.version ? ComponentKind.S2I: ComponentKind.DEVFILE,
             'component_type': componentType?.name,
             'component_version': componentType?.version,
             'starter_project': createStarter,
             'use_existing_devfile': useExistingDevfile,
         };
-        return result;
+        try {
+            await Progress.execFunctionWithProgress(
+                `Creating new Component '${componentName}'`,
+                () => Component.odo.createComponentFromFolder(
+                    application,
+                    componentType? componentType.name : undefined, // in case of using existing devfile
+                    componentType? componentType.version : undefined,
+                    componentName,
+                    folder,
+                    createStarter,
+                    useExistingDevfile
+                )
+            );
+
+            // when creating component based on existing workspace folder refresh components view
+            if (refreshComponentsView) {
+                commands.executeCommand('openshift.componentsView.refresh');
+            }
+
+            const result:any = new String(`Component '${componentName}' successfully created. To deploy it on cluster, perform 'Push' action.`);
+            result.properties = creatComponentProperties;
+            return result;
+        } catch (err) {
+            if (err instanceof VsCommandError) {
+                throw new VsCommandError(
+                    `Error occurred while creating Component '${componentName}': ${err.message}`,
+                    `Error occurred while creating Component: ${err.telemetryMessage}`, err,
+                    creatComponentProperties
+                );
+            }
+            throw err;
+        }
     }
 
     @vsCommand('openshift.component.createFromGit')

src/telemetry.ts

@@ -26,4 +26,22 @@ export default async function sendTelemetry(actionName: string, properties?: any
         return service?.sendStartupEvent();
     }
     return service?.send(createTrackingEvent(actionName, properties));
-}
+}
+
+export interface CommonCommandProps {
+    identifier: string;
+    error: string;
+    'stack_trace': string;
+    duration: number;
+    cancelled: boolean;
+}
+
+export interface NewComponentCommandProps {
+    'component_kind': string;
+    'component_type': string;
+    'component_version': string;
+    'starter_project': string;
+    'use_existing_devfile': boolean;
+}
+
+export type AllProps = Partial<CommonCommandProps & NewComponentCommandProps>;

src/vscommand.ts

@@ -4,10 +4,12 @@
  *-----------------------------------------------------------------------------------------------*/
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
+/* eslint-disable camelcase */
+/* eslint-disable @typescript-eslint/camelcase */
 
 import { commands, Disposable, window } from 'vscode';
 import * as stackTraceParser from 'stacktrace-parser';
-import sendTelemetry from './telemetry';
+import sendTelemetry, { AllProps, CommonCommandProps } from './telemetry';
 import { ExtenisonID } from './util/constants';
 
 type VsCommandFunction = (...args: any[]) => Promise<any> | any;
@@ -18,8 +20,13 @@ interface VsCommand {
     method: VsCommandFunction;
 }
 
+export interface Result<ReturnType> {
+    value: ReturnType;
+    properties: AllProps;
+}
+
 export class VsCommandError extends Error {
-    constructor(message: string, public readonly telemetryMessage = message, public parent?: Error) {
+    constructor(message: string, public readonly telemetryMessage = message, public readonly parent?, public readonly telemetryProps: AllProps = {}) {
         super(message);
         Object.setPrototypeOf(this, new.target.prototype);
     }
@@ -40,52 +47,50 @@ export async function registerCommands(...modules: string[]): Promise<Disposable
     await Promise.all(modules.map((module) => import(module)));
     return vsCommands.map((cmd) => {
         return commands.registerCommand(cmd.commandId, async (...params) => {
-            let telemetryProps: any = {
+            let telemetryProps: Partial<CommonCommandProps> = {
                 identifier: cmd.commandId,
             };
             let result: any;
+            let exception: any;
             const startTime = Date.now();
-            let stackTrace = {};
             try {
                 result = await Promise.resolve(cmd.method.call(null, ...params));
                 displayResult(result);
             } catch (err) {
+                let stack:stackTraceParser.StackFrame[];
                 if (err.stack) {
-                    const stack = stackTraceParser.parse(err.stack); // TODO: add recursive stacktrace parsing for parent errors
+                    stack = stackTraceParser.parse(err.stack); // TODO: add recursive stacktrace parsing for parent errors
                     if (stack.length > 0) {
                         const files = stack.map((value) => `${value.file.substring(value.file.lastIndexOf(ExtenisonID)-1)}:${value.lineNumber}:${value.column}`);
-                        stackTrace = {
-                            'stack_trace': files.join('\n')
-                        };
+                        telemetryProps.stack_trace  = files.join('\n')
                     }
                 }
                 if (err instanceof VsCommandError) {
                     // exception thrown by extension command with meaningful message
                     // just show it and return
                     telemetryProps.error = err.telemetryMessage;
-                    window.showErrorMessage(err.message);
                 } else {
-                    // Unexpected exception happened. Let vscode handle the error reporting.
-                    // This does not work when command started by pressing button in view title
-                    // TODO: Wrap view title commands in try/catch and re-throw as VsCommandError
-                    // TODO: telemetry cannot send not known exception stacktrace or message
-                    // because it can contain user's sensitive information
+                    // Unexpected exception happened.
                     if (err instanceof TypeError) {
                         telemetryProps.error = err.message;
                     } else {
                         telemetryProps.error = 'Unexpected error';
                     }
-                    window.showErrorMessage(err.toString());
                 }
+                window.showErrorMessage(err.message);
+                exception = err;
             } finally {
                 telemetryProps.duration = Date.now() - startTime;
                 telemetryProps.cancelled = result === null;
                 if (result?.properties) {
                     telemetryProps = {...telemetryProps, ...result.properties};
                 }
-                telemetryProps = {...telemetryProps, ...stackTrace };
+                if (exception?.telemetryProps) {
+                    telemetryProps = {...telemetryProps, ...exception.telemetryProps};
+                }
                 sendTelemetry('command', telemetryProps);
             }
+            return result;
         });
     });
 }

test/unit/extension.test.ts

@@ -156,14 +156,9 @@ suite('openshift connector Extension', () => {
         sandbox.stub(vscode.window, 'showWarningMessage').resolves('Yes');
         sandbox.stub(vscode.window, 'showInformationMessage');
         const semStub = sandbox.stub(vscode.window, 'showErrorMessage');
-        sandbox.stub(Progress, 'execFunctionWithProgress').rejects(Error('message'));
-        let error;
-        try {
-            await vscode.commands.executeCommand('openshift.app.delete', appItem);
-        } catch (err) {
-            error = err;
-        }
-        expect(error).not.null;
-        expect(semStub).calledWith('Failed to delete Application with error \'Error: message\'');
+        const error = new Error('message');
+        sandbox.stub(Progress, 'execFunctionWithProgress').rejects(error);
+        await vscode.commands.executeCommand('openshift.app.delete', appItem);
+        expect(semStub).calledWith(`Failed to delete Application with error '${error.message}'`);
     });
 });

test/unit/openshift/application.test.ts

@@ -12,6 +12,7 @@ import { Command } from '../../../src/odo/command';
 import { Application } from '../../../src/openshift/application';
 import { TestItem } from './testOSItem';
 import OpenShiftItem from '../../../src/openshift/openshiftItem';
+import { VsCommandError } from '../../../src/vscommand';
 
 const {expect} = chai;
 chai.use(sinonChai);
@@ -125,7 +126,7 @@ suite('OpenShift/Application', () => {
 
         test('throws an error message when odo command failed', async () => {
             warnStub.resolves('Yes');
-            execStub.rejects('ERROR');
+            execStub.rejects(new VsCommandError('ERROR', 'ERROR'));
             let expectedError;
             try {
                 await Application.del(appItem);

test/unit/openshift/component.test.ts

@@ -125,14 +125,14 @@ suite('OpenShift/Component', () => {
         });
 
         test('errors when a subcommand fails', async () => {
-            quickPickStub.onFirstCall().rejects(errorMessage);
+            quickPickStub.onFirstCall().rejects(new Error(errorMessage));
             let expectedError: Error;
             try {
                 await Component.create(appItem);
             } catch (error) {
                 expectedError = error;
             }
-            expect(expectedError.message).equals(`Failed to create Component with error '${errorMessage}'`);
+            expect(expectedError.message).equals(errorMessage);
         });
 
         suite('from local workspace', () => {