fix: various debug fixes

- feat: aded support for `debug/toolbar` and `debug/variables/context`, - feat: added support for `debugState` when context (eclipse-theia#11871), - feat: can customize debug session timeout, and error handling (eclipse-theia#11879), - fix: the `debugType` context that is not updated, - fix: `configure` must happen after receiving capabilities (eclipse-theia#11886), - fix: added missing conext menu in the _Variables_ view, - fix: handle `setFunctionBreakboints` response with no `body` (eclipse-theia#11885), - fix: `DebugExt` fires `didStart` event on `didCreate` (eclipse-theia#11916) Closes eclipse-theia#11871 Closes eclipse-theia#11879 Closes eclipse-theia#11885 Closes eclipse-theia#11886 Closes eclipse-theia#11916 Signed-off-by: Akos Kitta <[email protected]>
kittaakos · Dec 13, 2022 · d7f4e5c · d7f4e5c
1 parent 1446bca
commit d7f4e5c
Show file tree

Hide file tree

Showing 14 changed files with 146 additions and 48 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 
 - [core] returns of many methods of `MenuModelRegistry` changed from `CompositeMenuNode` to `MutableCompoundMenuNode`. To mutate a menu, use the `updateOptions` method or add a check for `instanceof CompositeMenuNode`, which will be true in most cases.
 - [plugin-ext] refactored the plugin RPC API - now also reuses the msgpackR based RPC protocol that is better suited for handling binary data and enables message tunneling [#11228](https://github.com/eclipse-theia/theia/pull/11261). All plugin protocol types now use `UInt8Array` as type for message parameters instead of `string`  - Contributed on behalf of STMicroelectronics.
+- [plugin-ext] `DebugExtImpl#sessionDidCreate` has been replaced with `DebugExtImpl#sessionDidStart` to avoid prematurely firing a `didStart` event on `didCreate` [#11916](https://github.com/eclipse-theia/theia/issues/11916)
 
 ## v1.32.0 - 11/24/2022
 

diff --git a/package.json b/package.json
@@ -113,7 +113,8 @@
     "vscode.typescript-language-features": "https://open-vsx.org/api/vscode/typescript-language-features/1.62.3/file/vscode.typescript-language-features-1.62.3.vsix",
     "EditorConfig.EditorConfig": "https://open-vsx.org/api/EditorConfig/EditorConfig/0.14.4/file/EditorConfig.EditorConfig-0.14.4.vsix",
     "dbaeumer.vscode-eslint": "https://open-vsx.org/api/dbaeumer/vscode-eslint/2.1.1/file/dbaeumer.vscode-eslint-2.1.1.vsix",
-    "ms-vscode.references-view": "https://open-vsx.org/api/ms-vscode/references-view/0.0.89/file/ms-vscode.references-view-0.0.89.vsix"
+    "ms-vscode.references-view": "https://open-vsx.org/api/ms-vscode/references-view/0.0.89/file/ms-vscode.references-view-0.0.89.vsix",
+    "vscode.mock-debug": "https://github.com/kittaakos/vscode-mock-debug/raw/theia/mock-debug-0.51.0.vsix"
   },
   "theiaPluginsExcludeIds": [
     "ms-vscode.js-debug-companion",

diff --git a/packages/debug/src/browser/debug-session-connection.ts b/packages/debug/src/browser/debug-session-connection.ts
@@ -215,7 +215,7 @@ export class DebugSessionConnection implements Disposable {
             };
 
             this.pendingRequests.set(request.seq, result);
-            if (timeout) {
+            if (typeof timeout === 'number') {
                 const handle = setTimeout(() => {
                     const pendingRequest = this.pendingRequests.get(request.seq);
                     if (pendingRequest) {

diff --git a/packages/debug/src/browser/debug-session-manager.ts b/packages/debug/src/browser/debug-session-manager.ts
@@ -27,7 +27,7 @@ import { DebugConfiguration } from '../common/debug-common';
 import { DebugError, DebugService } from '../common/debug-service';
 import { BreakpointManager } from './breakpoint/breakpoint-manager';
 import { DebugConfigurationManager } from './debug-configuration-manager';
-import { DebugSession, DebugState } from './debug-session';
+import { DebugSession, DebugState, debugStateLabel } from './debug-session';
 import { DebugSessionContributionRegistry, DebugSessionFactory } from './debug-session-contribution';
 import { DebugCompoundRoot, DebugCompoundSessionOptions, DebugConfigurationSessionOptions, DebugSessionOptions, InternalDebugSessionOptions } from './debug-session-options';
 import { DebugStackFrame } from './model/debug-stack-frame';
@@ -106,7 +106,9 @@ export class DebugSessionManager {
     protected readonly onDidChangeEmitter = new Emitter<DebugSession | undefined>();
     readonly onDidChange: Event<DebugSession | undefined> = this.onDidChangeEmitter.event;
     protected fireDidChange(current: DebugSession | undefined): void {
+        this.debugTypeKey.set(current?.configuration.type);
         this.inDebugModeKey.set(this.inDebugMode);
+        this.debugStateKey.set(debugStateLabel(this.state));
         this.onDidChangeEmitter.fire(current);
     }
 
@@ -154,11 +156,13 @@ export class DebugSessionManager {
 
     protected debugTypeKey: ContextKey<string>;
     protected inDebugModeKey: ContextKey<boolean>;
+    protected debugStateKey: ContextKey<string>;
 
     @postConstruct()
     protected init(): void {
         this.debugTypeKey = this.contextKeyService.createKey<string>('debugType', undefined);
         this.inDebugModeKey = this.contextKeyService.createKey<boolean>('inDebugMode', this.inDebugMode);
+        this.debugStateKey = this.contextKeyService.createKey<string>('debugState', debugStateLabel(this.state));
         this.breakpoints.onDidChangeMarkers(uri => this.fireDidChangeBreakpoints({ uri }));
         this.labelProvider.onDidChange(event => {
             for (const uriString of this.breakpoints.getUris()) {

diff --git a/packages/debug/src/browser/debug-session.tsx b/packages/debug/src/browser/debug-session.tsx
@@ -43,13 +43,22 @@ import { DebugContribution } from './debug-contribution';
 import { Deferred, waitForEvent } from '@theia/core/lib/common/promise-util';
 import { WorkspaceService } from '@theia/workspace/lib/browser';
 import { DebugInstructionBreakpoint } from './model/debug-instruction-breakpoint';
+import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
 
 export enum DebugState {
     Inactive,
     Initializing,
     Running,
     Stopped
 }
+export function debugStateLabel(state: DebugState): string {
+    switch (state) {
+        case DebugState.Initializing: return 'initializing';
+        case DebugState.Stopped: return 'stopped';
+        case DebugState.Running: return 'running';
+        default: return 'inactive';
+    }
+}
 
 // FIXME: make injectable to allow easily inject services
 export class DebugSession implements CompositeTreeElement {
@@ -74,7 +83,11 @@ export class DebugSession implements CompositeTreeElement {
     protected readonly childSessions = new Map<string, DebugSession>();
     protected readonly toDispose = new DisposableCollection();
 
-    private isStopping: boolean = false;
+    protected isStopping: boolean = false;
+    /**
+     * Number of millis after a `stop` request times out.
+     */
+    protected stopTimeout = 5_000;
 
     constructor(
         readonly id: string,
@@ -274,19 +287,26 @@ export class DebugSession implements CompositeTreeElement {
     }
 
     protected async initialize(): Promise<void> {
-        const response = await this.connection.sendRequest('initialize', {
-            clientID: 'Theia',
-            clientName: 'Theia IDE',
-            adapterID: this.configuration.type,
-            locale: 'en-US',
-            linesStartAt1: true,
-            columnsStartAt1: true,
-            pathFormat: 'path',
-            supportsVariableType: false,
-            supportsVariablePaging: false,
-            supportsRunInTerminalRequest: true
-        });
-        this.updateCapabilities(response?.body || {});
+        const clientName = FrontendApplicationConfigProvider.get().applicationName;
+        try {
+            const response = await this.connection.sendRequest('initialize', {
+                clientID: clientName.toLocaleLowerCase().replace(/\s+/g, '_'),
+                clientName,
+                adapterID: this.configuration.type,
+                locale: 'en-US',
+                linesStartAt1: true,
+                columnsStartAt1: true,
+                pathFormat: 'path',
+                supportsVariableType: false,
+                supportsVariablePaging: false,
+                supportsRunInTerminalRequest: true
+            });
+            this.updateCapabilities(response?.body || {});
+            this.didReceiveCapabilities.resolve();
+        } catch (err) {
+            this.didReceiveCapabilities.reject(err);
+            throw err;
+        }
     }
 
     protected async launchOrAttach(): Promise<void> {
@@ -304,8 +324,17 @@ export class DebugSession implements CompositeTreeElement {
         }
     }
 
+    /**
+     * The `send('initialize')` request could resolve later than `on('initialized')` emits the event.
+     * Hence, the `configure` would use the empty object `capabilities`.
+     * Using the empty `capabilities` could result in missing exception breakpoint filters, as
+     * always `capabilities.exceptionBreakpointFilters` is falsy. This deferred promise works
+     * around this timing issue. https://github.com/eclipse-theia/theia/issues/11886
+     */
+    protected didReceiveCapabilities = new Deferred<void>();
     protected initialized = false;
     protected async configure(): Promise<void> {
+        await this.didReceiveCapabilities.promise;
         if (this.capabilities.exceptionBreakpointFilters) {
             const exceptionBreakpoints = [];
             for (const filter of this.capabilities.exceptionBreakpointFilters) {
@@ -340,24 +369,40 @@ export class DebugSession implements CompositeTreeElement {
         if (!this.isStopping) {
             this.isStopping = true;
             if (this.canTerminate()) {
-                const terminated = this.waitFor('terminated', 5000);
                 try {
-                    await this.connection.sendRequest('terminate', { restart: isRestart }, 5000);
-                    await terminated;
+                    await Promise.all([
+                        this.connection.sendRequest('terminate', { restart: isRestart }, this.stopTimeout),
+                        this.waitFor('terminated', this.stopTimeout)
+                    ]);
                 } catch (e) {
-                    console.error('Did not receive terminated event in time', e);
+                    this.handleTerminateError(e);
                 }
             } else {
+                const terminateDebuggee = this.initialized && this.capabilities.supportTerminateDebuggee;
                 try {
-                    await this.sendRequest('disconnect', { restart: isRestart }, 5000);
+                    await this.sendRequest('disconnect', { restart: isRestart, terminateDebuggee }, this.stopTimeout);
                 } catch (e) {
-                    console.error('Error on disconnect', e);
+                    this.handleDisconnectError(e);
                 }
             }
             callback();
         }
     }
 
+    /**
+     * Invoked when sending the `terminate` request to the debugger is rejected or timed out.
+     */
+    protected handleTerminateError(err: unknown): void {
+        console.error('Did not receive terminated event in time', err);
+    }
+
+    /**
+     * Invoked when sending the `disconnect` request to the debugger is rejected or timed out.
+     */
+    protected handleDisconnectError(err: unknown): void {
+        console.error('Error on disconnect', err);
+    }
+
     async disconnect(isRestart: boolean, callback: () => void): Promise<void> {
         if (!this.isStopping) {
             this.isStopping = true;
@@ -665,12 +710,17 @@ export class DebugSession implements CompositeTreeElement {
                 const response = await this.sendRequest('setFunctionBreakpoints', {
                     breakpoints: enabled.map(b => b.origin.raw)
                 });
-                response.body.breakpoints.forEach((raw, index) => {
-                    // node debug adapter returns more breakpoints sometimes
-                    if (enabled[index]) {
-                        enabled[index].update({ raw });
-                    }
-                });
+                // Apparently, `body` and `breakpoints` can be missing.
+                // https://github.com/eclipse-theia/theia/issues/11885
+                // https://github.com/microsoft/vscode/blob/80004351ccf0884b58359f7c8c801c91bb827d83/src/vs/workbench/contrib/debug/browser/debugSession.ts#L448-L449
+                if (response && response.body) {
+                    response.body.breakpoints.forEach((raw, index) => {
+                        // node debug adapter returns more breakpoints sometimes
+                        if (enabled[index]) {
+                            enabled[index].update({ raw });
+                        }
+                    });
+                }
             } catch (error) {
                 // could be error or promise rejection of DebugProtocol.SetFunctionBreakpoints
                 if (error instanceof Error) {
@@ -699,10 +749,12 @@ export class DebugSession implements CompositeTreeElement {
         );
         const enabled = all.filter(b => b.enabled);
         try {
+            const breakpoints = enabled.map(({ origin }) => origin.raw);
             const response = await this.sendRequest('setBreakpoints', {
                 source: source.raw,
                 sourceModified,
-                breakpoints: enabled.map(({ origin }) => origin.raw)
+                breakpoints,
+                lines: breakpoints.map(({ line }) => line)
             });
             response.body.breakpoints.forEach((raw, index) => {
                 // node debug adapter returns more breakpoints sometimes

diff --git a/packages/debug/src/browser/model/debug-stack-frame.tsx b/packages/debug/src/browser/model/debug-stack-frame.tsx
@@ -22,12 +22,11 @@
 
 import * as React from '@theia/core/shared/react';
 import { WidgetOpenerOptions, DISABLED_CLASS } from '@theia/core/lib/browser';
-import { EditorWidget, Range, Position } from '@theia/editor/lib/browser';
+import { EditorWidget, Range } from '@theia/editor/lib/browser';
 import { DebugProtocol } from '@vscode/debugprotocol/lib/debugProtocol';
 import { TreeElement } from '@theia/core/lib/browser/source-tree';
 import { DebugScope } from '../console/debug-console-items';
 import { DebugSource } from './debug-source';
-import { RecursivePartial } from '@theia/core';
 import { DebugSession } from '../debug-session';
 import { DebugThread } from './debug-thread';
 import * as monaco from '@theia/monaco-editor-core';
@@ -70,16 +69,11 @@ export class DebugStackFrame extends DebugStackFrameData implements TreeElement
         if (!this.source) {
             return undefined;
         }
-        const { line, column, endLine, endColumn } = this.raw;
-        const selection: RecursivePartial<Range> = {
-            start: Position.create(line - 1, column - 1)
-        };
-        if (typeof endLine === 'number') {
-            selection.end = {
-                line: endLine - 1,
-                character: typeof endColumn === 'number' ? endColumn - 1 : undefined
-            };
+        const { line, column, endLine, endColumn, source } = this.raw;
+        if (!source) {
+            return undefined;
         }
+        const selection = Range.create(line, column, endLine || line, endColumn || column);
         this.source.open({
             ...options,
             selection

diff --git a/packages/debug/src/browser/style/index.css b/packages/debug/src/browser/style/index.css
@@ -146,6 +146,16 @@
     opacity: 1;
 }
 
+.debug-toolbar .debug-action>div {
+    font-family: var(--theia-ui-font-family);
+    font-size: var(--theia-ui-font-size0);
+    display: flex;
+    align-items: center;
+    align-self: center;
+    justify-content: center;
+    min-height: inherit;
+}
+
 /** Console */
 
 #debug-console .theia-console-info {

diff --git a/packages/debug/src/browser/view/debug-action.tsx b/packages/debug/src/browser/view/debug-action.tsx
@@ -29,7 +29,9 @@ export class DebugAction extends React.Component<DebugAction.Props> {
             className={classNames.join(' ')}
             title={label}
             onClick={this.props.run}
-            ref={this.setRef} />;
+            ref={this.setRef} >
+            {!iconClass || iconClass.match(/plugin-icon-\d+/) && <div>{label}</div>}
+        </span>;
     }
 
     focus(): void {

diff --git a/packages/debug/src/browser/view/debug-toolbar-widget.tsx b/packages/debug/src/browser/view/debug-toolbar-widget.tsx
@@ -16,7 +16,8 @@
 
 import * as React from '@theia/core/shared/react';
 import { inject, postConstruct, injectable } from '@theia/core/shared/inversify';
-import { Disposable, DisposableCollection, MenuPath } from '@theia/core';
+import { ActionMenuNode, CommandRegistry, CompositeMenuNode, Disposable, DisposableCollection, MenuModelRegistry, MenuPath } from '@theia/core';
+import { ContextKeyService } from '@theia/core/lib/browser/context-key-service';
 import { ReactWidget } from '@theia/core/lib/browser/widgets';
 import { DebugViewModel } from './debug-view-model';
 import { DebugState } from '../debug-session';
@@ -28,8 +29,10 @@ export class DebugToolBar extends ReactWidget {
 
     static readonly MENU: MenuPath = ['debug-toolbar-menu'];
 
-    @inject(DebugViewModel)
-    protected readonly model: DebugViewModel;
+    @inject(CommandRegistry) protected readonly commandRegistry: CommandRegistry;
+    @inject(MenuModelRegistry) protected readonly menuModelRegistry: MenuModelRegistry;
+    @inject(ContextKeyService) protected readonly contextKeyService: ContextKeyService;
+    @inject(DebugViewModel) protected readonly model: DebugViewModel;
 
     protected readonly onRender = new DisposableCollection();
 
@@ -65,6 +68,7 @@ export class DebugToolBar extends ReactWidget {
     protected render(): React.ReactNode {
         const { state } = this.model;
         return <React.Fragment>
+            {this.renderContributedCommands()}
             {this.renderContinue()}
             <DebugAction enabled={state === DebugState.Stopped} run={this.stepOver} label={nls.localizeByDefault('Step Over')}
                 iconClass='debug-step-over' ref={this.setStepRef} />
@@ -77,6 +81,29 @@ export class DebugToolBar extends ReactWidget {
             {this.renderStart()}
         </React.Fragment>;
     }
+
+    protected renderContributedCommands(): React.ReactNode {
+        return this.menuModelRegistry
+            .getMenu(DebugToolBar.MENU)
+            .children.filter(node => node instanceof CompositeMenuNode)
+            .map(node => (node as CompositeMenuNode).children)
+            .reduce((acc, curr) => acc.concat(curr), [])
+            .filter(node => node instanceof ActionMenuNode)
+            .map(node => this.debugAction(node as ActionMenuNode));
+    }
+
+    protected debugAction(node: ActionMenuNode): React.ReactNode {
+        const { label, command, when, icon: iconClass = '' } = node;
+        const run = () => this.commandRegistry.executeCommand(command);
+        const enabled = when ? this.contextKeyService.match(when) : true;
+        return enabled && <DebugAction
+            key={command}
+            enabled={enabled}
+            label={label}
+            iconClass={iconClass}
+            run={run} />;
+    }
+
     protected renderStart(): React.ReactNode {
         const { state } = this.model;
         if (state === DebugState.Inactive && this.model.sessionCount === 1) {

diff --git a/packages/plugin-ext/src/common/plugin-api-rpc.ts b/packages/plugin-ext/src/common/plugin-api-rpc.ts
@@ -1799,7 +1799,7 @@ export interface DebugConfigurationProviderDescriptor {
 export interface DebugExt {
     $onSessionCustomEvent(sessionId: string, event: string, body?: any): void;
     $breakpointsDidChange(added: Breakpoint[], removed: string[], changed: Breakpoint[]): void;
-    $sessionDidCreate(sessionId: string): void;
+    $sessionDidStart(sessionId: string): void;
     $sessionDidDestroy(sessionId: string): void;
     $sessionDidChange(sessionId: string | undefined): void;
     $provideDebugConfigurationsByHandle(handle: number, workspaceFolder: string | undefined): Promise<theia.DebugConfiguration[]>;

diff --git a/packages/plugin-ext/src/main/browser/debug/debug-main.ts b/packages/plugin-ext/src/main/browser/debug/debug-main.ts
@@ -113,7 +113,7 @@ export class DebugMainImpl implements DebugMain, Disposable {
         this.toDispose.pushAll([
             this.breakpointsManager.onDidChangeBreakpoints(fireDidChangeBreakpoints),
             this.breakpointsManager.onDidChangeFunctionBreakpoints(fireDidChangeBreakpoints),
-            this.sessionManager.onDidCreateDebugSession(debugSession => this.debugExt.$sessionDidCreate(debugSession.id)),
+            this.sessionManager.onDidStartDebugSession(debugSession => this.debugExt.$sessionDidStart(debugSession.id)),
             this.sessionManager.onDidDestroyDebugSession(debugSession => this.debugExt.$sessionDidDestroy(debugSession.id)),
             this.sessionManager.onDidChangeActiveDebugSession(event => this.debugExt.$sessionDidChange(event.current && event.current.id)),
             this.sessionManager.onDidReceiveDebugSessionCustomEvent(event => this.debugExt.$onSessionCustomEvent(event.session.id, event.event, event.body))

diff --git a/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts b/packages/plugin-ext/src/main/browser/menus/plugin-menu-command-adapter.ts
@@ -89,6 +89,7 @@ export class PluginMenuCommandAdapter implements MenuCommandAdapter {
             ['comments/comment/title', toCommentArgs],
             ['comments/commentThread/context', toCommentArgs],
             ['debug/callstack/context', firstArgOnly],
+            ['debug/variables/context', firstArgOnly],
             ['debug/toolBar', noArgs],
             ['editor/context', selectedResource],
             ['editor/title', widgetURI],