feat: don't attach debugger to terminal until a process starts (#221)

* feat: don't attach debugger to terminal until a process starts

As referenced in #199, the debugger terminal experience today is awkward
in that a session is 'running' as long as the terminal is open, even
when no Node.js processes are currently active. This PR addresses that.

We reuse the `TerminalNodeLauncher` to set up the terminal, but do so
outside of the debug adapter. Instead, when we see a process launches
and enters debug mode, we register it inside of a 'delegate' collection,
and then send the delegate ID into `vscode.debug.startDebugging`. This
pulls up the debug session as if it was just launched insider the
adapter, and from then on it's identical to any other debug session.

This PR also adds an configuration option that configures the default
launch config used for the debugger terminal, and there's churn from
typing the `targetOrigin` away from `any` throughout the process, now
that I've fully internalized what it is and what it's used for :)

This implementation has some indirection, but within the adapter's
architecture I believe this is the best general approach. Due to the way
in which we launch processes and the way the VS Code terminal behaves
(both of which are optimal), we can't ever know that we have a
debuggable target exists until it knocks on our door with a CDP
connection. At that point, we need to delegate it into a debug session
in some form or another. That's what I've tried to do here with a
minimal amount of changes or knowledge-leaks into the rest of the code.

Fixes #199

* fix: use terminal attach as delegate mode instead of different type

Author: connor4312

package-lock.json

@@ -57,7 +57,7 @@
     "micromatch": "^4.0.2",
     "source-map": "^0.7.3",
     "split2": "^3.1.1",
-    "typescript": "^3.7.2",
+    "typescript": "^3.7.4",
     "vscode-nls": "^4.1.1",
     "ws": "^7.0.1"
   },

package.json

@@ -24,6 +24,7 @@ import * as errors from './dap/errors';
 import { ILauncher, ILaunchResult, ITarget } from './targets/targets';
 import { RawTelemetryReporterToDap } from './telemetry/telemetryReporter';
 import { filterErrorsReportedToTelemetry } from './telemetry/unhandledErrorReporter';
+import { ITargetOrigin } from './targets/targetOrigin';
 import { IAsyncStackPolicy, getAsyncStackPolicy } from './adapter/asyncStackPolicy';
 
 const localize = nls.loadMessageBundle();
@@ -44,7 +45,7 @@ export class Binder implements IDisposable {
   private _onTargetListChangedEmitter = new EventEmitter<void>();
   readonly onTargetListChanged = this._onTargetListChangedEmitter.event;
   private _dap: Promise<Dap.Api>;
-  private _targetOrigin: any;
+  private _targetOrigin: ITargetOrigin;
   private _launchParams?: AnyLaunchConfiguration;
   private _rawTelemetryReporter: RawTelemetryReporterToDap | undefined;
   private _clientCapabilities: Dap.InitializeParams | undefined;
@@ -54,7 +55,7 @@ export class Binder implements IDisposable {
     delegate: IBinderDelegate,
     connection: DapConnection,
     launchers: ILauncher[],
-    targetOrigin: any,
+    targetOrigin: ITargetOrigin,
   ) {
     this._delegate = delegate;
     this._dap = connection.dap();

src/binder.ts

@@ -3,6 +3,7 @@
  *--------------------------------------------------------*/
 import { Contributions, IConfigurationTypes, Configuration } from '../common/contributionUtils';
 import {
+  IMandatedConfiguration,
   AnyLaunchConfiguration,
   ResolvingConfiguration,
   INodeAttachConfiguration,
@@ -14,20 +15,18 @@ import {
   IChromeBaseConfiguration,
   IChromeLaunchConfiguration,
   IChromeAttachConfiguration,
-  INodeTerminalConfiguration,
+  ITerminalLaunchConfiguration,
   baseDefaults,
 } from '../configuration';
 import { JSONSchema6 } from 'json-schema';
 import strings from './strings';
 import { walkObject, sortKeys } from '../common/objUtils';
 
 type OmittedKeysFromAttributes =
-  | 'type'
-  | 'request'
-  | 'internalConsoleOptions'
-  | 'name'
+  | keyof IMandatedConfiguration
   | 'rootPath'
-  | '__workspaceFolder';
+  | '__workspaceFolder'
+  | '__workspaceCachePath';
 
 type ConfigurationAttributes<T> = {
   [K in keyof Omit<T, OmittedKeysFromAttributes>]: JSONSchema6 &
@@ -468,7 +467,7 @@ const nodeLaunchConfig: IDebugger<INodeLaunchConfiguration> = {
   },
 };
 
-const nodeTerminalConfiguration: IDebugger<INodeTerminalConfiguration> = {
+const nodeTerminalConfiguration: IDebugger<ITerminalLaunchConfiguration> = {
   type: Contributions.TerminalDebugType,
   request: 'launch',
   label: refString('debug.terminal.label'),
@@ -730,6 +729,12 @@ const configurationSchema: ConfigurationAttributes<IConfigurationTypes> = {
     default: true,
     description: refString('configuration.warnOnLongPrediction'),
   },
+  [Configuration.TerminalDebugConfig]: {
+    type: 'object',
+    description: refString('configuration.terminalOptions'),
+    default: {},
+    properties: nodeTerminalConfiguration.configurationAttributes as { [key: string]: JSONSchema6 },
+  },
 };
 
 process.stdout.write(

src/build/generate-contributions.ts

@@ -176,6 +176,8 @@ const strings = {
     'Whether a loading prompt should be shown if breakpoint prediction takes a while.',
   'configuration.npmScriptLensLocation':
     'Where a "Run" and "Debug" code lens should be shown in your npm scripts. It may be on "all", scripts, on "top" of the script section, or "never".',
+  'configuration.terminalOptions':
+    'Default launch options for the JavaScript debug terminal and npm scripts.',
 };
 
 export default strings;

src/build/strings.ts

@@ -2,7 +2,8 @@
  * Copyright (C) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------*/
 
-import { WorkspaceConfiguration } from 'vscode';
+import { Command, WorkspaceConfiguration, WorkspaceFolder, commands } from 'vscode';
+import { ITerminalLaunchConfiguration } from '../configuration';
 
 export const enum Contributions {
   PrettyPrintCommand = 'extension.NAMESPACE(node-debug).prettyPrint',
@@ -27,6 +28,7 @@ export const enum Contributions {
 export const enum Configuration {
   NpmScriptLens = 'debug.javascript.codelens.npmScripts',
   WarnOnLongPrediction = 'debug.javascript.warnOnLongPrediction',
+  TerminalDebugConfig = 'debug.javascript.terminalOptions',
 }
 
 /**
@@ -35,8 +37,44 @@ export const enum Configuration {
 export interface IConfigurationTypes {
   [Configuration.NpmScriptLens]: 'all' | 'top' | 'never';
   [Configuration.WarnOnLongPrediction]: boolean;
+  [Configuration.TerminalDebugConfig]: Partial<ITerminalLaunchConfiguration>;
 }
 
+export interface ICommandTypes {
+  [Contributions.DebugNpmScript]: { args: [WorkspaceFolder?]; out: void };
+  [Contributions.PickProcessCommand]: { args: []; out: string | null };
+  [Contributions.AttachProcessCommand]: { args: []; out: void };
+  [Contributions.CreateDebuggerTerminal]: { args: [string?, WorkspaceFolder?]; out: void };
+}
+
+/**
+ * Typed guard for registering a command.
+ */
+export const registerCommand = <K extends keyof ICommandTypes>(
+  ns: typeof commands,
+  key: K,
+  fn: (...args: ICommandTypes[K]['args']) => Promise<ICommandTypes[K]['out']>,
+) => ns.registerCommand(key, fn);
+
+/**
+ * Typed guard for running a command.
+ */
+export const runCommand = async <K extends keyof ICommandTypes>(
+  ns: typeof commands,
+  key: K,
+  ...args: ICommandTypes[K]['args']
+): Promise<ICommandTypes[K]['out']> => await ns.executeCommand(key, ...args);
+
+/**
+ * Typed guard for creating a {@link Command} interface.
+ */
+export const asCommand = <K extends keyof ICommandTypes>(command: {
+  title: string;
+  command: K;
+  tooltip?: string;
+  arguments: ICommandTypes[K]['args'];
+}): Command => command;
+
 /**
  * Typed guard for reading a contributed config.
  */

src/common/contributionUtils.ts

@@ -187,6 +187,7 @@ export const assert = <T>(
 ): assertion is T => {
   if (assertion === false || assertion === undefined || assertion === null) {
     logger.error(LogTag.RuntimeAssertion, message, { error: new Error('Assertion failed') });
+    debugger; // break when running in development
     return false;
   }
 

src/common/logging/logger.ts

@@ -8,6 +8,14 @@ export const removeNulls = <V>(obj: { [key: string]: V | null }) =>
 export const removeUndefined = <V>(obj: { [key: string]: V | undefined }) =>
   filterValues(obj, (v): v is V => v !== undefined);
 
+/**
+ * Asserts that the value is never. If this function is reached, it throws.
+ */
+export const assertNever = (value: never, message: string): never => {
+  debugger;
+  throw new Error(message.replace('{value}', JSON.stringify(value)));
+};
+
 /**
  * Filters the object by value.
  */