Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug improvements #3094

Merged
merged 2 commits into from
Aug 31, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions PublicAPI.md
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,24 @@ tns.liveSyncService.on("userInteractionNeeded", data => {
});
```

* debuggerAttached - raised whenever CLI attaches the backend debugging socket and a frontend debugging client may be attached. The event is raised with an object containing the device's identifier:

Example:
```JavaScript
tns.liveSyncService.on("debuggerAttached", debugInfo => {
console.log(`Backend client connected, frontend client may be connected at ${debugInfo.url}`);
});
```

* debuggerDetached - raised whenever CLI detaches the backend debugging socket. The event is raised with an object of the `IDebugInformation` type:

Example:
```JavaScript
tns.liveSyncService.on("debuggerDetached", debugInfo => {
console.log(`Detached debugger for device with id ${debugInfo.deviceIdentifier}`);
});
```

## How to add a new method to Public API
CLI is designed as command line tool and when it is used as a library, it does not give you access to all of the methods. This is mainly implementation detail. Most of the CLI's code is created to work in command line, not as a library, so before adding method to public API, most probably it will require some modification.
For example the `$options` injected module contains information about all `--` options passed on the terminal. When the CLI is used as a library, the options are not populated. Before adding method to public API, make sure its implementation does not rely on `$options`.
Expand Down
3 changes: 2 additions & 1 deletion lib/commands/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export class DebugPlatformCommand implements ICommand {
debugData.deviceIdentifier = selectedDeviceForDebug.deviceInfo.identifier;

if (this.$options.start) {
return this.$liveSyncService.printDebugInformation(await this.$debugService.debug(debugData, debugOptions));
await this.$liveSyncService.printDebugInformation(await this.$debugService.debug(debugData, debugOptions));
return;
}

await this.$devicesService.detectCurrentlyAttachedDevices({ shouldReturnImmediateResult: false, platform: this.platform });
Expand Down
1 change: 1 addition & 0 deletions lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export const BUILD_OUTPUT_EVENT_NAME = "buildOutput";
export const CONNECTION_ERROR_EVENT_NAME = "connectionError";
export const USER_INTERACTION_NEEDED_EVENT_NAME = "userInteractionNeeded";
export const DEBUGGER_ATTACHED_EVENT_NAME = "debuggerAttached";
export const DEBUGGER_DETACHED_EVENT_NAME = "debuggerDetached";
export const VERSION_STRING = "version";
export const INSPECTOR_CACHE_DIRNAME = "ios-inspector";
export const POST_INSTALL_COMMAND_NAME = "post-install-cli";
Expand Down
11 changes: 9 additions & 2 deletions lib/declarations.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,15 @@ interface ICreateProjectOptions extends INpmInstallConfigurationOptionsBase {
pathToTemplate?: string;
}

interface IOptions extends ICommonOptions, IBundle, IPlatformTemplate, IEmulator, IClean, IProvision, ITeamIdentifier, IAndroidReleaseOptions, INpmInstallConfigurationOptions {
interface IDebugInformation extends IPort {
url: string;
}

interface IPort {
port: Number;
}

interface IOptions extends ICommonOptions, IBundle, IPlatformTemplate, IEmulator, IClean, IProvision, ITeamIdentifier, IAndroidReleaseOptions, INpmInstallConfigurationOptions, IPort {
all: boolean;
client: boolean;
compileSdk: number;
Expand All @@ -386,7 +394,6 @@ interface IOptions extends ICommonOptions, IBundle, IPlatformTemplate, IEmulator
tsc: boolean;
ng: boolean;
androidTypings: boolean;
port: Number;
production: boolean; //npm flag
sdk: string;
syncAllFiles: boolean;
Expand Down
14 changes: 11 additions & 3 deletions lib/definitions/debug.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ interface IDebugServiceBase extends NodeJS.EventEmitter {
* Starts debug operation based on the specified debug data.
* @param {IDebugData} debugData Describes information for device and application that will be debugged.
* @param {IDebugOptions} debugOptions Describe possible options to modify the behaivor of the debug operation, for example stop on the first line.
* @returns {Promise<T>} Array of URLs that can be used for debugging or a string representing a single url that can be used for debugging.
* @returns {Promise<IDebugInformation>} Full url and port where the frontend client can be connected.
*/
debug(debugData: IDebugData, debugOptions: IDebugOptions): Promise<string>;
debug(debugData: IDebugData, debugOptions: IDebugOptions): Promise<IDebugInformation>;
}

interface IDebugService extends IDebugServiceBase {
Expand All @@ -122,7 +122,7 @@ interface IDebugService extends IDebugServiceBase {
/**
* Describes actions required for debugging on specific platform (Android or iOS).
*/
interface IPlatformDebugService extends IDebugServiceBase, IPlatform {
interface IPlatformDebugService extends IPlatform, NodeJS.EventEmitter {
/**
* Starts debug operation.
* @param {IDebugData} debugData Describes information for device and application that will be debugged.
Expand All @@ -136,4 +136,12 @@ interface IPlatformDebugService extends IDebugServiceBase, IPlatform {
* @returns {Promise<void>}
*/
debugStop(): Promise<void>;

/**
* Starts debug operation based on the specified debug data.
* @param {IDebugData} debugData Describes information for device and application that will be debugged.
* @param {IDebugOptions} debugOptions Describe possible options to modify the behaivor of the debug operation, for example stop on the first line.
* @returns {Promise<string>} Full url where the frontend client may be connected.
*/
debug(debugData: IDebugData, debugOptions: IDebugOptions): Promise<string>;
}
14 changes: 7 additions & 7 deletions lib/definitions/livesync.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,18 +206,18 @@ interface ILiveSyncService {
interface IDebugLiveSyncService extends ILiveSyncService {
/**
* Prints debug information.
* @param {string[]} information Array of information to be printed. Note that false-like values will be stripped from the array.
* @returns {void}
* @param {IDebugInformation} debugInformation Information to be printed.
* @returns {IDebugInformation} Full url and port where the frontend client can be connected.
*/
printDebugInformation(information: string): void;
printDebugInformation(debugInformation: IDebugInformation): IDebugInformation;

/**
* Enables debugging for the specified devices
* @param {IEnableDebuggingDeviceOptions[]} deviceOpts Settings used for enabling debugging for each device.
* @param {IDebuggingAdditionalOptions} enableDebuggingOptions Settings used for enabling debugging.
* @returns {Promise<void>[]} Array of promises for each device.
* @returns {Promise<IDebugInformation>[]} Array of promises for each device.
*/
enableDebugging(deviceOpts: IEnableDebuggingDeviceOptions[], enableDebuggingOptions: IDebuggingAdditionalOptions): Promise<void>[];
enableDebugging(deviceOpts: IEnableDebuggingDeviceOptions[], enableDebuggingOptions: IDebuggingAdditionalOptions): Promise<IDebugInformation>[];

/**
* Disables debugging for the specified devices
Expand All @@ -230,9 +230,9 @@ interface IDebugLiveSyncService extends ILiveSyncService {
/**
* Attaches a debugger to the specified device.
* @param {IAttachDebuggerOptions} settings Settings used for controling the attaching process.
* @returns {Promise<void>}
* @returns {Promise<IDebugInformation>} Full url and port where the frontend client can be connected.
*/
attachDebugger(settings: IAttachDebuggerOptions): Promise<void>;
attachDebugger(settings: IAttachDebuggerOptions): Promise<IDebugInformation>;
}

/**
Expand Down
15 changes: 13 additions & 2 deletions lib/services/debug-service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { platform } from "os";
import { parse } from "url";
import { EventEmitter } from "events";
import { CONNECTION_ERROR_EVENT_NAME, DebugCommandErrors } from "../constants";
import { CONNECTED_STATUS } from "../common/constants";
Expand All @@ -14,7 +15,7 @@ export class DebugService extends EventEmitter implements IDebugService {
this._platformDebugServices = {};
}

public async debug(debugData: IDebugData, options: IDebugOptions): Promise<string> {
public async debug(debugData: IDebugData, options: IDebugOptions): Promise<IDebugInformation> {
const device = this.$devicesService.getDeviceByIdentifier(debugData.deviceIdentifier);

if (!device) {
Expand Down Expand Up @@ -57,7 +58,7 @@ export class DebugService extends EventEmitter implements IDebugService {
result = await debugService.debug(debugData, debugOptions);
}

return result;
return this.getDebugInformation(result);
}

public debugStop(deviceIdentifier: string): Promise<void> {
Expand Down Expand Up @@ -92,6 +93,16 @@ export class DebugService extends EventEmitter implements IDebugService {
connectionErrorHandler = connectionErrorHandler.bind(this);
platformDebugService.on(CONNECTION_ERROR_EVENT_NAME, connectionErrorHandler);
}

private getDebugInformation(fullUrl: string): IDebugInformation {
const parseQueryString = true;
const wsQueryParam = parse(fullUrl, parseQueryString).query.ws;
const hostPortSplit = wsQueryParam && wsQueryParam.split(":");
return {
url: fullUrl,
port: hostPortSplit && +hostPortSplit[1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case the wsQueryParam does not contain :, you'll end up with port = NaN, is this okay?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so - in case the ws query param isn't in the correct format I can't really tell the port so a null-like value seems okay

};
}
}

$injector.register("debugService", DebugService);
40 changes: 20 additions & 20 deletions lib/services/livesync/livesync-service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import * as path from "path";
import * as choki from "chokidar";
import { parse } from "url";
import { EOL } from "os";
import { EventEmitter } from "events";
import { hook } from "../../common/helpers";
import { APP_FOLDER_NAME, PACKAGE_JSON_FILE_NAME, LiveSyncTrackActionNames, USER_INTERACTION_NEEDED_EVENT_NAME, DEBUGGER_ATTACHED_EVENT_NAME } from "../../constants";
import { APP_FOLDER_NAME, PACKAGE_JSON_FILE_NAME, LiveSyncTrackActionNames, USER_INTERACTION_NEEDED_EVENT_NAME, DEBUGGER_ATTACHED_EVENT_NAME, DEBUGGER_DETACHED_EVENT_NAME } from "../../constants";
import { FileExtensions, DeviceTypes } from "../../common/constants";
const deviceDescriptorPrimaryKey = "identifier";

Expand Down Expand Up @@ -101,7 +100,7 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
return currentDescriptors || [];
}

private async refreshApplication(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, debugOpts?: IDebugOptions, outputPath?: string): Promise<void> {
private async refreshApplication(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, debugOpts?: IDebugOptions, outputPath?: string): Promise<void | IDebugInformation> {
const deviceDescriptor = this.getDeviceDescriptor(liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier, projectData.projectDir);

return deviceDescriptor && deviceDescriptor.debugggingEnabled ?
Expand Down Expand Up @@ -138,13 +137,14 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
this.$logger.info(`Successfully synced application ${liveSyncResultInfo.deviceAppData.appIdentifier} on device ${liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier}.`);
}

private async refreshApplicationWithDebug(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, debugOptions: IDebugOptions, outputPath?: string): Promise<void> {
private async refreshApplicationWithDebug(projectData: IProjectData, liveSyncResultInfo: ILiveSyncResultInfo, debugOptions: IDebugOptions, outputPath?: string): Promise<IDebugInformation> {
await this.$platformService.trackProjectType(projectData);

const deviceAppData = liveSyncResultInfo.deviceAppData;

const deviceIdentifier = liveSyncResultInfo.deviceAppData.device.deviceInfo.identifier;
await this.$debugService.debugStop(deviceIdentifier);
this.emit(DEBUGGER_DETACHED_EVENT_NAME, { deviceIdentifier });

const applicationId = deviceAppData.appIdentifier;
const attachDebuggerOptions: IAttachDebuggerOptions = {
Expand Down Expand Up @@ -181,7 +181,7 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
return this.enableDebuggingCoreWithoutWaitingCurrentAction(deviceOption, { projectDir: projectData.projectDir });
}

public async attachDebugger(settings: IAttachDebuggerOptions): Promise<void> {
public async attachDebugger(settings: IAttachDebuggerOptions): Promise<IDebugInformation> {
// Default values
if (settings.debugOptions) {
settings.debugOptions.chrome = settings.debugOptions.chrome === undefined ? true : settings.debugOptions.chrome;
Expand All @@ -208,22 +208,19 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
};
debugData.pathToAppPackage = this.$platformService.lastOutputPath(settings.platform, buildConfig, projectData, settings.outputPath);

this.printDebugInformation(await this.$debugService.debug(debugData, settings.debugOptions));
return this.printDebugInformation(await this.$debugService.debug(debugData, settings.debugOptions));
}

public printDebugInformation(information: string): void {
if (!!information) {
const wsQueryParam = parse(information).query.ws;
const hostPortSplit = wsQueryParam && wsQueryParam.split(":");
this.emit(DEBUGGER_ATTACHED_EVENT_NAME, {
url: information,
port: hostPortSplit && hostPortSplit[1]
});
this.$logger.info(`To start debugging, open the following URL in Chrome:${EOL}${information}${EOL}`.cyan);
public printDebugInformation(debugInformation: IDebugInformation): IDebugInformation {
if (!!debugInformation.url) {
this.emit(DEBUGGER_ATTACHED_EVENT_NAME, debugInformation);
this.$logger.info(`To start debugging, open the following URL in Chrome:${EOL}${debugInformation.url}${EOL}`.cyan);
}

return debugInformation;
}

public enableDebugging(deviceOpts: IEnableDebuggingDeviceOptions[], debuggingAdditionalOptions: IDebuggingAdditionalOptions): Promise<void>[] {
public enableDebugging(deviceOpts: IEnableDebuggingDeviceOptions[], debuggingAdditionalOptions: IDebuggingAdditionalOptions): Promise<IDebugInformation>[] {
return _.map(deviceOpts, d => this.enableDebuggingCore(d, debuggingAdditionalOptions));
}

Expand All @@ -233,7 +230,7 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
return _.find(deviceDescriptors, d => d.identifier === deviceIdentifier);
}

private async enableDebuggingCoreWithoutWaitingCurrentAction(deviceOption: IEnableDebuggingDeviceOptions, debuggingAdditionalOptions: IDebuggingAdditionalOptions): Promise<void> {
private async enableDebuggingCoreWithoutWaitingCurrentAction(deviceOption: IEnableDebuggingDeviceOptions, debuggingAdditionalOptions: IDebuggingAdditionalOptions): Promise<IDebugInformation> {
const currentDeviceDescriptor = this.getDeviceDescriptor(deviceOption.deviceIdentifier, debuggingAdditionalOptions.projectDir);
if (!currentDeviceDescriptor) {
this.$errors.failWithoutHelp(`Couldn't enable debugging for ${deviceOption.deviceIdentifier}`);
Expand All @@ -250,16 +247,19 @@ export class LiveSyncService extends EventEmitter implements IDebugLiveSyncServi
debugOptions: deviceOption.debugOptions
};

let debugInformation: IDebugInformation;
try {
await this.attachDebugger(attachDebuggerOptions);
debugInformation = await this.attachDebugger(attachDebuggerOptions);
} catch (err) {
this.$logger.trace("Couldn't attach debugger, will modify options and try again.", err);
attachDebuggerOptions.debugOptions.start = false;
await this.attachDebugger(attachDebuggerOptions);
debugInformation = await this.attachDebugger(attachDebuggerOptions);
}

return debugInformation;
}

private async enableDebuggingCore(deviceOption: IEnableDebuggingDeviceOptions, debuggingAdditionalOptions: IDebuggingAdditionalOptions): Promise<void> {
private async enableDebuggingCore(deviceOption: IEnableDebuggingDeviceOptions, debuggingAdditionalOptions: IDebuggingAdditionalOptions): Promise<IDebugInformation> {
const liveSyncProcessInfo: ILiveSyncProcessInfo = this.liveSyncProcessesInfo[debuggingAdditionalOptions.projectDir];
if (liveSyncProcessInfo && liveSyncProcessInfo.currentSyncAction) {
await liveSyncProcessInfo.currentSyncAction;
Expand Down
8 changes: 5 additions & 3 deletions test/services/debug-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { EventEmitter } from "events";
import * as constants from "../../lib/common/constants";
import { CONNECTION_ERROR_EVENT_NAME, DebugCommandErrors } from "../../lib/constants";

const fakeChromeDebugUrl = "fakeChromeDebugUrl";
const fakeChromeDebugPort = 123;
const fakeChromeDebugUrl = `fakeChromeDebugUrl?experiments=true&ws=localhost:${fakeChromeDebugPort}`;
class PlatformDebugService extends EventEmitter /* implements IPlatformDebugService */ {
public async debug(debugData: IDebugData, debugOptions: IDebugOptions): Promise<string> {
return fakeChromeDebugUrl;
Expand Down Expand Up @@ -202,7 +203,7 @@ describe("debugService", () => {
});
});

describe("returns chrome url returned by platform specific debug service", () => {
describe("returns chrome url along with port returned by platform specific debug service", () => {
_.each(["android", "iOS"], platform => {
it(`for ${platform} device`, async () => {
const testData = getDefaultTestData();
Expand All @@ -214,7 +215,8 @@ describe("debugService", () => {
const debugData = getDebugData();
const url = await debugService.debug(debugData, null);

assert.deepEqual(url, fakeChromeDebugUrl);
assert.deepEqual(url.url, fakeChromeDebugUrl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not asserting the whole object? Currently the object may have additional properties and the test will pass. I prefer asserting equality of the whole object, so in case we add new properties, we'll have to add/update tests for them.

assert.deepEqual(url.port, fakeChromeDebugPort);
});
});
});
Expand Down