Honor debug stop from configuration providers

Debug configuration providers can indicate to stop debugging by returning `undefined` or `null` on either of the functions - `resolveDebugConfiguration` or - `resolveDebugConfigurationWithSubstitutedVariables` The vscode API https://code.visualstudio.com/api/references/vscode-api#DebugConfigurationProvider specifies different handling for `undefined` and `null` as follows: "Returning the value 'undefined' prevents the debug session from starting. Returning the value 'null' prevents the debug session from starting and opens the underlying debug configuration instead." This implementation focuses on passing the `undefined` or `null` response from the extension to the debug session manager where the indication from the extension is handled. Signed-off-by: Alvaro Sanchez-Leon <alvaro.sanchez-leon@ericsson.com>
eclipse-theia · Apr 6, 2022 · e085336 · e085336
1 parent 1c7bdf7
commit e085336
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 62 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,7 +12,33 @@
 - [plugin] added support for `SnippetString.appendChoice` [#10969](https://github.com/eclipse-theia/theia/pull/10969) - Contributed on behalf of STMicroelectronics
 
 <a name="breaking_changes_1.25.0">[Breaking Changes:](#breaking_changes_1.25.0)</a>
-
+- [debug] 
+The following methods may now return `undefined | null` ([#10998](https://github.com/eclipse-theia/theia/issues/10998)):
+  - DebugSessionManager
+    - resolveConfiguration
+    - resolveDebugConfiguration
+    - resolveDebugConfigurationWithSubstitutedVariables
+  - DebugService
+    - resolveDebugConfiguration
+    - resolveDebugConfigurationWithSubstitutedVariables
+  - theia.d.ts DebugConfigurationProvider
+    - resolveDebugConfiguration
+    - resolveDebugConfigurationWithSubstitutedVariables
+  - plugin-api-rpc.ts DebugConfigurationProvider
+    - resolveDebugConfiguration
+    - resolveDebugConfigurationWithSubstitutedVariables
+  - DebugExt
+    - $resolveDebugConfigurationByHandle
+    - $resolveDebugConfigurationWithSubstitutedVariablesByHandle
+  - DebugExtImpl
+    - $resolveDebugConfigurationByHandle
+    - $resolveDebugConfigurationWithSubstitutedVariablesByHandle
+  - PluginDebugConfigurationProvider
+    - resolveDebugConfiguration
+    - resolveDebugConfigurationWithSubstitutedVariables
+  - PluginDebugService
+    - resolveDebugConfiguration
+    - resolveDebugConfigurationWithSubstitutedVariables
 
 ## v1.24.0 - 3/31/2022
 

diff --git a/packages/debug/src/browser/debug-session-manager.ts b/packages/debug/src/browser/debug-session-manager.ts
@@ -193,6 +193,19 @@ export class DebugSessionManager {
                 await this.fireWillStartDebugSession();
                 const resolved = await this.resolveConfiguration(options);
 
+                if (!resolved) {
+                    // As per vscode API: https://code.visualstudio.com/api/references/vscode-api#DebugConfigurationProvider
+                    // "Returning the value 'undefined' prevents the debug session from starting.
+                    // Returning the value 'null' prevents the debug session from starting and opens the
+                    // underlying debug configuration instead."
+
+                    // eslint-disable-next-line no-null/no-null
+                    if (resolved === null) {
+                        this.debugConfigurationManager.openConfiguration();
+                    }
+                    return undefined;
+                }
+
                 // preLaunchTask isn't run in case of auto restart as well as postDebugTask
                 if (!options.configuration.__restart) {
                     const taskRun = await this.runTask(options.workspaceFolderUri, resolved.configuration.preLaunchTask, true);
@@ -221,17 +234,31 @@ export class DebugSessionManager {
     }
 
     protected configurationIds = new Map<string, number>();
-    protected async resolveConfiguration(options: Readonly<DebugSessionOptions>): Promise<InternalDebugSessionOptions> {
+    protected async resolveConfiguration(
+        options: Readonly<DebugSessionOptions>
+    ): Promise<InternalDebugSessionOptions | undefined | null> {
         if (InternalDebugSessionOptions.is(options)) {
             return options;
         }
         const { workspaceFolderUri } = options;
         let configuration = await this.resolveDebugConfiguration(options.configuration, workspaceFolderUri);
-        configuration = await this.variableResolver.resolve(configuration, {
-            context: options.workspaceFolderUri ? new URI(options.workspaceFolderUri) : undefined,
-            configurationSection: 'launch'
-        });
-        configuration = await this.resolveDebugConfigurationWithSubstitutedVariables(configuration, workspaceFolderUri);
+
+        if (configuration) {
+            configuration = await this.variableResolver.resolve(configuration, {
+                context: options.workspaceFolderUri ? new URI(options.workspaceFolderUri) : undefined,
+                configurationSection: 'launch',
+            });
+
+            configuration = await this.resolveDebugConfigurationWithSubstitutedVariables(
+                configuration,
+                workspaceFolderUri
+            );
+        }
+
+        if (!configuration) {
+            return configuration;
+        }
+
         const key = configuration.name + workspaceFolderUri;
         const id = this.configurationIds.has(key) ? this.configurationIds.get(key)! + 1 : 0;
         this.configurationIds.set(key, id);
@@ -242,15 +269,22 @@ export class DebugSessionManager {
         };
     }
 
-    protected async resolveDebugConfiguration(configuration: DebugConfiguration, workspaceFolderUri: string | undefined): Promise<DebugConfiguration> {
+    protected async resolveDebugConfiguration(
+        configuration: DebugConfiguration,
+        workspaceFolderUri: string | undefined
+    ): Promise<DebugConfiguration | undefined | null> {
         await this.fireWillResolveDebugConfiguration(configuration.type);
         return this.debug.resolveDebugConfiguration(configuration, workspaceFolderUri);
     }
+
     protected async fireWillResolveDebugConfiguration(debugType: string): Promise<void> {
         await WaitUntilEvent.fire(this.onWillResolveDebugConfigurationEmitter, { debugType });
     }
 
-    protected async resolveDebugConfigurationWithSubstitutedVariables(configuration: DebugConfiguration, workspaceFolderUri: string | undefined): Promise<DebugConfiguration> {
+    protected async resolveDebugConfigurationWithSubstitutedVariables(
+        configuration: DebugConfiguration,
+        workspaceFolderUri: string | undefined
+    ): Promise<DebugConfiguration | undefined | null> {
         return this.debug.resolveDebugConfigurationWithSubstitutedVariables(configuration, workspaceFolderUri);
     }
 

diff --git a/packages/debug/src/common/debug-service.ts b/packages/debug/src/common/debug-service.ts
@@ -79,17 +79,23 @@ export interface DebugService extends Disposable {
      * Resolves a [debug configuration](#DebugConfiguration) by filling in missing values
      * or by adding/changing/removing attributes before variable substitution.
      * @param debugConfiguration The [debug configuration](#DebugConfiguration) to resolve.
-     * @returns The resolved debug configuration.
+     * @returns The resolved debug configuration, undefined or null.
      */
-    resolveDebugConfiguration(config: DebugConfiguration, workspaceFolderUri: string | undefined): Promise<DebugConfiguration>;
+    resolveDebugConfiguration(
+        config: DebugConfiguration,
+        workspaceFolderUri: string | undefined
+    ): Promise<DebugConfiguration | undefined | null>;
 
     /**
      * Resolves a [debug configuration](#DebugConfiguration) by filling in missing values
      * or by adding/changing/removing attributes with substituted variables.
      * @param debugConfiguration The [debug configuration](#DebugConfiguration) to resolve.
-     * @returns The resolved debug configuration.
+     * @returns The resolved debug configuration, undefined or null.
      */
-    resolveDebugConfigurationWithSubstitutedVariables(config: DebugConfiguration, workspaceFolderUri: string | undefined): Promise<DebugConfiguration>;
+    resolveDebugConfigurationWithSubstitutedVariables(
+        config: DebugConfiguration,
+        workspaceFolderUri: string | undefined
+    ): Promise<DebugConfiguration | undefined | null>;
 
     /**
      * Creates a new [debug adapter session](#DebugAdapterSession).

diff --git a/packages/plugin-ext/src/common/plugin-api-rpc.ts b/packages/plugin-ext/src/common/plugin-api-rpc.ts
@@ -1656,8 +1656,14 @@ export interface DebugConfigurationProvider {
     readonly type: string;
     readonly triggerKind: DebugConfigurationProviderTriggerKind;
     provideDebugConfigurations?(folder: string | undefined): Promise<theia.DebugConfiguration[]>;
-    resolveDebugConfiguration?(folder: string | undefined, debugConfiguration: theia.DebugConfiguration): Promise<theia.DebugConfiguration | undefined>;
-    resolveDebugConfigurationWithSubstitutedVariables?(folder: string | undefined, debugConfiguration: theia.DebugConfiguration): Promise<theia.DebugConfiguration | undefined>;
+    resolveDebugConfiguration?(
+        folder: string | undefined,
+        debugConfiguration: theia.DebugConfiguration
+    ): Promise<theia.DebugConfiguration | undefined | null>;
+    resolveDebugConfigurationWithSubstitutedVariables?(
+        folder: string | undefined,
+        debugConfiguration: theia.DebugConfiguration
+    ): Promise<theia.DebugConfiguration | undefined | null>;
 }
 
 export interface DebugConfigurationProviderDescriptor {
@@ -1692,8 +1698,16 @@ export interface DebugExt {
         Promise<theia.DebugConfiguration | undefined>;
 
     $provideDebugConfigurationsByHandle(handle: number, workspaceFolder: string | undefined): Promise<theia.DebugConfiguration[]>;
-    $resolveDebugConfigurationByHandle(handle: number, workspaceFolder: string | undefined, debugConfiguration: theia.DebugConfiguration): Promise<theia.DebugConfiguration | undefined>;
-    $resolveDebugConfigurationWithSubstitutedVariablesByHandle(handle: number, workspaceFolder: string | undefined, debugConfiguration: theia.DebugConfiguration): Promise<theia.DebugConfiguration | undefined>;
+    $resolveDebugConfigurationByHandle(
+        handle: number,
+        workspaceFolder: string | undefined,
+        debugConfiguration: theia.DebugConfiguration
+    ): Promise<theia.DebugConfiguration | undefined | null>;
+    $resolveDebugConfigurationWithSubstitutedVariablesByHandle(
+        handle: number,
+        workspaceFolder: string | undefined,
+        debugConfiguration: theia.DebugConfiguration
+    ): Promise<theia.DebugConfiguration | undefined | null>;
 
     $createDebugSession(debugConfiguration: theia.DebugConfiguration): Promise<string>;
     $terminateDebugSession(sessionId: string): Promise<void>;

diff --git a/packages/plugin-ext/src/main/browser/debug/plugin-debug-configuration-provider.ts b/packages/plugin-ext/src/main/browser/debug/plugin-debug-configuration-provider.ts
@@ -27,8 +27,14 @@ export class PluginDebugConfigurationProvider implements DebugConfigurationProvi
     public type: string;
     public triggerKind: DebugConfigurationProviderTriggerKind;
     provideDebugConfigurations: (folder: string | undefined) => Promise<DebugConfiguration[]>;
-    resolveDebugConfiguration: (folder: string | undefined, debugConfiguration: DebugConfiguration) => Promise<DebugConfiguration | undefined>;
-    resolveDebugConfigurationWithSubstitutedVariables: (folder: string | undefined, debugConfiguration: DebugConfiguration) => Promise<DebugConfiguration | undefined>;
+    resolveDebugConfiguration: (
+        folder: string | undefined,
+        debugConfiguration: DebugConfiguration
+    ) => Promise<DebugConfiguration | undefined | null>;
+    resolveDebugConfigurationWithSubstitutedVariables: (
+        folder: string | undefined,
+        debugConfiguration: DebugConfiguration
+    ) => Promise<DebugConfiguration | undefined | null>;
 
     constructor(
         description: DebugConfigurationProviderDescriptor,

diff --git a/packages/plugin-ext/src/main/browser/debug/plugin-debug-service.ts b/packages/plugin-ext/src/main/browser/debug/plugin-debug-service.ts
@@ -147,52 +147,74 @@ export class PluginDebugService implements DebugService {
         return configurationsRecord;
     }
 
-    async resolveDebugConfiguration(config: DebugConfiguration, workspaceFolderUri: string | undefined): Promise<DebugConfiguration> {
-        let resolved = config;
-
+    async resolveDebugConfiguration(
+        config: DebugConfiguration,
+        workspaceFolderUri: string | undefined
+    ): Promise<DebugConfiguration | undefined | null> {
         const allProviders = Array.from(this.configurationProviders.values());
+
+        const resolvers = allProviders
+            .filter(p => p.type === config.type && !!p.resolveDebugConfiguration)
+            .map(p => p.resolveDebugConfiguration);
+
         // Append debug type '*' at the end
-        const pluginProviders = allProviders.filter(p => p.type === config.type && !!p.resolveDebugConfiguration);
-        pluginProviders.push(...allProviders.filter(p => p.type === '*' && !!p.resolveDebugConfiguration));
+        resolvers.push(
+            ...allProviders
+                .filter(p => p.type === '*' && !!p.resolveDebugConfiguration)
+                .map(p => p.resolveDebugConfiguration)
+        );
 
-        for (const provider of pluginProviders) {
-            try {
-                const next = await provider.resolveDebugConfiguration(workspaceFolderUri, resolved);
-                if (next) {
-                    resolved = next;
-                } else {
-                    return resolved;
-                }
-            } catch (e) {
-                console.error(e);
-            }
-        }
+        const resolved = await this.resolveDebugConfigurationByResolversChain(config, workspaceFolderUri, resolvers);
 
-        return this.delegated.resolveDebugConfiguration(resolved, workspaceFolderUri);
+        return resolved ? this.delegated.resolveDebugConfiguration(resolved, workspaceFolderUri) : resolved;
     }
 
-    async resolveDebugConfigurationWithSubstitutedVariables(config: DebugConfiguration, workspaceFolderUri: string | undefined): Promise<DebugConfiguration> {
-        let resolved = config;
-
+    async resolveDebugConfigurationWithSubstitutedVariables(
+        config: DebugConfiguration,
+        workspaceFolderUri: string | undefined
+    ): Promise<DebugConfiguration | undefined | null> {
         const allProviders = Array.from(this.configurationProviders.values());
+
+        const resolvers = allProviders
+            .filter(p => p.type === config.type && !!p.resolveDebugConfigurationWithSubstitutedVariables)
+            .map(p => p.resolveDebugConfigurationWithSubstitutedVariables);
+
         // Append debug type '*' at the end
-        const pluginProviders = allProviders.filter(p => p.type === config.type && !!p.resolveDebugConfigurationWithSubstitutedVariables);
-        pluginProviders.push(...allProviders.filter(p => p.type === '*' && !!p.resolveDebugConfigurationWithSubstitutedVariables));
+        resolvers.push(
+            ...allProviders
+                .filter(p => p.type === '*' && !!p.resolveDebugConfigurationWithSubstitutedVariables)
+                .map(p => p.resolveDebugConfigurationWithSubstitutedVariables)
+        );
 
-        for (const provider of pluginProviders) {
+        const resolved = await this.resolveDebugConfigurationByResolversChain(config, workspaceFolderUri, resolvers);
+
+        return resolved
+            ? this.delegated.resolveDebugConfigurationWithSubstitutedVariables(resolved, workspaceFolderUri)
+            : resolved;
+    }
+
+    protected async resolveDebugConfigurationByResolversChain(
+        config: DebugConfiguration,
+        workspaceFolderUri: string | undefined,
+        resolvers: ((
+            folder: string | undefined,
+            debugConfiguration: DebugConfiguration
+        ) => Promise<DebugConfiguration | null | undefined>)[]
+    ): Promise<DebugConfiguration | undefined | null> {
+        let resolved: DebugConfiguration | undefined | null = config;
+        for (const resolver of resolvers) {
             try {
-                const next = await provider.resolveDebugConfigurationWithSubstitutedVariables(workspaceFolderUri, resolved);
-                if (next) {
-                    resolved = next;
-                } else {
-                    return resolved;
+                if (!resolved) {
+                    // A provider has indicated to stop and process undefined or null as per specified in the vscode API
+                    // https://code.visualstudio.com/api/references/vscode-api#DebugConfigurationProvider
+                    break;
                 }
+                resolved = await resolver(workspaceFolderUri, resolved);
             } catch (e) {
                 console.error(e);
             }
         }
-
-        return this.delegated.resolveDebugConfigurationWithSubstitutedVariables(resolved, workspaceFolderUri);
+        return resolved;
     }
 
     registerDebugger(contribution: DebuggerContribution): Disposable {

diff --git a/packages/plugin-ext/src/plugin/node/debug/debug.ts b/packages/plugin-ext/src/plugin/node/debug/debug.ts
@@ -346,12 +346,20 @@ export class DebugExtImpl implements DebugExt {
         return { provider, type };
     }
 
-    async $provideDebugConfigurationsByHandle(handle: number, workspaceFolderUri: string | undefined): Promise<theia.DebugConfiguration[]> {
+    async $provideDebugConfigurationsByHandle(
+        handle: number,
+        workspaceFolderUri: string | undefined
+    ): Promise<theia.DebugConfiguration[]> {
         const { provider, type } = this.getConfigurationProviderRecord(handle);
 
-        const configurations = await provider.provideDebugConfigurations?.(this.toWorkspaceFolder(workspaceFolderUri));
+        const configurations = await provider.provideDebugConfigurations?.(
+            this.toWorkspaceFolder(workspaceFolderUri)
+        );
+
         if (!configurations) {
-            throw new Error('nothing returned from DebugConfigurationProvider.provideDebugConfigurations, type: ' + type);
+            throw new Error(
+                'nothing returned from DebugConfigurationProvider.provideDebugConfigurations, type: ' + type
+            );
         }
 
         return configurations;
@@ -361,20 +369,24 @@ export class DebugExtImpl implements DebugExt {
         handle: number,
         workspaceFolderUri: string | undefined,
         debugConfiguration: theia.DebugConfiguration
-    ): Promise<theia.DebugConfiguration | undefined> {
-
+    ): Promise<theia.DebugConfiguration | undefined | null> {
         const { provider } = this.getConfigurationProviderRecord(handle);
-        return provider.resolveDebugConfiguration?.(this.toWorkspaceFolder(workspaceFolderUri), debugConfiguration);
+        return provider.resolveDebugConfiguration?.(
+            this.toWorkspaceFolder(workspaceFolderUri),
+            debugConfiguration
+        );
     }
 
     async $resolveDebugConfigurationWithSubstitutedVariablesByHandle(
         handle: number,
         workspaceFolderUri: string | undefined,
         debugConfiguration: theia.DebugConfiguration
-    ): Promise<theia.DebugConfiguration | undefined> {
-
+    ): Promise<theia.DebugConfiguration | undefined | null> {
         const { provider } = this.getConfigurationProviderRecord(handle);
-        return provider.resolveDebugConfigurationWithSubstitutedVariables?.(this.toWorkspaceFolder(workspaceFolderUri), debugConfiguration);
+        return provider.resolveDebugConfigurationWithSubstitutedVariables?.(
+            this.toWorkspaceFolder(workspaceFolderUri),
+            debugConfiguration
+        );
     }
 
     async $provideDebugConfigurations(debugType: string, workspaceFolderUri: string | undefined, dynamic: boolean = false): Promise<theia.DebugConfiguration[]> {
@@ -394,6 +406,9 @@ export class DebugExtImpl implements DebugExt {
         return result;
     }
 
+    /**
+     * @deprecated since 1.25.0. Use $registerDebugConfigurationProvider with $resolveDebugConfigurationByHandle instead.
+     */
     async $resolveDebugConfigurations(debugConfiguration: theia.DebugConfiguration, workspaceFolderUri: string | undefined): Promise<theia.DebugConfiguration | undefined> {
         let current = debugConfiguration;
 
@@ -419,6 +434,9 @@ export class DebugExtImpl implements DebugExt {
         return current;
     }
 
+    /**
+     * @deprecated since 1.25.0. Use $registerDebugConfigurationProvider with $resolveDebugConfigurationWithSubstitutedVariablesByHandle instead.
+     */
     async $resolveDebugConfigurationWithSubstitutedVariables(debugConfiguration: theia.DebugConfiguration, workspaceFolderUri: string | undefined):
         Promise<theia.DebugConfiguration | undefined> {
         let current = debugConfiguration;