Skip to content

Commit

Permalink
Fix debugger detaching when restarting on Expo Go (#985)
Browse files Browse the repository at this point in the history
This PR fixes issue where debugger is detached after restart, when
runing Expo Go app. This is due to the fact that when reloading metro,
the new instance of debugger with new websocket adress is created.

This PR solves it as follows:
- after running `metro.reload()` we wait for app to be ready
- once it's ready, we're checking if old debug session is available
- when old runtime is not available we close old debug session and start
the now one
- if old runtime is available, then we do nothing 

How to determine the old debug session is available:
 
 So there are to approches:
 - trying to execture something through the debugger
 - check if old debugger is on the list (#994)
 
The problem with the first one is that when debuger is stale, it might
just not answer, in that case we timeout after 1 sec.
The problem with the second one is that sometimes there is ws target
available that can't be used (Expo Go case), this the solution is not
working reliably. To overcome those two problems we combined both
solutions. We run both checks in `promise.any`, so:
- If ping returns true, we're good (connection alive)
- if checking list of ws targets returns false, we're good (connection
closed)
- if current ws target is on the list, we reject so `promise.any` waits
for ping and ping will check that reliably after at most 1 sec

### How Has This Been Tested: 

**For debugger that detaches during restart i.e. expo-go test app:**
- Lunch expo-go app 
- Test if debugger works (for example log or break point)
- Restart with `Reload JS`
- Wait until it restarts, and make sure the debugger is still attached
(there should be also nice ui with "launching", "attaching to debugger")

**For apps that debugger do not detach i.e. react-native-76**
- Lunch expo-go app 
- Test if debugger works (for example log or break point)
- Add breakpoint in
https://github.com/software-mansion/radon-ide/blob/59372cc4e83d28edb2ae6f183b2f0d0a2e48eeb9/packages/vscode-extension/src/debugging/DebugSession.ts#L107
- Restart with `Reload JS`
- Wait until it restarts (**it should not stop at the breakpoint**)
- Make sure the debugger is still attached (the UI should be similar as
above, but the "attaching debugger" will pass in the blink of an eye)

|Expo Go Before|Expo Go After|
|-|-|
|<video
src="https://github.com/user-attachments/assets/5ae32a1c-4c33-4a55-98a1-c645f4e92e59"
/>|<video
src="https://github.com/user-attachments/assets/eca7c8c2-8806-45c7-a386-10f79dd706d7"
/>|


|RN76 Before|RN 76 After|
|-|-|
|<video
src="https://github.com/user-attachments/assets/e0b81615-e2f7-412a-b3e5-6f83e141b9a7"
/>|<video
src="https://github.com/user-attachments/assets/70b6fae8-8ebe-4ba3-bc5d-141c9dc545ef"
/>|
  • Loading branch information
maciekstosio authored Mar 5, 2025
1 parent 439463e commit 2b96755
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 24 deletions.
24 changes: 24 additions & 0 deletions packages/vscode-extension/src/debugging/DebugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,27 @@ export class DebugAdapter extends DebugSession implements CDPSessionDelegate {
this.sendResponse(response);
}

private async ping() {
if (!this.cdpSession) {
Logger.warn("[DebugAdapter] [ping] The CDPSession was not initialized yet");
return;
}

try {
const res = await this.cdpSession.sendCDPMessage("Runtime.evaluate", {
expression: "('ping')",
});

const { result } = res;

if (result.value === "ping") {
this.sendEvent(new Event("RNIDE_pong"));
}
} catch (_) {
/** debugSession is waiting for an event, if it won't get any it will fail after timeout, so we don't need to do anything here */
}
}

protected async customRequest(
command: string,
response: DebugProtocol.Response,
Expand Down Expand Up @@ -400,6 +421,9 @@ export class DebugAdapter extends DebugSession implements CDPSessionDelegate {
this.sendEvent(new Event("RNIDE_profilingCPUStopped", { filePath }));
}
break;
case "RNIDE_ping":
this.ping();
break;
default:
Logger.debug(`Custom req ${command} ${args}`);
}
Expand Down
74 changes: 71 additions & 3 deletions packages/vscode-extension/src/debugging/DebugSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import {
DebugSession as VscDebugSession,
} from "vscode";
import { Metro } from "../project/metro";
import { Logger } from "../Logger";
import { CDPConfiguration } from "./DebugAdapter";

const PING_TIMEOUT = 1000;

export type DebugSessionDelegate = {
onConsoleLog(event: DebugSessionCustomEvent): void;
onDebuggerPaused(event: DebugSessionCustomEvent): void;
Expand All @@ -20,7 +23,10 @@ export type DebugSource = { filename?: string; line1based?: number; column0based
export class DebugSession implements Disposable {
private vscSession: VscDebugSession | undefined;
private debugEventsListener: Disposable;
private pingTimeout: NodeJS.Timeout | undefined;
private pingResolve: ((result: boolean) => void) | undefined;
private wasConnectedToCDP: boolean = false;
private currentWsTarget: string | undefined;

constructor(private delegate: DebugSessionDelegate) {
this.debugEventsListener = debug.onDidReceiveDebugSessionCustomEvent((event) => {
Expand All @@ -34,6 +40,13 @@ export class DebugSession implements Disposable {
case "RNIDE_continued":
this.delegate.onDebuggerResumed(event);
break;
case "RNIDE_pong":
if (this.pingResolve) {
this.pingResolve(true);
} else {
Logger.warn("[DEBUG SESSION] Received unexpected pong event");
}
break;
case "RNIDE_profilingCPUStarted":
this.delegate.onProfilingCPUStarted(event);
break;
Expand All @@ -47,6 +60,16 @@ export class DebugSession implements Disposable {
});
}

public async reconnectJSDebuggerIfNeeded(metro: Metro) {
const isAlive = await this.isWsTargetAlive(metro);

if (!isAlive) {
return this.connectJSDebugger(metro);
}

return true;
}

private async startInternal() {
const debugStarted = await debug.startDebugging(
undefined,
Expand Down Expand Up @@ -91,6 +114,9 @@ export class DebugSession implements Disposable {

private async stop() {
this.vscSession && (await debug.stopDebugging(this.vscSession));
this.vscSession = undefined;
this.wasConnectedToCDP = false;
this.currentWsTarget = undefined;
}

/**
Expand All @@ -104,8 +130,7 @@ export class DebugSession implements Disposable {

public async connectJSDebugger(metro: Metro) {
if (this.wasConnectedToCDP) {
this.vscSession && debug.stopDebugging(this.vscSession);
await this.startInternal();
await this.restart();
}

const websocketAddress = await metro.getDebuggerURL();
Expand All @@ -124,13 +149,14 @@ export class DebugSession implements Disposable {
}

await this.connectCDPDebugger({
websocketAddress: websocketAddress,
websocketAddress,
sourceMapAliases,
expoPreludeLineCount: metro.expoPreludeLineCount,
breakpointsAreRemovedOnContextCleared: isUsingNewDebugger ? false : true, // new debugger properly keeps all breakpoints in between JS reloads
});

this.wasConnectedToCDP = true;
this.currentWsTarget = websocketAddress;

return true;
}
Expand All @@ -143,6 +169,48 @@ export class DebugSession implements Disposable {
this.session.customRequest("next");
}

public async isWsTargetAlive(metro: Metro): Promise<boolean> {
/**
* This is a bit tricky, the idea is that we run both checks.
* pingCurrentWsTarget provides us reliable information about connection.
* isCurrentWsTargetStillVisible can say reliably only if the connection were lost (is missing on ws targets list).
* So what we do is promise any, but isCurrentWsTargetStillVisible rejects promise if the connection is on the list, so
* we can wait for ping to resolve.
*/
return Promise.any([this.pingCurrentWsTarget(), this.isCurrentWsTargetStillVisible(metro)]);
}

public async isCurrentWsTargetStillVisible(metro: Metro): Promise<boolean> {
return new Promise(async (resolve, reject) => {
const possibleWsTargets = await metro.fetchWsTargets();
const hasCurrentWsAddress = possibleWsTargets?.some(
(runtime) => runtime.webSocketDebuggerUrl === this.currentWsTarget
);

if (!this.currentWsTarget || !hasCurrentWsAddress) {
return resolve(false);
}
// We're rejecting as shouldDebuggerReconnect uses .any which waits for first promise to resolve.
// And th fact that current wsTarget is on the list is not enough, it might be stale, so in this case we wait for ping.
reject();
});
}

public async pingCurrentWsTarget(): Promise<boolean> {
this.session.customRequest("RNIDE_ping");
return new Promise((resolve, _) => {
this.pingResolve = (value) => {
clearTimeout(this.pingTimeout);
resolve(value);
this.pingResolve = undefined;
this.pingTimeout = undefined;
};
this.pingTimeout = setTimeout(() => {
this.pingResolve?.(false);
}, PING_TIMEOUT);
});
}

public async startProfilingCPU() {
await this.session.customRequest("RNIDE_startProfiling");
}
Expand Down
9 changes: 8 additions & 1 deletion packages/vscode-extension/src/project/deviceSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export class DeviceSession implements Disposable {
case "reloadJs":
if (this.devtools.hasConnectedClient) {
try {
await this.metro.reload();
await this.reloadMetro();
return true;
} catch (e) {
Logger.error("Failed to reload JS", e);
Expand All @@ -138,6 +138,13 @@ export class DeviceSession implements Disposable {

private launchAppCancelToken: CancelToken | undefined;

private async reloadMetro() {
this.eventDelegate.onStateChange(StartupMessage.WaitingForAppToLoad);
await Promise.all([this.metro.reload(), this.devtools.appReady()]);
this.eventDelegate.onStateChange(StartupMessage.AttachingDebugger);
await this.debugSession?.reconnectJSDebuggerIfNeeded(this.metro);
}

private async launchApp(previewReadyCallback?: PreviewReadyCallback) {
this.launchAppCancelToken && this.launchAppCancelToken.cancel();

Expand Down
56 changes: 37 additions & 19 deletions packages/vscode-extension/src/project/metro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Devtools } from "./devtools";
import { getLaunchConfiguration } from "../utilities/launchConfiguration";
import { EXPO_GO_BUNDLE_ID, EXPO_GO_PACKAGE_NAME } from "../builders/expoGo";
import { connectCDPAndEval } from "../utilities/connectCDPAndEval";
import { progressiveRetryTimeout, sleep } from "../utilities/retry";
import { getOpenPort } from "../utilities/common";
import { DebugSource } from "../debugging/DebugSession";

Expand Down Expand Up @@ -339,18 +340,6 @@ export class Metro implements Disposable {
await this.sendMessageToDevice("devMenu");
}

public async getDebuggerURL() {
const WAIT_FOR_DEBUGGER_TIMEOUT_MS = 15_000;

const startTime = Date.now();
let websocketAddress: string | undefined;
while (!websocketAddress && Date.now() - startTime < WAIT_FOR_DEBUGGER_TIMEOUT_MS) {
websocketAddress = await this.fetchDebuggerURL();
await new Promise((res) => setTimeout(res, 1000));
}
return websocketAddress;
}

private lookupWsAddressForOldDebugger(listJson: CDPTargetDescription[]) {
// Pre 0.76 RN metro lists debugger pages that are identified as "deviceId-pageId"
// After new device is connected, the deviceId is incremented while pageId could be
Expand Down Expand Up @@ -457,14 +446,43 @@ export class Metro implements Disposable {
return undefined;
}

private async fetchDebuggerURL() {
// query list from http://localhost:${metroPort}/json/list
const list = await fetch(`http://localhost:${this._port}/json/list`);
const listJson = await list.json();
public async fetchWsTargets(): Promise<CDPTargetDescription[] | undefined> {
const WAIT_FOR_DEBUGGER_TIMEOUT_MS = 15_000;

// fixup websocket addresses on the list
for (const page of listJson) {
page.webSocketDebuggerUrl = this.fixupWebSocketDebuggerUrl(page.webSocketDebuggerUrl);
let retryCount = 0;
const startTime = Date.now();

while (Date.now() - startTime < WAIT_FOR_DEBUGGER_TIMEOUT_MS) {
retryCount++;

try {
const list = await fetch(`http://localhost:${this._port}/json/list`);
const listJson = await list.json();

if (listJson.length > 0) {
// fixup websocket addresses on the list
for (const page of listJson) {
page.webSocketDebuggerUrl = this.fixupWebSocketDebuggerUrl(page.webSocketDebuggerUrl);
}

return listJson;
}
} catch (_) {
// It shouldn't happen, so lets warn about it. Except a warning we will retry anyway, so nothing to do here.
Logger.warn("[METRO] Fetching list of runtimes failed, retrying...");
}

await sleep(progressiveRetryTimeout(retryCount));
}

return undefined;
}

public async getDebuggerURL() {
const listJson = await this.fetchWsTargets();

if (listJson === undefined) {
return undefined;
}

// When there are pages that are identified as belonging to the new debugger, we
Expand Down
5 changes: 4 additions & 1 deletion packages/vscode-extension/src/utilities/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ export async function retry<T>(fn: RetryFn<T>, retries = 5, interval = 1000): Pr
return await call(retries);
}

function sleep(ms: number) {
export const progressiveRetryTimeout = (retryNumber: number) =>
Math.min(100 * Math.max(retryNumber, 1), 1000);

export function sleep(ms: number) {
return new Promise((resolve) => setTimeout(resolve, ms));
}

0 comments on commit 2b96755

Please sign in to comment.