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

Improve direct debug session termination #1747

Merged
merged 7 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/cdp-proxy/reactNativeCDPProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
IProtocolError,
IProtocolSuccess,
} from "vscode-cdp-proxy";
import { CancellationToken } from "vscode";
import { CancellationToken, EventEmitter } from "vscode";
import { OutputChannelLogger } from "../extension/log/OutputChannelLogger";
import { LogLevel } from "../extension/log/LogHelper";
import { DebuggerEndpointHelper } from "./debuggerEndpointHelper";
Expand All @@ -36,6 +36,9 @@ export class ReactNativeCDPProxy {
private applicationTargetPort: number;
private browserInspectUri: string;
private cancellationToken: CancellationToken | undefined;
private applicationTargetEventEmitter: EventEmitter<unknown> = new EventEmitter();

public readonly onApplicationTargetConnectionClosed = this.applicationTargetEventEmitter.event;

constructor(hostAddress: string, port: number, logLevel: LogLevel = LogLevel.None) {
this.port = port;
Expand Down Expand Up @@ -203,6 +206,7 @@ export class ReactNativeCDPProxy {

private async onApplicationTargetClosed() {
this.applicationTarget = null;
this.applicationTargetEventEmitter.fire({});
}

private async onDebuggerTargetClosed() {
Expand Down
2 changes: 1 addition & 1 deletion src/common/packager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class Packager {
private static OPN_PACKAGE_MAIN_FILENAME = "index.js";
private static fs: FileSystem = new FileSystem();
private expoHelper: ExponentHelper;
private runOptions: IRunOptions;
private runOptions?: IRunOptions;

constructor(
private workspacePath: string,
Expand Down
30 changes: 29 additions & 1 deletion src/debugger/debugSessionBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { ILaunchArgs, IRunOptions, PlatformType } from "../extension/launchArgs"
import { AppLauncher } from "../extension/appLauncher";
import { RNPackageVersions } from "../common/projectVersionHelper";
import { SettingsHelper } from "../extension/settingsHelper";
import { OutputChannelLogger } from "../extension/log/OutputChannelLogger";
import { RNSession } from "./debugSessionWrapper";

nls.config({
Expand All @@ -39,6 +40,10 @@ export enum DebugSessionStatus {
ConnectionDone,
/** A debuggee failed to connect */
ConnectionFailed,
/** The session is handling disconnect request now */
Stopping,
/** The session is stopped */
Stopped,
}

export interface TerminateEventArgs {
Expand Down Expand Up @@ -74,6 +79,7 @@ export abstract class DebugSessionBase extends LoggingDebugSession {
DebugSessionBase.rootSessionTerminatedEventEmitter.event;

protected readonly stopCommand: string;
protected readonly terminateCommand: string;
protected readonly pwaNodeSessionName: string;

protected appLauncher: AppLauncher;
Expand All @@ -82,6 +88,7 @@ export abstract class DebugSessionBase extends LoggingDebugSession {
protected previousAttachArgs: IAttachRequestArgs;
protected cdpProxyLogLevel: LogLevel;
protected debugSessionStatus: DebugSessionStatus;
protected nodeSession: vscode.DebugSession | null;
protected rnSession: RNSession;
protected vsCodeDebugSession: vscode.DebugSession;
protected cancellationTokenSource: vscode.CancellationTokenSource;
Expand All @@ -92,13 +99,15 @@ export abstract class DebugSessionBase extends LoggingDebugSession {
// constants definition
this.pwaNodeSessionName = "pwa-node"; // the name of node debug session created by js-debug extension
this.stopCommand = "workbench.action.debug.stop"; // the command which simulates a click on the "Stop" button
this.terminateCommand = "terminate"; // the "terminate" command is sent from the client to the debug adapter in order to give the debuggee a chance for terminating itself

// variables definition
this.rnSession = rnSession;
this.vsCodeDebugSession = rnSession.vsCodeDebugSession;
this.isSettingsInitialized = false;
this.debugSessionStatus = DebugSessionStatus.FirstConnection;
this.cancellationTokenSource = new vscode.CancellationTokenSource();
this.nodeSession = null;
}

protected initializeRequest(
Expand Down Expand Up @@ -204,6 +213,7 @@ export abstract class DebugSessionBase extends LoggingDebugSession {
}
}

this.debugSessionStatus = DebugSessionStatus.Stopped;
await logger.dispose();

DebugSessionBase.rootSessionTerminatedEventEmitter.fire({
Expand All @@ -216,7 +226,7 @@ export abstract class DebugSessionBase extends LoggingDebugSession {
this.sendResponse(response);
}

protected showError(error: Error, response: DebugProtocol.Response): void {
protected terminateWithErrorResponse(error: Error, response: DebugProtocol.Response): void {
// We can't print error messages after the debugging session is stopped. This could break the extension work.
if (
(error instanceof InternalError || error instanceof NestedError) &&
Expand Down Expand Up @@ -249,4 +259,22 @@ export abstract class DebugSessionBase extends LoggingDebugSession {
await this.appLauncher.getPackager().start();
}
}

protected showError(error: Error): void {
void vscode.window.showErrorMessage(error.message, {
modal: true,
});
// We can't print error messages via debug session logger after the session is stopped. This could break the extension work.
if (this.debugSessionStatus === DebugSessionStatus.Stopped) {
OutputChannelLogger.getMainChannel().error(error.message);
return;
}
logger.error(error.message);
}

protected async terminate(): Promise<void> {
await vscode.commands.executeCommand(this.stopCommand, undefined, {
sessionId: this.vsCodeDebugSession.id,
});
}
}
59 changes: 52 additions & 7 deletions src/debugger/direct/directDebugSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import * as nls from "vscode-nls";
import { ProjectVersionHelper } from "../../common/projectVersionHelper";
import { TelemetryHelper } from "../../common/telemetryHelper";
import { HermesCDPMessageHandler } from "../../cdp-proxy/CDPMessageHandlers/hermesCDPMessageHandler";
import { DebugSessionBase, IAttachRequestArgs, ILaunchRequestArgs } from "../debugSessionBase";
import {
DebugSessionBase,
DebugSessionStatus,
IAttachRequestArgs,
ILaunchRequestArgs,
} from "../debugSessionBase";
import { JsDebugConfigAdapter } from "../jsDebugConfigAdapter";
import { DebuggerEndpointHelper } from "../../cdp-proxy/debuggerEndpointHelper";
import { ErrorHelper } from "../../common/error/errorHelper";
Expand All @@ -29,16 +34,24 @@ const localize = nls.loadMessageBundle();
export class DirectDebugSession extends DebugSessionBase {
private debuggerEndpointHelper: DebuggerEndpointHelper;
private onDidTerminateDebugSessionHandler: vscode.Disposable;
private onDidStartDebugSessionHandler: vscode.Disposable;
private appTargetConnectionClosedHandlerDescriptor?: vscode.Disposable;
private attachSession: vscode.DebugSession | null;
private iOSWKDebugProxyHelper: IWDPHelper;

constructor(rnSession: RNSession) {
super(rnSession);
this.debuggerEndpointHelper = new DebuggerEndpointHelper();
this.iOSWKDebugProxyHelper = new IWDPHelper();
this.attachSession = null;

this.onDidTerminateDebugSessionHandler = vscode.debug.onDidTerminateDebugSession(
this.handleTerminateDebugSession.bind(this),
);

this.onDidStartDebugSessionHandler = vscode.debug.onDidStartDebugSession(
this.handleStartDebugSession.bind(this),
);
}

protected async launchRequest(
Expand Down Expand Up @@ -98,7 +111,7 @@ export class DirectDebugSession extends DebugSessionBase {
await this.vsCodeDebugSession.customRequest("attach", launchArgs);
this.sendResponse(response);
} catch (error) {
this.showError(error, response);
this.terminateWithErrorResponse(error, response);
}
}

Expand Down Expand Up @@ -144,8 +157,8 @@ export class DirectDebugSession extends DebugSessionBase {
ErrorHelper.getInternalError(
InternalErrorCode.AnotherDebuggerConnectedToPackager,
),
response,
);
void this.terminate();
},
() => {},
);
Expand Down Expand Up @@ -216,6 +229,20 @@ export class DirectDebugSession extends DebugSessionBase {
await this.preparePackagerBeforeAttach(attachArgs, versions);
}

this.appTargetConnectionClosedHandlerDescriptor = this.appLauncher
.getRnCdpProxy()
.onApplicationTargetConnectionClosed(() => {
if (this.attachSession) {
if (
this.debugSessionStatus !== DebugSessionStatus.Stopping &&
this.debugSessionStatus !== DebugSessionStatus.Stopped
) {
void this.terminate();
}
this.appTargetConnectionClosedHandlerDescriptor?.dispose();
}
});

const browserInspectUri = await this.debuggerEndpointHelper.retryGetWSEndpoint(
`http://localhost:${attachArgs.port}`,
90,
Expand All @@ -227,7 +254,7 @@ export class DirectDebugSession extends DebugSessionBase {
});
this.sendResponse(response);
} catch (error) {
this.showError(
this.terminateWithErrorResponse(
ErrorHelper.getInternalError(
InternalErrorCode.CouldNotAttachToDebugger,
error.message || error,
Expand All @@ -242,9 +269,13 @@ export class DirectDebugSession extends DebugSessionBase {
args: DebugProtocol.DisconnectArguments,
request?: DebugProtocol.Request,
): Promise<void> {
this.debugSessionStatus = DebugSessionStatus.Stopping;

this.iOSWKDebugProxyHelper.cleanUp();
this.onDidTerminateDebugSessionHandler.dispose();
this.onDidStartDebugSessionHandler.dispose();
this.appLauncher.getPackager().closeWsConnection();
this.appTargetConnectionClosedHandlerDescriptor?.dispose();
return super.disconnectRequest(response, args, request);
}

Expand Down Expand Up @@ -275,9 +306,23 @@ export class DirectDebugSession extends DebugSessionBase {
debugSession.configuration.rnDebugSessionId === this.rnSession.sessionId &&
debugSession.type === this.pwaNodeSessionName
) {
void vscode.commands.executeCommand(this.stopCommand, undefined, {
sessionId: this.vsCodeDebugSession.id,
});
void this.terminate();
}
}

private handleStartDebugSession(debugSession: vscode.DebugSession): void {
if (
this.nodeSession &&
(debugSession as any).parentSession &&
this.nodeSession.id === (debugSession as any).parentSession.id
) {
this.attachSession = debugSession;
}
if (
debugSession.configuration.rnDebugSessionId === this.rnSession.sessionId &&
debugSession.type === this.pwaNodeSessionName
) {
this.nodeSession = debugSession;
}
}

Expand Down
14 changes: 3 additions & 11 deletions src/debugger/rnDebugSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,13 @@ nls.config({
const localize = nls.loadMessageBundle();

export class RNDebugSession extends DebugSessionBase {
private readonly terminateCommand: string;

private appWorker: MultipleLifetimesAppWorker | null;
private nodeSession: vscode.DebugSession | null;
private onDidStartDebugSessionHandler: vscode.Disposable;
private onDidTerminateDebugSessionHandler: vscode.Disposable;

constructor(rnSession: RNSession) {
super(rnSession);

// constants definition
this.terminateCommand = "terminate"; // the "terminate" command is sent from the client to the debug adapter in order to give the debuggee a chance for terminating itself

// variables definition
this.appWorker = null;

Expand Down Expand Up @@ -83,7 +77,7 @@ export class RNDebugSession extends DebugSessionBase {
await this.vsCodeDebugSession.customRequest("attach", launchArgs);
this.sendResponse(response);
} catch (error) {
this.showError(error, response);
this.terminateWithErrorResponse(error, response);
}
}

Expand Down Expand Up @@ -201,7 +195,7 @@ export class RNDebugSession extends DebugSessionBase {
this.sendResponse(response);
})
.catch(err =>
this.showError(
this.terminateWithErrorResponse(
ErrorHelper.getInternalError(
InternalErrorCode.CouldNotAttachToDebugger,
err.message || err,
Expand Down Expand Up @@ -284,9 +278,7 @@ export class RNDebugSession extends DebugSessionBase {
if (this.debugSessionStatus === DebugSessionStatus.ConnectionPending) {
this.establishDebugSession(this.previousAttachArgs);
} else {
void vscode.commands.executeCommand(this.stopCommand, undefined, {
sessionId: this.vsCodeDebugSession.id,
});
void this.terminate();
}
}
}
Expand Down