Skip to content

Commit 27aab37

Browse files
Ravi Chandeakshita31
Ravi Chande
authored and
akshita31
committed
Refactor logger, reporter and channel into a unified EventStream using Rx Observable (#2084)
* Changes to refactor logging in server * Adding packages * Changes * Remove reporter from CSharpExtDownloader * remove telemtery reporter from server.ts * remove reporter from definitionProvider * Remove reporter from dotnetTest.ts * Debugger Activation + Commands * reduce message types * remove reporter from commands.ts * remove channel from status.ts * Remove reporter & logger from extension.ts * Build issues * Add missing rx dependency * Changed to download progress * Removed using and pass platformInfo * Moved files in observer folder * Renamed the files and added omnisharp channel observer * Remove unnecessary format * Changes in main.ts * Remove channel from global declaration * Preserving the context in onNext invocations * Pulled platformInfo out of server * Remove unnecessary variable * Formatting * Renamed observers * Add mocha+wallaby tests eventually the feature tests should be removed and most of our tests should become unit tests that are runnable from the command line or via wallaby. npm run tdd will enable using mocha's command line tdd capability * Code clean up * Fix `tdd` command * Fix test paths * Add initial DotnetChannelObserver test * Testing the download messages * Remove logger from requestQueue.ts * Fix builds * Use package manager factory * Remove Lines * Remove extra appendLine * Added test for csharp logger and channel * Extracted base class for observers * Test having dependency on vscode * vscode adapter changes * Changes for adapter * Refactored Omnisharp Manager * Moved from interfaces to classes * Renamed onNext to post * Created class EventStream * Removed comment * Added missing break * Added test for Omnisharp Logger * Test for OmnisharpLoggerObserver * Test for telemetry reporter observer * Added test for all the observers * minor nits * Changes * Remove unnecessary imports * remove import * Modified failing test * Make tests pass * Renamed platformInfo * CR feedback
1 parent 007bc0e commit 27aab37

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+2183
-850
lines changed

‎package-lock.json‎

Lines changed: 170 additions & 83 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,14 @@
4848
"fs-extra": "^5.0.0",
4949
"http-proxy-agent": "^2.0.0",
5050
"https-proxy-agent": "^2.1.1",
51+
"istanbul": "^0.4.5",
5152
"jsonc-parser": "^1.0.0",
5253
"lodash.debounce": "^4.0.8",
5354
"mkdirp": "^0.5.1",
5455
"open": "*",
5556
"request-light": "^0.2.0",
57+
"rx": "^4.1.0",
58+
"rxjs": "^5.5.6",
5659
"semver": "*",
5760
"tmp": "0.0.33",
5861
"vscode-debugprotocol": "^1.6.1",
@@ -66,6 +69,7 @@
6669
"@types/mkdirp": "0.5.1",
6770
"@types/mocha": "2.2.32",
6871
"@types/node": "8.0.53",
72+
"@types/rx": "^4.1.1",
6973
"@types/semver": "5.3.30",
7074
"@types/tmp": "0.0.33",
7175
"async-child-process": "^1.1.1",

‎src/CSharpExtDownloader.ts‎

Lines changed: 15 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,77 +3,57 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import * as vscode from 'vscode';
7-
import TelemetryReporter from 'vscode-extension-telemetry';
86
import * as util from './common';
9-
import { Logger } from './logger';
7+
import { GetNetworkConfiguration, GetStatus } from './downloader.helper';
108
import { PackageManager } from './packages';
119
import { PlatformInformation } from './platform';
12-
import { GetStatus, GetNetworkConfiguration, ReportInstallationError, SendInstallationTelemetry } from './downloader.helper';
10+
import { PackageInstallation, LogPlatformInfo, InstallationSuccess, InstallationFailure } from './omnisharp/loggingEvents';
11+
import { EventStream } from './EventStream';
1312

1413
/*
1514
* Class used to download the runtime dependencies of the C# Extension
1615
*/
1716
export class CSharpExtDownloader {
1817
public constructor(
19-
private channel: vscode.OutputChannel,
20-
private logger: Logger,
21-
private reporter: TelemetryReporter /* optional */,
22-
private packageJSON: any) {
18+
private eventStream: EventStream,
19+
private packageJSON: any,
20+
private platformInfo: PlatformInformation) {
2321
}
2422

2523
public async installRuntimeDependencies(): Promise<boolean> {
26-
this.logger.append('Installing C# dependencies...');
27-
this.channel.show();
24+
this.eventStream.post(new PackageInstallation("C# dependencies" ));
2825

2926
let status = GetStatus();
30-
31-
// Sends "AcquisitionStart" telemetry to indicate an acquisition started.
32-
if (this.reporter) {
33-
this.reporter.sendTelemetryEvent("AcquisitionStart");
34-
}
35-
36-
let platformInfo: PlatformInformation;
37-
let packageManager: PackageManager;
3827
let installationStage = 'touchBeginFile';
3928
let success = false;
4029

41-
let telemetryProps: any = {};
42-
4330
try {
4431
await util.touchInstallFile(util.InstallFileType.Begin);
45-
installationStage = 'getPlatformInfo';
46-
platformInfo = await PlatformInformation.GetCurrent();
4732

48-
packageManager = new PackageManager(platformInfo, this.packageJSON);
49-
this.logger.appendLine();
50-
// Display platform information and RID followed by a blank line
51-
this.logger.appendLine(`Platform: ${platformInfo.toString()}`);
52-
this.logger.appendLine();
33+
let packageManager = new PackageManager(this.platformInfo, this.packageJSON);
34+
// Display platform information and RID
35+
this.eventStream.post(new LogPlatformInfo(this.platformInfo ));
5336

5437
installationStage = 'downloadPackages';
55-
5638
let networkConfiguration = GetNetworkConfiguration();
5739
const proxy = networkConfiguration.Proxy;
5840
const strictSSL = networkConfiguration.StrictSSL;
5941

60-
await packageManager.DownloadPackages(this.logger, status, proxy, strictSSL);
61-
this.logger.appendLine();
42+
await packageManager.DownloadPackages(this.eventStream, status, proxy, strictSSL);
6243

6344
installationStage = 'installPackages';
64-
await packageManager.InstallPackages(this.logger, status);
45+
await packageManager.InstallPackages(this.eventStream, status);
6546

6647
installationStage = 'touchLockFile';
6748
await util.touchInstallFile(util.InstallFileType.Lock);
68-
69-
installationStage = 'completeSuccess';
49+
7050
success = true;
51+
this.eventStream.post(new InstallationSuccess());
7152
}
7253
catch (error) {
73-
ReportInstallationError(this.logger, error, telemetryProps, installationStage);
54+
this.eventStream.post(new InstallationFailure(installationStage, error));
7455
}
7556
finally {
76-
SendInstallationTelemetry(this.logger, this.reporter, telemetryProps, installationStage, platformInfo);
7757
status.dispose();
7858
// We do this step at the end so that we clean up the begin file in the case that we hit above catch block
7959
// Attach a an empty catch to this so that errors here do not propogate

‎src/EventStream.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
import { Subject } from "rx";
6+
import { BaseEvent } from "./omnisharp/loggingEvents";
7+
8+
export class EventStream {
9+
private sink: Subject<BaseEvent>;
10+
11+
constructor() {
12+
this.sink = new Subject<BaseEvent>();
13+
}
14+
15+
public post(event: BaseEvent) {
16+
this.sink.onNext(event);
17+
}
18+
19+
public subscribe(eventHandler: (event: BaseEvent) => void) {
20+
this.sink.subscribe(eventHandler);
21+
}
22+
}

‎src/coreclr-debug/activate.ts‎

Lines changed: 28 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -7,53 +7,38 @@
77
import * as path from 'path';
88
import * as vscode from 'vscode';
99
import * as common from './../common';
10-
1110
import { CoreClrDebugUtil, DotnetInfo, } from './util';
12-
13-
import { Logger } from './../logger';
1411
import { PlatformInformation } from './../platform';
15-
import TelemetryReporter from 'vscode-extension-telemetry';
12+
import { DebuggerPrerequisiteWarning, DebuggerPrerequisiteFailure, DebuggerNotInstalledFailure } from '../omnisharp/loggingEvents';
13+
import { EventStream } from '../EventStream';
1614

1715
let _debugUtil: CoreClrDebugUtil = null;
18-
let _logger: Logger = null;
1916

20-
export async function activate(thisExtension : vscode.Extension<any>, context: vscode.ExtensionContext, reporter: TelemetryReporter, logger: Logger, channel: vscode.OutputChannel) {
21-
_debugUtil = new CoreClrDebugUtil(context.extensionPath, logger);
22-
_logger = logger;
17+
export async function activate(thisExtension: vscode.Extension<any>, context: vscode.ExtensionContext, platformInformation: PlatformInformation, eventStream: EventStream) {
18+
_debugUtil = new CoreClrDebugUtil(context.extensionPath);
2319

2420
if (!CoreClrDebugUtil.existsSync(_debugUtil.debugAdapterDir())) {
25-
let isInvalidArchitecture: boolean = await checkForInvalidArchitecture(logger);
21+
let isInvalidArchitecture: boolean = await checkForInvalidArchitecture(platformInformation, eventStream);
2622
if (!isInvalidArchitecture) {
27-
logger.appendLine("[ERROR]: C# Extension failed to install the debugger package.");
28-
showInstallErrorMessage(channel);
23+
eventStream.post(new DebuggerPrerequisiteFailure("[ERROR]: C# Extension failed to install the debugger package."));
24+
showInstallErrorMessage(eventStream);
2925
}
3026
} else if (!CoreClrDebugUtil.existsSync(_debugUtil.installCompleteFilePath())) {
31-
completeDebuggerInstall(logger, channel);
27+
completeDebuggerInstall(platformInformation, eventStream);
3228
}
3329
}
3430

35-
async function checkForInvalidArchitecture(logger: Logger): Promise<boolean> {
36-
let platformInformation: PlatformInformation;
37-
38-
try {
39-
platformInformation = await PlatformInformation.GetCurrent();
40-
}
41-
catch (err) {
42-
// Somehow we couldn't figure out the platform we are on
43-
logger.appendLine("[ERROR] The debugger cannot be installed. Could not determine current platform.");
44-
return true;
45-
}
46-
31+
async function checkForInvalidArchitecture(platformInformation: PlatformInformation, eventStream: EventStream): Promise<boolean> {
4732
if (platformInformation) {
4833
if (platformInformation.isMacOS() && !CoreClrDebugUtil.isMacOSSupported()) {
49-
logger.appendLine("[ERROR] The debugger cannot be installed. The debugger requires macOS 10.12 (Sierra) or newer.");
34+
eventStream.post(new DebuggerPrerequisiteFailure("[ERROR] The debugger cannot be installed. The debugger requires macOS 10.12 (Sierra) or newer."));
5035
return true;
51-
}
36+
}
5237
else if (platformInformation.architecture !== "x86_64") {
5338
if (platformInformation.isWindows() && platformInformation.architecture === "x86") {
54-
logger.appendLine(`[WARNING]: x86 Windows is not currently supported by the .NET Core debugger. Debugging will not be available.`);
39+
eventStream.post(new DebuggerPrerequisiteWarning(`[WARNING]: x86 Windows is not currently supported by the .NET Core debugger. Debugging will not be available.`));
5540
} else {
56-
logger.appendLine(`[WARNING]: Processor architecture '${platformInformation.architecture}' is not currently supported by the .NET Core debugger. Debugging will not be available.`);
41+
eventStream.post(new DebuggerPrerequisiteWarning(`[WARNING]: Processor architecture '${platformInformation.architecture}' is not currently supported by the .NET Core debugger. Debugging will not be available.`));
5742
}
5843
return true;
5944
}
@@ -62,14 +47,14 @@ async function checkForInvalidArchitecture(logger: Logger): Promise<boolean> {
6247
return false;
6348
}
6449

65-
async function completeDebuggerInstall(logger: Logger, channel: vscode.OutputChannel) : Promise<boolean> {
50+
async function completeDebuggerInstall(platformInformation: PlatformInformation, eventStream: EventStream): Promise<boolean> {
6651
return _debugUtil.checkDotNetCli()
6752
.then(async (dotnetInfo: DotnetInfo) => {
68-
69-
let isInvalidArchitecture: boolean = await checkForInvalidArchitecture(logger);
53+
54+
let isInvalidArchitecture: boolean = await checkForInvalidArchitecture(platformInformation, eventStream);
7055

7156
if (isInvalidArchitecture) {
72-
channel.show();
57+
eventStream.post(new DebuggerNotInstalledFailure());
7358
vscode.window.showErrorMessage('Failed to complete the installation of the C# extension. Please see the error in the output window below.');
7459
return false;
7560
}
@@ -83,20 +68,18 @@ async function completeDebuggerInstall(logger: Logger, channel: vscode.OutputCha
8368
// Check for dotnet tools failed. pop the UI
8469
// err is a DotNetCliError but use defaults in the unexpected case that it's not
8570
showDotnetToolsWarning(err.ErrorMessage || _debugUtil.defaultDotNetCliErrorMessage());
86-
_logger.appendLine(err.ErrorString || err);
71+
eventStream.post(new DebuggerPrerequisiteWarning(err.ErrorString || err));
8772
// TODO: log telemetry?
88-
8973
return false;
9074
});
9175
}
9276

93-
function showInstallErrorMessage(channel: vscode.OutputChannel) {
94-
channel.show();
77+
function showInstallErrorMessage(eventStream : EventStream) {
78+
eventStream.post(new DebuggerNotInstalledFailure());
9579
vscode.window.showErrorMessage("An error occured during installation of the .NET Core Debugger. The C# extension may need to be reinstalled.");
9680
}
9781

98-
function showDotnetToolsWarning(message: string) : void
99-
{
82+
function showDotnetToolsWarning(message: string): void {
10083
const config = vscode.workspace.getConfiguration('csharp');
10184
if (!config.get('suppressDotnetInstallWarning', false)) {
10285
const getDotNetMessage = 'Get .NET CLI tools';
@@ -110,8 +93,7 @@ function showDotnetToolsWarning(message: string) : void
11093
let dotnetcoreURL = 'https://www.microsoft.com/net/core';
11194

11295
// Windows redirects https://www.microsoft.com/net/core to https://www.microsoft.com/net/core#windowsvs2015
113-
if (process.platform == "win32")
114-
{
96+
if (process.platform == "win32") {
11597
dotnetcoreURL = dotnetcoreURL + '#windowscmd';
11698
}
11799

@@ -130,9 +112,8 @@ interface AdapterExecutableCommand {
130112
// The default extension manifest calls this command as the adapterExecutableCommand
131113
// If the debugger components have not finished downloading, the proxy displays an error message to the user
132114
// Else it will launch the debug adapter
133-
export async function getAdapterExecutionCommand(channel: vscode.OutputChannel): Promise<AdapterExecutableCommand> {
134-
let logger = new Logger(text => channel.append(text));
135-
let util = new CoreClrDebugUtil(common.getExtensionPath(), logger);
115+
export async function getAdapterExecutionCommand(platformInfo: PlatformInformation, eventStream: EventStream): Promise<AdapterExecutableCommand> {
116+
let util = new CoreClrDebugUtil(common.getExtensionPath());
136117

137118
// Check for .debugger folder. Handle if it does not exist.
138119
if (!CoreClrDebugUtil.existsSync(util.debugAdapterDir())) {
@@ -142,19 +123,19 @@ export async function getAdapterExecutionCommand(channel: vscode.OutputChannel):
142123
// 2. install.Lock is created
143124
// 3. install.Begin is deleted
144125
// 4. install.complete is created
145-
126+
146127
// install.Lock does not exist, need to wait for packages to finish downloading.
147128
let installLock: boolean = await common.installFileExists(common.InstallFileType.Lock);
148129
if (!installLock) {
149-
channel.show();
130+
eventStream.post(new DebuggerNotInstalledFailure());
150131
throw new Error('The C# extension is still downloading packages. Please see progress in the output window below.');
151132
}
152133
// install.complete does not exist, check dotnetCLI to see if we can complete.
153134
else if (!CoreClrDebugUtil.existsSync(util.installCompleteFilePath())) {
154-
let success: boolean = await completeDebuggerInstall(logger, channel);
135+
let success: boolean = await completeDebuggerInstall(platformInfo, eventStream);
155136

156137
if (!success) {
157-
channel.show();
138+
eventStream.post(new DebuggerNotInstalledFailure());
158139
throw new Error('Failed to complete the installation of the C# extension. Please see the error in the output window below.');
159140
}
160141
}

‎src/coreclr-debug/util.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import * as fs from 'fs';
99
import * as semver from 'semver';
1010
import * as os from 'os';
1111
import { execChildProcess } from './../common';
12-
import { Logger } from './../logger';
1312

1413
const MINIMUM_SUPPORTED_DOTNET_CLI: string = '1.0.0-preview2-003121';
1514

@@ -31,7 +30,7 @@ export class CoreClrDebugUtil
3130
private _debugAdapterDir: string = '';
3231
private _installCompleteFilePath: string = '';
3332

34-
constructor(extensionDir: string, logger: Logger) {
33+
constructor(extensionDir: string) {
3534
this._extensionDir = extensionDir;
3635
this._debugAdapterDir = path.join(this._extensionDir, '.debugger');
3736
this._installCompleteFilePath = path.join(this._debugAdapterDir, 'install.complete');

‎src/downloader.helper.ts‎

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55
import * as vscode from 'vscode';
6-
import { Status, PackageError } from './packages';
7-
import { PlatformInformation } from './platform';
8-
import { Logger } from './logger';
9-
import TelemetryReporter from 'vscode-extension-telemetry';
6+
import { Status } from './packages';
107

118
export function GetNetworkConfiguration() {
129
const config = vscode.workspace.getConfiguration();
@@ -32,46 +29,4 @@ export function GetStatus(): Status {
3229
};
3330

3431
return status;
35-
}
36-
37-
export function ReportInstallationError(logger: Logger, error, telemetryProps: any, installationStage: string) {
38-
let errorMessage: string;
39-
if (error instanceof PackageError) {
40-
// we can log the message in a PackageError to telemetry as we do not put PII in PackageError messages
41-
telemetryProps['error.message'] = error.message;
42-
if (error.innerError) {
43-
errorMessage = error.innerError.toString();
44-
}
45-
else {
46-
errorMessage = error.message;
47-
}
48-
if (error.pkg) {
49-
telemetryProps['error.packageUrl'] = error.pkg.url;
50-
}
51-
}
52-
else {
53-
// do not log raw errorMessage in telemetry as it is likely to contain PII.
54-
errorMessage = error.toString();
55-
}
56-
57-
logger.appendLine();
58-
logger.appendLine(`Failed at stage: ${installationStage}`);
59-
logger.appendLine(errorMessage);
60-
}
61-
62-
export function SendInstallationTelemetry(logger: Logger, reporter: TelemetryReporter, telemetryProps: any, installationStage: string, platformInfo: PlatformInformation) {
63-
telemetryProps['installStage'] = installationStage;
64-
telemetryProps['platform.architecture'] = platformInfo.architecture;
65-
telemetryProps['platform.platform'] = platformInfo.platform;
66-
if (platformInfo.distribution) {
67-
telemetryProps['platform.distribution'] = platformInfo.distribution.toTelemetryString();
68-
}
69-
if (reporter) {
70-
reporter.sendTelemetryEvent('Acquisition', telemetryProps);
71-
}
72-
73-
logger.appendLine();
74-
installationStage = '';
75-
logger.appendLine('Finished');
76-
logger.appendLine();
7732
}

‎src/features/abstractProvider.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,16 @@
55

66
'use strict';
77

8-
import { OmniSharpServer } from '../omnisharp/server';
98
import { Disposable } from 'vscode';
10-
import TelemetryReporter from 'vscode-extension-telemetry';
9+
import { OmniSharpServer } from '../omnisharp/server';
1110

1211
export default abstract class AbstractProvider {
1312

1413
protected _server: OmniSharpServer;
15-
protected _reporter: TelemetryReporter;
1614
private _disposables: Disposable[];
1715

18-
constructor(server: OmniSharpServer, reporter: TelemetryReporter) {
16+
constructor(server: OmniSharpServer) {
1917
this._server = server;
20-
this._reporter = reporter;
2118
this._disposables = [];
2219
}
2320

0 commit comments

Comments
 (0)