Ensure debug adapter request/response sequence numbers tally

src/client/datascience/debugLocationTracker.ts

@@ -1,19 +1,23 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 'use strict';
+import { injectable } from 'inversify';
 import { DebugAdapterTracker, Event, EventEmitter } from 'vscode';
 import { DebugProtocol } from 'vscode-debugprotocol';
 
 import { IDebugLocation } from './types';
 
 // When a python debugging session is active keep track of the current debug location
+@injectable()
 export class DebugLocationTracker implements DebugAdapterTracker {
-    private waitingForStackTrace: boolean = false;
+    protected topMostFrameId = 0;
+    protected sequenceNumbersOfRequestsPendingResponses = new Set<number>();
+    private waitingForStackTrace = false;
     private _debugLocation: IDebugLocation | undefined;
     private debugLocationUpdatedEvent: EventEmitter<void> = new EventEmitter<void>();
     private sessionEndedEmitter: EventEmitter<DebugLocationTracker> = new EventEmitter<DebugLocationTracker>();
 
-    constructor(private _sessionId: string) {
+    constructor(private _sessionId: string | undefined) {
         this.DebugLocation = undefined;
     }
 
@@ -33,8 +37,22 @@ export class DebugLocationTracker implements DebugAdapterTracker {
         return this._debugLocation;
     }
 
-    // tslint:disable-next-line:no-any
-    public onDidSendMessage(message: DebugProtocol.ProtocolMessage) {
+    public onDidSendMessage(message: DebugProtocol.Response) {
+        if (this.isResponseForRequestToFetchAllFrames(message)) {
+            // This should be the top frame. We need to use this to compute the value of a variable
+            const topMostFrame = message.body.stackFrames[0];
+            this.topMostFrameId = topMostFrame?.id;
+            this.sequenceNumbersOfRequestsPendingResponses.delete(message.request_seq);
+            // If we are waiting for a stack trace, check our messages for one
+            if (this.waitingForStackTrace) {
+                this.DebugLocation = {
+                    lineNumber: topMostFrame?.line,
+                    fileName: this.normalizeFilePath(topMostFrame?.source?.path),
+                    column: topMostFrame.column
+                };
+                this.waitingForStackTrace = false;
+            }
+        }
         if (this.isStopEvent(message)) {
             // Some type of stop, wait to see our next stack trace to find our location
             this.waitingForStackTrace = true;
@@ -45,21 +63,23 @@ export class DebugLocationTracker implements DebugAdapterTracker {
             this.DebugLocation = undefined;
             this.waitingForStackTrace = false;
         }
-
-        if (this.waitingForStackTrace) {
-            // If we are waiting for a stack track, check our messages for one
-            const debugLoc = this.getStackTrace(message);
-            if (debugLoc) {
-                this.DebugLocation = debugLoc;
-                this.waitingForStackTrace = false;
-            }
-        }
     }
 
     public onWillStopSession() {
         this.sessionEndedEmitter.fire(this);
     }
 
+    public onWillReceiveMessage(message: DebugProtocol.Request) {
+        if (this.isRequestToFetchAllFrames(message)) {
+            // VSCode sometimes sends multiple stackTrace requests. The true topmost frame is determined
+            // based on the response to a stackTrace request where the startFrame is 0 or undefined (i.e.
+            // this request retrieves all frames). Here, remember the sequence number of the outgoing
+            // request whose startFrame === 0 or undefined, and update this.topMostFrameId only when we
+            // receive the response with a matching sequence number.
+            this.sequenceNumbersOfRequestsPendingResponses.add(message.seq);
+        }
+    }
+
     // Set our new location and fire our debug event
     private set DebugLocation(newLocation: IDebugLocation | undefined) {
         const oldLocation = this._debugLocation;
@@ -70,7 +90,15 @@ export class DebugLocationTracker implements DebugAdapterTracker {
         }
     }
 
-    // tslint:disable-next-line:no-any
+    private normalizeFilePath(path: string): string {
+        // Make the path match the os. Debugger seems to return
+        // invalid path chars on linux/darwin
+        if (process.platform !== 'win32') {
+            return path.replace(/\\/g, '/');
+        }
+        return path;
+    }
+
     private isStopEvent(message: DebugProtocol.ProtocolMessage) {
         if (message.type === 'event') {
             const eventMessage = message as DebugProtocol.Event;
@@ -82,34 +110,6 @@ export class DebugLocationTracker implements DebugAdapterTracker {
         return false;
     }
 
-    // tslint:disable-next-line:no-any
-    private getStackTrace(message: DebugProtocol.ProtocolMessage): IDebugLocation | undefined {
-        if (message.type === 'response') {
-            const responseMessage = message as DebugProtocol.Response;
-            if (responseMessage.command === 'stackTrace') {
-                const messageBody = responseMessage.body;
-                if (messageBody.stackFrames.length > 0) {
-                    const lineNumber = messageBody.stackFrames[0].line;
-                    const fileName = this.normalizeFilePath(messageBody.stackFrames[0].source.path);
-                    const column = messageBody.stackFrames[0].column;
-                    return { lineNumber, fileName, column };
-                }
-            }
-        }
-
-        return undefined;
-    }
-
-    private normalizeFilePath(path: string): string {
-        // Make the path match the os. Debugger seems to return
-        // invalid path chars on linux/darwin
-        if (process.platform !== 'win32') {
-            return path.replace(/\\/g, '/');
-        }
-        return path;
-    }
-
-    // tslint:disable-next-line:no-any
     private isContinueEvent(message: DebugProtocol.ProtocolMessage): boolean {
         if (message.type === 'event') {
             const eventMessage = message as DebugProtocol.Event;
@@ -125,4 +125,21 @@ export class DebugLocationTracker implements DebugAdapterTracker {
 
         return false;
     }
+
+    private isResponseForRequestToFetchAllFrames(message: DebugProtocol.Response) {
+        return (
+            message.type === 'response' &&
+            message.command === 'stackTrace' &&
+            message.body.stackFrames[0] &&
+            this.sequenceNumbersOfRequestsPendingResponses.has(message.request_seq)
+        );
+    }
+
+    private isRequestToFetchAllFrames(message: DebugProtocol.Request) {
+        return (
+            message.type === 'request' &&
+            message.command === 'stackTrace' &&
+            (message.arguments.startFrame === 0 || message.arguments.startFrame === undefined)
+        );
+    }
 }

src/client/datascience/debugLocationTrackerFactory.ts

@@ -50,7 +50,9 @@ export class DebugLocationTrackerFactory implements IDebugLocationTracker, Debug
     }
 
     private onSessionEnd(locationTracker: DebugLocationTracker) {
-        this.activeTrackers.delete(locationTracker.sessionId);
+        if (locationTracker.sessionId) {
+            this.activeTrackers.delete(locationTracker.sessionId);
+        }
     }
 
     private onLocationUpdated() {

src/client/datascience/jupyter/debuggerVariables.ts

@@ -10,6 +10,7 @@ import { traceError } from '../../common/logger';
 import { IConfigurationService, Resource } from '../../common/types';
 import { sendTelemetryEvent } from '../../telemetry';
 import { DataFrameLoading, Identifiers, Telemetry } from '../constants';
+import { DebugLocationTracker } from '../debugLocationTracker';
 import {
     IConditionalJupyterVariables,
     IJupyterDebugService,
@@ -23,17 +24,19 @@ const DataViewableTypes: Set<string> = new Set<string>(['DataFrame', 'list', 'di
 const KnownExcludedVariables = new Set<string>(['In', 'Out', 'exit', 'quit']);
 
 @injectable()
-export class DebuggerVariables implements IConditionalJupyterVariables, DebugAdapterTracker {
+export class DebuggerVariables extends DebugLocationTracker
+    implements IConditionalJupyterVariables, DebugAdapterTracker {
     private refreshEventEmitter = new EventEmitter<void>();
     private lastKnownVariables: IJupyterVariable[] = [];
-    private topMostFrameId = 0;
     private importedIntoKernel = new Set<string>();
     private watchedNotebooks = new Map<string, Disposable[]>();
     private debuggingStarted = false;
     constructor(
         @inject(IJupyterDebugService) @named(Identifiers.MULTIPLEXING_DEBUGSERVICE) private debugService: IDebugService,
         @inject(IConfigurationService) private configService: IConfigurationService
-    ) {}
+    ) {
+        super(undefined);
+    }
 
     public get refreshRequired(): Event<void> {
         return this.refreshEventEmitter.event;
@@ -110,10 +113,12 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
         );
 
         // Results should be the updated variable.
-        return {
-            ...targetVariable,
-            ...JSON.parse(results.result.slice(1, -1))
-        };
+        return results
+            ? {
+                  ...targetVariable,
+                  ...JSON.parse(results.result.slice(1, -1))
+              }
+            : targetVariable;
     }
 
     public async getDataFrameRows(
@@ -166,6 +171,7 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
 
     // tslint:disable-next-line: no-any
     public onDidSendMessage(message: any) {
+        super.onDidSendMessage(message);
         // When the initialize response comes back, indicate we have started.
         if (message.type === 'response' && message.command === 'initialize') {
             this.debuggingStarted = true;
@@ -174,9 +180,6 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
             // tslint:disable-next-line: no-suspicious-comment
             // TODO: Figure out what resource to use
             this.updateVariables(undefined, message as DebugProtocol.VariablesResponse);
-        } else if (message.type === 'response' && message.command === 'stackTrace') {
-            // This should be the top frame. We need to use this to compute the value of a variable
-            this.updateStackFrame(message as DebugProtocol.StackTraceResponse);
         } else if (message.type === 'event' && message.event === 'terminated') {
             // When the debugger exits, make sure the variables are cleared
             this.lastKnownVariables = [];
@@ -240,12 +243,6 @@ export class DebuggerVariables implements IConditionalJupyterVariables, DebugAda
         }
     }
 
-    private updateStackFrame(stackResponse: DebugProtocol.StackTraceResponse) {
-        if (stackResponse.body.stackFrames[0]) {
-            this.topMostFrameId = stackResponse.body.stackFrames[0].id;
-        }
-    }
-
     private async getFullVariable(variable: IJupyterVariable, notebook: INotebook): Promise<IJupyterVariable> {
         // See if we imported or not into the kernel our special function
         await this.importDataFrameScripts(notebook);

src/client/datascience/jupyterDebugService.ts

@@ -260,7 +260,18 @@ export class JupyterDebugService implements IJupyterDebugService, IDisposable {
     }
 
     private sendToTrackers(args: any) {
-        this.debugAdapterTrackers.forEach((d) => d.onDidSendMessage!(args));
+        switch (args.type) {
+            case 'request':
+                this.debugAdapterTrackers.forEach((d) => {
+                    if (d.onWillReceiveMessage) {
+                        d.onWillReceiveMessage(args);
+                    }
+                });
+                break;
+            default:
+                this.debugAdapterTrackers.forEach((d) => d.onDidSendMessage!(args));
+                break;
+        }
     }
 
     private sendCustomRequest(command: string, args?: any): Promise<any> {

src/test/datascience/debugLocationTracker.unit.test.ts

@@ -3,6 +3,7 @@
 'use strict';
 //tslint:disable:max-func-body-length match-default-export-name no-any no-multiline-string no-trailing-whitespace
 import { expect } from 'chai';
+import { DebugProtocol } from 'vscode-debugprotocol';
 
 import { DebugLocationTracker } from '../../client/datascience/debugLocationTracker';
 import { IDebugLocation } from '../../client/datascience/types';
@@ -21,6 +22,7 @@ suite('Debug Location Tracker', () => {
 
         expect(debugTracker.debugLocation).to.be.equal(undefined, 'After stop location is empty');
 
+        debugTracker.onWillReceiveMessage(makeStackTraceRequest());
         debugTracker.onDidSendMessage(makeStackTraceMessage());
 
         const testLocation: IDebugLocation = { lineNumber: 1, column: 1, fileName: 'testpath' };
@@ -40,12 +42,28 @@ function makeContinueMessage(): any {
     return { type: 'event', event: 'continue' };
 }
 
-function makeStackTraceMessage(): any {
+function makeStackTraceMessage(): DebugProtocol.Response {
     return {
         type: 'response',
         command: 'stackTrace',
+        request_seq: 42,
+        success: true,
+        seq: 43,
         body: {
-            stackFrames: [{ line: 1, column: 1, source: { path: 'testpath' } }]
+            stackFrames: [{ id: 9000, line: 1, column: 1, source: { path: 'testpath' } }]
+        }
+    };
+}
+
+function makeStackTraceRequest(): DebugProtocol.Request {
+    return {
+        type: 'request',
+        command: 'stackTrace',
+        seq: 42,
+        arguments: {
+            levels: 1,
+            startFrame: 0,
+            threadId: 1
         }
     };
 }