feat: 'intelligently' suggest using diagnostic tool for breakpoint is…

…sues

Fixes microsoft/vscode#57590

CHANGELOG.md

@@ -4,6 +4,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 
 ## TBA
 
+- feat: 'intelligently' suggest using diagnostic tool for breakpoint issues ([ref](https://github.com/microsoft/vscode/issues/57590))
 - fix: runtimeVersion overwriting default PATH ([ref](https://github.com/microsoft/vscode/issues/120140))
 - fix: skipFiles not skipping ranges in sourcemapped scripts ([ref](https://github.com/microsoft/vscode/issues/118282))
 - chore: update wording on debug terminal label to match new profiles

src/adapter/debugAdapter.ts

@@ -20,6 +20,7 @@ import { BreakpointManager } from './breakpoints';
 import { ICompletions } from './completions';
 import { IConsole } from './console';
 import { Diagnostics } from './diagnosics';
+import { DiagnosticToolSuggester } from './diagnosticToolSuggester';
 import { IEvaluator } from './evaluator';
 import { IExceptionPauseService, PauseOnExceptionsState } from './exceptionPauseService';
 import { IPerformanceProvider } from './performance';
@@ -333,6 +334,8 @@ export class DebugAdapter implements IDisposable {
       );
 
     this.breakpointManager.setThread(this._thread);
+    this._services.get(DiagnosticToolSuggester).attach(cdp);
+
     return this._thread;
   }
 

src/adapter/diagnosticToolSuggester.ts

@@ -0,0 +1,100 @@
+/*---------------------------------------------------------
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ *--------------------------------------------------------*/
+
+import { inject, injectable } from 'inversify';
+import Cdp from '../cdp/api';
+import { DisposableList } from '../common/disposable';
+import { disposableTimeout } from '../common/promiseUtil';
+import Dap from '../dap/api';
+import { IRootDapApi } from '../dap/connection';
+
+const ignoredModulePatterns = /\/node_modules\/|^node\:/;
+const consecutiveSessions = 2;
+const suggestDelay = 5000;
+const minDuration = suggestDelay / 2;
+
+/**
+ * Fires an event to indicate to the UI that it should suggest the user open
+ * the diagnostic tool. The indicator will be shown when all of the following
+ * are true:
+ *
+ * - At least one breakpoint was set, but no breakpoints bound,
+ * - For two consecutive debug sessions,
+ * - Where a sourcemap was used for a script outside of the node_modules, or
+ *   a remoteRoot is present (since sourcemaps and remote are the cases where
+ *   almost all path resolution issues happen)
+ */
+@injectable()
+export class DiagnosticToolSuggester {
+  /**
+   * Number of sessions that qualify for help. The DiagnosticToolSuggester is
+   * a global singleton and we don't care about persistence, so this is fine.
+   */
+  private static consecutiveQualifyingSessions = 0;
+
+  private readonly disposable = new DisposableList();
+  private hadBreakpoint = false;
+  private didVerifyBreakpoint = false;
+  private hadNonModuleSourcemap = false;
+  private startedAt = Date.now();
+
+  private get currentlyQualifying() {
+    return this.hadBreakpoint && !this.didVerifyBreakpoint && this.hadNonModuleSourcemap;
+  }
+
+  constructor(@inject(IRootDapApi) dap: Dap.Api) {
+    if (DiagnosticToolSuggester.consecutiveQualifyingSessions >= consecutiveSessions) {
+      this.disposable.push(
+        disposableTimeout(() => {
+          if (this.currentlyQualifying) {
+            dap.suggestDiagnosticTool({});
+          }
+        }, suggestDelay),
+      );
+    }
+  }
+
+  public notifyHadBreakpoint() {
+    this.hadBreakpoint = true;
+  }
+
+  /**
+   * Attaches the CDP API. Should be called for each
+   */
+  public attach(cdp: Cdp.Api) {
+    if (!this.hadNonModuleSourcemap) {
+      const listener = this.disposable.push(
+        cdp.Debugger.on('scriptParsed', evt => {
+          if (!!evt.sourceMapURL && !ignoredModulePatterns.test(evt.url)) {
+            this.hadNonModuleSourcemap = true;
+            this.disposable.disposeObject(listener);
+          }
+        }),
+      );
+    }
+
+    if (!this.didVerifyBreakpoint) {
+      const listener = this.disposable.push(
+        cdp.Debugger.on('breakpointResolved', () => {
+          this.didVerifyBreakpoint = true;
+          this.disposable.disposeObject(listener);
+        }),
+      );
+    }
+  }
+
+  /**
+   * Should be called before the root debug session ends. It'll fire a DAP
+   * message to show a notification if appropriate.
+   */
+  public dispose() {
+    if (this.currentlyQualifying && Date.now() - minDuration > this.startedAt) {
+      DiagnosticToolSuggester.consecutiveQualifyingSessions++;
+    } else {
+      DiagnosticToolSuggester.consecutiveQualifyingSessions = 0;
+    }
+
+    this.disposable.dispose();
+  }
+}

src/binder.ts

@@ -8,6 +8,7 @@ import { CancellationToken } from 'vscode';
 import * as nls from 'vscode-nls';
 import { getAsyncStackPolicy, IAsyncStackPolicy } from './adapter/asyncStackPolicy';
 import { DebugAdapter } from './adapter/debugAdapter';
+import { DiagnosticToolSuggester } from './adapter/diagnosticToolSuggester';
 import { SelfProfile } from './adapter/selfProfile';
 import { Thread } from './adapter/threads';
 import { CancellationTokenSource } from './common/cancellation';
@@ -99,6 +100,10 @@ export class Binder implements IDisposable {
       });
       dap.on('setExceptionBreakpoints', async () => ({}));
       dap.on('setBreakpoints', async params => {
+        if (params.breakpoints?.length) {
+          _rootServices.get(DiagnosticToolSuggester).notifyHadBreakpoint();
+        }
+
         return {
           breakpoints:
             params.breakpoints?.map(() => ({

src/build/dapCustom.ts

@@ -473,6 +473,12 @@ const dapCustom: JSONSchema4 = {
         required: ['file'],
       },
     ),
+
+    ...makeEvent(
+      'suggestDiagnosticTool',
+      "Shows a prompt to the user suggesting they use the diagnostic tool if breakpoints don't bind.",
+      {},
+    ),
   },
 };
 

src/build/strings.ts

@@ -306,7 +306,7 @@ A common case to disable certificate verification can be done by passing \`{ "ht
   'profile.start': 'Take Performance Profile',
   'profile.stop': 'Stop Performance Profile',
   'debugLink.label': 'Open Link',
-  'createDiagnostics.label': 'Create Diagnostic Information for Current Session',
+  'createDiagnostics.label': 'Diagnose Breakpoint Problems',
   'startWithStopOnEntry.label': 'Start Debugging and Stop on Entry',
 };