Skip to content

Commit

Permalink
Fix handling of numeric script ID when debug breakpoint is hit (#46)
Browse files Browse the repository at this point in the history
This PR fixes issue introduced in
fc3f800 where numeric script IDs were
expected to be numbers. This is not inline with CDP spec, as the IDs are
always strings. As a result we were attempting to parse those numeric
IDs as URLs which resulted in an exception being thrown and debugger
getting stuck without visual indicator.

In this PR I'm also addressing issue with touch events getting triggered
when debug overlay is present. This resulted in touches being sent to
device when someone would click the resume button. As a consequence when
button was placed over an item with click event and breakpoint set, we'd
be stuck in a breakpoint loop.
  • Loading branch information
kmagiera authored Mar 28, 2024
1 parent e8ad678 commit 420f6f1
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 22 deletions.
36 changes: 20 additions & 16 deletions packages/vscode-extension/src/debugging/DebugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,16 @@ import {
CDPPropertyDescriptor,
} from "./cdp";

function modifyURL(url: string, hostname: string, port: string): string {
const parsedURL = new URL(url);
parsedURL.hostname = hostname;
parsedURL.port = port;
return parsedURL.toString();
function compareIgnoringHost(url1: string, url2: string) {
try {
const firstURL = new URL(url1);
const secondURL = new URL(url2);
firstURL.hostname = secondURL.hostname = "localhost";
firstURL.port = secondURL.port = "8080";
return firstURL.href === secondURL.href;
} catch (e) {
return false;
}
}

function typeToCategory(type: string) {
Expand Down Expand Up @@ -67,7 +72,7 @@ export class DebugAdapter extends DebugSession {
private connection: WebSocket;
private configuration: DebugConfiguration;
private threads: Array<Thread> = [];
private sourceMaps: Array<[string, number, SourceMapConsumer]> = [];
private sourceMaps: Array<[string, string, SourceMapConsumer]> = [];

private linesStartAt1 = true;
private columnsStartAt1 = true;
Expand Down Expand Up @@ -179,21 +184,22 @@ export class DebugAdapter extends DebugSession {
}

private findOriginalPosition(
scriptIdOrURL: number | string,
scriptIdOrURL: string,
lineNumber1Based: number,
columnNumber0Based: number
) {
let scriptURL = "__script__";
let sourceURL = "__source__";
let sourceLine1Based = lineNumber1Based;
let sourceColumn0Based = columnNumber0Based;
this.sourceMaps.forEach(([url, id, consumer]) => {
if (typeof scriptIdOrURL === "string") {
const { port, hostname } = new URL(url);
scriptIdOrURL = modifyURL(scriptIdOrURL, hostname, port);
}

if (id === scriptIdOrURL || url === scriptIdOrURL) {
this.sourceMaps.forEach(([url, id, consumer]) => {
// when we identify script by its URL we need to deal with a situation when the URL is sent with a different
// hostname and port than the one we have registered in the source maps. The reason for that is that the request
// that populates the source map (scriptParsed) is sent by metro, while the requests from breakpoints or logs
// are sent directly from the device. In different setups, specifically on Android emulator, the device uses different URLs
// than localhost because it has a virtual network interface. Hence we need to unify the URL:
if (id === scriptIdOrURL || compareIgnoringHost(url, scriptIdOrURL)) {
scriptURL = url;
const pos = consumer.originalPositionFor({
line: lineNumber1Based,
Expand Down Expand Up @@ -288,9 +294,7 @@ export class DebugAdapter extends DebugSession {
stackObjEntry.variablesReference
);
const methodName = stackObjProperties.find((v) => v.name === "methodName")?.value || "";
let genUrl = stackObjProperties.find((v) => v.name === "file")?.value || "";
// genUrl = changeURLHostname(genUrl, "localhost", "");
Logger.debug("GENURL", genUrl);
const genUrl = stackObjProperties.find((v) => v.name === "file")?.value || "";
const genLine1Based = parseInt(
stackObjProperties.find((v) => v.name === "lineNumber")?.value || "0"
);
Expand Down
23 changes: 17 additions & 6 deletions packages/vscode-extension/src/webview/components/Preview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,30 @@ function Preview({ isInspecting, setIsInspecting }) {

const inspectFrame = inspectData?.frame;

const shouldPreventTouchInteraction =
debugPaused ||
debugException ||
hasBundleError ||
hasIncrementalBundleError ||
projectState?.status === "refreshing";

const touchHandlers = shouldPreventTouchInteraction
? {}
: {
onMouseDown,
onMouseMove,
onMouseUp,
onMouseLeave,
};

return (
<div
className="phone-wrapper"
style={cssPropertiesForDevice(device)}
tabIndex={0} // allows keyboard events to be captured
ref={wrapperDivRef}>
{!isStarting && !hasBuildError && (
<div
className="phone-content"
onMouseMove={onMouseMove}
onMouseLeave={onMouseLeave}
onMouseDown={onMouseDown}
onMouseUp={onMouseUp}>
<div className="phone-content" {...touchHandlers}>
<img
src={previewURL}
ref={previewRef}
Expand Down

0 comments on commit 420f6f1

Please sign in to comment.