fix: intermittent debug failures with browsers, especially Electron

Fixes microsoft/vscode#123420

CHANGELOG.md

@@ -10,6 +10,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 - fix: skipFiles working inconsistently in `attach` mode ([ref](https://github.com/microsoft/vscode/issues/118282))
 - fix: contribute js-debug to html ([ref](https://github.com/microsoft/vscode/issues/123106))
 - chore: log errors activating auto attach
+- fix: intermittent debug failures with browsers, especially Electron ([ref](https://github.com/microsoft/vscode/issues/123420)))
 
 ## v1.56 (April 2021)
 

src/targets/browser/launcher.ts

@@ -21,7 +21,7 @@ import {
   IBrowserProcess,
   NonTrackedBrowserProcess,
 } from './spawn/browserProcess';
-import { retryGetWSEndpoint } from './spawn/endpoints';
+import { retryGetBrowserEndpoint } from './spawn/endpoints';
 import { launchUnelevatedChrome } from './unelevatedChome';
 
 const noop = () => undefined;
@@ -206,7 +206,7 @@ export async function attach(
     );
     return new CdpConnection(connectionTransport, logger, telemetryReporter);
   } else if (browserURL) {
-    const connectionURL = await retryGetWSEndpoint(browserURL, cancellationToken, logger);
+    const connectionURL = await retryGetBrowserEndpoint(browserURL, cancellationToken, logger);
 
     const inspectWs = options.inspectUri
       ? constructInspectorWSUri(options.inspectUri, options.pageURL, connectionURL)

src/targets/browser/spawn/browserProcess.ts

@@ -16,7 +16,7 @@ import { ILogger } from '../../../common/logging';
 import { delay } from '../../../common/promiseUtil';
 import { killTree } from '../../node/killTree';
 import { constructInspectorWSUri } from '../constructInspectorWSUri';
-import { retryGetWSEndpoint } from './endpoints';
+import { retryGetBrowserEndpoint } from './endpoints';
 
 interface ITransportOptions {
   connection: 'pipe' | number;
@@ -68,7 +68,7 @@ const inspectWsConnection = async (
   const endpoint =
     options.connection === 0
       ? await waitForWSEndpoint(process, cancellationToken)
-      : await retryGetWSEndpoint(
+      : await retryGetBrowserEndpoint(
           `http://localhost:${options.connection}`,
           cancellationToken,
           logger,

src/targets/browser/spawn/endpoints.ts

@@ -18,13 +18,26 @@ export async function getWSEndpoint(
   browserURL: string,
   cancellationToken: CancellationToken,
   logger: ILogger,
+  isNode: boolean,
 ): Promise<string> {
   const provider = new BasicResourceProvider(fs);
-  const jsonVersion = await provider.fetchJson<{ webSocketDebuggerUrl?: string }>(
-    URL.resolve(browserURL, '/json/version'),
-    cancellationToken,
-    { host: 'localhost' },
-  );
+  const [jsonVersion, jsonList] = await Promise.all([
+    provider.fetchJson<{ webSocketDebuggerUrl?: string }>(
+      URL.resolve(browserURL, '/json/version'),
+      cancellationToken,
+      { host: 'localhost' },
+    ),
+    // Chrome publishes its top-level debugg on /json/version, while Node does not.
+    // Request both and return whichever one got us a string. ONLY try this on
+    // Node, since it'll cause a failure on browsers (vscode#123420)
+    isNode
+      ? provider.fetchJson<{ webSocketDebuggerUrl: string }[]>(
+          URL.resolve(browserURL, '/json/list'),
+          cancellationToken,
+          { host: 'localhost' },
+        )
+      : Promise.resolve(undefined),
+  ]);
 
   if (!jsonVersion.ok) {
     logger.verbose(LogTag.RuntimeLaunch, 'Error looking up /json/version', jsonVersion);
@@ -37,17 +50,11 @@ export async function getWSEndpoint(
     return fixed;
   }
 
-  // Chrome its top-level debugg on /json/version, while Node does not.
-  // Request both and return whichever one got us a string.
-  const jsonList = await provider.fetchJson<{ webSocketDebuggerUrl: string }[]>(
-    URL.resolve(browserURL, '/json/list'),
-    cancellationToken,
-    { host: 'localhost' },
-  );
-
-  if (!jsonList.ok) {
+  if (!jsonList) {
+    // no-op
+  } else if (!jsonList.ok) {
     logger.verbose(LogTag.RuntimeLaunch, 'Error looking up /json/list', jsonList);
-  } else if (jsonList.body.length) {
+  } else {
     const fixed = fixRemoteUrl(browserURL, jsonList.body[0].webSocketDebuggerUrl);
     logger.verbose(LogTag.RuntimeLaunch, 'Discovered target URL from /json/list', {
       url: jsonList.body[0].webSocketDebuggerUrl,
@@ -59,28 +66,39 @@ export async function getWSEndpoint(
   throw new Error('Could not find any debuggable target');
 }
 
+const makeRetryGetWSEndpoint = (isNode: boolean) => async (
+  browserURL: string,
+  cancellationToken: CancellationToken,
+  logger: ILogger,
+): Promise<string> => {
+  while (true) {
+    try {
+      return await getWSEndpoint(browserURL, cancellationToken, logger, isNode);
+    } catch (e) {
+      if (cancellationToken.isCancellationRequested) {
+        throw new Error(`Could not connect to debug target at ${browserURL}: ${e.message}`);
+      }
+
+      await delay(200);
+    }
+  }
+};
+
 /**
- * Attempts to retrieve the debugger websocket URL for a process listening
+ * Attempts to retrieve the debugger websocket URL for a Node process listening
  * at the given address, retrying until available.
  * @param browserURL -- Address like `http://localhost:1234`
  * @param cancellationToken -- Optional cancellation for this operation
  */
-export async function retryGetWSEndpoint(
-  browserURL: string,
-  cancellationToken: CancellationToken,
-  logger: ILogger,
-): Promise<string> {
-  try {
-    return await getWSEndpoint(browserURL, cancellationToken, logger);
-  } catch (e) {
-    if (cancellationToken.isCancellationRequested) {
-      throw new Error(`Could not connect to debug target at ${browserURL}: ${e.message}`);
-    }
+export const retryGetNodeEndpoint = makeRetryGetWSEndpoint(true);
 
-    await delay(200);
-    return retryGetWSEndpoint(browserURL, cancellationToken, logger);
-  }
-}
+/**
+ * Attempts to retrieve the debugger websocket URL for a browser listening
+ * at the given address, retrying until available.
+ * @param browserURL -- Address like `http://localhost:1234`
+ * @param cancellationToken -- Optional cancellation for this operation
+ */
+export const retryGetBrowserEndpoint = makeRetryGetWSEndpoint(false);
 
 function fixRemoteUrl(rawBrowserUrl: string, rawWebSocketUrl: string) {
   const browserUrl = new URL.URL(rawBrowserUrl);

src/targets/node/extensionHostAttacher.ts

@@ -13,7 +13,7 @@ import {
   IExtensionHostAttachConfiguration,
   KillBehavior,
 } from '../../configuration';
-import { retryGetWSEndpoint } from '../browser/spawn/endpoints';
+import { retryGetNodeEndpoint } from '../browser/spawn/endpoints';
 import { NodeAttacherBase } from './nodeAttacherBase';
 import { NodeBinary } from './nodeBinaryProvider';
 import { IRunData } from './nodeLauncherBase';
@@ -53,7 +53,7 @@ export class ExtensionHostAttacher extends NodeAttacherBase<IExtensionHostAttach
   protected async launchProgram(
     runData: IRunData<IExtensionHostAttachConfiguration>,
   ): Promise<void> {
-    const inspectorURL = await retryGetWSEndpoint(
+    const inspectorURL = await retryGetNodeEndpoint(
       `http://localhost:${runData.params.port}`,
       runData.context.cancellationToken,
       this.logger,

src/targets/node/nodeAttacher.ts

@@ -13,7 +13,7 @@ import { ILogger, LogTag } from '../../common/logging';
 import { delay } from '../../common/promiseUtil';
 import { isLoopback } from '../../common/urlUtils';
 import { AnyLaunchConfiguration, INodeAttachConfiguration } from '../../configuration';
-import { retryGetWSEndpoint } from '../browser/spawn/endpoints';
+import { retryGetNodeEndpoint } from '../browser/spawn/endpoints';
 import { ISourcePathResolverFactory } from '../sourcePathResolverFactory';
 import { IStopMetadata } from '../targets';
 import { LeaseFile } from './lease-file';
@@ -68,7 +68,7 @@ export class NodeAttacher extends NodeAttacherBase<INodeAttachConfiguration> {
         if (runData.params.websocketAddress) {
           inspectorURL = runData.params.websocketAddress;
         } else {
-          inspectorURL = await retryGetWSEndpoint(
+          inspectorURL = await retryGetNodeEndpoint(
             `http://${runData.params.address}:${runData.params.port}`,
             restarting
               ? CancellationTokenSource.withTimeout(runData.params.timeout).token

src/targets/node/nodeAttacherCluster.ts

@@ -45,7 +45,7 @@ export async function watchAllChildren(
     }
 
     todo.push(
-      getWSEndpoint(`http://${options.hostname}:${port}`, cancellation, logger)
+      getWSEndpoint(`http://${options.hostname}:${port}`, cancellation, logger, true)
         .then(inspectorURL =>
           WatchDog.attach({
             ipcAddress: options.ipcAddress,

src/targets/node/nodeLauncher.ts

@@ -16,7 +16,7 @@ import { ISourceMapMetadata } from '../../common/sourceMaps/sourceMap';
 import { absolutePathToFileUrl, urlToRegex } from '../../common/urlUtils';
 import { AnyLaunchConfiguration, INodeLaunchConfiguration } from '../../configuration';
 import { fixInspectFlags } from '../../ui/configurationUtils';
-import { retryGetWSEndpoint } from '../browser/spawn/endpoints';
+import { retryGetNodeEndpoint } from '../browser/spawn/endpoints';
 import { ISourcePathResolverFactory } from '../sourcePathResolverFactory';
 import { CallbackFile } from './callback-file';
 import { getRunScript, hideDebugInfoFromConsole, INodeBinaryProvider } from './nodeBinaryProvider';
@@ -145,7 +145,7 @@ export class NodeLauncher extends NodeLauncherBase<INodeLaunchConfiguration> {
         const wd = await WatchDog.attach({
           ipcAddress: runData.serverAddress,
           scriptName: 'Remote Process',
-          inspectorURL: await retryGetWSEndpoint(
+          inspectorURL: await retryGetNodeEndpoint(
             `http://127.0.0.1:${this.attachSimplePort}`,
             runData.context.cancellationToken,
             this.logger,