Skip to content
This repository has been archived by the owner on Oct 2, 2021. It is now read-only.

Add support for address param in launch config that allows remote debugging #50

Merged
merged 1 commit into from
Jun 21, 2016
Merged

Add support for address param in launch config that allows remote debugging #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 deletions src/chrome/chromeConnection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,17 @@ export class ChromeConnection {
/**
* Attach the websocket to the first available tab in the chrome instance with the given remote debugging port number.
*/
public attach(port: number, url?: string): Promise<void> {
public attach(port: number, url?: string, address?: string): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Logically I'd expect tha arguments to be : Address, port and url.

This smells of an after-thought... just one more param, right? I'd rather make sure the arguments are ordered logically, bump the sem-major version and update the two consumers of this debugger (edge + chrome)

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 thought about it. But since port is required, the other two are optional.
Having port, address, url looks equally weird as well.

On Tuesday, June 21, 2016, Kenneth Auchenberg notifications@github.com
wrote:

In src/chrome/chromeConnection.ts
#50 (comment)
:

@@ -162,16 +162,17 @@ export class ChromeConnection {
/**
* Attach the websocket to the first available tab in the chrome instance with the given remote debugging port number.
*/

  • public attach(port: number, url?: string): Promise {
  • public attach(port: number, url?: string, address?: string): Promise {

Logically I'd expect tha arguments to be : Address, port and url.

This smells of an after-thought... just one more param, right? I'd rather
make sure the arguments are ordered logically, bump the sem-major version
and update the two consumers of this debugger (edge + chrome)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/vscode-chrome-debug-core/pull/50/files/813face911502dab6c1b5d5bb3b3c8f2e2a7b9c9#r67997645,
or mute the thread
https://github.com/notifications/unsubscribe/AA-JVFpWtl7_wmkZg5pp2ywh03JzgMlGks5qOMzrgaJpZM4I7Oc5
.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point!

logger.log('Attempting to attach on port ' + port);
return utils.retryAsync(() => this._attach(port, url), 6000)
return utils.retryAsync(() => this._attach(port, url, address), /*timeoutMs*/ 6000)
.then(() => this.sendMessage('Debugger.enable'))
.then(() => this.sendMessage('Console.enable'))
.then(() => { });
}

public _attach(port: number, targetUrl?: string): Promise<void> {
return utils.getURL(`http://127.0.0.1:${port}/json`).then(jsonResponse => {
public _attach(port: number, targetUrl?: string, address?: string): Promise<void> {
address = address || '127.0.0.1';
return utils.getURL(`http://${address}:${port}/json`).then(jsonResponse => {
// Validate every step of processing the response
try {
const responseArray = JSON.parse(jsonResponse);
Expand Down
8 changes: 4 additions & 4 deletions src/chrome/chromeDebugAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export class ChromeDebugAdapter implements IDebugAdapter {
this.terminateSession();
});

return this._attach(port, launchUrl);
return this._attach(port, launchUrl, args.address);
}

public attach(args: IAttachRequestArgs): Promise<void> {
Expand All @@ -149,7 +149,7 @@ export class ChromeDebugAdapter implements IDebugAdapter {

this.setupLogging(args);

return this._attach(args.port, args.url);
return this._attach(args.port, args.url, args.address);
}

public setupLogging(args: IAttachRequestArgs | ILaunchRequestArgs): void {
Expand Down Expand Up @@ -187,7 +187,7 @@ export class ChromeDebugAdapter implements IDebugAdapter {
}
}

private _attach(port: number, url?: string): Promise<void> {
private _attach(port: number, url?: string, address?: string): Promise<void> {
// Client is attaching - if not attached to the chrome target, create a connection and attach
this._clientAttached = true;
if (!this._chromeConnection.isAttached) {
Expand All @@ -203,7 +203,7 @@ export class ChromeDebugAdapter implements IDebugAdapter {
this._chromeConnection.on('close', () => this.terminateSession());
this._chromeConnection.on('error', () => this.terminateSession());

return this._chromeConnection.attach(port, url).then(
return this._chromeConnection.attach(port, url, address).then(
() => this.fireEvent(new InitializedEvent()),
e => {
this.clearEverything();
Expand Down
2 changes: 2 additions & 0 deletions src/chrome/debugAdapterInterfaces.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface ILaunchRequestArgs extends DebugProtocol.LaunchRequestArguments
url?: string;
stopOnEntry?: boolean;
sourceMaps?: boolean;
address?: string;
port?: number;
diagnosticLogging?: boolean;
verboseDiagnosticLogging?: boolean;
Expand All @@ -25,6 +26,7 @@ export interface ILaunchRequestArgs extends DebugProtocol.LaunchRequestArguments
export interface IAttachRequestArgs extends DebugProtocol.AttachRequestArguments {
url?: string;
webRoot?: string;
address?: string;
port: number;
sourceMaps?: boolean;
diagnosticLogging?: boolean;
Expand Down
4 changes: 2 additions & 2 deletions test/chrome/chromeDebugAdapter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ suite('ChromeDebugAdapter', () => {
.setup(x => x.on(It.isAnyString(), It.isAny()))
.callback((eventName: string, handler: (msg: any) => void) => mockEventEmitter.on(eventName, handler));
mockChromeConnection
.setup(x => x.attach(It.isAnyNumber(), It.isValue(undefined)))
.setup(x => x.attach(It.isAnyNumber(), It.isValue(undefined), It.isValue(undefined)))
.returns(() => Promise.resolve<void>());
mockChromeConnection
.setup(x => x.isAttached)
Expand Down Expand Up @@ -276,7 +276,7 @@ suite('ChromeDebugAdapter', () => {
require('fs').statSync = () => true;

mockChromeConnection
.setup(x => x.attach(It.isAnyNumber(), It.isAnyString()))
.setup(x => x.attach(It.isAnyNumber(), It.isAnyString(), It.isValue(undefined)))
.returns(() => Promise.resolve<void>())
.verifiable();

Expand Down