Merge pull request #2251 from akshita31/immediate_zip_error

Immediate zip error on invalid zip

Author: akshita31

src/main.ts

@@ -30,6 +30,7 @@ import { ProjectStatusBarObserver } from './observers/ProjectStatusBarObserver';
 import CSharpExtensionExports from './CSharpExtensionExports';
 import { Options } from './omnisharp/options';
 import { vscodeNetworkSettingsProvider, NetworkSettingsProvider } from './NetworkSettings';
+import { ErrorMessageObserver } from './observers/ErrorMessageObserver';
 
 export async function activate(context: vscode.ExtensionContext): Promise<CSharpExtensionExports> {
 
@@ -67,6 +68,9 @@ export async function activate(context: vscode.ExtensionContext): Promise<CSharp
     let informationMessageObserver = new InformationMessageObserver(vscode);
     eventStream.subscribe(informationMessageObserver.post);
 
+    let errorMessageObserver = new ErrorMessageObserver(vscode);
+    eventStream.subscribe(errorMessageObserver.post);
+
     let omnisharpStatusBar = new StatusBarItemAdapter(vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, Number.MIN_VALUE));
     let omnisharpStatusBarObserver = new OmnisharpStatusBarObserver(omnisharpStatusBar);
     eventStream.subscribe(omnisharpStatusBarObserver.post);

src/observers/ErrorMessageObserver.ts

@@ -0,0 +1,35 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+ 
+import { BaseEvent, ZipError } from "../omnisharp/loggingEvents";
+import { vscode } from "../vscodeAdapter";
+ 
+export class ErrorMessageObserver {
+
+    constructor(private vscode: vscode) {
+    }
+
+    public post = (event: BaseEvent) => {
+        switch (event.constructor.name) {
+            case ZipError.name:
+                this.handleZipError(<ZipError>event);
+                break;
+        }
+    }
+
+    private async handleZipError(event: ZipError) {
+        await showErrorMessage(this.vscode, event.message);
+    }
+}
+
+async function showErrorMessage(vscode: vscode, message: string, ...items: string[]) {
+    try {
+        await vscode.window.showErrorMessage(message, ...items);
+    }
+    catch (err) {
+        console.log(err);
+    }
+}

src/observers/WarningMessageObserver.ts

@@ -20,21 +20,7 @@ export class WarningMessageObserver {
         this.warningMessageDebouncer = new Subject<BaseEvent>();
         this.warningMessageDebouncer.debounceTime(1500, scheduler).subscribe(async event => {
             let message = "Some projects have trouble loading. Please review the output for more details.";
-            let value: MessageItemWithCommand;
-            try {
-                value = await this.vscode.window.showWarningMessage<MessageItemWithCommand>(message, { title: "Show Output", command: 'o.showOutput' });
-            }
-            catch (err){
-                console.log(err);
-            }
-            if (value) {
-                try {
-                    await this.vscode.commands.executeCommand<string>(value.command);
-                }
-                catch (err) {
-                    console.log(err);
-                }
-            }
+            await showWarningMessage(this.vscode, message, { title: "Show Output", command: 'o.showOutput' });
         });
     }
 
@@ -54,4 +40,16 @@ export class WarningMessageObserver {
             this.warningMessageDebouncer.next(event);
         }
     }
+}
+
+async function showWarningMessage(vscode: vscode, message: string, ...items: MessageItemWithCommand[]) {
+    try {
+        let value = await vscode.window.showWarningMessage<MessageItemWithCommand>(message, ...items);
+        if (value && value.command) {
+            await vscode.commands.executeCommand<string>(value.command);
+        }
+    }
+    catch (err) {
+        console.log(err);
+    }
 }

src/omnisharp/loggingEvents.ts

@@ -118,6 +118,10 @@ export class DownloadSizeObtained implements BaseEvent {
     constructor(public packageSize: number) { }
 }
 
+export class ZipError implements BaseEvent {
+    constructor(public message: string) { }
+}
+
 export class DebuggerPrerequisiteFailure extends EventWithMessage { }
 export class DebuggerPrerequisiteWarning extends EventWithMessage { }
 export class CommandDotNetRestoreProgress extends EventWithMessage { }

src/packageManager/ZipInstaller.ts

@@ -8,7 +8,7 @@ import * as mkdirp from 'mkdirp';
 import * as path from 'path';
 import * as yauzl from 'yauzl';
 import { EventStream } from "../EventStream";
-import { InstallationStart } from "../omnisharp/loggingEvents";
+import { InstallationStart, ZipError } from "../omnisharp/loggingEvents";
 import { NestedError } from '../NestedError';
 
 export async function InstallZip(buffer: Buffer, description: string, destinationInstallPath: string, binaries: string[], eventStream: EventStream): Promise<void> {
@@ -17,7 +17,9 @@ export async function InstallZip(buffer: Buffer, description: string, destinatio
     return new Promise<void>((resolve, reject) => {
         yauzl.fromBuffer(buffer, { lazyEntries: true }, (err, zipFile) => {
             if (err) {
-                return reject(new NestedError('Immediate zip file error', err));
+                let message = "C# Extension was unable to download its dependencies. Please check your internet connection. If you use a proxy server, please visit https://aka.ms/VsCodeCsharpNetworking";
+                eventStream.post(new ZipError(message));
+                return reject(new NestedError(message));
             }
 
             zipFile.readEntry();

src/vscodeAdapter.ts

@@ -900,6 +900,7 @@ export interface vscode {
         activeTextEditor: TextEditor | undefined;
         showInformationMessage: (message: string, ...items: string[]) => Thenable<string | undefined>;
         showWarningMessage: <T extends MessageItem>(message: string, ...items: T[]) => Thenable<T | undefined>;
+        showErrorMessage(message: string, ...items: string[]): Thenable<string | undefined>;
     };
     workspace: {
         getConfiguration: (section?: string, resource?: Uri) => WorkspaceConfiguration;

test/unitTests/Packages/ZipInstaller.test.ts

@@ -11,7 +11,7 @@ import { CreateTmpDir, TmpAsset } from '../../../src/CreateTmpAsset';
 import { InstallZip } from '../../../src/packageManager/ZipInstaller';
 import { EventStream } from '../../../src/EventStream';
 import { PlatformInformation } from '../../../src/platform';
-import { BaseEvent, InstallationStart } from '../../../src/omnisharp/loggingEvents';
+import { BaseEvent, InstallationStart, ZipError } from '../../../src/omnisharp/loggingEvents';
 import { Files, Binaries, createTestZipAsync } from '../testAssets/CreateTestZip';
 
 chai.use(require("chai-as-promised"));
@@ -65,10 +65,23 @@ suite('ZipInstaller', () => {
         }
     });
 
-    test('Error is thrown when the file is not a zip', async () => {
+    test('Error is thrown when the buffer contains an invalid zip', async () => {
         expect(InstallZip(new Buffer("My file", "utf8"), "Text File", installationPath, [], eventStream)).to.be.rejected;
     });
 
+    test('Error event is created when the buffer contains an invalid zip', async () => {
+        try {
+            await InstallZip(new Buffer("some content", "utf8"), "Text File", installationPath, [], eventStream);
+        }
+        catch{
+            let eventSequence: BaseEvent[] = [
+                new InstallationStart("Text File"),
+                new ZipError("C# Extension was unable to download its dependencies. Please check your internet connection. If you use a proxy server, please visit https://aka.ms/VsCodeCsharpNetworking")
+            ];
+            expect(eventBus).to.be.deep.equal(eventSequence);
+        }
+    });
+
     teardown(async () => {
         if (tmpInstallDir) {
             tmpInstallDir.dispose();

test/unitTests/logging/ErrorMessageObserver.test.ts

@@ -0,0 +1,38 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { use as chaiUse, expect, should } from 'chai';
+import { vscode } from '../../../src/vscodeAdapter';
+import { getFakeVsCode } from '../testAssets/Fakes';
+import 'rxjs/add/observable/fromPromise';
+import 'rxjs/add/operator/timeout';
+import { ErrorMessageObserver } from '../../../src/observers/ErrorMessageObserver';
+import { ZipError } from '../../../src/omnisharp/loggingEvents';
+
+chaiUse(require('chai-as-promised'));
+chaiUse(require('chai-string'));
+
+suite("ErrorMessageObserver", () => {
+    suiteSetup(() => should());
+
+    let vscode: vscode = getFakeVsCode();
+    let errorMessage: string;
+    let observer = new ErrorMessageObserver(vscode);
+
+    vscode.window.showErrorMessage = async (message: string, ...items: string[]) => {
+        errorMessage = message;
+        return Promise.resolve<string>("Done");
+    };
+
+    setup(() => {
+        errorMessage = undefined;
+    });
+
+    test('ZipError: Error message is shown', () => {
+        let event = new ZipError("This is an error");
+            observer.post(event);
+            expect(errorMessage).to.be.equal("This is an error");
+    });   
+});

test/unitTests/testAssets/Fakes.ts

@@ -132,6 +132,9 @@ export function getFakeVsCode(): vscode.vscode {
             },
             showWarningMessage: <T extends MessageItem>(message: string, ...items: T[]) => {
                 throw new Error("Not Implemented");
+            },
+            showErrorMessage: (message: string, ...items: string[]) => {
+                throw new Error("Not Implemented");
             }
         },
         workspace: {