From 2feb6429c59d78171cdd50a7820682afec30ec4f Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 26 Sep 2022 10:11:22 -0700 Subject: [PATCH 01/10] Support debug "restart" microsoft/vscode-jupyter#7670 --- .../debugger/jupyter/debuggingManager.ts | 12 +- .../debugger/debugCellControllers.ts | 2 +- src/notebooks/debugger/debugger.ts | 35 +-- src/notebooks/debugger/debuggingManager.ts | 208 ++++++++---------- .../debugger/debuggingManagerBase.ts | 59 ++--- src/notebooks/debugger/debuggingTypes.ts | 5 +- src/notebooks/debugger/runByLineController.ts | 2 +- 7 files changed, 129 insertions(+), 194 deletions(-) diff --git a/src/interactive-window/debugger/jupyter/debuggingManager.ts b/src/interactive-window/debugger/jupyter/debuggingManager.ts index b30ca7a08f9..efe1404f15d 100644 --- a/src/interactive-window/debugger/jupyter/debuggingManager.ts +++ b/src/interactive-window/debugger/jupyter/debuggingManager.ts @@ -148,12 +148,10 @@ export class InteractiveWindowDebuggingManager __cellIndex: cell.index }; const opts: DebugSessionOptions = { suppressSaveBeforeStart: true }; - return this.startDebuggingConfig(doc, config, opts); + return this.startDebuggingConfig(config, opts); } - protected override async createDebugAdapterDescriptor( - session: DebugSession - ): Promise { + protected async createDebugAdapterDescriptor(session: DebugSession): Promise { const config = session.configuration; assertIsDebugConfig(config); @@ -194,9 +192,7 @@ export class InteractiveWindowDebuggingManager const cell = activeDoc.cellAt(config.__cellIndex); const controller = new DebugCellController(adapter, cell, kernel!); adapter.setDebuggingDelegate(controller); - controller.ready - .then(() => debug.resolve(session)) - .catch((ex) => console.error('Failed waiting for controller to be ready', ex)); + controller.ready.catch((ex) => console.error('Failed waiting for controller to be ready', ex)); // ?? TODO this.trackDebugAdapter(activeDoc, adapter); return new DebugAdapterInlineImplementation(adapter); @@ -205,7 +201,7 @@ export class InteractiveWindowDebuggingManager // TODO: This will likely be needed for mapping breakpoints and such public async updateSourceMaps(notebookEditor: NotebookEditor, hashes: IFileGeneratedCodes[]): Promise { // Make sure that we have an active debugging session at this point - let debugSession = await this.getDebugSession(notebookEditor.notebook); + let debugSession = this.getDebugSession(notebookEditor.notebook); if (debugSession) { traceInfoIfCI(`Sending debug request for source map`); await Promise.all( diff --git a/src/notebooks/debugger/debugCellControllers.ts b/src/notebooks/debugger/debugCellControllers.ts index 6218440fb6d..14396012554 100644 --- a/src/notebooks/debugger/debugCellControllers.ts +++ b/src/notebooks/debugger/debugCellControllers.ts @@ -51,7 +51,7 @@ export class DebugCellController implements IDebuggingDelegate { this.commandManager .executeCommand('notebook.cell.execute', { ranges: [{ start: this.debugCell.index, end: this.debugCell.index + 1 }], - document: this.debugCell.document.uri + document: this.debugCell.notebook.uri }) .then(noop, noop); } diff --git a/src/notebooks/debugger/debugger.ts b/src/notebooks/debugger/debugger.ts index 158905281e9..f834bfef0ae 100644 --- a/src/notebooks/debugger/debugger.ts +++ b/src/notebooks/debugger/debugger.ts @@ -2,44 +2,15 @@ // Licensed under the MIT License. 'use strict'; -import { debug, NotebookDocument, DebugSession, DebugSessionOptions, DebugConfiguration } from 'vscode'; -import { noop } from '../../platform/common/utils/misc'; +import { DebugConfiguration, DebugSession, NotebookDocument } from 'vscode'; /** * Wraps debug start in a promise */ export class Debugger { - private resolveFunc?: (value: DebugSession) => void; - private rejectFunc?: (reason?: Error) => void; - - readonly session: Promise; - constructor( public readonly document: NotebookDocument, public readonly config: DebugConfiguration, - options?: DebugSessionOptions - ) { - this.session = new Promise((resolve, reject) => { - this.resolveFunc = resolve; - this.rejectFunc = reject; - - debug.startDebugging(undefined, config, options).then(undefined, reject); - }); - } - - resolve(session: DebugSession) { - if (this.resolveFunc) { - this.resolveFunc(session); - } - } - - reject(reason: Error) { - if (this.rejectFunc) { - this.rejectFunc(reason); - } - } - - async stop() { - void debug.stopDebugging(await this.session).then(noop, noop); - } + public readonly session: DebugSession + ) {} } diff --git a/src/notebooks/debugger/debuggingManager.ts b/src/notebooks/debugger/debuggingManager.ts index ba8def1dee6..4edff80fe40 100644 --- a/src/notebooks/debugger/debuggingManager.ts +++ b/src/notebooks/debugger/debuggingManager.ts @@ -30,18 +30,18 @@ import { IConfigurationService } from '../../platform/common/types'; import { DataScience } from '../../platform/common/utils/localize'; import { noop } from '../../platform/common/utils/misc'; import { IServiceContainer } from '../../platform/ioc/types'; -import { traceError, traceInfo, traceInfoIfCI } from '../../platform/logging'; +import { traceInfo, traceInfoIfCI } from '../../platform/logging'; import { ResourceSet } from '../../platform/vscode-path/map'; import * as path from '../../platform/vscode-path/path'; import { sendTelemetryEvent } from '../../telemetry'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; import { DebuggingTelemetry, pythonKernelDebugAdapter } from './constants'; import { DebugCellController } from './debugCellControllers'; +import { Debugger } from './debugger'; import { DebuggingManagerBase } from './debuggingManagerBase'; import { IDebuggingManager, IKernelDebugAdapterConfig, KernelDebugMode } from './debuggingTypes'; import { assertIsDebugConfig, IpykernelCheckResult } from './helper'; import { KernelDebugAdapter } from './kernelDebugAdapter'; -import { KernelDebugAdapterBase } from './kernelDebugAdapterBase'; import { RunByLineController } from './runByLineController'; /** @@ -202,67 +202,34 @@ export class DebuggingManager return; } - if (this.notebookInProgress.has(editor.notebook)) { - traceInfo(`Cannot start debugging. Already debugging this notebook`); - return; - } - - if (this.isDebugging(editor.notebook)) { - traceInfo(`Cannot start debugging. Already debugging this notebook document.`); - return; + await this.checkIpykernel(editor); + if (mode === KernelDebugMode.RunByLine || mode === KernelDebugMode.Cell) { + await this.startDebuggingCell(editor.notebook, mode, cell!); + } else { + await this.startDebugging(editor.notebook); } + } - const checkIpykernelAndStart = async (allowSelectKernel = true): Promise => { - const ipykernelResult = await this.checkForIpykernel6(editor.notebook); - switch (ipykernelResult) { - case IpykernelCheckResult.NotInstalled: - // User would have been notified about this, nothing more to do. - return; - case IpykernelCheckResult.Outdated: - case IpykernelCheckResult.Unknown: { - this.promptInstallIpykernel6().then(noop, noop); - return; - } - case IpykernelCheckResult.Ok: { - switch (mode) { - case KernelDebugMode.Everything: { - await this.startDebugging(editor.notebook); - return; - } - case KernelDebugMode.Cell: - if (cell) { - await this.startDebuggingCell(editor.notebook, KernelDebugMode.Cell, cell); - } - return; - case KernelDebugMode.RunByLine: - if (cell) { - await this.startDebuggingCell(editor.notebook, KernelDebugMode.RunByLine, cell); - } - return; - default: - return; - } - } - case IpykernelCheckResult.ControllerNotSelected: { - if (allowSelectKernel) { - await this.commandManager.executeCommand('notebook.selectKernel', { notebookEditor: editor }); - await checkIpykernelAndStart(false); - } + protected async checkIpykernel(editor: NotebookEditor, allowSelectKernel: boolean = true): Promise { + const ipykernelResult = await this.checkForIpykernel6(editor.notebook); + switch (ipykernelResult) { + case IpykernelCheckResult.NotInstalled: + // User would have been notified about this, nothing more to do. + return; + case IpykernelCheckResult.Outdated: + case IpykernelCheckResult.Unknown: { + this.promptInstallIpykernel6().then(noop, noop); + return; + } + case IpykernelCheckResult.ControllerNotSelected: { + if (allowSelectKernel) { + await this.commandManager.executeCommand('notebook.selectKernel', { notebookEditor: editor }); + await this.checkIpykernel(editor, false); } } - }; - - try { - this.notebookInProgress.add(editor.notebook); - this.updateDebugContextKey(); - await checkIpykernelAndStart(); - } catch (e) { - traceInfo(`Error starting debugging: ${e}`); - } finally { - this.notebookInProgress.delete(editor.notebook); - this.updateDebugContextKey(); } } + private async startDebuggingCell( doc: NotebookDocument, mode: KernelDebugMode.Cell | KernelDebugMode.RunByLine, @@ -275,7 +242,8 @@ export class DebuggingManager justMyCode: true, // add a property to the config to know if the session is runByLine __mode: mode, - __cellIndex: cell.index + __cellIndex: cell.index, + __notebookUri: doc.uri.toString() }; const opts: DebugSessionOptions | undefined = mode === KernelDebugMode.RunByLine @@ -286,7 +254,7 @@ export class DebuggingManager suppressSaveBeforeStart: true } : { suppressSaveBeforeStart: true }; - return this.startDebuggingConfig(doc, config, opts); + return this.startDebuggingConfig(config, opts); } private async startDebugging(doc: NotebookDocument) { @@ -296,71 +264,81 @@ export class DebuggingManager request: 'attach', internalConsoleOptions: 'neverOpen', justMyCode: false, - __mode: KernelDebugMode.Everything + __mode: KernelDebugMode.Everything, + __notebookUri: doc.uri.toString() }; - return this.startDebuggingConfig(doc, config); - } - - protected override trackDebugAdapter(notebook: NotebookDocument, adapter: KernelDebugAdapterBase): void { - super.trackDebugAdapter(notebook, adapter); - this.updateDebugContextKey(); + return this.startDebuggingConfig(config); } - protected override async createDebugAdapterDescriptor( - session: DebugSession - ): Promise { + protected async createDebugAdapterDescriptor(session: DebugSession): Promise { const config = session.configuration; assertIsDebugConfig(config); - const activeDoc = config.__interactiveWindowNotebookUri - ? this.vscNotebook.notebookDocuments.find( - (doc) => doc.uri.toString() === config.__interactiveWindowNotebookUri - ) - : this.vscNotebook.activeNotebookEditor?.notebook; - if (activeDoc) { - // TODO we apparently always have a kernel here, clean up typings - const kernel = await this.ensureKernelIsRunning(activeDoc); - const debug = this.getDebuggerByUri(activeDoc); - - if (debug) { - if (kernel?.session) { - const adapter = new KernelDebugAdapter( - session, - debug.document, - kernel.session, - kernel, - this.platform, - this.debugService - ); - - if (config.__mode === KernelDebugMode.RunByLine && typeof config.__cellIndex === 'number') { - const cell = activeDoc.cellAt(config.__cellIndex); - const controller = new RunByLineController( - adapter, - cell, - this.commandManager, - kernel!, - this.settings - ); - adapter.setDebuggingDelegate(controller); - this.notebookToRunByLineController.set(debug.document, controller); - this.updateRunByLineContextKeys(); - } else if (config.__mode === KernelDebugMode.Cell && typeof config.__cellIndex === 'number') { - const cell = activeDoc.cellAt(config.__cellIndex); - const controller = new DebugCellController(adapter, cell, kernel!, this.commandManager); - adapter.setDebuggingDelegate(controller); - } - this.trackDebugAdapter(debug.document, adapter); + const notebookUri = config.__interactiveWindowNotebookUri ?? config.__notebookUri; + const notebook = this.vscNotebook.notebookDocuments.find((doc) => doc.uri.toString() === notebookUri); - // Wait till we're attached before resolving the session - debug.resolve(session); - return new DebugAdapterInlineImplementation(adapter); - } else { - this.appShell.showInformationMessage(DataScience.kernelWasNotStarted()).then(noop, noop); - } + if (!notebook) { + traceInfo(`Cannot start debugging. Notebook ${notebookUri} not found.`); + return; + } + + if (this.notebookInProgress.has(notebook)) { + traceInfo(`Cannot start debugging. Already debugging this notebook`); + return; + } + + if (this.isDebugging(notebook)) { + traceInfo(`Cannot start debugging. Already debugging this notebook document.`); + return; + } + + this.notebookToDebugger.set(notebook, new Debugger(notebook, config, session)); + try { + this.notebookInProgress.add(notebook); + this.updateDebugContextKey(); + return await this.doCreateDebugAdapterDescriptor(config, session, notebook); + } finally { + this.notebookInProgress.delete(notebook); + this.updateDebugContextKey(); + } + } + + private async doCreateDebugAdapterDescriptor( + config: IKernelDebugAdapterConfig, + session: DebugSession, + notebook: NotebookDocument + ): Promise { + const kernel = await this.ensureKernelIsRunning(notebook); + if (kernel?.session) { + const adapter = new KernelDebugAdapter( + session, + notebook, + kernel.session, + kernel, + this.platform, + this.debugService + ); + + if (config.__mode === KernelDebugMode.RunByLine && typeof config.__cellIndex === 'number') { + const cell = notebook.cellAt(config.__cellIndex); + const controller = new RunByLineController(adapter, cell, this.commandManager, kernel!, this.settings); + adapter.setDebuggingDelegate(controller); + this.notebookToRunByLineController.set(notebook, controller); + this.updateRunByLineContextKeys(); + } else if (config.__mode === KernelDebugMode.Cell && typeof config.__cellIndex === 'number') { + const cell = notebook.cellAt(config.__cellIndex); + const controller = new DebugCellController(adapter, cell, kernel!, this.commandManager); + adapter.setDebuggingDelegate(controller); } + + this.trackDebugAdapter(notebook, adapter); + this.updateDebugContextKey(); + + return new DebugAdapterInlineImplementation(adapter); + } else { + this.appShell.showInformationMessage(DataScience.kernelWasNotStarted()).then(noop, noop); } - traceError('Debug sessions should start only from the cell toolbar command'); + return; } } diff --git a/src/notebooks/debugger/debuggingManagerBase.ts b/src/notebooks/debugger/debuggingManagerBase.ts index 8dac8b6708e..3c3534ad83e 100644 --- a/src/notebooks/debugger/debuggingManagerBase.ts +++ b/src/notebooks/debugger/debuggingManagerBase.ts @@ -5,28 +5,27 @@ import { debug, - NotebookDocument, - workspace, DebugSession, DebugSessionOptions, - DebugAdapterDescriptor, Event, EventEmitter, - NotebookCell + NotebookCell, + NotebookDocument, + workspace } from 'vscode'; import { IKernel, IKernelProvider } from '../../kernels/types'; -import { IDisposable } from '../../platform/common/types'; import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../platform/common/application/types'; -import { DebuggingTelemetry } from './constants'; -import { sendTelemetryEvent } from '../../telemetry'; -import { traceError, traceInfoIfCI } from '../../platform/logging'; +import { IDisposable } from '../../platform/common/types'; import { DataScience } from '../../platform/common/utils/localize'; -import { IKernelDebugAdapterConfig } from './debuggingTypes'; -import { Debugger } from './debugger'; -import { KernelDebugAdapterBase } from './kernelDebugAdapterBase'; -import { IpykernelCheckResult, isUsingIpykernel6OrLater } from './helper'; import { noop } from '../../platform/common/utils/misc'; +import { traceError, traceInfoIfCI } from '../../platform/logging'; +import { sendTelemetryEvent } from '../../telemetry'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; +import { DebuggingTelemetry } from './constants'; +import { Debugger } from './debugger'; +import { IKernelDebugAdapterConfig } from './debuggingTypes'; +import { IpykernelCheckResult, isUsingIpykernel6OrLater } from './helper'; +import { KernelDebugAdapterBase } from './kernelDebugAdapterBase'; import { KernelConnector } from '../controllers/kernelConnector'; import { IServiceContainer } from '../../platform/ioc/types'; import { DisplayOptions } from '../../kernels/displayOptions'; @@ -35,7 +34,7 @@ import { DisplayOptions } from '../../kernels/displayOptions'; * The DebuggingManager maintains the mapping between notebook documents and debug sessions. */ export abstract class DebuggingManagerBase implements IDisposable { - private notebookToDebugger = new Map(); + protected notebookToDebugger = new Map(); protected notebookToDebugAdapter = new Map(); protected notebookInProgress = new Set(); protected readonly disposables: IDisposable[] = []; @@ -60,7 +59,7 @@ export abstract class DebuggingManagerBase implements IDisposable { workspace.onDidCloseNotebookDocument(async (document) => { const dbg = this.notebookToDebugger.get(document); if (dbg) { - await dbg.stop(); + await debug.stopDebugging(dbg.session); this.onDidStopDebugging(document); } }) @@ -82,12 +81,13 @@ export abstract class DebuggingManagerBase implements IDisposable { return this.notebookToDebugger.has(notebook); } - public getDebugSession(notebook: NotebookDocument): Promise | undefined { + public getDebugSession(notebook: NotebookDocument): DebugSession | undefined { const dbg = this.notebookToDebugger.get(notebook); if (dbg) { return dbg.session; } } + public getDebugAdapter(notebook: NotebookDocument): KernelDebugAdapterBase | undefined { return this.notebookToDebugAdapter.get(notebook); } @@ -96,26 +96,14 @@ export abstract class DebuggingManagerBase implements IDisposable { // } - protected async startDebuggingConfig( - doc: NotebookDocument, - config: IKernelDebugAdapterConfig, - options?: DebugSessionOptions - ) { + protected async startDebuggingConfig(config: IKernelDebugAdapterConfig, options?: DebugSessionOptions) { traceInfoIfCI(`Attempting to start debugging with config ${JSON.stringify(config)}`); - let dbg = this.notebookToDebugger.get(doc); - if (!dbg) { - dbg = new Debugger(doc, config, options); - this.notebookToDebugger.set(doc, dbg); - - try { - const session = await dbg.session; - traceInfoIfCI(`Debugger session is ready. Should be debugging ${session.id} now`); - } catch (err) { - traceError(`Can't start debugging (${err})`); - this.appShell.showErrorMessage(DataScience.cantStartDebugging()).then(noop, noop); - } - } else { - traceInfoIfCI(`Not starting debugging because already debugging in this notebook`); + + try { + await debug.startDebugging(undefined, config, options); + } catch (err) { + traceError(`Can't start debugging (${err})`); + this.appShell.showErrorMessage(DataScience.cantStartDebugging()).then(noop, noop); } } @@ -123,6 +111,7 @@ export abstract class DebuggingManagerBase implements IDisposable { this.notebookToDebugAdapter.set(notebook, adapter); this.disposables.push(adapter.onDidEndSession(this.endSession.bind(this))); } + protected async endSession(session: DebugSession) { traceInfoIfCI(`Ending debug session ${session.id}`); this._doneDebugging.fire(); @@ -136,8 +125,6 @@ export abstract class DebuggingManagerBase implements IDisposable { } } - protected abstract createDebugAdapterDescriptor(session: DebugSession): Promise; - protected getDebuggerByUri(document: NotebookDocument): Debugger | undefined { for (const [doc, dbg] of this.notebookToDebugger.entries()) { if (document === doc) { diff --git a/src/notebooks/debugger/debuggingTypes.ts b/src/notebooks/debugger/debuggingTypes.ts index 7d8831ccc1e..83278661aaa 100644 --- a/src/notebooks/debugger/debuggingTypes.ts +++ b/src/notebooks/debugger/debuggingTypes.ts @@ -76,7 +76,7 @@ export interface IDebuggingManager { readonly onDoneDebugging: Event; isDebugging(notebook: NotebookDocument): boolean; getDebugMode(notebook: NotebookDocument): KernelDebugMode | undefined; - getDebugSession(notebook: NotebookDocument): Promise | undefined; + getDebugSession(notebook: NotebookDocument): DebugSession | undefined; getDebugCell(notebook: NotebookDocument): NotebookCell | undefined; getDebugAdapter(notebook: NotebookDocument): IKernelDebugAdapter | undefined; } @@ -122,6 +122,9 @@ export enum KernelDebugMode { export interface IKernelDebugAdapterConfig extends DebugConfiguration { __mode: KernelDebugMode; __cellIndex?: number; + + // TODO + __notebookUri?: string; __interactiveWindowNotebookUri?: string; } diff --git a/src/notebooks/debugger/runByLineController.ts b/src/notebooks/debugger/runByLineController.ts index 20464c2d953..4635e777f2f 100644 --- a/src/notebooks/debugger/runByLineController.ts +++ b/src/notebooks/debugger/runByLineController.ts @@ -150,7 +150,7 @@ export class RunByLineController implements IDebuggingDelegate { this.commandManager .executeCommand('notebook.cell.execute', { ranges: [{ start: this.debugCell.index, end: this.debugCell.index + 1 }], - document: this.debugCell.document.uri + document: this.debugCell.notebook.uri }) .then(noop, noop); } From b331f96380faeb70d505c39b3f50b1017991bb16 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 26 Sep 2022 21:00:53 -0700 Subject: [PATCH 02/10] More debug refactoring, set up IW for restart --- .../debugger/jupyter/debugCellControllers.ts | 4 - .../debugger/jupyter/debuggingManager.ts | 145 ++++++++---------- src/notebooks/debugger/debuggingManager.ts | 40 ++--- .../debugger/debuggingManagerBase.ts | 34 +++- src/notebooks/debugger/debuggingTypes.ts | 33 +++- src/notebooks/debugger/helper.ts | 18 ++- .../debugger/kernelDebugAdapterBase.ts | 6 +- 7 files changed, 146 insertions(+), 134 deletions(-) diff --git a/src/interactive-window/debugger/jupyter/debugCellControllers.ts b/src/interactive-window/debugger/jupyter/debugCellControllers.ts index 4d3b2d8cbb4..bae98706e17 100644 --- a/src/interactive-window/debugger/jupyter/debugCellControllers.ts +++ b/src/interactive-window/debugger/jupyter/debugCellControllers.ts @@ -8,7 +8,6 @@ import { DebuggingTelemetry } from '../../../notebooks/debugger/constants'; import { isJustMyCodeNotification } from '../../../notebooks/debugger/debugCellControllers'; import { IDebuggingDelegate, IKernelDebugAdapter } from '../../../notebooks/debugger/debuggingTypes'; import { cellDebugSetup } from '../../../notebooks/debugger/helper'; -import { createDeferred } from '../../../platform/common/utils/async'; import { traceVerbose } from '../../../platform/logging'; import { sendTelemetryEvent } from '../../../telemetry'; import { getInteractiveCellMetadata } from '../../helpers'; @@ -18,8 +17,6 @@ import { getInteractiveCellMetadata } from '../../helpers'; * Dumping a cell is how the IPython kernel determines the file path of a cell */ export class DebugCellController implements IDebuggingDelegate { - private readonly _ready = createDeferred(); - public readonly ready = this._ready.promise; private cellDumpInvoked?: boolean; constructor( private readonly debugAdapter: IKernelDebugAdapter, @@ -58,7 +55,6 @@ export class DebugCellController implements IDebuggingDelegate { this.debugCellDumped = cellDebugSetup(this.kernel, this.debugAdapter); } await this.debugCellDumped; - this._ready.resolve(); } } } diff --git a/src/interactive-window/debugger/jupyter/debuggingManager.ts b/src/interactive-window/debugger/jupyter/debuggingManager.ts index efe1404f15d..9015d84f3dc 100644 --- a/src/interactive-window/debugger/jupyter/debuggingManager.ts +++ b/src/interactive-window/debugger/jupyter/debuggingManager.ts @@ -3,45 +3,46 @@ import { inject, injectable } from 'inversify'; import { - NotebookDocument, + debug, + DebugAdapterDescriptor, DebugAdapterInlineImplementation, DebugSession, - NotebookCell, DebugSessionOptions, - DebugAdapterDescriptor, - NotebookEditor, - debug + NotebookCell, + NotebookDocument, + NotebookEditor } from 'vscode'; +import { IKernelProvider } from '../../../kernels/types'; +import { IControllerLoader, IControllerSelection } from '../../../notebooks/controllers/types'; import { pythonIWKernelDebugAdapter } from '../../../notebooks/debugger/constants'; +import { Debugger } from '../../../notebooks/debugger/debugger'; +import { DebuggingManagerBase } from '../../../notebooks/debugger/debuggingManagerBase'; import { IDebuggingManager, - KernelDebugMode, - IKernelDebugAdapterConfig, - IDebugLocationTrackerFactory + IDebugLocationTrackerFactory, + IInteractiveWindowDebugConfig, + KernelDebugMode } from '../../../notebooks/debugger/debuggingTypes'; -import { IKernelProvider } from '../../../kernels/types'; -import { IpykernelCheckResult, assertIsDebugConfig } from '../../../notebooks/debugger/helper'; -import { KernelDebugAdapter } from './kernelDebugAdapter'; +import { assertIsInteractiveWindowDebugConfig, IpykernelCheckResult } from '../../../notebooks/debugger/helper'; import { IExtensionSingleActivationService } from '../../../platform/activation/types'; import { - ICommandManager, IApplicationShell, - IVSCodeNotebook, - IDebugService + ICommandManager, + IDebugService, + IVSCodeNotebook } from '../../../platform/common/application/types'; import { IPlatformService } from '../../../platform/common/platform/types'; +import { IConfigurationService } from '../../../platform/common/types'; import { DataScience } from '../../../platform/common/utils/localize'; -import { traceInfoIfCI, traceInfo, traceError } from '../../../platform/logging'; +import { noop } from '../../../platform/common/utils/misc'; +import { IServiceContainer } from '../../../platform/ioc/types'; +import { traceError, traceInfo, traceInfoIfCI } from '../../../platform/logging'; import * as path from '../../../platform/vscode-path/path'; -import { DebugCellController } from './debugCellControllers'; -import { DebuggingManagerBase } from '../../../notebooks/debugger/debuggingManagerBase'; -import { IConfigurationService } from '../../../platform/common/types'; import { IFileGeneratedCodes } from '../../editor-integration/types'; -import { buildSourceMap } from '../helper'; -import { noop } from '../../../platform/common/utils/misc'; import { IInteractiveWindowDebuggingManager } from '../../types'; -import { IControllerLoader, IControllerSelection } from '../../../notebooks/controllers/types'; -import { IServiceContainer } from '../../../platform/ioc/types'; +import { buildSourceMap } from '../helper'; +import { DebugCellController } from './debugCellControllers'; +import { KernelDebugAdapter } from './kernelDebugAdapter'; /** * The DebuggingManager maintains the mapping between notebook documents and debug sessions. @@ -85,64 +86,28 @@ export class InteractiveWindowDebuggingManager }) ); } + public getDebugMode(_notebook: NotebookDocument): KernelDebugMode | undefined { return KernelDebugMode.InteractiveWindow; } + public async start(editor: NotebookEditor, cell: NotebookCell) { traceInfoIfCI(`Starting debugging IW`); - if (this.notebookInProgress.has(editor.notebook)) { - traceInfo(`Cannot start debugging. Already debugging this notebook`); - return; - } - - if (this.isDebugging(editor.notebook)) { - traceInfo(`Cannot start debugging. Already debugging this notebook document. Toolbar should update`); - return; - } - - const checkIpykernelAndStart = async (allowSelectKernel = true): Promise => { - const ipykernelResult = await this.checkForIpykernel6(editor.notebook); - switch (ipykernelResult) { - case IpykernelCheckResult.NotInstalled: - // User would have been notified about this, nothing more to do. - return; - case IpykernelCheckResult.Outdated: - case IpykernelCheckResult.Unknown: { - this.promptInstallIpykernel6().then(noop, noop); - return; - } - case IpykernelCheckResult.Ok: { - await this.startDebuggingCell(editor.notebook, cell); - return; - } - case IpykernelCheckResult.ControllerNotSelected: { - if (allowSelectKernel) { - await this.commandManager.executeCommand('notebook.selectKernel', { notebookEditor: editor }); - await checkIpykernelAndStart(false); - } - } - } - }; - - try { - this.notebookInProgress.add(editor.notebook); - await checkIpykernelAndStart(); - } catch (e) { - traceInfo(`Error starting debugging: ${e}`); - } finally { - this.notebookInProgress.delete(editor.notebook); + const ipykernelResult = await this.checkIpykernelAndPrompt(editor); + if (ipykernelResult === IpykernelCheckResult.Ok) { + await this.startDebuggingCell(editor.notebook, cell); } } private async startDebuggingCell(doc: NotebookDocument, cell: NotebookCell) { const settings = this.configService.getSettings(doc.uri); - const config: IKernelDebugAdapterConfig = { + const config: IInteractiveWindowDebugConfig = { type: pythonIWKernelDebugAdapter, name: path.basename(doc.uri.toString()), request: 'attach', justMyCode: settings.debugJustMyCode, - __interactiveWindowNotebookUri: doc.uri.toString(), + __notebookUri: doc.uri.toString(), // add a property to the config to know if the session is runByLine __mode: KernelDebugMode.InteractiveWindow, __cellIndex: cell.index @@ -152,33 +117,47 @@ export class InteractiveWindowDebuggingManager } protected async createDebugAdapterDescriptor(session: DebugSession): Promise { - const config = session.configuration; - assertIsDebugConfig(config); - - const activeDoc = config.__interactiveWindowNotebookUri - ? this.vscNotebook.notebookDocuments.find( - (doc) => doc.uri.toString() === config.__interactiveWindowNotebookUri - ) - : this.vscNotebook.activeNotebookEditor?.notebook; - if (!activeDoc || typeof config.__cellIndex !== 'number') { - // This cannot happen. + const config = session.configuration as IInteractiveWindowDebugConfig; + assertIsInteractiveWindowDebugConfig(config); + + const notebook = this.vscNotebook.notebookDocuments.find((doc) => doc.uri.toString() === config.__notebookUri); + if (!notebook || typeof config.__cellIndex !== 'number') { traceError('Invalid debug session for debugging of IW using Jupyter Protocol'); return; } - // TODO we apparently always have a kernel here, clean up typings - const kernel = await this.ensureKernelIsRunning(activeDoc); - const debug = this.getDebuggerByUri(activeDoc); - if (!debug) { + if (this.notebookInProgress.has(notebook)) { + traceInfo(`Cannot start debugging. Already debugging this notebook`); + return; + } + + if (this.isDebugging(notebook)) { + traceInfo(`Cannot start debugging. Already debugging this notebook document. Toolbar should update`); return; } + + this.notebookToDebugger.set(notebook, new Debugger(notebook, config, session)); + try { + this.notebookInProgress.add(notebook); + return await this.doCreateDebugAdapterDescriptor(config, session, notebook); + } finally { + this.notebookInProgress.delete(notebook); + } + } + + private async doCreateDebugAdapterDescriptor( + config: IInteractiveWindowDebugConfig, + session: DebugSession, + notebook: NotebookDocument + ): Promise { + const kernel = await this.ensureKernelIsRunning(notebook); if (!kernel?.session) { this.appShell.showInformationMessage(DataScience.kernelWasNotStarted()).then(noop, noop); return; } const adapter = new KernelDebugAdapter( session, - debug.document, + notebook, kernel.session, kernel, this.platform, @@ -188,13 +167,11 @@ export class InteractiveWindowDebuggingManager this.disposables.push(adapter.onDidEndSession(this.endSession.bind(this))); - // Wait till we're attached before resolving the session - const cell = activeDoc.cellAt(config.__cellIndex); + const cell = notebook.cellAt(config.__cellIndex); const controller = new DebugCellController(adapter, cell, kernel!); adapter.setDebuggingDelegate(controller); - controller.ready.catch((ex) => console.error('Failed waiting for controller to be ready', ex)); // ?? TODO - this.trackDebugAdapter(activeDoc, adapter); + this.trackDebugAdapter(notebook, adapter); return new DebugAdapterInlineImplementation(adapter); } diff --git a/src/notebooks/debugger/debuggingManager.ts b/src/notebooks/debugger/debuggingManager.ts index 4edff80fe40..fb9340618a1 100644 --- a/src/notebooks/debugger/debuggingManager.ts +++ b/src/notebooks/debugger/debuggingManager.ts @@ -39,7 +39,7 @@ import { DebuggingTelemetry, pythonKernelDebugAdapter } from './constants'; import { DebugCellController } from './debugCellControllers'; import { Debugger } from './debugger'; import { DebuggingManagerBase } from './debuggingManagerBase'; -import { IDebuggingManager, IKernelDebugAdapterConfig, KernelDebugMode } from './debuggingTypes'; +import { IDebuggingManager, INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; import { assertIsDebugConfig, IpykernelCheckResult } from './helper'; import { KernelDebugAdapter } from './kernelDebugAdapter'; import { RunByLineController } from './runByLineController'; @@ -202,30 +202,12 @@ export class DebuggingManager return; } - await this.checkIpykernel(editor); - if (mode === KernelDebugMode.RunByLine || mode === KernelDebugMode.Cell) { - await this.startDebuggingCell(editor.notebook, mode, cell!); - } else { - await this.startDebugging(editor.notebook); - } - } - - protected async checkIpykernel(editor: NotebookEditor, allowSelectKernel: boolean = true): Promise { - const ipykernelResult = await this.checkForIpykernel6(editor.notebook); - switch (ipykernelResult) { - case IpykernelCheckResult.NotInstalled: - // User would have been notified about this, nothing more to do. - return; - case IpykernelCheckResult.Outdated: - case IpykernelCheckResult.Unknown: { - this.promptInstallIpykernel6().then(noop, noop); - return; - } - case IpykernelCheckResult.ControllerNotSelected: { - if (allowSelectKernel) { - await this.commandManager.executeCommand('notebook.selectKernel', { notebookEditor: editor }); - await this.checkIpykernel(editor, false); - } + const ipykernelResult = await this.checkIpykernelAndPrompt(editor); + if (ipykernelResult === IpykernelCheckResult.Ok) { + if (mode === KernelDebugMode.RunByLine || mode === KernelDebugMode.Cell) { + await this.startDebuggingCell(editor.notebook, mode, cell!); + } else { + await this.startDebugging(editor.notebook); } } } @@ -235,7 +217,7 @@ export class DebuggingManager mode: KernelDebugMode.Cell | KernelDebugMode.RunByLine, cell: NotebookCell ) { - const config: IKernelDebugAdapterConfig = { + const config: INotebookDebugConfig = { type: pythonKernelDebugAdapter, name: path.basename(doc.uri.toString()), request: 'attach', @@ -258,7 +240,7 @@ export class DebuggingManager } private async startDebugging(doc: NotebookDocument) { - const config: IKernelDebugAdapterConfig = { + const config: INotebookDebugConfig = { type: pythonKernelDebugAdapter, name: path.basename(doc.uri.toString()), request: 'attach', @@ -274,7 +256,7 @@ export class DebuggingManager const config = session.configuration; assertIsDebugConfig(config); - const notebookUri = config.__interactiveWindowNotebookUri ?? config.__notebookUri; + const notebookUri = config.__notebookUri; const notebook = this.vscNotebook.notebookDocuments.find((doc) => doc.uri.toString() === notebookUri); if (!notebook) { @@ -304,7 +286,7 @@ export class DebuggingManager } private async doCreateDebugAdapterDescriptor( - config: IKernelDebugAdapterConfig, + config: INotebookDebugConfig, session: DebugSession, notebook: NotebookDocument ): Promise { diff --git a/src/notebooks/debugger/debuggingManagerBase.ts b/src/notebooks/debugger/debuggingManagerBase.ts index 3c3534ad83e..42356c0f6e7 100644 --- a/src/notebooks/debugger/debuggingManagerBase.ts +++ b/src/notebooks/debugger/debuggingManagerBase.ts @@ -11,6 +11,7 @@ import { EventEmitter, NotebookCell, NotebookDocument, + NotebookEditor, workspace } from 'vscode'; import { IKernel, IKernelProvider } from '../../kernels/types'; @@ -23,7 +24,7 @@ import { sendTelemetryEvent } from '../../telemetry'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; import { DebuggingTelemetry } from './constants'; import { Debugger } from './debugger'; -import { IKernelDebugAdapterConfig } from './debuggingTypes'; +import { INotebookDebugConfig } from './debuggingTypes'; import { IpykernelCheckResult, isUsingIpykernel6OrLater } from './helper'; import { KernelDebugAdapterBase } from './kernelDebugAdapterBase'; import { KernelConnector } from '../controllers/kernelConnector'; @@ -96,7 +97,7 @@ export abstract class DebuggingManagerBase implements IDisposable { // } - protected async startDebuggingConfig(config: IKernelDebugAdapterConfig, options?: DebugSessionOptions) { + protected async startDebuggingConfig(config: INotebookDebugConfig, options?: DebugSessionOptions) { traceInfoIfCI(`Attempting to start debugging with config ${JSON.stringify(config)}`); try { @@ -154,7 +155,32 @@ export abstract class DebuggingManagerBase implements IDisposable { return kernel; } - protected async checkForIpykernel6(doc: NotebookDocument): Promise { + protected async checkIpykernelAndPrompt( + editor: NotebookEditor, + allowSelectKernel: boolean = true + ): Promise { + const ipykernelResult = await this.checkForIpykernel6(editor.notebook); + switch (ipykernelResult) { + case IpykernelCheckResult.NotInstalled: + // User would have been notified about this, nothing more to do. + break; + case IpykernelCheckResult.Outdated: + case IpykernelCheckResult.Unknown: { + this.promptInstallIpykernel6().then(noop, noop); + break; + } + case IpykernelCheckResult.ControllerNotSelected: { + if (allowSelectKernel) { + await this.commandManager.executeCommand('notebook.selectKernel', { notebookEditor: editor }); + await this.checkIpykernelAndPrompt(editor, false); + } + } + } + + return ipykernelResult; + } + + private async checkForIpykernel6(doc: NotebookDocument): Promise { try { let kernel = this.kernelProvider.get(doc); if (!kernel) { @@ -180,7 +206,7 @@ export abstract class DebuggingManagerBase implements IDisposable { return IpykernelCheckResult.Unknown; } - protected async promptInstallIpykernel6() { + private async promptInstallIpykernel6() { const response = await this.appShell.showInformationMessage( DataScience.needIpykernel6(), { modal: true }, diff --git a/src/notebooks/debugger/debuggingTypes.ts b/src/notebooks/debugger/debuggingTypes.ts index 83278661aaa..26d9bad66fa 100644 --- a/src/notebooks/debugger/debuggingTypes.ts +++ b/src/notebooks/debugger/debuggingTypes.ts @@ -68,7 +68,7 @@ export interface IKernelDebugAdapter extends DebugAdapter { disconnect(): Promise; onDidEndSession: Event; dumpAllCells(): Promise; - getConfiguration(): IKernelDebugAdapterConfig; + getConfiguration(): IBaseNotebookDebugConfig; } export const IDebuggingManager = Symbol('IDebuggingManager'); @@ -119,13 +119,34 @@ export enum KernelDebugMode { InteractiveWindow } -export interface IKernelDebugAdapterConfig extends DebugConfiguration { +interface IBaseNotebookDebugConfig extends DebugConfiguration { __mode: KernelDebugMode; - __cellIndex?: number; + __notebookUri: string; +} + +export type INotebookDebugConfig = + | IRunByLineDebugConfig + | ICellDebugConfig + | IEverythingDebugConfig + | IInteractiveWindowDebugConfig; + +export interface IRunByLineDebugConfig extends IBaseNotebookDebugConfig { + __mode: KernelDebugMode.RunByLine; + __cellIndex: number; +} + +export interface ICellDebugConfig extends IBaseNotebookDebugConfig { + __mode: KernelDebugMode.Cell; + __cellIndex: number; +} + +export interface IEverythingDebugConfig extends IBaseNotebookDebugConfig { + __mode: KernelDebugMode.Everything; +} - // TODO - __notebookUri?: string; - __interactiveWindowNotebookUri?: string; +export interface IInteractiveWindowDebugConfig extends IBaseNotebookDebugConfig { + __mode: KernelDebugMode.InteractiveWindow; + __cellIndex: number; } export interface IDebugLocation { diff --git a/src/notebooks/debugger/helper.ts b/src/notebooks/debugger/helper.ts index a5bcb68edd2..481d8ccc501 100644 --- a/src/notebooks/debugger/helper.ts +++ b/src/notebooks/debugger/helper.ts @@ -1,10 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import { IDebugEventMsg } from '@jupyterlab/services/lib/kernel/messages'; import { DebugProtocol } from 'vscode-debugprotocol'; -import { IKernelDebugAdapter, IKernelDebugAdapterConfig, KernelDebugMode } from './debuggingTypes'; import { IKernel } from '../../kernels/types'; -import { IDebugEventMsg } from '@jupyterlab/services/lib/kernel/messages'; +import { IInteractiveWindowDebugConfig, IKernelDebugAdapter, INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; export enum IpykernelCheckResult { Unknown, @@ -29,9 +29,10 @@ builtins.print(ipykernel.__version__)`; return IpykernelCheckResult.Unknown; } -export function assertIsDebugConfig(thing: unknown): asserts thing is IKernelDebugAdapterConfig { - const config = thing as IKernelDebugAdapterConfig; +export function assertIsDebugConfig(thing: unknown): asserts thing is INotebookDebugConfig { + const config = thing as INotebookDebugConfig; if ( + typeof config.__notebookUri === 'undefined' || typeof config.__mode === 'undefined' || ((config.__mode === KernelDebugMode.Cell || config.__mode === KernelDebugMode.InteractiveWindow || @@ -42,6 +43,15 @@ export function assertIsDebugConfig(thing: unknown): asserts thing is IKernelDeb } } +export function assertIsInteractiveWindowDebugConfig( + thing: unknown +): asserts thing is IInteractiveWindowDebugConfig { + assertIsDebugConfig(thing); + if (thing.__mode !== KernelDebugMode.InteractiveWindow) { + throw new Error('Invalid launch configuration'); + } +} + export function getMessageSourceAndHookIt( msg: DebugProtocol.ProtocolMessage, sourceHook: ( diff --git a/src/notebooks/debugger/kernelDebugAdapterBase.ts b/src/notebooks/debugger/kernelDebugAdapterBase.ts index 5d2dba511a9..fa49a77bf23 100644 --- a/src/notebooks/debugger/kernelDebugAdapterBase.ts +++ b/src/notebooks/debugger/kernelDebugAdapterBase.ts @@ -36,7 +36,7 @@ import { IDebuggingDelegate, IDebugInfoResponse, IKernelDebugAdapter, - IKernelDebugAdapterConfig, + INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; import { @@ -60,7 +60,7 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb protected readonly fileToCell = new Map(); private readonly sendMessage = new EventEmitter(); private readonly endSession = new EventEmitter(); - private readonly configuration: IKernelDebugAdapterConfig; + private readonly configuration: INotebookDebugConfig; protected readonly disposables: IDisposable[] = []; private delegate: IDebuggingDelegate | undefined; onDidSendMessage: Event = this.sendMessage.event; @@ -199,7 +199,7 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb } } - public getConfiguration(): IKernelDebugAdapterConfig { + public getConfiguration(): INotebookDebugConfig { return this.configuration; } From 87f3d55a326344bb48b7b4ea6ba26112f1f5e225 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 14 Oct 2022 12:52:32 -0700 Subject: [PATCH 03/10] Hook up Restart for rbl, via a custom Restart request implementation. Move commands out of DebuggingManager, and expose methods on that class for use by the rbl controller --- package.json | 12 -- src/commands.ts | 3 +- ...lControllers.ts => debugCellController.ts} | 6 +- .../debugger/jupyter/debuggingManager.ts | 7 +- src/notebooks/debugger/commandRegistry.ts | 93 ++++++++++++ ...lControllers.ts => debugCellController.ts} | 4 +- src/notebooks/debugger/debuggerVariables.ts | 28 ++-- src/notebooks/debugger/debuggingManager.ts | 134 +++++------------- .../debugger/debuggingManagerBase.ts | 31 ++-- src/notebooks/debugger/debuggingTypes.ts | 28 ++-- .../debugger/kernelDebugAdapterBase.ts | 16 ++- src/notebooks/debugger/runByLineController.ts | 30 +++- src/notebooks/serviceRegistry.node.ts | 59 ++++---- src/notebooks/serviceRegistry.web.ts | 59 ++++---- src/platform/common/constants.ts | 1 - src/test/datascience/debugger.vscode.test.ts | 36 ++--- 16 files changed, 306 insertions(+), 241 deletions(-) rename src/interactive-window/debugger/jupyter/{debugCellControllers.ts => debugCellController.ts} (97%) create mode 100644 src/notebooks/debugger/commandRegistry.ts rename src/notebooks/debugger/{debugCellControllers.ts => debugCellController.ts} (98%) diff --git a/package.json b/package.json index 3de8718173c..fba07b7028c 100644 --- a/package.json +++ b/package.json @@ -288,13 +288,6 @@ "icon": "$(debug-start)", "enablement": "jupyter.development && notebookType == jupyter-notebook && isWorkspaceTrusted && jupyter.replayLogLoaded && !jupyter.webExtension" }, - { - "command": "jupyter.debugNotebook", - "title": "%jupyter.command.jupyter.debug.title%", - "icon": "$(bug)", - "category": "Jupyter", - "enablement": "notebookKernelCount > 0 && resource not in jupyter.notebookeditor.runByLineDocuments" - }, { "command": "jupyter.filterKernels", "title": "%jupyter.command.jupyter.filterKernels.title%", @@ -1099,11 +1092,6 @@ "title": "%jupyter.commandPalette.jupyter.replayPylanceLog.title%", "when": "jupyter.development && isWorkspaceTrusted" }, - { - "command": "jupyter.debugNotebook", - "title": "%jupyter.command.jupyter.debug.title%", - "when": "false" - }, { "command": "jupyter.interactive.copyCell", "when": "false" diff --git a/src/commands.ts b/src/commands.ts index 8ffc0467548..9d81db437e8 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -193,11 +193,10 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu NotebookDocument | undefined ]; [DSCommands.SelectNativeJupyterUriFromToolBar]: []; - [DSCommands.DebugNotebook]: []; [DSCommands.RunByLine]: [NotebookCell]; [DSCommands.RunAndDebugCell]: [NotebookCell]; [DSCommands.RunByLineNext]: [NotebookCell]; - [DSCommands.RunByLineStop]: []; + [DSCommands.RunByLineStop]: [NotebookCell]; [DSCommands.ReplayPylanceLog]: [Uri]; [DSCommands.ReplayPylanceLogStep]: []; [DSCommands.InstallPythonExtensionViaKernelPicker]: []; diff --git a/src/interactive-window/debugger/jupyter/debugCellControllers.ts b/src/interactive-window/debugger/jupyter/debugCellController.ts similarity index 97% rename from src/interactive-window/debugger/jupyter/debugCellControllers.ts rename to src/interactive-window/debugger/jupyter/debugCellController.ts index bae98706e17..3035a4c918a 100644 --- a/src/interactive-window/debugger/jupyter/debugCellControllers.ts +++ b/src/interactive-window/debugger/jupyter/debugCellController.ts @@ -5,7 +5,7 @@ import { NotebookCell } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; import { IKernel } from '../../../kernels/types'; import { DebuggingTelemetry } from '../../../notebooks/debugger/constants'; -import { isJustMyCodeNotification } from '../../../notebooks/debugger/debugCellControllers'; +import { isJustMyCodeNotification } from '../../../notebooks/debugger/debugCellController'; import { IDebuggingDelegate, IKernelDebugAdapter } from '../../../notebooks/debugger/debuggingTypes'; import { cellDebugSetup } from '../../../notebooks/debugger/helper'; import { traceVerbose } from '../../../platform/logging'; @@ -42,7 +42,7 @@ export class DebugCellController implements IDebuggingDelegate { } private debugCellDumped?: Promise; - public async willSendRequest(request: DebugProtocol.Request): Promise { + public async willSendRequest(request: DebugProtocol.Request): Promise { const metadata = getInteractiveCellMetadata(this.debugCell); if (request.command === 'setBreakpoints' && metadata && metadata.generatedCode && !this.cellDumpInvoked) { if (!this.debugCellDumped) { @@ -56,5 +56,7 @@ export class DebugCellController implements IDebuggingDelegate { } await this.debugCellDumped; } + + return false; } } diff --git a/src/interactive-window/debugger/jupyter/debuggingManager.ts b/src/interactive-window/debugger/jupyter/debuggingManager.ts index 9015d84f3dc..33a50d7ea41 100644 --- a/src/interactive-window/debugger/jupyter/debuggingManager.ts +++ b/src/interactive-window/debugger/jupyter/debuggingManager.ts @@ -18,7 +18,6 @@ import { pythonIWKernelDebugAdapter } from '../../../notebooks/debugger/constant import { Debugger } from '../../../notebooks/debugger/debugger'; import { DebuggingManagerBase } from '../../../notebooks/debugger/debuggingManagerBase'; import { - IDebuggingManager, IDebugLocationTrackerFactory, IInteractiveWindowDebugConfig, KernelDebugMode @@ -41,7 +40,7 @@ import * as path from '../../../platform/vscode-path/path'; import { IFileGeneratedCodes } from '../../editor-integration/types'; import { IInteractiveWindowDebuggingManager } from '../../types'; import { buildSourceMap } from '../helper'; -import { DebugCellController } from './debugCellControllers'; +import { DebugCellController } from './debugCellController'; import { KernelDebugAdapter } from './kernelDebugAdapter'; /** @@ -50,7 +49,7 @@ import { KernelDebugAdapter } from './kernelDebugAdapter'; @injectable() export class InteractiveWindowDebuggingManager extends DebuggingManagerBase - implements IExtensionSingleActivationService, IDebuggingManager, IInteractiveWindowDebuggingManager + implements IExtensionSingleActivationService, IInteractiveWindowDebuggingManager { public constructor( @inject(IKernelProvider) kernelProvider: IKernelProvider, @@ -94,7 +93,7 @@ export class InteractiveWindowDebuggingManager public async start(editor: NotebookEditor, cell: NotebookCell) { traceInfoIfCI(`Starting debugging IW`); - const ipykernelResult = await this.checkIpykernelAndPrompt(editor); + const ipykernelResult = await this.checkIpykernelAndPrompt(cell); if (ipykernelResult === IpykernelCheckResult.Ok) { await this.startDebuggingCell(editor.notebook, cell); } diff --git a/src/notebooks/debugger/commandRegistry.ts b/src/notebooks/debugger/commandRegistry.ts new file mode 100644 index 00000000000..182b0cd6aa6 --- /dev/null +++ b/src/notebooks/debugger/commandRegistry.ts @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import { NotebookCell } from 'vscode'; +import { ICommandManager, IVSCodeNotebook } from '../../platform/common/application/types'; + +import { IExtensionSingleActivationService } from '../../platform/activation/types'; +import { Commands } from '../../platform/common/constants'; +import { IDisposable, IDisposableRegistry } from '../../platform/common/types'; +import { sendTelemetryEvent } from '../../telemetry'; +import { DebuggingTelemetry } from './constants'; +import { INotebookDebuggingManager, KernelDebugMode } from './debuggingTypes'; + +/** + * Class that registers command handlers for interactive window commands. + */ +@injectable() +export class CommandRegistry implements IDisposable, IExtensionSingleActivationService { + constructor( + @inject(INotebookDebuggingManager) private readonly debuggingManager: INotebookDebuggingManager, + @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(ICommandManager) private readonly commandManager: ICommandManager + ) {} + + public async activate(): Promise { + this.disposables.push(this.commandManager.registerCommand(Commands.RunByLine, this.runByLine, this)); + this.disposables.push(this.commandManager.registerCommand(Commands.RunByLineNext, this.runByLineNext, this)); + this.disposables.push(this.commandManager.registerCommand(Commands.RunByLineStop, this.runByLineStop, this)); + this.disposables.push( + this.commandManager.registerCommand(Commands.RunAndDebugCell, this.runAndDebugCell, this) + ); + } + + private async runByLine(cell: NotebookCell | undefined) { + sendTelemetryEvent(DebuggingTelemetry.clickedRunByLine); + cell ??= this.getCellFromActiveEditor(); + if (!cell) { + return; + } + + if (!cell) { + return; + } + + await this.debuggingManager.tryToStartDebugging(KernelDebugMode.RunByLine, cell); + } + + private async runByLineNext(cell: NotebookCell | undefined) { + cell ??= this.getCellFromActiveEditor(); + if (!cell) { + return; + } + + this.debuggingManager.runByLineNext(cell); + } + + private async runByLineStop(cell: NotebookCell | undefined) { + cell ??= this.getCellFromActiveEditor(); + if (!cell) { + return; + } + + this.debuggingManager.runByLineStop(cell); + } + + private async runAndDebugCell(cell: NotebookCell | undefined) { + sendTelemetryEvent(DebuggingTelemetry.clickedRunAndDebugCell); + cell ??= this.getCellFromActiveEditor(); + if (!cell) { + return; + } + + await this.debuggingManager.tryToStartDebugging(KernelDebugMode.Cell, cell); + } + + private getCellFromActiveEditor(): NotebookCell | undefined { + const editor = this.vscNotebook.activeNotebookEditor; + if (editor) { + const range = editor.selections[0]; + if (range) { + return editor.notebook.cellAt(range.start); + } + } + } + + public dispose() { + this.disposables.forEach((d) => d.dispose()); + } +} diff --git a/src/notebooks/debugger/debugCellControllers.ts b/src/notebooks/debugger/debugCellController.ts similarity index 98% rename from src/notebooks/debugger/debugCellControllers.ts rename to src/notebooks/debugger/debugCellController.ts index 14396012554..289a845bac5 100644 --- a/src/notebooks/debugger/debugCellControllers.ts +++ b/src/notebooks/debugger/debugCellController.ts @@ -44,7 +44,7 @@ export class DebugCellController implements IDebuggingDelegate { return false; } - public async willSendRequest(request: DebugProtocol.Request): Promise { + public async willSendRequest(request: DebugProtocol.Request): Promise { if (request.command === 'configurationDone') { await cellDebugSetup(this.kernel, this.debugAdapter); @@ -55,5 +55,7 @@ export class DebugCellController implements IDebuggingDelegate { }) .then(noop, noop); } + + return false; } } diff --git a/src/notebooks/debugger/debuggerVariables.ts b/src/notebooks/debugger/debuggerVariables.ts index d76a64e6100..a7326049a1d 100644 --- a/src/notebooks/debugger/debuggerVariables.ts +++ b/src/notebooks/debugger/debuggerVariables.ts @@ -8,19 +8,8 @@ import * as uriPath from '../../platform/vscode-path/resources'; import { DebugAdapterTracker, Disposable, Event, EventEmitter } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; -import { Identifiers } from '../../platform/common/constants'; -import { IDebugService, IVSCodeNotebook } from '../../platform/common/application/types'; -import { traceError, traceVerbose } from '../../platform/logging'; -import { - IConfigurationService, - IDataFrameScriptGenerator, - IVariableScriptGenerator, - Resource -} from '../../platform/common/types'; -import { DebugLocationTracker } from './debugLocationTracker'; -import { sendTelemetryEvent, Telemetry } from '../../telemetry'; -import { IDebuggingManager, IJupyterDebugService, KernelDebugMode } from './debuggingTypes'; import { IKernel } from '../../kernels/types'; +import { convertDebugProtocolVariableToIJupyterVariable, DataViewableTypes } from '../../kernels/variables/helpers'; import { parseDataFrame } from '../../kernels/variables/pythonVariableRequester'; import { IConditionalJupyterVariables, @@ -28,8 +17,19 @@ import { IJupyterVariablesRequest, IJupyterVariablesResponse } from '../../kernels/variables/types'; -import { convertDebugProtocolVariableToIJupyterVariable, DataViewableTypes } from '../../kernels/variables/helpers'; +import { IDebugService, IVSCodeNotebook } from '../../platform/common/application/types'; +import { Identifiers } from '../../platform/common/constants'; +import { + IConfigurationService, + IDataFrameScriptGenerator, + IVariableScriptGenerator, + Resource +} from '../../platform/common/types'; import { noop } from '../../platform/common/utils/misc'; +import { traceError, traceVerbose } from '../../platform/logging'; +import { sendTelemetryEvent, Telemetry } from '../../telemetry'; +import { IJupyterDebugService, INotebookDebuggingManager, KernelDebugMode } from './debuggingTypes'; +import { DebugLocationTracker } from './debugLocationTracker'; const KnownExcludedVariables = new Set(['In', 'Out', 'exit', 'quit']); const MaximumRowChunkSizeForDebugger = 100; @@ -53,7 +53,7 @@ export class DebuggerVariables constructor( @inject(IJupyterDebugService) @named(Identifiers.MULTIPLEXING_DEBUGSERVICE) private debugService: IDebugService, - @inject(IDebuggingManager) private readonly debuggingManager: IDebuggingManager, + @inject(INotebookDebuggingManager) private readonly debuggingManager: INotebookDebuggingManager, @inject(IConfigurationService) private configService: IConfigurationService, @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook, @inject(IVariableScriptGenerator) private readonly varScriptGenerator: IVariableScriptGenerator, diff --git a/src/notebooks/debugger/debuggingManager.ts b/src/notebooks/debugger/debuggingManager.ts index fb9340618a1..8f7915e7250 100644 --- a/src/notebooks/debugger/debuggingManager.ts +++ b/src/notebooks/debugger/debuggingManager.ts @@ -12,7 +12,6 @@ import { DebugSessionOptions, NotebookCell, NotebookDocument, - NotebookEditor, Uri } from 'vscode'; import { IKernelProvider } from '../../kernels/types'; @@ -23,7 +22,7 @@ import { IDebugService, IVSCodeNotebook } from '../../platform/common/application/types'; -import { Commands as DSCommands, EditorContexts } from '../../platform/common/constants'; +import { EditorContexts } from '../../platform/common/constants'; import { ContextKey } from '../../platform/common/contextKey'; import { IPlatformService } from '../../platform/common/platform/types'; import { IConfigurationService } from '../../platform/common/types'; @@ -36,7 +35,7 @@ import * as path from '../../platform/vscode-path/path'; import { sendTelemetryEvent } from '../../telemetry'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; import { DebuggingTelemetry, pythonKernelDebugAdapter } from './constants'; -import { DebugCellController } from './debugCellControllers'; +import { DebugCellController } from './debugCellController'; import { Debugger } from './debugger'; import { DebuggingManagerBase } from './debuggingManagerBase'; import { IDebuggingManager, INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; @@ -89,76 +88,6 @@ export class DebuggingManager // factory for kernel debug adapters debug.registerDebugAdapterDescriptorFactory(pythonKernelDebugAdapter, { createDebugAdapterDescriptor: async (session) => this.createDebugAdapterDescriptor(session) - }), - this.commandManager.registerCommand(DSCommands.DebugNotebook, async () => { - const editor = this.vscNotebook.activeNotebookEditor; - await this.tryToStartDebugging(KernelDebugMode.Everything, editor); - }), - - this.commandManager.registerCommand(DSCommands.RunByLine, async (cell: NotebookCell | undefined) => { - sendTelemetryEvent(DebuggingTelemetry.clickedRunByLine); - const editor = this.vscNotebook.activeNotebookEditor; - if (!cell) { - const range = editor?.selections[0]; - if (range) { - cell = editor?.notebook.cellAt(range.start); - } - } - - if (!cell) { - return; - } - - await this.tryToStartDebugging(KernelDebugMode.RunByLine, editor, cell); - }), - - this.commandManager.registerCommand(DSCommands.RunByLineNext, (cell: NotebookCell | undefined) => { - if (!cell) { - const editor = this.vscNotebook.activeNotebookEditor; - const range = editor?.selections[0]; - if (range) { - cell = editor?.notebook.cellAt(range.start); - } - } - - if (!cell) { - return; - } - - const controller = this.notebookToRunByLineController.get(cell.notebook); - if (controller && controller.debugCell.document.uri.toString() === cell.document.uri.toString()) { - controller.continue(); - } - }), - - this.commandManager.registerCommand(DSCommands.RunByLineStop, () => { - const editor = this.vscNotebook.activeNotebookEditor; - if (editor) { - const controller = this.notebookToRunByLineController.get(editor.notebook); - if (controller) { - sendTelemetryEvent(DebuggingTelemetry.endedSession, undefined, { - reason: 'withKeybinding' - }); - controller.stop(); - } - } - }), - - this.commandManager.registerCommand(DSCommands.RunAndDebugCell, async (cell: NotebookCell | undefined) => { - sendTelemetryEvent(DebuggingTelemetry.clickedRunAndDebugCell); - const editor = this.vscNotebook.activeNotebookEditor; - if (!cell) { - const range = editor?.selections[0]; - if (range) { - cell = editor?.notebook.cellAt(range.start); - } - } - - if (!cell) { - return; - } - - await this.tryToStartDebugging(KernelDebugMode.Cell, editor, cell); }) ); } @@ -194,29 +123,36 @@ export class DebuggingManager this.debugDocuments.set(Array.from(debugDocumentUris.values())).ignoreErrors(); } - private async tryToStartDebugging(mode: KernelDebugMode, editor?: NotebookEditor, cell?: NotebookCell) { + public async tryToStartDebugging(mode: KernelDebugMode, cell: NotebookCell) { traceInfoIfCI(`Starting debugging with mode ${mode}`); - if (!editor) { - this.appShell.showErrorMessage(DataScience.noNotebookToDebug()).then(noop, noop); - return; - } - - const ipykernelResult = await this.checkIpykernelAndPrompt(editor); + const ipykernelResult = await this.checkIpykernelAndPrompt(cell); if (ipykernelResult === IpykernelCheckResult.Ok) { if (mode === KernelDebugMode.RunByLine || mode === KernelDebugMode.Cell) { - await this.startDebuggingCell(editor.notebook, mode, cell!); - } else { - await this.startDebugging(editor.notebook); + await this.startDebuggingCell(mode, cell!); } } } - private async startDebuggingCell( - doc: NotebookDocument, - mode: KernelDebugMode.Cell | KernelDebugMode.RunByLine, - cell: NotebookCell - ) { + public runByLineNext(cell: NotebookCell) { + const controller = this.notebookToRunByLineController.get(cell.notebook); + if (controller && controller.debugCell.document.uri.toString() === cell.document.uri.toString()) { + controller.continue(); + } + } + + public runByLineStop(cell: NotebookCell) { + const controller = this.notebookToRunByLineController.get(cell.notebook); + if (controller) { + sendTelemetryEvent(DebuggingTelemetry.endedSession, undefined, { + reason: 'withKeybinding' + }); + controller.stop(); + } + } + + private async startDebuggingCell(mode: KernelDebugMode.Cell | KernelDebugMode.RunByLine, cell: NotebookCell) { + const doc = cell.notebook; const config: INotebookDebugConfig = { type: pythonKernelDebugAdapter, name: path.basename(doc.uri.toString()), @@ -239,19 +175,6 @@ export class DebuggingManager return this.startDebuggingConfig(config, opts); } - private async startDebugging(doc: NotebookDocument) { - const config: INotebookDebugConfig = { - type: pythonKernelDebugAdapter, - name: path.basename(doc.uri.toString()), - request: 'attach', - internalConsoleOptions: 'neverOpen', - justMyCode: false, - __mode: KernelDebugMode.Everything, - __notebookUri: doc.uri.toString() - }; - return this.startDebuggingConfig(config); - } - protected async createDebugAdapterDescriptor(session: DebugSession): Promise { const config = session.configuration; assertIsDebugConfig(config); @@ -303,7 +226,14 @@ export class DebuggingManager if (config.__mode === KernelDebugMode.RunByLine && typeof config.__cellIndex === 'number') { const cell = notebook.cellAt(config.__cellIndex); - const controller = new RunByLineController(adapter, cell, this.commandManager, kernel!, this.settings); + const controller = new RunByLineController( + adapter, + cell, + this.commandManager, + kernel!, + this.settings, + this.serviceContainer + ); adapter.setDebuggingDelegate(controller); this.notebookToRunByLineController.set(notebook, controller); this.updateRunByLineContextKeys(); diff --git a/src/notebooks/debugger/debuggingManagerBase.ts b/src/notebooks/debugger/debuggingManagerBase.ts index 42356c0f6e7..3736ff38417 100644 --- a/src/notebooks/debugger/debuggingManagerBase.ts +++ b/src/notebooks/debugger/debuggingManagerBase.ts @@ -12,19 +12,20 @@ import { NotebookCell, NotebookDocument, NotebookEditor, - workspace + workspace, + window } from 'vscode'; import { IKernel, IKernelProvider } from '../../kernels/types'; import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../platform/common/application/types'; import { IDisposable } from '../../platform/common/types'; import { DataScience } from '../../platform/common/utils/localize'; import { noop } from '../../platform/common/utils/misc'; -import { traceError, traceInfoIfCI } from '../../platform/logging'; +import { traceError, traceInfo, traceInfoIfCI } from '../../platform/logging'; import { sendTelemetryEvent } from '../../telemetry'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; import { DebuggingTelemetry } from './constants'; import { Debugger } from './debugger'; -import { INotebookDebugConfig } from './debuggingTypes'; +import { IDebuggingManager, INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; import { IpykernelCheckResult, isUsingIpykernel6OrLater } from './helper'; import { KernelDebugAdapterBase } from './kernelDebugAdapterBase'; import { KernelConnector } from '../controllers/kernelConnector'; @@ -34,7 +35,7 @@ import { DisplayOptions } from '../../kernels/displayOptions'; /** * The DebuggingManager maintains the mapping between notebook documents and debug sessions. */ -export abstract class DebuggingManagerBase implements IDisposable { +export abstract class DebuggingManagerBase implements IDisposable, IDebuggingManager { protected notebookToDebugger = new Map(); protected notebookToDebugAdapter = new Map(); protected notebookInProgress = new Set(); @@ -66,6 +67,9 @@ export abstract class DebuggingManagerBase implements IDisposable { }) ); } + + abstract getDebugMode(notebook: NotebookDocument): KernelDebugMode | undefined; + public getDebugCell(notebook: NotebookDocument): NotebookCell | undefined { return this.notebookToDebugAdapter.get(notebook)?.debugCell; } @@ -114,10 +118,10 @@ export abstract class DebuggingManagerBase implements IDisposable { } protected async endSession(session: DebugSession) { - traceInfoIfCI(`Ending debug session ${session.id}`); + traceInfo(`Ending debug session ${session.id}`); this._doneDebugging.fire(); for (const [doc, dbg] of this.notebookToDebugger.entries()) { - if (dbg && session.id === (await dbg.session).id) { + if (dbg && session.id === dbg.session.id) { this.notebookToDebugger.delete(doc); this.notebookToDebugAdapter.delete(doc); this.onDidStopDebugging(doc); @@ -155,10 +159,21 @@ export abstract class DebuggingManagerBase implements IDisposable { return kernel; } + private findEditorForCell(cell: NotebookCell): NotebookEditor | undefined { + const notebookUri = cell.notebook.uri.toString(); + return window.visibleNotebookEditors.find((e) => e.notebook.uri.toString() === notebookUri.toString()); + } + protected async checkIpykernelAndPrompt( - editor: NotebookEditor, + cell: NotebookCell, allowSelectKernel: boolean = true ): Promise { + const editor = this.findEditorForCell(cell); + if (!editor) { + this.appShell.showErrorMessage(DataScience.noNotebookToDebug()).then(noop, noop); + return IpykernelCheckResult.Unknown; + } + const ipykernelResult = await this.checkForIpykernel6(editor.notebook); switch (ipykernelResult) { case IpykernelCheckResult.NotInstalled: @@ -172,7 +187,7 @@ export abstract class DebuggingManagerBase implements IDisposable { case IpykernelCheckResult.ControllerNotSelected: { if (allowSelectKernel) { await this.commandManager.executeCommand('notebook.selectKernel', { notebookEditor: editor }); - await this.checkIpykernelAndPrompt(editor, false); + return await this.checkIpykernelAndPrompt(cell, false); } } } diff --git a/src/notebooks/debugger/debuggingTypes.ts b/src/notebooks/debugger/debuggingTypes.ts index 26d9bad66fa..ac633b92ab4 100644 --- a/src/notebooks/debugger/debuggingTypes.ts +++ b/src/notebooks/debugger/debuggingTypes.ts @@ -71,7 +71,6 @@ export interface IKernelDebugAdapter extends DebugAdapter { getConfiguration(): IBaseNotebookDebugConfig; } -export const IDebuggingManager = Symbol('IDebuggingManager'); export interface IDebuggingManager { readonly onDoneDebugging: Event; isDebugging(notebook: NotebookDocument): boolean; @@ -81,6 +80,13 @@ export interface IDebuggingManager { getDebugAdapter(notebook: NotebookDocument): IKernelDebugAdapter | undefined; } +export const INotebookDebuggingManager = Symbol('INotebookDebuggingManager'); +export interface INotebookDebuggingManager extends IDebuggingManager { + tryToStartDebugging(mode: KernelDebugMode, cell: NotebookCell): Promise; + runByLineNext(cell: NotebookCell): void; + runByLineStop(cell: NotebookCell): void; +} + export interface IDebuggingDelegate { /** * Called for every event sent from the debug adapter to the client. Returns true to signal that sending the message is vetoed. @@ -88,9 +94,14 @@ export interface IDebuggingDelegate { willSendEvent?(msg: DebugProtocol.Event): Promise; /** - * Called for every request sent from the client to the debug adapter. + * Called for every request sent from the client to the debug adapter. Returns true to signal that the request was handled by the delegate. */ - willSendRequest?(request: DebugProtocol.Request): Promise; + willSendRequest?(request: DebugProtocol.Request): Promise; + + /** + * Called for every response returned from the debug adapter to the client. + */ + willSendResponse?(request: DebugProtocol.Response): Promise; } export interface IDumpCellResponse { @@ -115,7 +126,6 @@ export interface IDebugInfoResponseBreakpoint { export enum KernelDebugMode { RunByLine, Cell, - Everything, InteractiveWindow } @@ -124,11 +134,7 @@ interface IBaseNotebookDebugConfig extends DebugConfiguration { __notebookUri: string; } -export type INotebookDebugConfig = - | IRunByLineDebugConfig - | ICellDebugConfig - | IEverythingDebugConfig - | IInteractiveWindowDebugConfig; +export type INotebookDebugConfig = IRunByLineDebugConfig | ICellDebugConfig | IInteractiveWindowDebugConfig; export interface IRunByLineDebugConfig extends IBaseNotebookDebugConfig { __mode: KernelDebugMode.RunByLine; @@ -140,10 +146,6 @@ export interface ICellDebugConfig extends IBaseNotebookDebugConfig { __cellIndex: number; } -export interface IEverythingDebugConfig extends IBaseNotebookDebugConfig { - __mode: KernelDebugMode.Everything; -} - export interface IInteractiveWindowDebugConfig extends IBaseNotebookDebugConfig { __mode: KernelDebugMode.InteractiveWindow; __cellIndex: number; diff --git a/src/notebooks/debugger/kernelDebugAdapterBase.ts b/src/notebooks/debugger/kernelDebugAdapterBase.ts index fa49a77bf23..5b7aa4257c5 100644 --- a/src/notebooks/debugger/kernelDebugAdapterBase.ts +++ b/src/notebooks/debugger/kernelDebugAdapterBase.ts @@ -165,6 +165,10 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb } } } + + /** + * Handle a message from the client to the debug adapter. + */ async handleMessage(message: DebugProtocol.ProtocolMessage) { try { traceInfoIfCI(`KernelDebugAdapter::handleMessage ${JSON.stringify(message, undefined, ' ')}`); @@ -300,10 +304,14 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb true ); - control.onReply = (msg) => { - const message = msg.content as DebugProtocol.ProtocolMessage; + control.onReply = async (msg) => { + const message = msg.content as DebugProtocol.Response; getMessageSourceAndHookIt(message, this.translateDebuggerFileToRealFile.bind(this)); + if (this.delegate?.willSendResponse) { + await this.delegate?.willSendResponse(message); + } + this.trace('response', JSON.stringify(message)); this.sendMessage.fire(message); }; @@ -343,7 +351,7 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb ): void; protected abstract getDumpFilesForDeletion(): string[]; - private async deleteDumpedFiles() { + private async deleteDumpedFiles(): Promise { const fileValues = this.getDumpFilesForDeletion(); // Need to have our Jupyter Session and some dumpCell files to delete if (this.jupyterSession && fileValues.length) { @@ -365,7 +373,7 @@ for file in _VSCODE_fileList: pass del _VSCODE_fileList`; - return executeSilently(this.jupyterSession, deleteFilesCode, { + await executeSilently(this.jupyterSession, deleteFilesCode, { traceErrors: true, traceErrorsMessage: 'Error deleting temporary debugging files' }); diff --git a/src/notebooks/debugger/runByLineController.ts b/src/notebooks/debugger/runByLineController.ts index 4635e777f2f..2c165f082a5 100644 --- a/src/notebooks/debugger/runByLineController.ts +++ b/src/notebooks/debugger/runByLineController.ts @@ -9,12 +9,13 @@ import { Commands } from '../../platform/common/constants'; import { IConfigurationService } from '../../platform/common/types'; import { parseForComments } from '../../platform/common/utils'; import { noop } from '../../platform/common/utils/misc'; +import { IServiceContainer } from '../../platform/ioc/types'; import { traceInfoIfCI, traceVerbose } from '../../platform/logging'; import * as path from '../../platform/vscode-path/path'; import { sendTelemetryEvent } from '../../telemetry'; import { DebuggingTelemetry } from './constants'; -import { isJustMyCodeNotification } from './debugCellControllers'; -import { IDebuggingDelegate, IKernelDebugAdapter, KernelDebugMode } from './debuggingTypes'; +import { isJustMyCodeNotification } from './debugCellController'; +import { IDebuggingDelegate, IKernelDebugAdapter, INotebookDebuggingManager, KernelDebugMode } from './debuggingTypes'; import { cellDebugSetup } from './helper'; /** @@ -22,15 +23,18 @@ import { cellDebugSetup } from './helper'; */ export class RunByLineController implements IDebuggingDelegate { private lastPausedThreadId: number | undefined; + private debuggingManager: INotebookDebuggingManager; constructor( private readonly debugAdapter: IKernelDebugAdapter, public readonly debugCell: NotebookCell, private readonly commandManager: ICommandManager, private readonly kernel: IKernel, - private readonly settings: IConfigurationService + private readonly settings: IConfigurationService, + private readonly serviceContainer: IServiceContainer ) { sendTelemetryEvent(DebuggingTelemetry.successfullyStartedRunByLine); + this.debuggingManager = this.serviceContainer.get(INotebookDebuggingManager); } public continue(): void { @@ -74,11 +78,25 @@ export class RunByLineController implements IDebuggingDelegate { return false; } - public async willSendRequest(request: DebugProtocol.Request): Promise { + public async willSendRequest(request: DebugProtocol.Request): Promise { traceInfoIfCI(`willSendRequest: ${request.command}`); if (request.command === 'configurationDone') { await this.initializeExecute(); } + + if (request.command === 'restart') { + await this.debugAdapter.disconnect(); + this.doRestart().then(noop, noop); + return true; + } + + return false; + } + + public async willSendResponse(response: DebugProtocol.Response): Promise { + if (response.command === 'initialize' && response.body) { + (response as DebugProtocol.InitializeResponse).body!.supportsRestartRequest = true; + } } private async handleStoppedEvent(threadId: number): Promise { @@ -154,4 +172,8 @@ export class RunByLineController implements IDebuggingDelegate { }) .then(noop, noop); } + + private async doRestart() { + return this.debuggingManager.tryToStartDebugging!(KernelDebugMode.RunByLine, this.debugCell); + } } diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index 160c1352144..1b90d401d36 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -3,36 +3,36 @@ 'use strict'; +import { ITracebackFormatter } from '../kernels/types'; +import { IJupyterVariables } from '../kernels/variables/types'; import { IExtensionSingleActivationService, IExtensionSyncActivationService } from '../platform/activation/types'; +import { Identifiers } from '../platform/common/constants'; +import { IConfigurationService, IDataScienceCommandListener } from '../platform/common/types'; import { IServiceManager } from '../platform/ioc/types'; +import { InstallPythonControllerCommands } from './controllers/commands/installPythonControllerCommands'; +import { ServerConnectionControllerCommands } from './controllers/commands/serverConnectionControllerCommands'; import { KernelFilterService } from './controllers/kernelFilter/kernelFilterService'; import { KernelFilterUI } from './controllers/kernelFilter/kernelFilterUI'; import { LiveKernelSwitcher } from './controllers/liveKernelSwitcher'; -import { RemoteSwitcher } from './controllers/remoteSwitcher'; -import { NotebookCommandListener } from './notebookCommandListener'; -import { NotebookEditorProvider } from './notebookEditorProvider'; -import { ErrorRendererCommunicationHandler } from './outputs/errorRendererComms'; -import { INotebookCompletionProvider, INotebookEditorProvider } from './types'; -import { IConfigurationService, IDataScienceCommandListener } from '../platform/common/types'; +import { NotebookIPyWidgetCoordinator } from './controllers/notebookIPyWidgetCoordinator'; +import { RemoteKernelConnectionHandler } from './controllers/remoteKernelConnectionHandler'; import { RemoteKernelControllerWatcher } from './controllers/remoteKernelControllerWatcher'; -import { ITracebackFormatter } from '../kernels/types'; -import { NotebookTracebackFormatter } from './outputs/tracebackFormatter'; +import { RemoteSwitcher } from './controllers/remoteSwitcher'; +import { registerTypes as registerControllerTypes } from './controllers/serviceRegistry.node'; +import { CommandRegistry } from './debugger/commandRegistry'; +import { DebuggerVariableRegistration } from './debugger/debuggerVariableRegistration.node'; +import { DebuggerVariables } from './debugger/debuggerVariables'; +import { DebuggingManager } from './debugger/debuggingManager'; import { IDebuggingManager, IDebugLocationTracker, IDebugLocationTrackerFactory, - IJupyterDebugService + IJupyterDebugService, + INotebookDebuggingManager } from './debugger/debuggingTypes'; -import { Identifiers } from '../platform/common/constants'; +import { DebugLocationTrackerFactory } from './debugger/debugLocationTrackerFactory'; import { JupyterDebugService } from './debugger/jupyterDebugService.node'; -import { NotebookIPyWidgetCoordinator } from './controllers/notebookIPyWidgetCoordinator'; -import { RemoteKernelConnectionHandler } from './controllers/remoteKernelConnectionHandler'; -import { JupyterServerSelectorCommand } from './serverSelectorCommand'; -import { InterpreterPackageTracker } from './telemetry/interpreterPackageTracker'; -import { InstallPythonControllerCommands } from './controllers/commands/installPythonControllerCommands'; -import { NotebookCellLanguageService } from './languages/cellLanguageService'; -import { EmptyNotebookCellLanguageService } from './languages/emptyNotebookCellLanguageService'; -import { DebuggingManager } from './debugger/debuggingManager'; +import { MultiplexingDebugService } from './debugger/multiplexingDebugService'; import { ExportBase } from './export/exportBase.node'; import { ExportDialog } from './export/exportDialog'; import { ExportFileOpener } from './export/exportFileOpener'; @@ -41,17 +41,19 @@ import { ExportToHTML } from './export/exportToHTML'; import { ExportToPDF } from './export/exportToPDF'; import { ExportToPython } from './export/exportToPython'; import { ExportToPythonPlain } from './export/exportToPythonPlain'; +import { ExportUtilBase } from './export/exportUtil'; import { ExportUtil } from './export/exportUtil.node'; import { FileConverter } from './export/fileConverter.node'; -import { IFileConverter, INbConvertExport, ExportFormat, IExport, IExportDialog, IExportBase } from './export/types'; -import { ExportUtilBase } from './export/exportUtil'; -import { registerTypes as registerControllerTypes } from './controllers/serviceRegistry.node'; -import { ServerConnectionControllerCommands } from './controllers/commands/serverConnectionControllerCommands'; -import { DebuggerVariableRegistration } from './debugger/debuggerVariableRegistration.node'; -import { IJupyterVariables } from '../kernels/variables/types'; -import { DebuggerVariables } from './debugger/debuggerVariables'; -import { MultiplexingDebugService } from './debugger/multiplexingDebugService'; -import { DebugLocationTrackerFactory } from './debugger/debugLocationTrackerFactory'; +import { ExportFormat, IExport, IExportBase, IExportDialog, IFileConverter, INbConvertExport } from './export/types'; +import { NotebookCellLanguageService } from './languages/cellLanguageService'; +import { EmptyNotebookCellLanguageService } from './languages/emptyNotebookCellLanguageService'; +import { NotebookCommandListener } from './notebookCommandListener'; +import { NotebookEditorProvider } from './notebookEditorProvider'; +import { ErrorRendererCommunicationHandler } from './outputs/errorRendererComms'; +import { NotebookTracebackFormatter } from './outputs/tracebackFormatter'; +import { JupyterServerSelectorCommand } from './serverSelectorCommand'; +import { InterpreterPackageTracker } from './telemetry/interpreterPackageTracker'; +import { INotebookCompletionProvider, INotebookEditorProvider } from './types'; import { PickDocumentKernelSourceCommand } from './controllers/commands/pickDocumentKernelSourceCommand'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { @@ -115,7 +117,7 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea ); // Debugging - serviceManager.addSingleton(IDebuggingManager, DebuggingManager, undefined, [ + serviceManager.addSingleton(INotebookDebuggingManager, DebuggingManager, undefined, [ IExtensionSingleActivationService ]); serviceManager.addSingleton( @@ -135,6 +137,7 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea serviceManager.addSingleton(IDebugLocationTracker, DebugLocationTrackerFactory, undefined, [ IDebugLocationTrackerFactory ]); + serviceManager.addSingleton(IExtensionSingleActivationService, CommandRegistry); // File export/import serviceManager.addSingleton(IFileConverter, FileConverter); diff --git a/src/notebooks/serviceRegistry.web.ts b/src/notebooks/serviceRegistry.web.ts index 0cc5f93cf67..6d124366eab 100644 --- a/src/notebooks/serviceRegistry.web.ts +++ b/src/notebooks/serviceRegistry.web.ts @@ -3,50 +3,52 @@ 'use strict'; +import { ITracebackFormatter } from '../kernels/types'; +import { IJupyterVariables } from '../kernels/variables/types'; import { IExtensionSingleActivationService, IExtensionSyncActivationService } from '../platform/activation/types'; +import { Identifiers } from '../platform/common/constants'; +import { IConfigurationService, IDataScienceCommandListener } from '../platform/common/types'; import { IServiceManager } from '../platform/ioc/types'; +import { ServerConnectionControllerCommands } from './controllers/commands/serverConnectionControllerCommands'; import { KernelFilterService } from './controllers/kernelFilter/kernelFilterService'; import { KernelFilterUI } from './controllers/kernelFilter/kernelFilterUI'; import { LiveKernelSwitcher } from './controllers/liveKernelSwitcher'; -import { RemoteSwitcher } from './controllers/remoteSwitcher'; -import { INotebookEditorProvider } from './types'; -import { NotebookEditorProvider } from './notebookEditorProvider'; -import { RemoteKernelControllerWatcher } from './controllers/remoteKernelControllerWatcher'; -import { ITracebackFormatter } from '../kernels/types'; -import { NotebookTracebackFormatter } from './outputs/tracebackFormatter'; import { NotebookIPyWidgetCoordinator } from './controllers/notebookIPyWidgetCoordinator'; import { RemoteKernelConnectionHandler } from './controllers/remoteKernelConnectionHandler'; -import { JupyterServerSelectorCommand } from './serverSelectorCommand'; -import { IConfigurationService, IDataScienceCommandListener } from '../platform/common/types'; -import { NotebookCommandListener } from './notebookCommandListener'; -import { InterpreterPackageTracker } from './telemetry/interpreterPackageTracker'; -import { NotebookCellLanguageService } from './languages/cellLanguageService'; -import { EmptyNotebookCellLanguageService } from './languages/emptyNotebookCellLanguageService'; +import { RemoteKernelControllerWatcher } from './controllers/remoteKernelControllerWatcher'; +import { RemoteSwitcher } from './controllers/remoteSwitcher'; +import { registerTypes as registerControllerTypes } from './controllers/serviceRegistry.web'; +import { CommandRegistry } from './debugger/commandRegistry'; +import { DebuggerVariables } from './debugger/debuggerVariables'; +import { DebuggingManager } from './debugger/debuggingManager'; import { IDebuggingManager, IDebugLocationTracker, IDebugLocationTrackerFactory, - IJupyterDebugService + IJupyterDebugService, + INotebookDebuggingManager } from './debugger/debuggingTypes'; -import { DebuggingManager } from './debugger/debuggingManager'; -import { ErrorRendererCommunicationHandler } from './outputs/errorRendererComms'; +import { DebugLocationTrackerFactory } from './debugger/debugLocationTrackerFactory'; +import { MultiplexingDebugService } from './debugger/multiplexingDebugService'; +import { ExportBase } from './export/exportBase.web'; import { ExportDialog } from './export/exportDialog'; -import { ExportFormat, IExport, IExportBase, IExportDialog, IFileConverter, INbConvertExport } from './export/types'; -import { FileConverter } from './export/fileConverter'; import { ExportFileOpener } from './export/exportFileOpener'; -import { ExportToPythonPlain } from './export/exportToPythonPlain'; -import { ExportBase } from './export/exportBase.web'; -import { ExportUtilBase } from './export/exportUtil'; import { ExportToHTML } from './export/exportToHTML'; import { ExportToPDF } from './export/exportToPDF'; import { ExportToPython } from './export/exportToPython'; -import { registerTypes as registerControllerTypes } from './controllers/serviceRegistry.web'; -import { ServerConnectionControllerCommands } from './controllers/commands/serverConnectionControllerCommands'; -import { MultiplexingDebugService } from './debugger/multiplexingDebugService'; -import { Identifiers } from '../platform/common/constants'; -import { DebugLocationTrackerFactory } from './debugger/debugLocationTrackerFactory'; -import { IJupyterVariables } from '../kernels/variables/types'; -import { DebuggerVariables } from './debugger/debuggerVariables'; +import { ExportToPythonPlain } from './export/exportToPythonPlain'; +import { ExportUtilBase } from './export/exportUtil'; +import { FileConverter } from './export/fileConverter'; +import { ExportFormat, IExport, IExportBase, IExportDialog, IFileConverter, INbConvertExport } from './export/types'; +import { NotebookCellLanguageService } from './languages/cellLanguageService'; +import { EmptyNotebookCellLanguageService } from './languages/emptyNotebookCellLanguageService'; +import { NotebookCommandListener } from './notebookCommandListener'; +import { NotebookEditorProvider } from './notebookEditorProvider'; +import { ErrorRendererCommunicationHandler } from './outputs/errorRendererComms'; +import { NotebookTracebackFormatter } from './outputs/tracebackFormatter'; +import { JupyterServerSelectorCommand } from './serverSelectorCommand'; +import { InterpreterPackageTracker } from './telemetry/interpreterPackageTracker'; +import { INotebookEditorProvider } from './types'; import { PickDocumentKernelSourceCommand } from './controllers/commands/pickDocumentKernelSourceCommand'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { @@ -90,7 +92,7 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea EmptyNotebookCellLanguageService ); - serviceManager.addSingleton(IDebuggingManager, DebuggingManager, undefined, [ + serviceManager.addSingleton(INotebookDebuggingManager, DebuggingManager, undefined, [ IExtensionSingleActivationService ]); serviceManager.addSingleton( @@ -106,6 +108,7 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea DebuggerVariables, Identifiers.DEBUGGER_VARIABLES ); + serviceManager.addSingleton(IExtensionSingleActivationService, CommandRegistry); serviceManager.addSingleton( IExtensionSyncActivationService, diff --git a/src/platform/common/constants.ts b/src/platform/common/constants.ts index 3450e96c2ac..2bc40eb890b 100644 --- a/src/platform/common/constants.ts +++ b/src/platform/common/constants.ts @@ -276,7 +276,6 @@ export namespace Commands { export const InteractiveCopyCell = 'jupyter.interactive.copyCell'; export const InteractiveExportAsNotebook = 'jupyter.interactive.exportasnotebook'; export const InteractiveExportAs = 'jupyter.interactive.exportas'; - export const DebugNotebook = 'jupyter.debugNotebook'; export const RunByLine = 'jupyter.runByLine'; export const RunAndDebugCell = 'jupyter.runAndDebugCell'; export const RunByLineNext = 'jupyter.runByLineNext'; diff --git a/src/test/datascience/debugger.vscode.test.ts b/src/test/datascience/debugger.vscode.test.ts index da087c503ab..f1ffccd5df5 100644 --- a/src/test/datascience/debugger.vscode.test.ts +++ b/src/test/datascience/debugger.vscode.test.ts @@ -2,37 +2,37 @@ // Licensed under the MIT License. 'use strict'; -import * as sinon from 'sinon'; +import { assert } from 'chai'; import * as fs from 'fs'; import * as os from 'os'; -import * as path from '../../platform/vscode-path/path'; -import { noop, sleep } from '../core'; +import * as sinon from 'sinon'; +import { debug } from 'vscode'; +import { DebugProtocol } from 'vscode-debugprotocol'; +import { IDebuggingManager, INotebookDebuggingManager } from '../../notebooks/debugger/debuggingTypes'; import { ICommandManager, IVSCodeNotebook } from '../../platform/common/application/types'; +import { Commands } from '../../platform/common/constants'; import { IDisposable } from '../../platform/common/types'; +import { traceInfo } from '../../platform/logging'; +import * as path from '../../platform/vscode-path/path'; +import { IVariableViewProvider } from '../../webviews/extension-side/variablesView/types'; import { captureScreenShot, IExtensionTestApi, waitForCondition } from '../common.node'; +import { noop, sleep } from '../core'; import { initialize, IS_REMOTE_NATIVE_TEST } from '../initialize.node'; import { closeNotebooks, closeNotebooksAndCleanUpAfterTests, createEmptyPythonNotebook, + defaultNotebookTestTimeout, + getCellOutputs, + getDebugSessionAndAdapter, insertCodeCell, prewarmNotebooks, - getCellOutputs, - defaultNotebookTestTimeout, - waitForStoppedEvent, runCell, - getDebugSessionAndAdapter + waitForStoppedEvent } from './notebook/helper.node'; -import { ITestVariableViewProvider } from './variableView/variableViewTestInterfaces'; -import { traceInfo } from '../../platform/logging'; -import { assert } from 'chai'; -import { debug } from 'vscode'; import { ITestWebviewHost } from './testInterfaces'; -import { DebugProtocol } from 'vscode-debugprotocol'; import { waitForVariablesToMatch } from './variableView/variableViewHelpers'; -import { Commands } from '../../platform/common/constants'; -import { IVariableViewProvider } from '../../webviews/extension-side/variablesView/types'; -import { IDebuggingManager } from '../../notebooks/debugger/debuggingTypes'; +import { ITestVariableViewProvider } from './variableView/variableViewTestInterfaces'; suite('VSCode Notebook - Run By Line', function () { let api: IExtensionTestApi; @@ -58,7 +58,7 @@ suite('VSCode Notebook - Run By Line', function () { const coreVariableViewProvider = api.serviceContainer.get(IVariableViewProvider); // eslint-disable-next-line @typescript-eslint/no-explicit-any variableViewProvider = coreVariableViewProvider as any as ITestVariableViewProvider; // Cast to expose the test interfaces - debuggingManager = api.serviceContainer.get(IDebuggingManager); + debuggingManager = api.serviceContainer.get(INotebookDebuggingManager); vscodeNotebook = api.serviceContainer.get(IVSCodeNotebook); traceInfo(`Start Test Suite (completed)`); }); @@ -107,7 +107,7 @@ suite('VSCode Notebook - Run By Line', function () { await waitForStoppedEvent(debugAdapter!); // Go head and run to the end now - await commandManager.executeCommand(Commands.RunByLineStop); + await commandManager.executeCommand(Commands.RunByLineStop, cell); // Wait until we have finished and have output await waitForCondition( @@ -284,7 +284,7 @@ suite('VSCode Notebook - Run By Line', function () { `Print during time loop is not working. Outputs: ${getCellOutputs(cell)}}`, 1000 ); - await commandManager.executeCommand(Commands.RunByLineStop); + await commandManager.executeCommand(Commands.RunByLineStop, cell); await waitForCondition( async () => !debug.activeDebugSession, defaultNotebookTestTimeout, From 972b9f1219432e3f22f88bf5b7fa2220e38bf1db Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 14 Oct 2022 14:02:04 -0700 Subject: [PATCH 04/10] Hook up restart for "debug cell" --- .../debugger/jupyter/debugCellController.ts | 2 +- .../debugger/jupyter/debuggingManager.ts | 2 +- .../{ => controllers}/debugCellController.ts | 16 +++---- .../debugger/controllers/restartController.ts | 47 +++++++++++++++++++ .../{ => controllers}/runByLineController.ts | 46 +++++------------- src/notebooks/debugger/debuggingManager.ts | 22 +++++---- .../debugger/kernelDebugAdapterBase.ts | 24 ++++++---- 7 files changed, 100 insertions(+), 59 deletions(-) rename src/notebooks/debugger/{ => controllers}/debugCellController.ts (78%) create mode 100644 src/notebooks/debugger/controllers/restartController.ts rename src/notebooks/debugger/{ => controllers}/runByLineController.ts (75%) diff --git a/src/interactive-window/debugger/jupyter/debugCellController.ts b/src/interactive-window/debugger/jupyter/debugCellController.ts index 3035a4c918a..539a20b2b29 100644 --- a/src/interactive-window/debugger/jupyter/debugCellController.ts +++ b/src/interactive-window/debugger/jupyter/debugCellController.ts @@ -5,7 +5,7 @@ import { NotebookCell } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; import { IKernel } from '../../../kernels/types'; import { DebuggingTelemetry } from '../../../notebooks/debugger/constants'; -import { isJustMyCodeNotification } from '../../../notebooks/debugger/debugCellController'; +import { isJustMyCodeNotification } from '../../../notebooks/debugger/controllers/debugCellController'; import { IDebuggingDelegate, IKernelDebugAdapter } from '../../../notebooks/debugger/debuggingTypes'; import { cellDebugSetup } from '../../../notebooks/debugger/helper'; import { traceVerbose } from '../../../platform/logging'; diff --git a/src/interactive-window/debugger/jupyter/debuggingManager.ts b/src/interactive-window/debugger/jupyter/debuggingManager.ts index 33a50d7ea41..f6af4fe558d 100644 --- a/src/interactive-window/debugger/jupyter/debuggingManager.ts +++ b/src/interactive-window/debugger/jupyter/debuggingManager.ts @@ -168,7 +168,7 @@ export class InteractiveWindowDebuggingManager const cell = notebook.cellAt(config.__cellIndex); const controller = new DebugCellController(adapter, cell, kernel!); - adapter.setDebuggingDelegate(controller); + adapter.setDebuggingDelegates([controller]); this.trackDebugAdapter(notebook, adapter); return new DebugAdapterInlineImplementation(adapter); diff --git a/src/notebooks/debugger/debugCellController.ts b/src/notebooks/debugger/controllers/debugCellController.ts similarity index 78% rename from src/notebooks/debugger/debugCellController.ts rename to src/notebooks/debugger/controllers/debugCellController.ts index 289a845bac5..9bd64153318 100644 --- a/src/notebooks/debugger/debugCellController.ts +++ b/src/notebooks/debugger/controllers/debugCellController.ts @@ -3,14 +3,14 @@ import { NotebookCell } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; -import { IKernel } from '../../kernels/types'; -import { ICommandManager } from '../../platform/common/application/types'; -import { noop } from '../../platform/common/utils/misc'; -import { traceVerbose } from '../../platform/logging'; -import { sendTelemetryEvent } from '../../telemetry'; -import { DebuggingTelemetry } from './constants'; -import { IDebuggingDelegate, IKernelDebugAdapter } from './debuggingTypes'; -import { cellDebugSetup } from './helper'; +import { IKernel } from '../../../kernels/types'; +import { ICommandManager } from '../../../platform/common/application/types'; +import { noop } from '../../../platform/common/utils/misc'; +import { traceVerbose } from '../../../platform/logging'; +import { sendTelemetryEvent } from '../../../telemetry'; +import { DebuggingTelemetry } from '../constants'; +import { IDebuggingDelegate, IKernelDebugAdapter } from '../debuggingTypes'; +import { cellDebugSetup } from '../helper'; export function isJustMyCodeNotification(msg: string): boolean { return msg.includes('Frame skipped from debugging during step-in'); diff --git a/src/notebooks/debugger/controllers/restartController.ts b/src/notebooks/debugger/controllers/restartController.ts new file mode 100644 index 00000000000..22bd7223f2b --- /dev/null +++ b/src/notebooks/debugger/controllers/restartController.ts @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { NotebookCell } from 'vscode'; +import { DebugProtocol } from 'vscode-debugprotocol'; +import { noop } from '../../../platform/common/utils/misc'; +import { IServiceContainer } from '../../../platform/ioc/types'; +import { sendTelemetryEvent } from '../../../telemetry'; +import { DebuggingTelemetry } from '../constants'; +import { IDebuggingDelegate, IKernelDebugAdapter, INotebookDebuggingManager, KernelDebugMode } from '../debuggingTypes'; + +/** + * Listens to event when doing run by line and controls the behavior of the debugger (like auto stepping on moving out of a cell) + */ +export class RestartController implements IDebuggingDelegate { + private debuggingManager: INotebookDebuggingManager; + + constructor( + private readonly mode: KernelDebugMode, + private readonly debugAdapter: IKernelDebugAdapter, + public readonly debugCell: NotebookCell, + private readonly serviceContainer: IServiceContainer + ) { + sendTelemetryEvent(DebuggingTelemetry.successfullyStartedRunByLine); + this.debuggingManager = this.serviceContainer.get(INotebookDebuggingManager); + } + + public async willSendRequest(request: DebugProtocol.Request): Promise { + if (request.command === 'restart') { + await this.debugAdapter.disconnect(); + this.doRestart().then(noop, noop); + return true; + } + + return false; + } + + public async willSendResponse(response: DebugProtocol.Response): Promise { + if (response.command === 'initialize' && response.body) { + (response as DebugProtocol.InitializeResponse).body!.supportsRestartRequest = true; + } + } + + private async doRestart() { + return this.debuggingManager.tryToStartDebugging(this.mode, this.debugCell); + } +} diff --git a/src/notebooks/debugger/runByLineController.ts b/src/notebooks/debugger/controllers/runByLineController.ts similarity index 75% rename from src/notebooks/debugger/runByLineController.ts rename to src/notebooks/debugger/controllers/runByLineController.ts index 2c165f082a5..4ad29095b73 100644 --- a/src/notebooks/debugger/runByLineController.ts +++ b/src/notebooks/debugger/controllers/runByLineController.ts @@ -3,38 +3,34 @@ import { NotebookCell } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; -import { IKernel } from '../../kernels/types'; -import { ICommandManager } from '../../platform/common/application/types'; -import { Commands } from '../../platform/common/constants'; -import { IConfigurationService } from '../../platform/common/types'; -import { parseForComments } from '../../platform/common/utils'; -import { noop } from '../../platform/common/utils/misc'; -import { IServiceContainer } from '../../platform/ioc/types'; -import { traceInfoIfCI, traceVerbose } from '../../platform/logging'; -import * as path from '../../platform/vscode-path/path'; -import { sendTelemetryEvent } from '../../telemetry'; -import { DebuggingTelemetry } from './constants'; +import { IKernel } from '../../../kernels/types'; +import { ICommandManager } from '../../../platform/common/application/types'; +import { Commands } from '../../../platform/common/constants'; +import { IConfigurationService } from '../../../platform/common/types'; +import { parseForComments } from '../../../platform/common/utils'; +import { noop } from '../../../platform/common/utils/misc'; +import { traceInfoIfCI, traceVerbose } from '../../../platform/logging'; +import * as path from '../../../platform/vscode-path/path'; +import { sendTelemetryEvent } from '../../../telemetry'; +import { DebuggingTelemetry } from '../constants'; +import { IDebuggingDelegate, IKernelDebugAdapter, KernelDebugMode } from '../debuggingTypes'; +import { cellDebugSetup } from '../helper'; import { isJustMyCodeNotification } from './debugCellController'; -import { IDebuggingDelegate, IKernelDebugAdapter, INotebookDebuggingManager, KernelDebugMode } from './debuggingTypes'; -import { cellDebugSetup } from './helper'; /** * Listens to event when doing run by line and controls the behavior of the debugger (like auto stepping on moving out of a cell) */ export class RunByLineController implements IDebuggingDelegate { private lastPausedThreadId: number | undefined; - private debuggingManager: INotebookDebuggingManager; constructor( private readonly debugAdapter: IKernelDebugAdapter, public readonly debugCell: NotebookCell, private readonly commandManager: ICommandManager, private readonly kernel: IKernel, - private readonly settings: IConfigurationService, - private readonly serviceContainer: IServiceContainer + private readonly settings: IConfigurationService ) { sendTelemetryEvent(DebuggingTelemetry.successfullyStartedRunByLine); - this.debuggingManager = this.serviceContainer.get(INotebookDebuggingManager); } public continue(): void { @@ -84,21 +80,9 @@ export class RunByLineController implements IDebuggingDelegate { await this.initializeExecute(); } - if (request.command === 'restart') { - await this.debugAdapter.disconnect(); - this.doRestart().then(noop, noop); - return true; - } - return false; } - public async willSendResponse(response: DebugProtocol.Response): Promise { - if (response.command === 'initialize' && response.body) { - (response as DebugProtocol.InitializeResponse).body!.supportsRestartRequest = true; - } - } - private async handleStoppedEvent(threadId: number): Promise { if (await this.shouldStepIn(threadId)) { this.debugAdapter.stepIn(threadId).then(noop, noop); @@ -172,8 +156,4 @@ export class RunByLineController implements IDebuggingDelegate { }) .then(noop, noop); } - - private async doRestart() { - return this.debuggingManager.tryToStartDebugging!(KernelDebugMode.RunByLine, this.debugCell); - } } diff --git a/src/notebooks/debugger/debuggingManager.ts b/src/notebooks/debugger/debuggingManager.ts index 8f7915e7250..ebcd38f18fd 100644 --- a/src/notebooks/debugger/debuggingManager.ts +++ b/src/notebooks/debugger/debuggingManager.ts @@ -35,13 +35,14 @@ import * as path from '../../platform/vscode-path/path'; import { sendTelemetryEvent } from '../../telemetry'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; import { DebuggingTelemetry, pythonKernelDebugAdapter } from './constants'; -import { DebugCellController } from './debugCellController'; +import { DebugCellController } from './controllers/debugCellController'; import { Debugger } from './debugger'; import { DebuggingManagerBase } from './debuggingManagerBase'; import { IDebuggingManager, INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; import { assertIsDebugConfig, IpykernelCheckResult } from './helper'; import { KernelDebugAdapter } from './kernelDebugAdapter'; -import { RunByLineController } from './runByLineController'; +import { RunByLineController } from './controllers/runByLineController'; +import { RestartController } from './controllers/restartController'; /** * The DebuggingManager maintains the mapping between notebook documents and debug sessions. @@ -226,21 +227,26 @@ export class DebuggingManager if (config.__mode === KernelDebugMode.RunByLine && typeof config.__cellIndex === 'number') { const cell = notebook.cellAt(config.__cellIndex); - const controller = new RunByLineController( + const rblController = new RunByLineController( adapter, cell, this.commandManager, kernel!, - this.settings, - this.serviceContainer + this.settings ); - adapter.setDebuggingDelegate(controller); - this.notebookToRunByLineController.set(notebook, controller); + adapter.setDebuggingDelegates([ + rblController, + new RestartController(KernelDebugMode.RunByLine, adapter, cell, this.serviceContainer) + ]); + this.notebookToRunByLineController.set(notebook, rblController); this.updateRunByLineContextKeys(); } else if (config.__mode === KernelDebugMode.Cell && typeof config.__cellIndex === 'number') { const cell = notebook.cellAt(config.__cellIndex); const controller = new DebugCellController(adapter, cell, kernel!, this.commandManager); - adapter.setDebuggingDelegate(controller); + adapter.setDebuggingDelegates([ + controller, + new RestartController(KernelDebugMode.Cell, adapter, cell, this.serviceContainer) + ]); } this.trackDebugAdapter(notebook, adapter); diff --git a/src/notebooks/debugger/kernelDebugAdapterBase.ts b/src/notebooks/debugger/kernelDebugAdapterBase.ts index 5b7aa4257c5..c7c38e0c6d6 100644 --- a/src/notebooks/debugger/kernelDebugAdapterBase.ts +++ b/src/notebooks/debugger/kernelDebugAdapterBase.ts @@ -62,7 +62,7 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb private readonly endSession = new EventEmitter(); private readonly configuration: INotebookDebugConfig; protected readonly disposables: IDisposable[] = []; - private delegate: IDebuggingDelegate | undefined; + private delegates: IDebuggingDelegate[] | undefined; onDidSendMessage: Event = this.sendMessage.event; onDidEndSession: Event = this.endSession.event; public readonly debugCell: NotebookCell | undefined; @@ -148,8 +148,8 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb ); } - public setDebuggingDelegate(delegate: IDebuggingDelegate) { - this.delegate = delegate; + public setDebuggingDelegates(delegates: IDebuggingDelegate[]) { + this.delegates = delegates; } private trace(tag: string, msg: string) { @@ -160,9 +160,13 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb traceInfoIfCI(`Debug IO Pub message: ${JSON.stringify(msg)}`); if (isDebugEventMsg(msg)) { this.trace('event', JSON.stringify(msg)); - if (!this.delegate?.willSendEvent || !(await this.delegate.willSendEvent(msg.content))) { - this.sendMessage.fire(msg.content); + for (const d of this.delegates ?? []) { + if (await d?.willSendEvent?.(msg.content)) { + return; + } } + + this.sendMessage.fire(msg.content); } } @@ -194,7 +198,11 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb } if (message.type === 'request') { - await this.delegate?.willSendRequest?.(message as DebugProtocol.Request); + for (const d of this.delegates ?? []) { + if (await d?.willSendRequest?.(message as DebugProtocol.Request)) { + return; + } + } } return this.sendRequestToJupyterSession(message); @@ -308,8 +316,8 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb const message = msg.content as DebugProtocol.Response; getMessageSourceAndHookIt(message, this.translateDebuggerFileToRealFile.bind(this)); - if (this.delegate?.willSendResponse) { - await this.delegate?.willSendResponse(message); + for (const d of this.delegates ?? []) { + await d?.willSendResponse?.(message); } this.trace('response', JSON.stringify(message)); From 25ca6ff9abd5388ea1a548a7979d427f52775060 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 14 Oct 2022 14:25:25 -0700 Subject: [PATCH 05/10] Clean up dumped cell files in disconnect, so it can be awaited --- src/notebooks/debugger/kernelDebugAdapterBase.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/notebooks/debugger/kernelDebugAdapterBase.ts b/src/notebooks/debugger/kernelDebugAdapterBase.ts index c7c38e0c6d6..e7778d6fed4 100644 --- a/src/notebooks/debugger/kernelDebugAdapterBase.ts +++ b/src/notebooks/debugger/kernelDebugAdapterBase.ts @@ -224,7 +224,12 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb this.disconnected = true; if (this.debugService.activeDebugSession === this.session) { try { - await this.session.customRequest('disconnect', { restart: false }); + await Promise.all([ + this.deleteDumpedFiles().catch((ex) => + traceWarning('Error deleting temporary debug files.', ex) + ), + this.session.customRequest('disconnect', { restart: false }) + ]); } catch (e) { traceError(`Failed to disconnect debug session`, e); } @@ -235,7 +240,6 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb } dispose() { - this.deleteDumpedFiles().catch((ex) => traceWarning('Error deleting temporary debug files.', ex)); this.disposables.forEach((d) => d.dispose()); } From b14e66b129d15ab106c239a14a06752d87922cdb Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 14 Oct 2022 14:34:56 -0700 Subject: [PATCH 06/10] Fix comments in controller --- src/notebooks/debugger/controllers/restartController.ts | 3 ++- src/notebooks/debugger/controllers/runByLineController.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/notebooks/debugger/controllers/restartController.ts b/src/notebooks/debugger/controllers/restartController.ts index 22bd7223f2b..64827d7c158 100644 --- a/src/notebooks/debugger/controllers/restartController.ts +++ b/src/notebooks/debugger/controllers/restartController.ts @@ -10,7 +10,7 @@ import { DebuggingTelemetry } from '../constants'; import { IDebuggingDelegate, IKernelDebugAdapter, INotebookDebuggingManager, KernelDebugMode } from '../debuggingTypes'; /** - * Listens to event when doing run by line and controls the behavior of the debugger (like auto stepping on moving out of a cell) + * Implements the "restart" request. */ export class RestartController implements IDebuggingDelegate { private debuggingManager: INotebookDebuggingManager; @@ -42,6 +42,7 @@ export class RestartController implements IDebuggingDelegate { } private async doRestart() { + // We have to implement restart manually because the previous launch config includes the cell index, but the cell index may have changed. return this.debuggingManager.tryToStartDebugging(this.mode, this.debugCell); } } diff --git a/src/notebooks/debugger/controllers/runByLineController.ts b/src/notebooks/debugger/controllers/runByLineController.ts index 4ad29095b73..867393e71fc 100644 --- a/src/notebooks/debugger/controllers/runByLineController.ts +++ b/src/notebooks/debugger/controllers/runByLineController.ts @@ -18,7 +18,7 @@ import { cellDebugSetup } from '../helper'; import { isJustMyCodeNotification } from './debugCellController'; /** - * Listens to event when doing run by line and controls the behavior of the debugger (like auto stepping on moving out of a cell) + * Implements the business logic of RBL (like auto stepping on moving out of a cell) */ export class RunByLineController implements IDebuggingDelegate { private lastPausedThreadId: number | undefined; From 08ae7229ee23327b7c82e787121d2d182e1d110e Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 14 Oct 2022 14:38:47 -0700 Subject: [PATCH 07/10] formatting --- src/notebooks/debugger/helper.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/notebooks/debugger/helper.ts b/src/notebooks/debugger/helper.ts index 481d8ccc501..50dece16444 100644 --- a/src/notebooks/debugger/helper.ts +++ b/src/notebooks/debugger/helper.ts @@ -4,7 +4,12 @@ import { IDebugEventMsg } from '@jupyterlab/services/lib/kernel/messages'; import { DebugProtocol } from 'vscode-debugprotocol'; import { IKernel } from '../../kernels/types'; -import { IInteractiveWindowDebugConfig, IKernelDebugAdapter, INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; +import { + IInteractiveWindowDebugConfig, + IKernelDebugAdapter, + INotebookDebugConfig, + KernelDebugMode +} from './debuggingTypes'; export enum IpykernelCheckResult { Unknown, @@ -43,9 +48,7 @@ export function assertIsDebugConfig(thing: unknown): asserts thing is INotebookD } } -export function assertIsInteractiveWindowDebugConfig( - thing: unknown -): asserts thing is IInteractiveWindowDebugConfig { +export function assertIsInteractiveWindowDebugConfig(thing: unknown): asserts thing is IInteractiveWindowDebugConfig { assertIsDebugConfig(thing); if (thing.__mode !== KernelDebugMode.InteractiveWindow) { throw new Error('Invalid launch configuration'); From 2628495888ec15cbffa20c5875c4095435fe9b62 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 17 Oct 2022 18:17:09 -0700 Subject: [PATCH 08/10] Fix PR comments --- src/notebooks/debugger/commandRegistry.ts | 4 ---- src/notebooks/debugger/debuggingManagerBase.ts | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/notebooks/debugger/commandRegistry.ts b/src/notebooks/debugger/commandRegistry.ts index 182b0cd6aa6..cd73e8a9387 100644 --- a/src/notebooks/debugger/commandRegistry.ts +++ b/src/notebooks/debugger/commandRegistry.ts @@ -42,10 +42,6 @@ export class CommandRegistry implements IDisposable, IExtensionSingleActivationS return; } - if (!cell) { - return; - } - await this.debuggingManager.tryToStartDebugging(KernelDebugMode.RunByLine, cell); } diff --git a/src/notebooks/debugger/debuggingManagerBase.ts b/src/notebooks/debugger/debuggingManagerBase.ts index 3736ff38417..ac655851f37 100644 --- a/src/notebooks/debugger/debuggingManagerBase.ts +++ b/src/notebooks/debugger/debuggingManagerBase.ts @@ -161,7 +161,7 @@ export abstract class DebuggingManagerBase implements IDisposable, IDebuggingMan private findEditorForCell(cell: NotebookCell): NotebookEditor | undefined { const notebookUri = cell.notebook.uri.toString(); - return window.visibleNotebookEditors.find((e) => e.notebook.uri.toString() === notebookUri.toString()); + return window.visibleNotebookEditors.find((e) => e.notebook.uri.toString() === notebookUri); } protected async checkIpykernelAndPrompt( From 764e5a03712a2b282cc60ffed77fdf1da814b980 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 18 Oct 2022 10:20:24 -0700 Subject: [PATCH 09/10] Wait for debugger to be totally set up before executing cell in IW --- .../debugger/jupyter/debugCellController.ts | 5 +++++ .../debugger/jupyter/debugger.ts | 15 +++++++++++++ .../debugger/jupyter/debuggingManager.ts | 21 ++++++++++++++----- src/notebooks/debugger/debugger.ts | 3 --- 4 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 src/interactive-window/debugger/jupyter/debugger.ts diff --git a/src/interactive-window/debugger/jupyter/debugCellController.ts b/src/interactive-window/debugger/jupyter/debugCellController.ts index 539a20b2b29..0c10cadac6a 100644 --- a/src/interactive-window/debugger/jupyter/debugCellController.ts +++ b/src/interactive-window/debugger/jupyter/debugCellController.ts @@ -8,6 +8,7 @@ import { DebuggingTelemetry } from '../../../notebooks/debugger/constants'; import { isJustMyCodeNotification } from '../../../notebooks/debugger/controllers/debugCellController'; import { IDebuggingDelegate, IKernelDebugAdapter } from '../../../notebooks/debugger/debuggingTypes'; import { cellDebugSetup } from '../../../notebooks/debugger/helper'; +import { createDeferred } from '../../../platform/common/utils/async'; import { traceVerbose } from '../../../platform/logging'; import { sendTelemetryEvent } from '../../../telemetry'; import { getInteractiveCellMetadata } from '../../helpers'; @@ -17,6 +18,9 @@ import { getInteractiveCellMetadata } from '../../helpers'; * Dumping a cell is how the IPython kernel determines the file path of a cell */ export class DebugCellController implements IDebuggingDelegate { + private _ready = createDeferred(); + public readonly ready = this._ready.promise; + private cellDumpInvoked?: boolean; constructor( private readonly debugAdapter: IKernelDebugAdapter, @@ -55,6 +59,7 @@ export class DebugCellController implements IDebuggingDelegate { this.debugCellDumped = cellDebugSetup(this.kernel, this.debugAdapter); } await this.debugCellDumped; + this._ready.resolve(); } return false; diff --git a/src/interactive-window/debugger/jupyter/debugger.ts b/src/interactive-window/debugger/jupyter/debugger.ts new file mode 100644 index 00000000000..16ab7ae3818 --- /dev/null +++ b/src/interactive-window/debugger/jupyter/debugger.ts @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +'use strict'; +import { Debugger } from '../../../notebooks/debugger/debugger'; +import { createDeferred } from '../../../platform/common/utils/async'; + +export class IWDebugger extends Debugger { + private readonly _ready = createDeferred(); + public readonly ready = this._ready.promise; + + public resolve() { + this._ready.resolve(); + } +} diff --git a/src/interactive-window/debugger/jupyter/debuggingManager.ts b/src/interactive-window/debugger/jupyter/debuggingManager.ts index f6af4fe558d..85d79b363b1 100644 --- a/src/interactive-window/debugger/jupyter/debuggingManager.ts +++ b/src/interactive-window/debugger/jupyter/debuggingManager.ts @@ -15,7 +15,6 @@ import { import { IKernelProvider } from '../../../kernels/types'; import { IControllerLoader, IControllerSelection } from '../../../notebooks/controllers/types'; import { pythonIWKernelDebugAdapter } from '../../../notebooks/debugger/constants'; -import { Debugger } from '../../../notebooks/debugger/debugger'; import { DebuggingManagerBase } from '../../../notebooks/debugger/debuggingManagerBase'; import { IDebugLocationTrackerFactory, @@ -41,6 +40,7 @@ import { IFileGeneratedCodes } from '../../editor-integration/types'; import { IInteractiveWindowDebuggingManager } from '../../types'; import { buildSourceMap } from '../helper'; import { DebugCellController } from './debugCellController'; +import { IWDebugger } from './debugger'; import { KernelDebugAdapter } from './kernelDebugAdapter'; /** @@ -112,7 +112,13 @@ export class InteractiveWindowDebuggingManager __cellIndex: cell.index }; const opts: DebugSessionOptions = { suppressSaveBeforeStart: true }; - return this.startDebuggingConfig(config, opts); + await this.startDebuggingConfig(config, opts); + const dbgr = this.notebookToDebugger.get(doc); + if (!dbgr) { + traceError('Debugger not found, could not start debugging.'); + return; + } + await (dbgr as IWDebugger).ready; } protected async createDebugAdapterDescriptor(session: DebugSession): Promise { @@ -135,10 +141,11 @@ export class InteractiveWindowDebuggingManager return; } - this.notebookToDebugger.set(notebook, new Debugger(notebook, config, session)); + const dbgr = new IWDebugger(notebook, config, session); + this.notebookToDebugger.set(notebook, dbgr); try { this.notebookInProgress.add(notebook); - return await this.doCreateDebugAdapterDescriptor(config, session, notebook); + return await this.doCreateDebugAdapterDescriptor(config, session, notebook, dbgr); } finally { this.notebookInProgress.delete(notebook); } @@ -147,7 +154,8 @@ export class InteractiveWindowDebuggingManager private async doCreateDebugAdapterDescriptor( config: IInteractiveWindowDebugConfig, session: DebugSession, - notebook: NotebookDocument + notebook: NotebookDocument, + dbgr: IWDebugger ): Promise { const kernel = await this.ensureKernelIsRunning(notebook); if (!kernel?.session) { @@ -169,6 +177,9 @@ export class InteractiveWindowDebuggingManager const cell = notebook.cellAt(config.__cellIndex); const controller = new DebugCellController(adapter, cell, kernel!); adapter.setDebuggingDelegates([controller]); + controller.ready + .then(() => dbgr.resolve()) + .catch((ex) => console.error('Failed waiting for controller to be ready', ex)); this.trackDebugAdapter(notebook, adapter); return new DebugAdapterInlineImplementation(adapter); diff --git a/src/notebooks/debugger/debugger.ts b/src/notebooks/debugger/debugger.ts index f834bfef0ae..69e54c69014 100644 --- a/src/notebooks/debugger/debugger.ts +++ b/src/notebooks/debugger/debugger.ts @@ -4,9 +4,6 @@ 'use strict'; import { DebugConfiguration, DebugSession, NotebookDocument } from 'vscode'; -/** - * Wraps debug start in a promise - */ export class Debugger { constructor( public readonly document: NotebookDocument, From 0729d7a51c136e97cc896d835b16f4319faf601a Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 21 Oct 2022 09:12:03 -0700 Subject: [PATCH 10/10] Add test for restarting debug. Also fix the restart request handler to avoid cancelling the restart request on accident. --- src/commands.ts | 1 + .../debugger/jupyter/debugCellController.ts | 4 +- .../controllers/debugCellController.ts | 4 +- .../debugger/controllers/restartController.ts | 44 ++++++++++++++----- .../controllers/runByLineController.ts | 4 +- src/notebooks/debugger/debuggerVariables.ts | 2 +- src/notebooks/debugger/debuggingManager.ts | 12 ++--- src/notebooks/debugger/debuggingTypes.ts | 2 +- .../debugger/kernelDebugAdapterBase.ts | 5 ++- src/test/datascience/debugger.vscode.test.ts | 22 ++++++++++ src/test/datascience/notebook/helper.ts | 12 +++-- 11 files changed, 82 insertions(+), 30 deletions(-) diff --git a/src/commands.ts b/src/commands.ts index 9d81db437e8..9ed432c9bb9 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -30,6 +30,7 @@ interface ICommandNameWithoutArgumentTypeMapping { ['workbench.action.showCommands']: []; ['workbench.action.debug.continue']: []; ['workbench.action.debug.stepOver']: []; + ['workbench.action.debug.restart']: []; ['workbench.action.debug.stop']: []; ['workbench.action.reloadWindow']: []; ['workbench.action.closeActiveEditor']: []; diff --git a/src/interactive-window/debugger/jupyter/debugCellController.ts b/src/interactive-window/debugger/jupyter/debugCellController.ts index 0c10cadac6a..64e1363669b 100644 --- a/src/interactive-window/debugger/jupyter/debugCellController.ts +++ b/src/interactive-window/debugger/jupyter/debugCellController.ts @@ -46,7 +46,7 @@ export class DebugCellController implements IDebuggingDelegate { } private debugCellDumped?: Promise; - public async willSendRequest(request: DebugProtocol.Request): Promise { + public async willSendRequest(request: DebugProtocol.Request): Promise { const metadata = getInteractiveCellMetadata(this.debugCell); if (request.command === 'setBreakpoints' && metadata && metadata.generatedCode && !this.cellDumpInvoked) { if (!this.debugCellDumped) { @@ -62,6 +62,6 @@ export class DebugCellController implements IDebuggingDelegate { this._ready.resolve(); } - return false; + return undefined; } } diff --git a/src/notebooks/debugger/controllers/debugCellController.ts b/src/notebooks/debugger/controllers/debugCellController.ts index 9bd64153318..e3ada7e3edc 100644 --- a/src/notebooks/debugger/controllers/debugCellController.ts +++ b/src/notebooks/debugger/controllers/debugCellController.ts @@ -44,7 +44,7 @@ export class DebugCellController implements IDebuggingDelegate { return false; } - public async willSendRequest(request: DebugProtocol.Request): Promise { + public async willSendRequest(request: DebugProtocol.Request): Promise { if (request.command === 'configurationDone') { await cellDebugSetup(this.kernel, this.debugAdapter); @@ -56,6 +56,6 @@ export class DebugCellController implements IDebuggingDelegate { .then(noop, noop); } - return false; + return undefined; } } diff --git a/src/notebooks/debugger/controllers/restartController.ts b/src/notebooks/debugger/controllers/restartController.ts index 64827d7c158..3ecea1639bf 100644 --- a/src/notebooks/debugger/controllers/restartController.ts +++ b/src/notebooks/debugger/controllers/restartController.ts @@ -3,8 +3,8 @@ import { NotebookCell } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; -import { noop } from '../../../platform/common/utils/misc'; import { IServiceContainer } from '../../../platform/ioc/types'; +import { traceError, traceVerbose } from '../../../platform/logging'; import { sendTelemetryEvent } from '../../../telemetry'; import { DebuggingTelemetry } from '../constants'; import { IDebuggingDelegate, IKernelDebugAdapter, INotebookDebuggingManager, KernelDebugMode } from '../debuggingTypes'; @@ -25,14 +25,41 @@ export class RestartController implements IDebuggingDelegate { this.debuggingManager = this.serviceContainer.get(INotebookDebuggingManager); } - public async willSendRequest(request: DebugProtocol.Request): Promise { + private trace(tag: string, msg: string) { + traceVerbose(`[Debug-Restart] ${tag}: ${msg}`); + } + + private error(tag: string, msg: string) { + traceError(`[Debug-Restart] ${tag}: ${msg}`); + } + + public async willSendRequest(request: DebugProtocol.Request): Promise { if (request.command === 'restart') { - await this.debugAdapter.disconnect(); - this.doRestart().then(noop, noop); - return true; + // We have to implement restart manually because the previous launch config includes the cell index, but the cell index may have changed. + this.trace('restart', 'Handling restart request'); + setTimeout(() => { + // The restart response has to be sent _before_ the debug session is disconnected - otherwise the pending restart request will be canceled, + // and considered to have failed. eg a call to executeCommand('workbench.action.debug.restart') would fail. + this.debugAdapter + .disconnect() + .then(() => { + this.trace('restart', 'doRestart'); + return this.debuggingManager.tryToStartDebugging(this.mode, this.debugCell); + }) + .catch((err) => { + this.error('restart', `Error restarting: ${err}`); + }); + }, 0); + return { + command: request.command, + request_seq: request.seq, + seq: request.seq, + success: true, + type: 'response' + }; } - return false; + return undefined; } public async willSendResponse(response: DebugProtocol.Response): Promise { @@ -40,9 +67,4 @@ export class RestartController implements IDebuggingDelegate { (response as DebugProtocol.InitializeResponse).body!.supportsRestartRequest = true; } } - - private async doRestart() { - // We have to implement restart manually because the previous launch config includes the cell index, but the cell index may have changed. - return this.debuggingManager.tryToStartDebugging(this.mode, this.debugCell); - } } diff --git a/src/notebooks/debugger/controllers/runByLineController.ts b/src/notebooks/debugger/controllers/runByLineController.ts index 867393e71fc..dbf585084a0 100644 --- a/src/notebooks/debugger/controllers/runByLineController.ts +++ b/src/notebooks/debugger/controllers/runByLineController.ts @@ -74,13 +74,13 @@ export class RunByLineController implements IDebuggingDelegate { return false; } - public async willSendRequest(request: DebugProtocol.Request): Promise { + public async willSendRequest(request: DebugProtocol.Request): Promise { traceInfoIfCI(`willSendRequest: ${request.command}`); if (request.command === 'configurationDone') { await this.initializeExecute(); } - return false; + return undefined; } private async handleStoppedEvent(threadId: number): Promise { diff --git a/src/notebooks/debugger/debuggerVariables.ts b/src/notebooks/debugger/debuggerVariables.ts index a7326049a1d..6d4ae9a98e8 100644 --- a/src/notebooks/debugger/debuggerVariables.ts +++ b/src/notebooks/debugger/debuggerVariables.ts @@ -434,7 +434,7 @@ export class DebuggerVariables const threadId = stoppedMessage.body.threadId; if (doc) { - const session = await this.debuggingManager.getDebugSession(doc); + const session = this.debuggingManager.getDebugSession(doc); if (session) { // Call stack trace const stResponse: DebugProtocol.StackTraceResponse['body'] = await session.customRequest('stackTrace', { diff --git a/src/notebooks/debugger/debuggingManager.ts b/src/notebooks/debugger/debuggingManager.ts index e17948e3b4c..7c815537882 100644 --- a/src/notebooks/debugger/debuggingManager.ts +++ b/src/notebooks/debugger/debuggingManager.ts @@ -29,20 +29,20 @@ import { IConfigurationService } from '../../platform/common/types'; import { DataScience } from '../../platform/common/utils/localize'; import { noop } from '../../platform/common/utils/misc'; import { IServiceContainer } from '../../platform/ioc/types'; -import { traceInfo, traceInfoIfCI } from '../../platform/logging'; +import { traceInfo } from '../../platform/logging'; import { ResourceSet } from '../../platform/vscode-path/map'; import * as path from '../../platform/vscode-path/path'; import { sendTelemetryEvent } from '../../telemetry'; import { IControllerLoader, IControllerSelection } from '../controllers/types'; import { DebuggingTelemetry, pythonKernelDebugAdapter } from './constants'; import { DebugCellController } from './controllers/debugCellController'; +import { RestartController } from './controllers/restartController'; +import { RunByLineController } from './controllers/runByLineController'; import { Debugger } from './debugger'; import { DebuggingManagerBase } from './debuggingManagerBase'; -import { IDebuggingManager, INotebookDebugConfig, KernelDebugMode } from './debuggingTypes'; +import { INotebookDebugConfig, INotebookDebuggingManager, KernelDebugMode } from './debuggingTypes'; import { assertIsDebugConfig, IpykernelCheckResult } from './helper'; import { KernelDebugAdapter } from './kernelDebugAdapter'; -import { RunByLineController } from './controllers/runByLineController'; -import { RestartController } from './controllers/restartController'; /** * The DebuggingManager maintains the mapping between notebook documents and debug sessions. @@ -50,7 +50,7 @@ import { RestartController } from './controllers/restartController'; @injectable() export class DebuggingManager extends DebuggingManagerBase - implements IExtensionSingleActivationService, IDebuggingManager + implements IExtensionSingleActivationService, INotebookDebuggingManager { private runByLineCells: ContextKey; private runByLineDocuments: ContextKey; @@ -125,7 +125,7 @@ export class DebuggingManager } public async tryToStartDebugging(mode: KernelDebugMode, cell: NotebookCell) { - traceInfoIfCI(`Starting debugging with mode ${mode}`); + traceInfo(`Starting debugging with mode ${mode}`); const ipykernelResult = await this.checkIpykernelAndPrompt(cell); if (ipykernelResult === IpykernelCheckResult.Ok) { diff --git a/src/notebooks/debugger/debuggingTypes.ts b/src/notebooks/debugger/debuggingTypes.ts index ac633b92ab4..e218a59b2b6 100644 --- a/src/notebooks/debugger/debuggingTypes.ts +++ b/src/notebooks/debugger/debuggingTypes.ts @@ -96,7 +96,7 @@ export interface IDebuggingDelegate { /** * Called for every request sent from the client to the debug adapter. Returns true to signal that the request was handled by the delegate. */ - willSendRequest?(request: DebugProtocol.Request): Promise; + willSendRequest?(request: DebugProtocol.Request): undefined | Promise; /** * Called for every response returned from the debug adapter to the client. diff --git a/src/notebooks/debugger/kernelDebugAdapterBase.ts b/src/notebooks/debugger/kernelDebugAdapterBase.ts index e7778d6fed4..237ebd110f9 100644 --- a/src/notebooks/debugger/kernelDebugAdapterBase.ts +++ b/src/notebooks/debugger/kernelDebugAdapterBase.ts @@ -199,7 +199,10 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb if (message.type === 'request') { for (const d of this.delegates ?? []) { - if (await d?.willSendRequest?.(message as DebugProtocol.Request)) { + const response = await d?.willSendRequest?.(message as DebugProtocol.Request); + if (response) { + this.trace('response', JSON.stringify(response)); + this.sendMessage.fire(response); return; } } diff --git a/src/test/datascience/debugger.vscode.test.ts b/src/test/datascience/debugger.vscode.test.ts index f1ffccd5df5..0d838938fc4 100644 --- a/src/test/datascience/debugger.vscode.test.ts +++ b/src/test/datascience/debugger.vscode.test.ts @@ -222,6 +222,28 @@ suite('VSCode Notebook - Run By Line', function () { assert.equal(stack2.stackFrames[0].line, 4, 'Stopped at the wrong line'); }); + test('Restart while debugging', async function () { + const cell = await insertCodeCell('def foo():\n print(1)\n\nfoo()', { index: 0 }); + const doc = vscodeNotebook.activeNotebookEditor?.notebook!; + + await commandManager.executeCommand(Commands.RunByLine, cell); + const { debugAdapter, session } = await getDebugSessionAndAdapter(debuggingManager, doc); + await waitForStoppedEvent(debugAdapter!); // First line + await commandManager.executeCommand('workbench.action.debug.restart'); + const { debugAdapter: debugAdapter2, session: session2 } = await getDebugSessionAndAdapter( + debuggingManager, + doc, + session.id + ); + const stoppedEvent = await waitForStoppedEvent(debugAdapter2!); // First line + const stack: DebugProtocol.StackTraceResponse['body'] = await session2!.customRequest('stackTrace', { + threadId: stoppedEvent.body.threadId + }); + assert.isTrue(stack.stackFrames.length > 0, 'has frames'); + assert.equal(stack.stackFrames[0].source?.path, cell.document.uri.toString(), 'Stopped at the wrong path'); + assert.equal(stack.stackFrames[0].line, 1, 'Stopped at the wrong line'); + }); + test.skip('Does not stop in other cell', async function () { // https://github.com/microsoft/vscode-jupyter/issues/8757 const cell0 = await insertCodeCell('def foo():\n print(1)'); diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index a362ee98b5b..9b33666edc7 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -1445,7 +1445,8 @@ export async function waitForDebugEvent( return asPromise( debugAdapter.onDidSendMessage, (message) => (message as DebugProtocol.Event).event === eventType, - timeout + timeout, + `waitForDebugEvent: ${eventType}` ) as Promise; } @@ -1456,14 +1457,17 @@ export async function waitForStoppedEvent(debugAdapter: IKernelDebugAdapter): Pr export async function getDebugSessionAndAdapter( debuggingManager: IDebuggingManager, - doc: NotebookDocument + doc: NotebookDocument, + prevSessionId?: string ): Promise<{ session: DebugSession; debugAdapter: IKernelDebugAdapter }> { await waitForCondition( - async () => !!debuggingManager.getDebugSession(doc), + async () => + !!debuggingManager.getDebugSession(doc) && + (!prevSessionId || prevSessionId !== debuggingManager.getDebugSession(doc)?.id), defaultNotebookTestTimeout, 'DebugSession should start' ); - const session = await debuggingManager.getDebugSession(doc)!; + const session = debuggingManager.getDebugSession(doc)!; const debugAdapter = debuggingManager.getDebugAdapter(doc)!; assert.isOk(debugAdapter, 'DebugAdapter not started');