Stop downloading odo/oc when cluster is down (#406)

* Propagate errors from odo

* Do not download oc again in case of error

* Fix comment for the change

* Fix test for exception being thrown

* Fix error flow

* Fix test error

src/cli.ts

@@ -36,15 +36,9 @@ export class Cli implements ICli {
             childProcess.exec(cmd, opts, (error: ExecException, stdout: string, stderr: string) => {
                 this.odoChannel.print(stdout);
                 this.odoChannel.print(stderr);
-                if (error) {
-                    if (error.code === 1 && stderr.trim().length === 0) {
-                        resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr });
-                    } else {
-                        reject(error);
-                    }
-                } else {
-                    resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr });
-                }
+                // do not reject it here, because caller in some cases need the error and the streams
+                // to make a decision
+                resolve({ error, stdout: stdout.replace(/---[\s\S]*---/g, '').trim(), stderr });
             });
         });
     }

src/odo.ts

@@ -111,7 +111,7 @@ export interface Odo {
     getServiceTemplatePlans(svc: string): Promise<string[]>;
     getServices(application: OpenShiftObject): Promise<OpenShiftObject[]>;
     getApplicationChildren(application: OpenShiftObject): Promise<OpenShiftObject[]>;
-    execute(command: string, cwd?: string): Promise<CliExitData>;
+    execute(command: string, cwd?: string, fail?: boolean): Promise<CliExitData>;
     requireLogin(): Promise<boolean>;
 }
 
@@ -207,9 +207,9 @@ export class OdoImpl implements Odo {
         return clusters;
     }
 
-    public async getClustersWithOc(): Promise<OpenShiftObject[]> {
+    private async getClustersWithOc(): Promise<OpenShiftObject[]> {
         let clusters: OpenShiftObject[] = [];
-        const result: cliInstance.CliExitData = await this.execute(`oc version`);
+        const result: cliInstance.CliExitData = await this.execute(`oc version`, process.cwd(), false);
         clusters = result.stdout.trim().split('\n').filter((value) => {
             return value.indexOf('Server ') !== -1;
         }).map((value) => {
@@ -219,7 +219,7 @@ export class OdoImpl implements Odo {
         return clusters;
     }
 
-    public async getClustersWithOdo(): Promise<OpenShiftObject[]> {
+    private async getClustersWithOdo(): Promise<OpenShiftObject[]> {
         let clusters: OpenShiftObject[] = [];
         const result: cliInstance.CliExitData = await this.execute(
             `odo version && odo project list`
@@ -294,13 +294,19 @@ export class OdoImpl implements Odo {
         terminal.show();
     }
 
-    public async execute(command: string, cwd?: string): Promise<CliExitData> {
+    public async execute(command: string, cwd?: string, fail: boolean = true): Promise<CliExitData> {
         const cmd = command.split(' ')[0];
         const toolLocation = await ToolsConfig.detectOrDownload(cmd);
         return OdoImpl.cli.execute(
             toolLocation ? command.replace(cmd, `"${toolLocation}"`).replace(new RegExp(`&& ${cmd}`, 'g'), `&& "${toolLocation}"`) : command,
             cwd ? {cwd} : { }
-        );
+        ).then(async (result) => {
+            if (result.error && fail) {
+                return Promise.reject(result.error);
+            } else {
+                return result;
+            }
+        });
     }
 
     public async requireLogin(): Promise<boolean> {

src/tools.ts

@@ -162,18 +162,14 @@ export class ToolsConfig {
         let detectedVersion: string;
         if (fs.existsSync(location)) {
             const version = new RegExp(`${cmd} v([\\d\\.]+)`);
-            try {
-                const result = await Cli.getInstance().execute(`"${location}" version`);
-                if (!result.error) {
-                    const toolVersion: string[] = result.stdout.trim().split('\n').filter((value) => {
-                        return value.match(version);
-                    }).map((value)=>version.exec(value)[1]);
-                    if (toolVersion.length) {
-                        detectedVersion = toolVersion[0];
-                    }
+            const result = await Cli.getInstance().execute(`"${location}" version`);    
+            if (result.stdout) {
+                const toolVersion: string[] = result.stdout.trim().split('\n').filter((value) => {
+                    return value.match(version);
+                }).map((value)=>version.exec(value)[1]);
+                if (toolVersion.length) {
+                    detectedVersion = toolVersion[0];
                 }
-            } catch (ignore) {
-                // if `${tool} version` failed, then there is no tool at specified location
             }
         }
         return detectedVersion;

src/util/progress.ts

@@ -7,6 +7,8 @@
 
 import * as vscode from 'vscode';
 import * as odoctl from '../odo';
+import { CliExitData } from '../cli';
+import { ExecException } from 'child_process';
 
 export interface Step {
     command: string;
@@ -17,33 +19,29 @@ export interface Step {
 export class Progress {
     static execWithProgress(options, steps: Step[], odo: odoctl.Odo): Thenable<void> {
         return vscode.window.withProgress(options,
-        (progress: vscode.Progress<{increment: number, message: string}>, token: vscode.CancellationToken) => {
-            const calls: (()=>Promise<any>)[] = [];
-            steps.reduce((previous: Step, current: Step, currentIndex: number, steps: Step[])=> {
-                calls.push(()=> {
-                    let result: Promise<any> = Promise.resolve();
-                    progress.report({
-                        increment: previous.increment,
-                        message: `${previous.total}%`
-                    });
-                    result = odo.execute(current.command);
+            (progress: vscode.Progress<{increment: number, message: string}>, token: vscode.CancellationToken) => {
+                const calls: (()=>Promise<any>)[] = [];
+                steps.reduce((previous: Step, current: Step, currentIndex: number, steps: Step[])=> {
                     current.total = previous.total + current.increment;
-                    return currentIndex+1 === steps.length ? result.then(()=> {
-                        progress.report({
-                            increment: previous.increment,
-                            message: `${previous.total}%`
-                        });
-                    }) : result;
-                });
-                return current;
-            }, {increment: 0, command: "", total: 0});
+                    calls.push (() => {
+                        return Promise.resolve()
+                            .then(() => progress.report({increment: previous.increment, message: `${previous.total}%` }))
+                            .then(() => odo.execute(current.command))
+                            .then(() => {
+                                if (currentIndex+1 === steps.length) {
+                                    progress.report({
+                                        increment: current.increment,
+                                        message: `${current.total}%`
+                                    });
+                                }
+                            });
+                    });
+                    return current;
+                }, {increment: 0, command: "", total: 0});
 
-            return calls.reduce<Promise<any>>((previous: Promise<any>, current: ()=>Promise<any>, index: number, calls: (()=>Promise<any>)[])=> {
-                return previous.then(current);
-            }, Promise.resolve()).catch((error) => {
-                vscode.window.showErrorMessage(`${error}`);
-                return;
+                return calls.reduce<Promise<any>>((previous: Promise<any>, current: ()=>Promise<any>, index: number, calls: (()=>Promise<any>)[])=> {
+                    return previous.then(current);
+                }, Promise.resolve());
             });
-        });
     }
 }

test/util/progress.test.ts

@@ -30,14 +30,14 @@ suite('Progress Utility', () => {
 
     setup(() => {
         sandbox = sinon.createSandbox();
-        execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves();
     });
 
     teardown(() => {
         sandbox.restore();
     });
 
     test('calls cli commands in sequence', async () => {
+        execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves({ error: undefined, stdout: "", stderr: "" });
         await Progress.execWithProgress(options, steps, OdoImpl.getInstance());
 
         expect(execStub).calledTwice;
@@ -46,17 +46,26 @@ suite('Progress Utility', () => {
     });
 
     test('calls progress with given options', async () => {
+        execStub = sandbox.stub(OdoImpl.prototype, 'execute').resolves({ error: undefined, stdout: "", stderr: "" });
         const spy = sandbox.spy(vscode.window, 'withProgress');
         await Progress.execWithProgress(options, steps, OdoImpl.getInstance());
 
         expect(spy).calledOnceWith(options, sinon.match.func);
     });
 
-    test('shows an error message if a command fails', async () => {
-        execStub.rejects(errorMessage);
+    test('throw an error if a command fails', async () => {
+        const error = new Error(errorMessage);
+        execStub = sandbox.stub(OdoImpl.prototype, 'execute').rejects(error);
         const spy = sandbox.spy(vscode.window, 'showErrorMessage');
-        await Progress.execWithProgress(options, steps, OdoImpl.getInstance());
-
-        expect(spy).calledOnceWith(errorMessage);
+        let e;
+        try {
+            await Progress.execWithProgress(options, steps, OdoImpl.getInstance());
+        } catch (err)  {
+            e = err;
+            expect(err.message).equals(errorMessage);
+        }
+        if (!e) {
+            expect.fail('no error thrown');
+        }
     });
 });