From f129fc0485ca07d4a263dc49142ffcef33df6b46 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 30 May 2022 10:42:28 +1000 Subject: [PATCH 1/3] Shared kernelconnection wrapper --- TELEMETRY.md | 227 ++++++++++-------- src/kernels/common/baseJupyterSession.ts | 44 ++-- src/kernels/common/kernelConnectionWrapper.ts | 73 ++++++ .../debugger/kernelDebugAdapterBase.ts | 27 ++- .../baseKernelConnectionWrapper.ts} | 96 ++------ src/kernels/jupyter/session/jupyterSession.ts | 19 +- .../raw/session/rawJupyterSession.node.ts | 3 - src/kernels/raw/session/rawKernel.node.ts | 13 +- src/kernels/raw/session/rawSocket.node.ts | 8 +- src/kernels/types.ts | 1 - src/platform/api/kernelApi.ts | 2 +- src/platform/api/kernelConnectionWrapper.ts | 90 +++++++ src/test/common.node.ts | 87 +------ src/test/common.ts | 82 +++++++ src/test/datascience/notebook/helper.ts | 1 + .../notebook/kernelEvents.vscode.common.ts | 172 +++++++++++++ .../notebook/kernelEvents.vscode.test.ts | 10 + .../notebook/kernelEvents.vscode.web.test.ts | 10 + 18 files changed, 643 insertions(+), 322 deletions(-) create mode 100644 src/kernels/common/kernelConnectionWrapper.ts rename src/kernels/{kernelConnectionWrapper.ts => jupyter/baseKernelConnectionWrapper.ts} (72%) create mode 100644 src/platform/api/kernelConnectionWrapper.ts create mode 100644 src/test/datascience/notebook/kernelEvents.vscode.common.ts create mode 100644 src/test/datascience/notebook/kernelEvents.vscode.test.ts create mode 100644 src/test/datascience/notebook/kernelEvents.vscode.web.test.ts diff --git a/TELEMETRY.md b/TELEMETRY.md index 3cdc29c70ce..317eb470bc1 100644 --- a/TELEMETRY.md +++ b/TELEMETRY.md @@ -147,7 +147,7 @@ No properties for event ## Locations Used -[src/interactive-window/interactiveWindowCommandListener.ts#L357](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L357) +[src/interactive-window/interactiveWindowCommandListener.ts#L358](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L358) ```typescript } } @@ -178,7 +178,7 @@ No properties for event ## Locations Used -[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L326](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L326) +[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L322](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L322) ```typescript private maybeSendSliceDataDimensionalityTelemetry(numberOfDimensions: number) { @@ -210,7 +210,7 @@ No properties for event ## Locations Used -[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L208](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L208) +[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L204](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L204) ```typescript break; @@ -241,7 +241,7 @@ No properties for event ## Locations Used -[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L269](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L269) +[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L265](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L265) ```typescript if (payload.shape?.length) { this.maybeSendSliceDataDimensionalityTelemetry(payload.shape.length); @@ -421,7 +421,7 @@ No properties for event ## Locations Used -[src/platform/debugger/jupyter/notebook/debuggingManager.ts#L481](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/debuggingManager.ts#L481) +[src/kernels/debugger/debuggingManagerBase.ts#L198](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugger/debuggingManagerBase.ts#L198) ```typescript ); @@ -449,7 +449,7 @@ No properties for event ## Locations Used -[src/platform/debugger/jupyter/notebook/debuggingManager.ts#L161](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/debuggingManager.ts#L161) +[src/notebooks/debugger/debuggingManager.ts#L144](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/debugger/debuggingManager.ts#L144) ```typescript }), @@ -477,7 +477,7 @@ No properties for event ## Locations Used -[src/platform/debugger/jupyter/notebook/debuggingManager.ts#L108](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/debuggingManager.ts#L108) +[src/notebooks/debugger/debuggingManager.ts#L91](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/debugger/debuggingManager.ts#L91) ```typescript }), @@ -505,7 +505,7 @@ No properties for event ## Locations Used -[src/platform/debugger/jupyter/notebook/debuggingManager.ts#L486](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/debuggingManager.ts#L486) +[src/kernels/debugger/debuggingManagerBase.ts#L203](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugger/debuggingManagerBase.ts#L203) ```typescript 'https://github.com/microsoft/vscode-jupyter/wiki/Setting-Up-Run-by-Line-and-Debugging-for-Notebooks' ); @@ -532,7 +532,7 @@ No description provided ## Locations Used -[src/platform/debugger/jupyter/kernelDebugAdapter.ts#L94](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/kernelDebugAdapter.ts#L94) +[src/kernels/debugger/kernelDebugAdapterBase.ts#L104](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugger/kernelDebugAdapterBase.ts#L104) ```typescript this.kernel.onDisposed(() => { void debug.stopDebugging(this.session); @@ -544,7 +544,7 @@ No description provided ``` -[src/platform/debugger/jupyter/kernelDebugAdapter.ts#L108](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/kernelDebugAdapter.ts#L108) +[src/kernels/debugger/kernelDebugAdapterBase.ts#L118](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugger/kernelDebugAdapterBase.ts#L118) ```typescript cellStateChange.state === NotebookCellExecutionState.Idle && !this.disconnected @@ -556,7 +556,7 @@ No description provided ``` -[src/platform/debugger/jupyter/notebook/debuggingManager.ts#L152](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/debuggingManager.ts#L152) +[src/notebooks/debugger/debuggingManager.ts#L135](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/debugger/debuggingManager.ts#L135) ```typescript if (editor) { const controller = this.notebookToRunByLineController.get(editor.notebook); @@ -583,7 +583,7 @@ No description provided ## Locations Used -[src/platform/debugger/jupyter/notebook/debuggingManager.ts#L463](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/debuggingManager.ts#L463) +[src/kernels/debugger/debuggingManagerBase.ts#L180](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugger/debuggingManagerBase.ts#L180) ```typescript } @@ -594,6 +594,27 @@ No description provided return result; ``` + +
+ DATASCIENCE.DEBUGGING.SUCCESSFULLY_STARTED_IW_JUPYTER + +## Description + + + + + Telemetry sent when we have managed to successfully start the Interactive Window debugger using the Jupyter protocol. + +## Properties + + +No properties for event + + +## Locations Used + +Event can be removed. Not referenced anywhere +
DATASCIENCE.DEBUGGING.SUCCESSFULLY_STARTED_RUN_AND_DEBUG_CELL @@ -611,7 +632,7 @@ No properties for event ## Locations Used -[src/platform/debugger/jupyter/notebook/debugCellControllers.ts#L20](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/debugCellControllers.ts#L20) +[src/notebooks/debugger/debugCellControllers.ts#L20](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/debugger/debugCellControllers.ts#L20) ```typescript private readonly kernel: IKernel, private readonly commandManager: ICommandManager @@ -622,6 +643,18 @@ No properties for event public async willSendEvent(_msg: DebugProtocolMessage): Promise { ``` + +[src/interactive-window/debugger/jupyter/debugCellControllers.ts#L23](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/debugger/jupyter/debugCellControllers.ts#L23) +```typescript + public readonly debugCell: NotebookCell, + private readonly kernel: IKernel + ) { + sendTelemetryEvent(DebuggingTelemetry.successfullyStartedRunAndDebugCell); + } + + public async willSendEvent(_msg: DebugProtocolMessage): Promise { +``` +
DATASCIENCE.DEBUGGING.SUCCESSFULLY_STARTED_RUNBYLINE @@ -639,7 +672,7 @@ No properties for event ## Locations Used -[src/platform/debugger/jupyter/notebook/runByLineController.ts#L29](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/debugger/jupyter/notebook/runByLineController.ts#L29) +[src/notebooks/debugger/runByLineController.ts#L29](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/debugger/runByLineController.ts#L29) ```typescript private readonly kernel: IKernel, private readonly settings: IConfigurationService @@ -916,7 +949,7 @@ No properties for event ``` -[src/notebooks/controllers/vscodeNotebookController.ts#L257](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/vscodeNotebookController.ts#L257) +[src/notebooks/controllers/vscodeNotebookController.ts#L256](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/vscodeNotebookController.ts#L256) ```typescript return; } @@ -928,7 +961,7 @@ No properties for event ``` -[src/kernels/kernel.base.ts#L191](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L191) +[src/kernels/kernel.base.ts#L197](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L197) ```typescript } public async executeCell(cell: NotebookCell): Promise { @@ -1050,7 +1083,7 @@ No description provided ``` -[src/platform/export/fileConverter.ts#L64](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/export/fileConverter.ts#L64) +[src/platform/export/fileConverter.ts#L65](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/export/fileConverter.ts#L65) ```typescript } @@ -1102,7 +1135,7 @@ No description provided ## Locations Used -[src/platform/export/fileConverter.ts#L84](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/export/fileConverter.ts#L84) +[src/platform/export/fileConverter.ts#L85](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/export/fileConverter.ts#L85) ```typescript await this.performExport(format, sourceDocument, target, token, candidateInterpreter); } catch (e) { @@ -1130,7 +1163,7 @@ No properties for event ## Locations Used -[src/interactive-window/interactiveWindowCommandListener.ts#L198](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L198) +[src/interactive-window/interactiveWindowCommandListener.ts#L199](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L199) ```typescript return result; } @@ -1158,7 +1191,7 @@ No properties for event ## Locations Used -[src/interactive-window/interactiveWindowCommandListener.ts#L247](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L247) +[src/interactive-window/interactiveWindowCommandListener.ts#L248](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L248) ```typescript } } @@ -1186,7 +1219,7 @@ No properties for event ## Locations Used -[src/webviews/extension-side/variablesView/variableView.node.ts#L174](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L174) +[src/webviews/extension-side/variablesView/variableView.node.ts#L170](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L170) ```typescript } } catch (e) { @@ -1215,9 +1248,9 @@ No properties for event ## Locations Used -[src/notebooks/controllers/notebookControllerManager.ts#L899](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/notebookControllerManager.ts#L899) +[src/notebooks/controllers/notebookControllerManager.ts#L918](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/notebookControllerManager.ts#L918) ```typescript - } catch (ex) { + } // We know that this fails when we have xeus kernels installed (untill that's resolved thats one instance when we can have duplicates). sendTelemetryEvent( Telemetry.FailedToCreateNotebookController, @@ -1266,7 +1299,7 @@ No properties for event ## Locations Used -[src/kernels/kernelFinder.base.ts#L236](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernelFinder.base.ts#L236) +[src/kernels/kernelFinder.base.ts#L242](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernelFinder.base.ts#L242) ```typescript const key = `${kind}:${useCache}`; if (this.startTimeForFetching && !this.fetchingTelemetrySent.has(key)) { @@ -1294,10 +1327,10 @@ No properties for event ## Locations Used -[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L33](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L33) +[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L39](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L39) ```typescript - @inject(IJupyterRequestCreator) private readonly requestCreator: IJupyterRequestCreator - ) {} + this.serverUriStorage.onDidRemoveUris(this.onDidRemoveUris, this, this.disposables); + } @captureTelemetry(Telemetry.GetPasswordAttempt) public getPasswordConnectionInfo(url: string): Promise { @@ -1395,7 +1428,7 @@ No description provided ## Locations Used -[src/interactive-window/interactiveWindowCommandListener.ts#L372](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L372) +[src/interactive-window/interactiveWindowCommandListener.ts#L373](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L373) ```typescript return this.statusProvider.waitWithStatus(promise, message, undefined, canceled); } @@ -1407,7 +1440,7 @@ No description provided ``` -[src/interactive-window/interactiveWindowCommandListener.ts#L395](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L395) +[src/interactive-window/interactiveWindowCommandListener.ts#L396](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/interactiveWindowCommandListener.ts#L396) ```typescript } } @@ -1436,7 +1469,7 @@ No description provided ## Locations Used -[src/kernels/debugging/interactiveWindowDebugger.node.ts#L112](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugging/interactiveWindowDebugger.node.ts#L112) +[src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L111](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L111) ```typescript executeSilently(kernel.session, this.tracingEnableCode, { traceErrors: true, @@ -1448,7 +1481,7 @@ No description provided ``` -[src/kernels/debugging/interactiveWindowDebugger.node.ts#L123](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugging/interactiveWindowDebugger.node.ts#L123) +[src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L122](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L122) ```typescript executeSilently(kernel.session, this.tracingDisableCode, { traceErrors: true, @@ -1460,7 +1493,7 @@ No description provided ``` -[src/kernels/debugging/interactiveWindowDebugger.node.ts#L152](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugging/interactiveWindowDebugger.node.ts#L152) +[src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L151](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L151) ```typescript const importResults = await executeSilently(kernel.session, this.waitForDebugClientCode, { traceErrors: true, @@ -1472,7 +1505,7 @@ No description provided ``` -[src/kernels/debugging/interactiveWindowDebugger.node.ts#L275](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugging/interactiveWindowDebugger.node.ts#L275) +[src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L268](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L268) ```typescript { traceErrors: true, @@ -1484,7 +1517,7 @@ No description provided ``` -[src/kernels/debugging/interactiveWindowDebugger.node.ts#L304](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/debugging/interactiveWindowDebugger.node.ts#L304) +[src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L297](https://github.com/microsoft/vscode-jupyter/tree/main/src/interactive-window/debugger/interactiveWindowDebugger.node.ts#L297) ```typescript ? await executeSilently(kernel.session, this.enableDebuggerCode, { traceErrors: true, @@ -1512,7 +1545,7 @@ No properties for event ## Locations Used -[src/notebooks/execution/kernelExecution.ts#L207](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L207) +[src/notebooks/execution/kernelExecution.ts#L199](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L199) ```typescript this.documentExecutions.set(document, newCellExecutionQueue); return newCellExecutionQueue; @@ -1856,7 +1889,7 @@ No description provided ## Locations Used -[src/kernels/kernel.base.ts#L527](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L527) +[src/kernels/kernel.base.ts#L545](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L545) ```typescript await this.executeSilently(session, startupCode, { traceErrors: true, @@ -2656,7 +2689,7 @@ function resetData(resource: Resource, eventName: string, properties: any) { ``` -[src/notebooks/execution/kernelExecution.ts#L263](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L263) +[src/notebooks/execution/kernelExecution.ts#L255](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L255) ```typescript // Otherwise a real error occurred. sendKernelTelemetryEvent( @@ -2668,7 +2701,7 @@ function resetData(resource: Resource, eventName: string, properties: any) { ``` -[src/notebooks/execution/kernelExecution.ts#L275](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L275) +[src/notebooks/execution/kernelExecution.ts#L267](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L267) ```typescript })(); @@ -2765,7 +2798,7 @@ No properties for event ``` -[src/kernels/kernel.base.ts#L282](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L282) +[src/kernels/kernel.base.ts#L291](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L291) ```typescript await (this._jupyterSessionPromise ? this.kernelExecution.restart(this._jupyterSessionPromise) @@ -2777,7 +2810,7 @@ No properties for event ``` -[src/kernels/kernel.base.ts#L293](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L293) +[src/kernels/kernel.base.ts#L302](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L302) ```typescript this.restarting = undefined; // If we get a kernel promise failure, then restarting timed out. Just shutdown and restart the entire server. @@ -3114,7 +3147,7 @@ No properties for event ## Locations Used -[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L204](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L204) +[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L200](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L200) ```typescript case DataViewerMessages.RefreshDataViewer: @@ -3307,7 +3340,7 @@ No properties for event ## Locations Used -[src/kernels/variables/debuggerVariables.node.ts#L121](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/variables/debuggerVariables.node.ts#L121) +[src/kernels/variables/debuggerVariables.node.ts#L120](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/variables/debuggerVariables.node.ts#L120) ```typescript // Note, full variable results isn't necessary for this call. It only really needs the variable value. const result = this.lastKnownVariables.find((v) => v.name === name); @@ -4119,7 +4152,7 @@ No properties for event ``` -[src/platform/errors/errorHandler.ts#L302](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/errors/errorHandler.ts#L302) +[src/platform/errors/errorHandler.ts#L301](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/errors/errorHandler.ts#L301) ```typescript ConfigurationTarget.Workspace ); @@ -4131,7 +4164,7 @@ No properties for event ``` -[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L394](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L394) +[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L395](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L395) ```typescript ); return this.requestCreator.getFetchMethod()(url, this.addAllowUnauthorized(url, true, options)); @@ -4183,7 +4216,7 @@ No properties for event ``` -[src/platform/errors/errorHandler.ts#L294](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/errors/errorHandler.ts#L294) +[src/platform/errors/errorHandler.ts#L293](https://github.com/microsoft/vscode-jupyter/tree/main/src/platform/errors/errorHandler.ts#L293) ```typescript .showErrorMessage(DataScience.jupyterSelfCertFail().format(err.message), enableOption, closeOption) .then((value) => { @@ -4195,7 +4228,7 @@ No properties for event ``` -[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L385](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L385) +[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L386](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L386) ```typescript closeOption ); @@ -4307,7 +4340,7 @@ No description provided ## Locations Used -[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L237](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L237) +[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L233](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L233) ```typescript // Log telemetry about number of rows @@ -4319,7 +4352,7 @@ No description provided ``` -[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L320](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L320) +[src/webviews/extension-side/dataviewer/dataViewer.node.ts#L316](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/dataviewer/dataViewer.node.ts#L316) ```typescript private sendElapsedTimeTelemetry() { @@ -4574,7 +4607,7 @@ No description provided ## Locations Used -[src/kernels/kernel.base.ts#L534](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L534) +[src/kernels/kernel.base.ts#L552](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L552) ```typescript await this.executeSilently(session, this.getUserStartupCommands(), { traceErrors: true, @@ -5205,7 +5238,7 @@ No properties for event ## Locations Used -[src/notebooks/execution/cellExecution.ts#L463](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/cellExecution.ts#L463) +[src/notebooks/execution/cellExecution.ts#L449](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/cellExecution.ts#L449) ```typescript const props = { notebook: true }; if (!CellExecution.sentExecuteCellTelemetry) { @@ -5248,7 +5281,7 @@ No properties for event ## Locations Used -[src/notebooks/execution/cellExecution.ts#L465](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/cellExecution.ts#L465) +[src/notebooks/execution/cellExecution.ts#L451](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/cellExecution.ts#L451) ```typescript CellExecution.sentExecuteCellTelemetry = true; sendTelemetryEvent(Telemetry.ExecuteCellPerceivedCold, this.stopWatchForTelemetry.elapsedTime, props); @@ -5300,7 +5333,7 @@ No properties for event ## Locations Used -[src/kernels/jupyter/jupyterKernelService.node.ts#L216](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L216) +[src/kernels/jupyter/jupyterKernelService.node.ts#L217](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L217) ```typescript await this.fs.writeLocalFile(kernelSpecFilePath.fsPath, JSON.stringify(contents, undefined, 4)); } catch (ex) { @@ -5312,7 +5345,7 @@ No properties for event ``` -[src/kernels/jupyter/jupyterKernelService.node.ts#L356](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L356) +[src/kernels/jupyter/jupyterKernelService.node.ts#L349](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L349) ```typescript await this.fs.writeLocalFile(kernelSpecFilePath, JSON.stringify(specModel, undefined, 2)); } catch (ex) { @@ -5506,7 +5539,7 @@ No properties for event ## Locations Used -[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L258](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L258) +[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L259](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L259) ```typescript const requestHeaders = { Cookie: cookieString, 'X-XSRFToken': xsrfCookie }; return { requestHeaders }; @@ -5534,7 +5567,7 @@ No properties for event ## Locations Used -[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L253](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L253) +[src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L254](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/jupyterPasswordConnect.ts#L254) ```typescript // If we found everything return it all back if not, undefined as partial is useless @@ -5766,7 +5799,7 @@ No properties for event ## Locations Used -[src/notebooks/execution/kernelExecution.ts#L208](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L208) +[src/notebooks/execution/kernelExecution.ts#L200](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L200) ```typescript return newCellExecutionQueue; } @@ -6378,7 +6411,7 @@ No properties for event ## Locations Used -[src/kernels/jupyter/jupyterKernelService.node.ts#L159](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L159) +[src/kernels/jupyter/jupyterKernelService.node.ts#L160](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L160) ```typescript */ // eslint-disable-next-line @@ -6512,7 +6545,7 @@ No properties for event ## Locations Used -[src/kernels/raw/launcher/kernelLauncher.node.ts#L116](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/launcher/kernelLauncher.node.ts#L116) +[src/kernels/raw/launcher/kernelLauncher.node.ts#L117](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/launcher/kernelLauncher.node.ts#L117) ```typescript // Should be available now, wait with a timeout return await this.launchProcess(kernelConnectionMetadata, resource, workingDirectory, timeout, cancelToken); @@ -6737,14 +6770,14 @@ No properties for event ## Locations Used -[src/webviews/extension-side/variablesView/variableView.node.ts#L84](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L84) +[src/webviews/extension-side/variablesView/variableView.node.ts#L80](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L80) ```typescript console.log(`Done initing variables`); } @captureTelemetry(Telemetry.NativeVariableViewLoaded) public async load(codeWebview: vscodeWebviewView) { - await super.loadWebview(process.cwd(), codeWebview).catch(traceError); + await super.loadWebview(Uri.file(process.cwd()), codeWebview).catch(traceError); ``` @@ -6765,7 +6798,7 @@ No properties for event ## Locations Used -[src/webviews/extension-side/variablesView/variableView.node.ts#L143](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L143) +[src/webviews/extension-side/variablesView/variableView.node.ts#L139](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L139) ```typescript // I've we've been made visible, make sure that we are updated @@ -6939,7 +6972,7 @@ No properties for event ## Locations Used -[src/kernels/kernel.base.ts#L323](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L323) +[src/kernels/kernel.base.ts#L332](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L332) ```typescript // Setup telemetry if (!this.perceivedJupyterStartupTelemetryCaptured) { @@ -6951,7 +6984,7 @@ No properties for event ``` -[src/kernels/kernel.base.ts#L406](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L406) +[src/kernels/kernel.base.ts#L424](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L424) ```typescript sendKernelTelemetryEvent( @@ -6981,7 +7014,7 @@ No properties for event ## Locations Used -[src/notebooks/controllers/notebookControllerManager.ts#L768](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/notebookControllerManager.ts#L768) +[src/notebooks/controllers/notebookControllerManager.ts#L774](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/notebookControllerManager.ts#L774) ```typescript ? PYTHON_LANGUAGE : getTelemetrySafeLanguage(getLanguageInNotebookMetadata(notebookMetadata) || ''); @@ -7008,7 +7041,7 @@ No description provided ## Locations Used -[src/notebooks/controllers/notebookControllerManager.ts#L748](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/notebookControllerManager.ts#L748) +[src/notebooks/controllers/notebookControllerManager.ts#L754](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/notebookControllerManager.ts#L754) ```typescript onlyConnection && (matchReason |= PreferredKernelExactMatchReason.OnlyKernel); topMatchIsPreferredInterpreter && (matchReason |= PreferredKernelExactMatchReason.WasPreferredInterpreter); @@ -7479,7 +7512,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L347](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L347) +[src/kernels/raw/session/rawJupyterSession.node.ts#L350](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L350) ```typescript } else { traceWarning(`Didn't get response for requestKernelInfo after ${stopWatch.elapsedTime}ms.`); @@ -7507,7 +7540,7 @@ No properties for event ## Locations Used -[src/kernels/raw/launcher/kernelProcess.node.ts#L131](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/launcher/kernelProcess.node.ts#L131) +[src/kernels/raw/launcher/kernelProcess.node.ts#L135](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/launcher/kernelProcess.node.ts#L135) ```typescript } } @@ -7536,7 +7569,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L140](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L140) +[src/kernels/raw/session/rawJupyterSession.node.ts#L142](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L142) ```typescript throw error; } @@ -7597,7 +7630,7 @@ No properties for event ## Locations Used -[src/kernels/raw/launcher/kernelLauncher.node.ts#L219](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/launcher/kernelLauncher.node.ts#L219) +[src/kernels/raw/launcher/kernelLauncher.node.ts#L224](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/launcher/kernelLauncher.node.ts#L224) ```typescript const disposable = kernelProcess.exited( @@ -7621,7 +7654,7 @@ No properties for event ``` -[src/kernels/raw/session/rawJupyterSession.node.ts#L184](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L184) +[src/kernels/raw/session/rawJupyterSession.node.ts#L186](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L186) ```typescript if (session !== this.session) { return; @@ -7649,7 +7682,7 @@ No properties for event ## Locations Used -[src/kernels/kernelConnector.ts#L143](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernelConnector.ts#L143) +[src/kernels/kernelConnector.ts#L144](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernelConnector.ts#L144) ```typescript const rawNotebookProvider = serviceContainer.tryGet(IRawNotebookProvider); const rawLocalKernel = rawNotebookProvider?.isSupported && isLocal; @@ -7680,7 +7713,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L158](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L158) +[src/kernels/raw/session/rawJupyterSession.node.ts#L160](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L160) ```typescript // We want to know why we got shut down const stacktrace = new Error().stack; @@ -7709,7 +7742,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L83](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L83) +[src/kernels/raw/session/rawJupyterSession.node.ts#L85](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L85) ```typescript Cancellation.throwIfCanceled(options.token); // Only connect our session if we didn't cancel or timeout @@ -7721,9 +7754,9 @@ No properties for event ``` -[src/kernels/raw/session/rawJupyterSession.node.ts#L98](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L98) +[src/kernels/raw/session/rawJupyterSession.node.ts#L100](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L100) ```typescript - if (error instanceof CancellationError) { + if (isCancellationError(error) || options.token.isCancellationRequested) { sendKernelTelemetryEvent( this.resource, Telemetry.RawKernelSessionStart, @@ -7733,7 +7766,7 @@ No properties for event ``` -[src/kernels/raw/session/rawJupyterSession.node.ts#L109](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L109) +[src/kernels/raw/session/rawJupyterSession.node.ts#L111](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L111) ```typescript } else if (error instanceof TimedOutError) { sendKernelTelemetryEvent( @@ -7745,7 +7778,7 @@ No properties for event ``` -[src/kernels/raw/session/rawJupyterSession.node.ts#L121](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L121) +[src/kernels/raw/session/rawJupyterSession.node.ts#L123](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L123) ```typescript // Send our telemetry event with the error included sendKernelTelemetryEvent( @@ -7773,7 +7806,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L129](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L129) +[src/kernels/raw/session/rawJupyterSession.node.ts#L131](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L131) ```typescript ); sendKernelTelemetryEvent( @@ -7801,7 +7834,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L82](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L82) +[src/kernels/raw/session/rawJupyterSession.node.ts#L84](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L84) ```typescript newSession = await this.startRawSession(options); Cancellation.throwIfCanceled(options.token); @@ -7829,7 +7862,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L114](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L114) +[src/kernels/raw/session/rawJupyterSession.node.ts#L116](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L116) ```typescript undefined, error @@ -7857,13 +7890,13 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L103](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L103) +[src/kernels/raw/session/rawJupyterSession.node.ts#L105](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L105) ```typescript undefined, error ); sendKernelTelemetryEvent(this.resource, Telemetry.RawKernelSessionStartUserCancel); - traceInfo('Starting of raw session cancelled by user'); + traceVerbose('Starting of raw session cancelled by user'); throw error; } else if (error instanceof TimedOutError) { ``` @@ -7885,7 +7918,7 @@ No properties for event ## Locations Used -[src/kernels/raw/session/rawJupyterSession.node.ts#L235](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L235) +[src/kernels/raw/session/rawJupyterSession.node.ts#L238](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/raw/session/rawJupyterSession.node.ts#L238) ```typescript return this.startRawSession({ token: cancelToken, ui: new DisplayOptions(disableUI) }); } @@ -7913,7 +7946,7 @@ No properties for event ## Locations Used -[src/kernels/jupyter/jupyterKernelService.node.ts#L238](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L238) +[src/kernels/jupyter/jupyterKernelService.node.ts#L239](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/jupyterKernelService.node.ts#L239) ```typescript ); } @@ -7962,7 +7995,7 @@ No properties for event ## Locations Used -[src/notebooks/execution/kernelExecution.ts#L283](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L283) +[src/notebooks/execution/kernelExecution.ts#L275](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L275) ```typescript } @@ -7990,7 +8023,7 @@ No properties for event ## Locations Used -[src/notebooks/execution/kernelExecution.ts#L282](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L282) +[src/notebooks/execution/kernelExecution.ts#L274](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/execution/kernelExecution.ts#L274) ```typescript }); } @@ -8278,7 +8311,7 @@ No properties for event ## Locations Used -[src/kernels/kernel.base.ts#L325](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L325) +[src/kernels/kernel.base.ts#L334](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/kernel.base.ts#L334) ```typescript this.perceivedJupyterStartupTelemetryCaptured = true; sendTelemetryEvent(Telemetry.PerceivedJupyterStartupNotebook, stopWatch.elapsedTime); @@ -8306,7 +8339,7 @@ No properties for event ## Locations Used -[src/kernels/jupyter/launcher/notebookStarter.node.ts#L152](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/notebookStarter.node.ts#L152) +[src/kernels/jupyter/launcher/notebookStarter.node.ts#L156](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/launcher/notebookStarter.node.ts#L156) ```typescript } @@ -8359,7 +8392,7 @@ No properties for event ## Locations Used -[src/kernels/common/baseJupyterSession.ts#L69](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/common/baseJupyterSession.ts#L69) +[src/kernels/common/baseJupyterSession.ts#L70](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/common/baseJupyterSession.ts#L70) ```typescript export class JupyterSessionStartError extends WrappedError { constructor(originalException: Error) { @@ -8611,7 +8644,7 @@ No description provided ## Locations Used -[src/webviews/extension-side/variablesView/variableView.node.ts#L187](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L187) +[src/webviews/extension-side/variablesView/variableView.node.ts#L183](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/variablesView/variableView.node.ts#L183) ```typescript const response = await this.variables.getVariables(args, activeNotebook); @@ -8638,7 +8671,7 @@ No description provided ## Locations Used -[src/notebooks/helpers.ts#L749](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/helpers.ts#L749) +[src/notebooks/helpers.ts#L671](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/helpers.ts#L671) ```typescript // Unless we already know its an unknown output type. const outputType: nbformat.OutputType = @@ -8666,7 +8699,7 @@ No properties for event ## Locations Used -[src/kernels/jupyter/session/jupyterSession.ts#L74](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/session/jupyterSession.ts#L74) +[src/kernels/jupyter/session/jupyterSession.ts#L67](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/session/jupyterSession.ts#L67) ```typescript return true; } @@ -8722,7 +8755,7 @@ No description provided ## Locations Used -[src/webviews/extension-side/webviewHost.node.ts#L281](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/webviewHost.node.ts#L281) +[src/webviews/extension-side/webviewHost.ts#L279](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/webviewHost.ts#L279) ```typescript protected webViewRendered() { if (this.webviewInit && !this.webviewInit.resolved) { @@ -8750,15 +8783,15 @@ No properties for event ## Locations Used -[src/webviews/extension-side/webviewHost.node.ts#L299](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/webviewHost.node.ts#L299) +[src/webviews/extension-side/webviewHost.ts#L297](https://github.com/microsoft/vscode-jupyter/tree/main/src/webviews/extension-side/webviewHost.ts#L297) ```typescript this.dispose(); }; @captureTelemetry(Telemetry.WebviewStyleUpdate) - private async handleCssRequest(request: IGetCssRequest): Promise { - const settings = await this.generateDataScienceExtraSettings(); - const requestIsDark = settings.ignoreVscodeTheme ? false : request?.isDark; + private async handleCssRequest(): Promise { + const isDark = await this.isDark(); + return this.postMessageInternal(CssMessages.GetCssResponse, { ```
diff --git a/src/kernels/common/baseJupyterSession.ts b/src/kernels/common/baseJupyterSession.ts index 881c210ec71..172ccf3b343 100644 --- a/src/kernels/common/baseJupyterSession.ts +++ b/src/kernels/common/baseJupyterSession.ts @@ -31,6 +31,7 @@ import { ChainingExecuteRequester } from './chainingExecuteRequester'; import { getResourceType } from '../../platform/common/utils'; import { KernelProgressReporter } from '../../platform/progress/kernelProgressReporter'; import { isTestExecution } from '../../platform/common/constants'; +import { KernelConnectionWrapper } from './kernelConnectionWrapper'; // eslint-disable-next-line @typescript-eslint/no-explicit-any export function suppressShutdownErrors(realKernel: any) { @@ -71,6 +72,13 @@ export class JupyterSessionStartError extends WrappedError { } export abstract class BaseJupyterSession implements IJupyterSession { + /** + * Keep a single instance of KernelConnectionWrapper. + * This way when sessions change, we still have a single Kernel.IKernelConnection proxy (wrapper), + * which will have all of the event handlers bound to it. + * This allows consumers to add event handlers hand not worry about internals & can use the lower level Jupyter API. + */ + private _wrappedKernel?: KernelConnectionWrapper; private _isDisposed?: boolean; private readonly _disposed = new EventEmitter(); protected readonly disposables: IDisposable[] = []; @@ -83,28 +91,30 @@ export abstract class BaseJupyterSession implements IJupyterSession { protected get session(): ISessionWithSocket | undefined { return this._session; } + public get kernelId(): string { + return this.session?.kernel?.id || ''; + } public get kernel(): Kernel.IKernelConnection | undefined { - return this._session?.kernel || undefined; + if (this._wrappedKernel) { + return this._wrappedKernel; + } + if (!this._session?.kernel) { + return; + } + this._wrappedKernel = new KernelConnectionWrapper(this._session.kernel, this.disposables); + return this._wrappedKernel; } + public get kernelSocket(): Observable { return this._kernelSocket; } public get onSessionStatusChanged(): Event { return this.onStatusChangedEvent.event; } - public get onIOPubMessage(): Event { - if (!this.ioPubEventEmitter) { - this.ioPubEventEmitter = new EventEmitter(); - } - return this.ioPubEventEmitter.event; - } - public get status(): KernelMessage.Status { return this.getServerStatus(); } - public abstract get kernelId(): string; - public get isConnected(): boolean { return this.connected; } @@ -115,8 +125,6 @@ export abstract class BaseJupyterSession implements IJupyterSession { protected restartSessionPromise?: { token: CancellationTokenSource; promise: Promise }; private _session: ISessionWithSocket | undefined; private _kernelSocket = new ReplaySubject(); - private ioPubEventEmitter = new EventEmitter(); - private ioPubHandler: Slot; private unhandledMessageHandler: Slot; private chainingExecute = new ChainingExecuteRequester(); @@ -128,7 +136,6 @@ export abstract class BaseJupyterSession implements IJupyterSession { private readonly interruptTimeout: number ) { this.statusHandler = this.onStatusChanged.bind(this); - this.ioPubHandler = (_s, m) => this.ioPubEventEmitter.fire(m); this.unhandledMessageHandler = (_s, m) => { traceInfo(`Unhandled message found: ${m.header.msg_type}`); }; @@ -346,9 +353,6 @@ export abstract class BaseJupyterSession implements IJupyterSession { protected setSession(session: ISessionWithSocket | undefined, forceUpdateKernelSocketInfo: boolean = false) { const oldSession = this._session; if (oldSession) { - if (this.ioPubHandler) { - oldSession.iopubMessage.disconnect(this.ioPubHandler); - } if (this.unhandledMessageHandler) { oldSession.unhandledMessage.disconnect(this.unhandledMessageHandler); } @@ -358,12 +362,12 @@ export abstract class BaseJupyterSession implements IJupyterSession { } this._session = session; if (session) { + if (session.kernel && this._wrappedKernel) { + this._wrappedKernel.changeKernel(session.kernel); + } + // Listen for session status changes session.statusChanged.connect(this.statusHandler); - - if (session.iopubMessage) { - session.iopubMessage.connect(this.ioPubHandler); - } if (session.unhandledMessage) { session.unhandledMessage.connect(this.unhandledMessageHandler); } diff --git a/src/kernels/common/kernelConnectionWrapper.ts b/src/kernels/common/kernelConnectionWrapper.ts new file mode 100644 index 00000000000..df0b9730a43 --- /dev/null +++ b/src/kernels/common/kernelConnectionWrapper.ts @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import type { Kernel } from '@jupyterlab/services'; +import { IDisposable } from '../../platform/common/types'; +import { BaseKernelConnectionWrapper } from '../jupyter/baseKernelConnectionWrapper'; + +export class KernelConnectionWrapper extends BaseKernelConnectionWrapper { + /** + * Use `kernelConnection` to access the value as its not a constant (can change over time). + * E.g. when restarting kernels or the like. + */ + private _kernelConnection!: Kernel.IKernelConnection; + protected get possibleKernelConnection(): undefined | Kernel.IKernelConnection { + return this._kernelConnection; + } + public get kernel() { + return this._kernelConnection; + } + + constructor(kernel: Kernel.IKernelConnection, disposables: IDisposable[]) { + super(kernel, disposables); + this._kernelConnection = kernel; + } + public changeKernel(kernel: Kernel.IKernelConnection) { + if (this.kernel === kernel) { + return; + } + this.stopHandlingKernelMessages(this.possibleKernelConnection!); + this._kernelConnection = kernel; + this.startHandleKernelMessages(kernel); + } + async shutdown(): Promise { + await this._kernelConnection.shutdown(); + } + dispose(): void { + this._kernelConnection.dispose(); + } + async interrupt(): Promise { + await this._kernelConnection.interrupt(); + } + async restart(): Promise { + await this._kernelConnection.restart(); + } + protected override startHandleKernelMessages(kernelConnection: Kernel.IKernelConnection) { + super.startHandleKernelMessages(kernelConnection); + this._kernelConnection = kernelConnection; + kernelConnection.connectionStatusChanged.connect(this.onConnectionStatusChanged, this); + kernelConnection.statusChanged.connect(this.onStatusChanged, this); + kernelConnection.disposed.connect(this.onDisposed, this); + } + protected override stopHandlingKernelMessages(kernelConnection: Kernel.IKernelConnection): void { + super.stopHandlingKernelMessages(kernelConnection); + kernelConnection.connectionStatusChanged.disconnect(this.onConnectionStatusChanged, this); + kernelConnection.statusChanged.disconnect(this.onStatusChanged, this); + kernelConnection.disposed.disconnect(this.onDisposed, this); + } + private onDisposed(connection: Kernel.IKernelConnection) { + if (connection === this.possibleKernelConnection) { + this.disposed.emit(); + } + } + private onStatusChanged(connection: Kernel.IKernelConnection, args: Kernel.Status) { + if (connection === this.possibleKernelConnection) { + this.statusChanged.emit(args); + } + } + private onConnectionStatusChanged(connection: Kernel.IKernelConnection, args: Kernel.ConnectionStatus) { + if (connection === this.possibleKernelConnection) { + this.connectionStatusChanged.emit(args); + } + } +} diff --git a/src/kernels/debugger/kernelDebugAdapterBase.ts b/src/kernels/debugger/kernelDebugAdapterBase.ts index d2a616a8e26..e07cb305dda 100644 --- a/src/kernels/debugger/kernelDebugAdapterBase.ts +++ b/src/kernels/debugger/kernelDebugAdapterBase.ts @@ -10,6 +10,7 @@ import { DebugAdapter, DebugProtocolMessage, DebugSession, + Disposable, Event, EventEmitter, NotebookCell, @@ -33,7 +34,6 @@ import { IDebugInfoResponse } from './types'; import { sendTelemetryEvent } from '../../telemetry'; -import { IDisposable } from '../../platform/common/types'; import { traceError, traceInfo, traceInfoIfCI, traceVerbose } from '../../platform/logging'; import { assertIsDebugConfig, @@ -42,6 +42,7 @@ import { getMessageSourceAndHookIt } from '../../notebooks/debugger/helper'; import { ResourceMap } from '../../platform/vscode-path/map'; +import { IDisposable } from '../../platform/common/types'; /** * For info on the custom requests implemented by jupyter see: @@ -89,18 +90,9 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb this.debugCell = notebookDocument.cellAt(configuration.__cellIndex!); } + this.jupyterSession.kernel?.iopubMessage.connect(this.onIOPubMessage, this); this.disposables.push( - this.jupyterSession.onIOPubMessage(async (msg: KernelMessage.IIOPubMessage) => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const anyMsg = msg as any; - traceInfoIfCI(`Debug IO Pub message: ${JSON.stringify(msg)}`); - if (anyMsg.header.msg_type === 'debug_event') { - this.trace('event', JSON.stringify(msg)); - if (!(await this.delegate?.willSendEvent(anyMsg))) { - this.sendMessage.fire(msg.content); - } - } - }) + new Disposable(() => this.jupyterSession.kernel?.iopubMessage.disconnect(this.onIOPubMessage, this)) ); if (this.kernel) { @@ -157,6 +149,17 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb traceVerbose(`[Debug] ${tag}: ${msg}`); } + async onIOPubMessage(_: unknown, msg: KernelMessage.IIOPubMessage) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const anyMsg = msg as any; + traceInfoIfCI(`Debug IO Pub message: ${JSON.stringify(msg)}`); + if (anyMsg.header.msg_type === 'debug_event') { + this.trace('event', JSON.stringify(msg)); + if (!(await this.delegate?.willSendEvent(anyMsg))) { + this.sendMessage.fire(msg.content); + } + } + } async handleMessage(message: DebugProtocol.ProtocolMessage) { try { traceInfoIfCI(`KernelDebugAdapter::handleMessage ${JSON.stringify(message, undefined, ' ')}`); diff --git a/src/kernels/kernelConnectionWrapper.ts b/src/kernels/jupyter/baseKernelConnectionWrapper.ts similarity index 72% rename from src/kernels/kernelConnectionWrapper.ts rename to src/kernels/jupyter/baseKernelConnectionWrapper.ts index 8c5c0a766c4..6a828ed19e2 100644 --- a/src/kernels/kernelConnectionWrapper.ts +++ b/src/kernels/jupyter/baseKernelConnectionWrapper.ts @@ -34,11 +34,9 @@ import { ISpecModel } from '@jupyterlab/services/lib/kernelspec/restapi'; import { JSONObject } from '@lumino/coreutils'; import { Signal } from '@lumino/signaling'; import { Disposable } from 'vscode'; -import { IDisposable } from '../platform/common/types'; -import { noop } from '../platform/common/utils/misc'; -import { IKernel } from './types'; +import { IDisposable } from '../../platform/common/types'; -export class KernelConnectionWrapper implements Kernel.IKernelConnection { +export abstract class BaseKernelConnectionWrapper implements Kernel.IKernelConnection { public readonly statusChanged = new Signal(this); public readonly connectionStatusChanged = new Signal(this); public readonly iopubMessage = new Signal>(this); @@ -48,23 +46,8 @@ export class KernelConnectionWrapper implements Kernel.IKernelConnection { return (this.possibleKernelConnection || this._previousKernelConnection).serverSettings; } public readonly disposed = new Signal(this); - /** - * Use `kernelConnection` to access the value as its not a constant (can change over time). - * E.g. when restarting kernels or the like. - */ - private _kernelConnection!: Kernel.IKernelConnection; - private readonly _previousKernelConnection: Kernel.IKernelConnection; // private _isRestarting?: boolean; - private get possibleKernelConnection(): undefined | Kernel.IKernelConnection { - if (this.kernel.session?.kernel === this._kernelConnection) { - return this._kernelConnection; - } - this.stopHandlingKernelMessages(this._kernelConnection); - if (this.kernel.session?.kernel) { - this.startHandleKernelMessages(this.kernel.session.kernel); - return this._kernelConnection; - } - } + protected abstract get possibleKernelConnection(): undefined | Kernel.IKernelConnection; private getKernelConnection(): Kernel.IKernelConnection { if (!this.possibleKernelConnection) { throw new Error( @@ -74,27 +57,8 @@ export class KernelConnectionWrapper implements Kernel.IKernelConnection { return this.possibleKernelConnection; } - constructor(readonly kernel: IKernel, disposables: IDisposable[]) { - const emiStatusChangeEvents = () => { - this.statusChanged.emit(kernel.status); - if (kernel.status === 'dead' && !kernel.disposed && !kernel.disposing) { - this.connectionStatusChanged.emit('disconnected'); - } - }; - kernel.onDisposed( - () => { - // this._isRestarting = false; - emiStatusChangeEvents(); - this.disposed.emit(); - }, - this, - disposables - ); - kernel.onStarted(emiStatusChangeEvents, this, disposables); - kernel.onRestarted(emiStatusChangeEvents, this, disposables); - kernel.onStatusChanged(emiStatusChangeEvents, this, disposables); - this._previousKernelConnection = kernel.session!.kernel!; - this.startHandleKernelMessages(kernel.session!.kernel!); + constructor(private _previousKernelConnection: Kernel.IKernelConnection, disposables: IDisposable[]) { + this.startHandleKernelMessages(_previousKernelConnection); disposables.push( new Disposable(() => { if (this.possibleKernelConnection) { @@ -103,6 +67,11 @@ export class KernelConnectionWrapper implements Kernel.IKernelConnection { }) ); } + abstract shutdown(): Promise; + abstract dispose(): void; + abstract interrupt(): Promise; + abstract restart(): Promise; + public get id(): string { return (this.possibleKernelConnection || this._previousKernelConnection).id; } @@ -110,7 +79,7 @@ export class KernelConnectionWrapper implements Kernel.IKernelConnection { return (this.possibleKernelConnection || this._previousKernelConnection).name; } public get isDisposed(): boolean { - return this.kernel.disposed; + return this.possibleKernelConnection ? this.possibleKernelConnection?.isDisposed === true : true; } public get model(): Kernel.IModel { @@ -233,55 +202,18 @@ export class KernelConnectionWrapper implements Kernel.IKernelConnection { ): void { return this.getKernelConnection().removeMessageHook(msgId, hook); } - async shutdown(): Promise { - if ( - this.kernel.kernelConnectionMetadata.kind === 'startUsingRemoteKernelSpec' || - this.kernel.kernelConnectionMetadata.kind === 'connectToLiveRemoteKernel' - ) { - await this.kernel.session?.shutdown(); - } - await this.kernel.dispose(); - } + clone( _options?: Pick ): Kernel.IKernelConnection { throw new Error('Method not implemented.'); } - dispose(): void { - this.kernel.dispose().catch(noop); - } - async interrupt(): Promise { - // Sometimes we end up starting a new session. - // Hence assume a new session was created, meaning we need to bind to the kernel connection all over again. - this.stopHandlingKernelMessages(this.possibleKernelConnection!); - - await this.kernel.interrupt(); - - if (!this.kernel.session?.kernel) { - throw new Error('Restart failed'); - } - this.startHandleKernelMessages(this.kernel.session?.kernel); - } - async restart(): Promise { - if (this.possibleKernelConnection) { - this.stopHandlingKernelMessages(this.possibleKernelConnection); - } - - // If this is a remote, then we do something special. - await this.kernel.restart(); - - if (!this.kernel.session?.kernel) { - throw new Error('Restart failed'); - } - this.startHandleKernelMessages(this.kernel.session?.kernel); - } - private startHandleKernelMessages(kernelConnection: Kernel.IKernelConnection) { - this._kernelConnection = kernelConnection; + protected startHandleKernelMessages(kernelConnection: Kernel.IKernelConnection) { kernelConnection.anyMessage.connect(this.onAnyMessage, this); kernelConnection.iopubMessage.connect(this.onIOPubMessage, this); kernelConnection.unhandledMessage.connect(this.onUnhandledMessage, this); } - private stopHandlingKernelMessages(kernelConnection: Kernel.IKernelConnection) { + protected stopHandlingKernelMessages(kernelConnection: Kernel.IKernelConnection) { kernelConnection.anyMessage.disconnect(this.onAnyMessage, this); kernelConnection.iopubMessage.disconnect(this.onIOPubMessage, this); kernelConnection.unhandledMessage.disconnect(this.onUnhandledMessage, this); diff --git a/src/kernels/jupyter/session/jupyterSession.ts b/src/kernels/jupyter/session/jupyterSession.ts index 375a85277d9..93940918d5e 100644 --- a/src/kernels/jupyter/session/jupyterSession.ts +++ b/src/kernels/jupyter/session/jupyterSession.ts @@ -1,14 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import type { - Contents, - ContentsManager, - Kernel, - KernelSpecManager, - Session, - SessionManager -} from '@jupyterlab/services'; +import type { Contents, ContentsManager, KernelSpecManager, Session, SessionManager } from '@jupyterlab/services'; import * as uuid from 'uuid/v4'; import { CancellationToken, CancellationTokenSource } from 'vscode-jsonrpc'; import { Cancellation } from '../../../platform/common/cancellation'; @@ -76,15 +69,6 @@ export class JupyterSession extends BaseJupyterSession implements IJupyterServer // Wait for idle on this session return this.waitForIdleOnSession(this.session, timeout); } - - public override get kernel(): Kernel.IKernelConnection | undefined { - return this.session?.kernel || undefined; - } - - public get kernelId(): string { - return this.session?.kernel?.id || ''; - } - public async connect(options: { token: CancellationToken; ui: IDisplayOptions }): Promise { // Start a new session this.setSession(await this.createNewKernelSession(options)); @@ -158,7 +142,6 @@ export class JupyterSession extends BaseJupyterSession implements IJupyterServer return newSession; } - protected async createRestartSession( disableUI: boolean, session: ISessionWithSocket, diff --git a/src/kernels/raw/session/rawJupyterSession.node.ts b/src/kernels/raw/session/rawJupyterSession.node.ts index 5eab7f33b16..4afc7237b31 100644 --- a/src/kernels/raw/session/rawJupyterSession.node.ts +++ b/src/kernels/raw/session/rawJupyterSession.node.ts @@ -51,9 +51,6 @@ export class RawJupyterSession extends BaseJupyterSession { } return super.status; } - public get kernelId(): string { - return this.session?.kernel?.id || ''; - } constructor( private readonly kernelLauncher: IKernelLauncher, resource: Resource, diff --git a/src/kernels/raw/session/rawKernel.node.ts b/src/kernels/raw/session/rawKernel.node.ts index 4df68ec1e05..b335c21c2fd 100644 --- a/src/kernels/raw/session/rawKernel.node.ts +++ b/src/kernels/raw/session/rawKernel.node.ts @@ -276,9 +276,17 @@ export function createRawKernel(kernelProcess: IKernelProcess, clientId: string) // Dummy websocket we give to the underlying real kernel let socketInstance: any; + let rawKernel: RawKernel; class RawSocketWrapper extends RawSocket { constructor() { - super(kernelProcess.connection, jupyterLabSerialize.serialize, jupyterLabSerialize.deserialize); + super(kernelProcess.connection, jupyterLabSerialize.serialize, jupyterLabSerialize.deserialize, (msg) => { + if (rawKernel) { + // These messages are sent directly to the kernel bypassing the Jupyter lab npm libraries. + // As a result, we don't get any notification that messages were sent (on the anymessage signal). + // To ensure those signals can still be used to monitor such messages, send them via a callback so that we can emit these messages on the anymessage signal. + rawKernel?.anyMessage.emit({ direction: 'send', msg }); + } + }); socketInstance = this; } } @@ -309,5 +317,6 @@ export function createRawKernel(kernelProcess: IKernelProcess, clientId: string) }); // Use this real kernel in result. - return new RawKernel(realKernel, socketInstance, kernelProcess); + rawKernel = new RawKernel(realKernel, socketInstance, kernelProcess); + return rawKernel; } diff --git a/src/kernels/raw/session/rawSocket.node.ts b/src/kernels/raw/session/rawSocket.node.ts index b5b2d1a424e..1915e6c99df 100644 --- a/src/kernels/raw/session/rawSocket.node.ts +++ b/src/kernels/raw/session/rawSocket.node.ts @@ -48,7 +48,8 @@ export class RawSocket implements IWebSocketLike, IKernelSocket, IDisposable { constructor( private connection: IKernelConnection, private serialize: (msg: KernelMessage.IMessage) => string | ArrayBuffer, - private deserialize: (data: ArrayBuffer | string) => KernelMessage.IMessage + private deserialize: (data: ArrayBuffer | string) => KernelMessage.IMessage, + private readonly onAnyMessage: (msg: KernelMessage.IMessage) => void ) { // Setup our ZMQ channels now this.channels = this.generateChannels(connection); @@ -100,7 +101,10 @@ export class RawSocket implements IWebSocketLike, IKernelSocket, IDisposable { // If from ipywidgets, this will be serialized already, so turn it back into a message so // we can add the special hash to it. const message = this.deserialize(data); - + // These messages are sent directly to the kernel bypassing the Jupyter lab npm libraries. + // As a result, we don't get any notification that messages were sent (on the anymessage signal). + // To ensure those signals can still be used to monitor such messages, send them via a callback so that we can emit these messages on the anymessage signal. + this.onAnyMessage(message as any); // Send this directly (don't call back into the hooks) this.sendMessage(message, true); } diff --git a/src/kernels/types.ts b/src/kernels/types.ts index 7ba266a512f..cbcccd76de5 100644 --- a/src/kernels/types.ts +++ b/src/kernels/types.ts @@ -270,7 +270,6 @@ export interface IJupyterSession extends IAsyncDisposable { isServerSession(): this is IJupyterServerSession; onSessionStatusChanged: Event; onDidDispose: Event; - onIOPubMessage: Event; interrupt(): Promise; restart(): Promise; waitForIdle(timeout: number): Promise; diff --git a/src/platform/api/kernelApi.ts b/src/platform/api/kernelApi.ts index 60be576b92d..eb8ffa3fbe7 100644 --- a/src/platform/api/kernelApi.ts +++ b/src/platform/api/kernelApi.ts @@ -3,7 +3,7 @@ import { injectable, inject } from 'inversify'; import { Disposable, Event, EventEmitter, Uri } from 'vscode'; -import { KernelConnectionWrapper } from '../../kernels/kernelConnectionWrapper'; +import { KernelConnectionWrapper } from './kernelConnectionWrapper'; import { IKernelProvider, IKernel, diff --git a/src/platform/api/kernelConnectionWrapper.ts b/src/platform/api/kernelConnectionWrapper.ts new file mode 100644 index 00000000000..826351b68b8 --- /dev/null +++ b/src/platform/api/kernelConnectionWrapper.ts @@ -0,0 +1,90 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import type { Kernel } from '@jupyterlab/services'; +import { IDisposable } from '../common/types'; +import { noop } from '../common/utils/misc'; +import { BaseKernelConnectionWrapper } from '../../kernels/jupyter/baseKernelConnectionWrapper'; +import { IKernel } from '../../kernels/types'; + +export class KernelConnectionWrapper extends BaseKernelConnectionWrapper { + /** + * Use `kernelConnection` to access the value as its not a constant (can change over time). + * E.g. when restarting kernels or the like. + */ + private _kernelConnection!: Kernel.IKernelConnection; + protected get possibleKernelConnection(): undefined | Kernel.IKernelConnection { + if (this.kernel.session?.kernel === this._kernelConnection) { + return this._kernelConnection; + } + this.stopHandlingKernelMessages(this._kernelConnection); + if (this.kernel.session?.kernel) { + this.startHandleKernelMessages(this.kernel.session.kernel); + return this._kernelConnection; + } + } + + constructor(readonly kernel: IKernel, disposables: IDisposable[]) { + super(kernel.session!.kernel!, disposables); + const emiStatusChangeEvents = () => { + this.statusChanged.emit(kernel.status); + if (kernel.status === 'dead' && !kernel.disposed && !kernel.disposing) { + this.connectionStatusChanged.emit('disconnected'); + } + }; + kernel.onDisposed( + () => { + // this._isRestarting = false; + emiStatusChangeEvents(); + this.disposed.emit(); + }, + this, + disposables + ); + kernel.onStarted(emiStatusChangeEvents, this, disposables); + kernel.onRestarted(emiStatusChangeEvents, this, disposables); + kernel.onStatusChanged(emiStatusChangeEvents, this, disposables); + this.startHandleKernelMessages(kernel.session!.kernel!); + } + async shutdown(): Promise { + if ( + this.kernel.kernelConnectionMetadata.kind === 'startUsingRemoteKernelSpec' || + this.kernel.kernelConnectionMetadata.kind === 'connectToLiveRemoteKernel' + ) { + await this.kernel.session?.shutdown(); + } + await this.kernel.dispose(); + } + dispose(): void { + this.kernel.dispose().catch(noop); + } + async interrupt(): Promise { + // Sometimes we end up starting a new session. + // Hence assume a new session was created, meaning we need to bind to the kernel connection all over again. + this.stopHandlingKernelMessages(this.possibleKernelConnection!); + + await this.kernel.interrupt(); + + if (!this.kernel.session?.kernel) { + throw new Error('Restart failed'); + } + this.startHandleKernelMessages(this.kernel.session?.kernel); + } + async restart(): Promise { + if (this.possibleKernelConnection) { + this.stopHandlingKernelMessages(this.possibleKernelConnection); + } + + // If this is a remote, then we do something special. + await this.kernel.restart(); + + if (!this.kernel.session?.kernel) { + throw new Error('Restart failed'); + } + this.startHandleKernelMessages(this.kernel.session?.kernel); + } + protected override startHandleKernelMessages(kernelConnection: Kernel.IKernelConnection) { + this._kernelConnection = kernelConnection; + super.startHandleKernelMessages(kernelConnection); + } +} diff --git a/src/test/common.node.ts b/src/test/common.node.ts index 6e27e840ee8..16bf55135b7 100644 --- a/src/test/common.node.ts +++ b/src/test/common.node.ts @@ -9,14 +9,14 @@ import * as fs from 'fs-extra'; import * as path from '../platform/vscode-path/path'; import * as uuid from 'uuid/v4'; import { coerce, SemVer } from 'semver'; -import type { ConfigurationTarget, Event, TextDocument, Uri } from 'vscode'; +import type { ConfigurationTarget, TextDocument, Uri } from 'vscode'; import { IProcessService } from '../platform/common/process/types.node'; -import { IDisposable } from '../platform/common/types'; import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_MULTI_ROOT_TEST, IS_PERF_TEST, IS_SMOKE_TEST } from './constants.node'; import { noop } from './core'; import { isCI } from '../platform/common/constants'; import { IWorkspaceService } from '../platform/common/application/types'; -import { waitForCondition } from './common'; + +export { createEventHandler } from './common'; const StreamZip = require('node-stream-zip'); @@ -302,87 +302,6 @@ export async function openFile(file: string): Promise { assert(vscode.window.activeTextEditor, 'No active editor'); return textDocument; } - -/** - * Helper class to test events. - * - * Usage: Assume xyz.onDidSave is the event we want to test. - * const handler = new TestEventHandler(xyz.onDidSave); - * // Do something that would trigger the event. - * assert.ok(handler.fired) - * assert.equal(handler.first, 'Args Passed to first onDidSave') - * assert.equal(handler.count, 1)// Only one should have been fired. - */ -export class TestEventHandler implements IDisposable { - public get fired() { - return this.handledEvents.length > 0; - } - public get first(): T { - return this.handledEvents[0]; - } - public get second(): T { - return this.handledEvents[1]; - } - public get last(): T { - return this.handledEvents[this.handledEvents.length - 1]; - } - public get count(): number { - return this.handledEvents.length; - } - public get all(): T[] { - return this.handledEvents; - } - private readonly handler: IDisposable; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private readonly handledEvents: any[] = []; - constructor(event: Event, private readonly eventNameForErrorMessages: string, disposables: IDisposable[] = []) { - disposables.push(this); - this.handler = event(this.listener, this); - } - public reset() { - while (this.handledEvents.length) { - this.handledEvents.pop(); - } - } - public async assertFired(waitPeriod: number = 100): Promise { - await waitForCondition(async () => this.fired, waitPeriod, `${this.eventNameForErrorMessages} event not fired`); - } - public async assertFiredExactly(numberOfTimesFired: number, waitPeriod: number = 2_000): Promise { - await waitForCondition( - async () => this.count === numberOfTimesFired, - waitPeriod, - `${this.eventNameForErrorMessages} event fired ${this.count}, expected ${numberOfTimesFired}` - ); - } - public async assertFiredAtLeast(numberOfTimesFired: number, waitPeriod: number = 2_000): Promise { - await waitForCondition( - async () => this.count >= numberOfTimesFired, - waitPeriod, - `${this.eventNameForErrorMessages} event fired ${this.count}, expected at least ${numberOfTimesFired}.` - ); - } - public atIndex(index: number): T { - return this.handledEvents[index]; - } - - public dispose() { - this.handler.dispose(); - } - - private listener(e: T) { - this.handledEvents.push(e); - } -} - -export function createEventHandler( - obj: T, - eventName: K, - disposables: IDisposable[] = [] -): T[K] extends Event ? TestEventHandler : TestEventHandler { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return new TestEventHandler(obj[eventName] as any, eventName as string, disposables) as any; -} - /** * Captures screenshots (png format) & dumpts into root directory (only on CI). * If there's a failure, it will be logged (errors are swallowed). diff --git a/src/test/common.ts b/src/test/common.ts index 7d829232a1a..6ad2b82f2dd 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -3,7 +3,9 @@ // Licensed under the MIT License. 'use strict'; +import { Event } from 'vscode'; import { IExtensionApi } from '../platform/api'; +import { IDisposable } from '../platform/common/types'; import { IServiceContainer, IServiceManager } from '../platform/ioc/types'; export interface IExtensionTestApi extends IExtensionApi { @@ -80,3 +82,83 @@ export async function waitForCondition( pendingTimers.push(timeout); }); } + +/** + * Helper class to test events. + * + * Usage: Assume xyz.onDidSave is the event we want to test. + * const handler = new TestEventHandler(xyz.onDidSave); + * // Do something that would trigger the event. + * assert.ok(handler.fired) + * assert.equal(handler.first, 'Args Passed to first onDidSave') + * assert.equal(handler.count, 1)// Only one should have been fired. + */ +export class TestEventHandler implements IDisposable { + public get fired() { + return this.handledEvents.length > 0; + } + public get first(): T { + return this.handledEvents[0]; + } + public get second(): T { + return this.handledEvents[1]; + } + public get last(): T { + return this.handledEvents[this.handledEvents.length - 1]; + } + public get count(): number { + return this.handledEvents.length; + } + public get all(): T[] { + return this.handledEvents; + } + private readonly handler: IDisposable; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private readonly handledEvents: any[] = []; + constructor(event: Event, private readonly eventNameForErrorMessages: string, disposables: IDisposable[] = []) { + disposables.push(this); + this.handler = event(this.listener, this); + } + public reset() { + while (this.handledEvents.length) { + this.handledEvents.pop(); + } + } + public async assertFired(waitPeriod: number = 100): Promise { + await waitForCondition(async () => this.fired, waitPeriod, `${this.eventNameForErrorMessages} event not fired`); + } + public async assertFiredExactly(numberOfTimesFired: number, waitPeriod: number = 2_000): Promise { + await waitForCondition( + async () => this.count === numberOfTimesFired, + waitPeriod, + `${this.eventNameForErrorMessages} event fired ${this.count}, expected ${numberOfTimesFired}` + ); + } + public async assertFiredAtLeast(numberOfTimesFired: number, waitPeriod: number = 2_000): Promise { + await waitForCondition( + async () => this.count >= numberOfTimesFired, + waitPeriod, + `${this.eventNameForErrorMessages} event fired ${this.count}, expected at least ${numberOfTimesFired}.` + ); + } + public atIndex(index: number): T { + return this.handledEvents[index]; + } + + public dispose() { + this.handler.dispose(); + } + + private listener(e: T) { + this.handledEvents.push(e); + } +} + +export function createEventHandler( + obj: T, + eventName: K, + disposables: IDisposable[] = [] +): T[K] extends Event ? TestEventHandler : TestEventHandler { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return new TestEventHandler(obj[eventName] as any, eventName as string, disposables) as any; +} diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index b271585210f..07b1d915a7a 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -239,6 +239,7 @@ export async function createEmptyPythonNotebook( await waitForKernelToGetAutoSelected(); } await deleteAllCellsAndWait(); + return vscodeNotebook.activeNotebookEditor!.notebook; } async function shutdownAllNotebooks() { diff --git a/src/test/datascience/notebook/kernelEvents.vscode.common.ts b/src/test/datascience/notebook/kernelEvents.vscode.common.ts new file mode 100644 index 00000000000..9f343f4908d --- /dev/null +++ b/src/test/datascience/notebook/kernelEvents.vscode.common.ts @@ -0,0 +1,172 @@ +/* eslint-disable no-void */ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +/* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import { Disposable, NotebookDocument } from 'vscode'; +import { traceInfo } from '../../../platform/logging'; +import { + IConfigurationService, + IDisposable, + IWatchableJupyterSettings, + ReadWrite +} from '../../../platform/common/types'; +import { IExtensionTestApi, waitForCondition } from '../../common'; +import { initialize } from '../../initialize'; +import { + runCell, + insertCodeCell, + waitForTextOutput, + closeNotebooksAndCleanUpAfterTests, + createEmptyPythonNotebook +} from './helper'; +import { createEventHandler } from '../../common'; +import { IKernelProvider } from '../../../kernels/types'; + +/* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */ +export function sharedKernelEventTests( + this: Mocha.Suite, + options: { + startJupyterServer: (notebook?: NotebookDocument) => Promise; + } +) { + let api: IExtensionTestApi; + const disposables: IDisposable[] = []; + let configSettings: ReadWrite; + let kernelProvider: IKernelProvider; + let previousDisableJupyterAutoStartValue: boolean; + this.timeout(120_000); + suiteSetup(async function () { + traceInfo(`Suite Setup ${this.currentTest?.title}`); + this.timeout(120_000); + try { + api = await initialize(); + sinon.restore(); + kernelProvider = api.serviceContainer.get(IKernelProvider); + const configService = api.serviceContainer.get(IConfigurationService); + configSettings = configService.getSettings(undefined) as any; + previousDisableJupyterAutoStartValue = configSettings.disableJupyterAutoStart; + configSettings.disableJupyterAutoStart = true; + traceInfo('Suite Setup (completed)'); + } catch (e) { + traceInfo('Suite Setup (failed)'); + throw e; + } + }); + // Use same notebook without starting kernel in every single test (use one for whole suite). + setup(async function () { + try { + traceInfo(`Start Test ${this.currentTest?.title}`); + sinon.restore(); + const configService = api.serviceContainer.get(IConfigurationService); + configSettings = configService.getSettings(undefined) as any; + configSettings.disableJupyterAutoStart = true; + await options.startJupyterServer(); + traceInfo(`Start Test (completed) ${this.currentTest?.title}`); + } catch (e) { + throw e; + } + }); + teardown(async function () { + traceInfo(`Ended Test ${this.currentTest?.title}`); + await closeNotebooksAndCleanUpAfterTests(disposables); + configSettings.disableJupyterAutoStart = previousDisableJupyterAutoStartValue; + traceInfo(`Ended Test (completed) ${this.currentTest?.title}`); + }); + suiteTeardown(() => closeNotebooksAndCleanUpAfterTests(disposables)); + test('Kernel Events', async () => { + const kernelCreated = createEventHandler(kernelProvider, 'onDidCreateKernel', disposables); + const kernelStarted = createEventHandler(kernelProvider, 'onDidStartKernel', disposables); + const kernelDisposed = createEventHandler(kernelProvider, 'onDidDisposeKernel', disposables); + const kernelRestarted = createEventHandler(kernelProvider, 'onDidRestartKernel', disposables); + const kernelStatusChanged = createEventHandler(kernelProvider, 'onKernelStatusChanged', disposables); + + const nb = await createEmptyPythonNotebook(disposables); + await waitForCondition(async () => !!kernelProvider.get(nb.uri), 5_000, 'Kernel not created'); + const kernel = kernelProvider.get(nb.uri)!; + const startedEvent = createEventHandler(kernel, 'onStarted', disposables); + const onPreExecuteEvent = createEventHandler(kernel, 'onPreExecute', disposables); + const onStatusChangeEvent = createEventHandler(kernel, 'onStatusChanged', disposables); + const onDisposed = createEventHandler(kernel, 'onDisposed', disposables); + const restartEvent = createEventHandler(kernel, 'onRestarted', disposables); + + const cell = await insertCodeCell('print("cell1")', { index: 0 }); + await Promise.all([runCell(cell), waitForTextOutput(cell, 'cell1')]); + + assert.isTrue(kernelCreated.fired, 'IKernelProvider.onDidCreateKernel not fired'); + assert.isTrue(kernelStarted.fired, 'IKernelProvider.onDidStartKernel not fired'); + assert.isTrue(kernelStatusChanged.fired, 'IKernelProvider.onKernelStatusChanged not fired'); + assert.isFalse(kernelRestarted.fired, 'IKernelProvider.onDidRestartKernel should not have fired'); + assert.isFalse(kernelDisposed.fired, 'IKernelProvider.onDidDisposeKernel should not have fired'); + + assert.equal(onPreExecuteEvent.first, cell, 'Incorrect cell'); + assert.isTrue(startedEvent.fired, 'IKernel.onStarted not fired'); + assert.isTrue(onPreExecuteEvent.fired, 'IKernel.onPreExecute not fired'); + assert.isTrue(onStatusChangeEvent.fired, 'IKernel.onStatusChanged not fired'); + assert.isFalse(restartEvent.fired, 'IKernel.onRestarted event should not have fired'); + assert.isFalse(onDisposed.fired, 'IKernel.onDisposed event should not have fired'); + + await kernel.restart(); + assert.isTrue(restartEvent.fired, 'IKernel.onRestarted event not fired'); + assert.isFalse(onDisposed.fired, 'IKernel.onDisposed event should not have fired'); + assert.isTrue(kernelRestarted.fired, 'IKernelProvider.onDidRestartKernel not fired'); + assert.isFalse(kernelDisposed.fired, 'IKernelProvider.onDidDisposeKernel should not have fired'); + + await kernel.dispose(); + assert.isTrue(onDisposed.fired, 'Disposed event not fired'); + assert.isTrue(kernelDisposed.fired, 'IKernelProvider.onDidDisposeKernel not fired'); + }); + test('Kernel.IKernelConnection Events', async () => { + const nb = await createEmptyPythonNotebook(disposables); + await waitForCondition(async () => !!kernelProvider.get(nb.uri), 5_000, 'Kernel not created'); + const kernel = kernelProvider.get(nb.uri)!; + const onPreExecuteEvent = createEventHandler(kernel, 'onPreExecute', disposables); + + const cell = await insertCodeCell('print("cell1")', { index: 0 }); + await Promise.all([runCell(cell), waitForTextOutput(cell, 'cell1')]); + + const kernelConnection = kernelProvider.get(nb.uri)!.session!.kernel!; + assert.strictEqual(onPreExecuteEvent.count, 1, 'Pre-execute should be fired once'); + assert.equal(onPreExecuteEvent.first, cell, 'Incorrect cell'); + + let gotAnyMessage = false; + let gotIOPubMessage = false; + let statusChanged = false; + const onAnyMessage = () => (gotAnyMessage = true); + const onIOPubMessage = () => (gotIOPubMessage = true); + const onStatusChanged = () => (statusChanged = true); + kernelConnection.anyMessage.connect(onAnyMessage); + kernelConnection.iopubMessage.connect(onIOPubMessage); + kernelConnection.statusChanged.connect(onStatusChanged); + disposables.push(new Disposable(() => void kernelConnection.anyMessage.disconnect(onAnyMessage))); + disposables.push(new Disposable(() => void kernelConnection.iopubMessage.disconnect(onIOPubMessage))); + disposables.push(new Disposable(() => void kernelConnection.statusChanged.disconnect(onStatusChanged))); + + const cell2 = await insertCodeCell('print("cell2")', { index: 0 }); + await Promise.all([runCell(cell2), waitForTextOutput(cell2, 'cell2')]); + + assert.strictEqual(onPreExecuteEvent.count, 2, 'Pre-execute should be fired twice'); + assert.equal(onPreExecuteEvent.second, cell2, 'Incorrect cell'); + + assert.isTrue(gotAnyMessage, 'AnyMessage event not fired'); + assert.isTrue(gotIOPubMessage, 'IOPubMessage event fired'); + assert.isTrue(statusChanged, 'StatusChange event fired'); + + // Restart the kernel & verify we still get the events fired. + gotAnyMessage = false; + gotIOPubMessage = false; + statusChanged = false; + await kernel.restart(); + + const cell3 = await insertCodeCell('print("cell3")', { index: 0 }); + await Promise.all([runCell(cell3), waitForTextOutput(cell3, 'cell3')]); + + assert.isTrue(gotAnyMessage, 'AnyMessage event not fired after restarting the kernel'); + assert.isTrue(gotIOPubMessage, 'IOPubMessage event fired after restarting the kernel'); + assert.isTrue(statusChanged, 'StatusChange event fired after restarting the kernel'); + }); +} diff --git a/src/test/datascience/notebook/kernelEvents.vscode.test.ts b/src/test/datascience/notebook/kernelEvents.vscode.test.ts new file mode 100644 index 00000000000..40c5a4dfb1c --- /dev/null +++ b/src/test/datascience/notebook/kernelEvents.vscode.test.ts @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { startJupyterServer } from './helper.node'; +import { sharedKernelEventTests } from './kernelEvents.vscode.common'; + +/* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */ +suite('Kernel Events', function () { + sharedKernelEventTests.bind(this)({ startJupyterServer }); +}); diff --git a/src/test/datascience/notebook/kernelEvents.vscode.web.test.ts b/src/test/datascience/notebook/kernelEvents.vscode.web.test.ts new file mode 100644 index 00000000000..e727cd21b32 --- /dev/null +++ b/src/test/datascience/notebook/kernelEvents.vscode.web.test.ts @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { startJupyterServer } from './helper.web'; +import { sharedKernelEventTests } from './kernelEvents.vscode.common'; + +/* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */ +suite('Kernel Events', function () { + sharedKernelEventTests.bind(this)({ startJupyterServer }); +}); From 313ac070e80e3620150dde35acedc2c6f8afee0a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 30 May 2022 11:05:19 +1000 Subject: [PATCH 2/3] Move cell exec result handler into a seprate class --- src/notebooks/execution/cellExecution.ts | 597 +----------------- .../execution/cellExecutionMessageHandler.ts | 574 +++++++++++++++++ .../cellExecutionMessageHandlerFactory.ts | 40 ++ src/notebooks/execution/kernelExecution.ts | 10 +- .../common/{refBool.node.ts => refBool.ts} | 3 + 5 files changed, 660 insertions(+), 564 deletions(-) create mode 100644 src/notebooks/execution/cellExecutionMessageHandler.ts create mode 100644 src/notebooks/execution/cellExecutionMessageHandlerFactory.ts rename src/platform/common/{refBool.node.ts => refBool.ts} (67%) diff --git a/src/notebooks/execution/cellExecution.ts b/src/notebooks/execution/cellExecution.ts index 020709cd91b..06db387a21c 100644 --- a/src/notebooks/execution/cellExecution.ts +++ b/src/notebooks/execution/cellExecution.ts @@ -3,99 +3,46 @@ 'use strict'; -import * as fastDeepEqual from 'fast-deep-equal'; -import type * as nbformat from '@jupyterlab/nbformat'; import type * as KernelMessage from '@jupyterlab/services/lib/kernel/messages'; import { NotebookCell, NotebookCellExecution, - NotebookCellKind, - NotebookCellExecutionSummary, - NotebookDocument, workspace, NotebookController, - WorkspaceEdit, - NotebookCellData, - Range, NotebookCellOutput, NotebookCellExecutionState, - CancellationTokenSource, Event, - EventEmitter, - ExtensionMode, - NotebookEdit + EventEmitter } from 'vscode'; import { Kernel } from '@jupyterlab/services'; -import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; import { CellExecutionCreator } from './cellExecutionCreator'; -import { IApplicationShell } from '../../platform/common/application/types'; import { analyzeKernelErrors, createOutputWithErrorMessageForDisplay } from '../../platform/errors/errorUtils'; import { BaseError } from '../../platform/errors/types'; import { disposeAllDisposables } from '../../platform/common/helpers'; import { traceError, traceInfoIfCI, traceWarning } from '../../platform/logging'; -import { RefBool } from '../../platform/common/refBool.node'; -import { IDisposable, IExtensionContext } from '../../platform/common/types'; -import { Deferred, createDeferred } from '../../platform/common/utils/async'; +import { IDisposable } from '../../platform/common/types'; +import { createDeferred } from '../../platform/common/utils/async'; import { StopWatch } from '../../platform/common/utils/stopWatch'; -import { - NotebookCellStateTracker, - traceCellMessage, - cellOutputToVSCCellOutput, - translateCellDisplayOutput, - isJupyterNotebook -} from '../helpers'; +import { NotebookCellStateTracker, traceCellMessage } from '../helpers'; import { sendTelemetryEvent } from '../../telemetry'; -import { formatStreamText, concatMultilineString } from '../../webviews/webview-side/common'; import { Telemetry } from '../../webviews/webview-side/common/constants'; -import { swallowExceptions } from '../../platform/common/utils/decorators'; import { noop } from '../../platform/common/utils/misc'; import { getDisplayNameOrNameOfKernelConnection, isPythonKernelConnection } from '../../kernels/helpers'; -import { - IJupyterSession, - ITracebackFormatter, - KernelConnectionMetadata, - NotebookCellRunState -} from '../../kernels/types'; -import { handleTensorBoardDisplayDataOutput } from './executionHelpers'; -import { WIDGET_MIMETYPE } from '../../kernels/ipywidgets-message-coordination/constants'; import { isCancellationError } from '../../platform/common/cancellation'; - -// Helper interface for the set_next_input execute reply payload -interface ISetNextInputPayload { - replace: boolean; - source: 'set_next_input'; - text: string; -} - -type ExecuteResult = nbformat.IExecuteResult & { - transient?: { display_id?: string }; -}; -type DisplayData = nbformat.IDisplayData & { - transient?: { display_id?: string }; -}; +import { activeNotebookCellExecution, CellExecutionMessageHandler } from './cellExecutionMessageHandler'; +import { CellExecutionMessageHandlerFactory } from './cellExecutionMessageHandlerFactory'; +import { IJupyterSession, KernelConnectionMetadata, NotebookCellRunState } from '../../kernels/types'; export class CellExecutionFactory { constructor( - private readonly appShell: IApplicationShell, private readonly controller: NotebookController, - private readonly outputTracker: CellOutputDisplayIdTracker, - private readonly context: IExtensionContext, - private readonly formatters: ITracebackFormatter[] + private readonly factory: CellExecutionMessageHandlerFactory ) {} public create(cell: NotebookCell, code: string | undefined, metadata: Readonly) { // eslint-disable-next-line @typescript-eslint/no-use-before-define - return CellExecution.fromCell( - cell, - code, - this.appShell, - metadata, - this.controller, - this.outputTracker, - this.context, - this.formatters - ); + return CellExecution.fromCell(cell, code, metadata, this.controller, this.factory); } } @@ -115,16 +62,6 @@ export class CellExecution implements IDisposable { public get preExecute(): Event { return this._preExecuteEmitter.event; } - /** - * To be used only in tests. - */ - public static cellsCompletedForTesting = new WeakMap>(); - /** - * At any given point in time, we can only have one cell actively running. - * This will keep track of that task. - */ - private static activeNotebookCellExecution = new WeakMap(); - private static sentExecuteCellTelemetry?: boolean; private stopWatch = new StopWatch(); @@ -138,30 +75,17 @@ export class CellExecution implements IDisposable { private startTime?: number; private endTime?: number; private execution?: NotebookCellExecution; - private temporaryExecution?: NotebookCellExecution; - private previousResultsToRestore?: NotebookCellExecutionSummary; private cancelHandled = false; - private cellHasErrorsInOutput?: boolean; - /** - * We keep track of the last output that was used to store stream text. - * We need this so that we can update it later on (when we get new data for the same stream). - * If users clear outputs or if we have a new output other than stream, then clear this item. - * Because if after the stream we have an image, then the stream is not the last output item, hence its cleared. - */ - private lastUsedStreamOutput?: { stream: 'stdout' | 'stderr'; text: string; output: NotebookCellOutput }; private request: Kernel.IShellFuture | undefined; private readonly disposables: IDisposable[] = []; - private readonly prompts = new Set(); private _preExecuteEmitter = new EventEmitter(); + private cellExecutionHandler?: CellExecutionMessageHandler; private constructor( public readonly cell: NotebookCell, private readonly codeOverride: string | undefined, - private readonly applicationService: IApplicationShell, private readonly kernelConnection: Readonly, private readonly controller: NotebookController, - private readonly outputDisplayIdTracker: CellOutputDisplayIdTracker, - private readonly context: IExtensionContext, - private readonly formatters: ITracebackFormatter[] + private readonly factory: CellExecutionMessageHandlerFactory ) { workspace.onDidCloseTextDocument( (e) => { @@ -181,23 +105,6 @@ export class CellExecution implements IDisposable { this, this.disposables ); - workspace.onDidChangeNotebookDocument( - (e) => { - if (!isJupyterNotebook(e.notebook)) { - return; - } - const thisCellChange = e.cellChanges.find(({ cell }) => cell === this.cell); - if (!thisCellChange) { - return; - } - if (thisCellChange.outputs?.length === 0) { - // keep track of the fact that user has cleared the output. - this.clearLastUsedStreamOutput(); - } - }, - this, - this.disposables - ); NotebookCellStateTracker.setCellState(cell, NotebookCellExecutionState.Idle); const execution = CellExecutionCreator.get(cell); if (this.canExecuteCell()) { @@ -221,14 +128,11 @@ export class CellExecution implements IDisposable { public static fromCell( cell: NotebookCell, code: string | undefined, - appService: IApplicationShell, metadata: Readonly, controller: NotebookController, - outputTracker: CellOutputDisplayIdTracker, - context: IExtensionContext, - formatters: ITracebackFormatter[] + factory: CellExecutionMessageHandlerFactory ) { - return new CellExecution(cell, code, appService, metadata, controller, outputTracker, context, formatters); + return new CellExecution(cell, code, metadata, controller, factory); } public async start(session: IJupyterSession) { if (this.cancelHandled) { @@ -253,10 +157,9 @@ export class CellExecution implements IDisposable { this.started = true; this.startTime = new Date().getTime(); - CellExecution.activeNotebookCellExecution.set(this.cell.notebook, this.execution); + activeNotebookCellExecution.set(this.cell.notebook, this.execution); this.execution?.start(this.startTime); NotebookCellStateTracker.setCellState(this.cell, NotebookCellExecutionState.Executing); - this.clearLastUsedStreamOutput(); // Await here, so that the UI updates on the progress & we clear the output. // Else when running cells with existing outputs, the outputs don't get cleared & it doesn't look like its running. // Ideally we shouldn't have any awaits, but here we want the UI to get updated. @@ -283,8 +186,6 @@ export class CellExecution implements IDisposable { if (this.cancelHandled) { return; } - // Close all of the prompts (if we any any UI prompts asking user for input). - this.prompts.forEach((item) => item.cancel()); if (this.started && !forced) { // At this point the cell execution can only be stopped from kernel & we should not // stop handling execution results & the like from the kernel. @@ -309,11 +210,6 @@ export class CellExecution implements IDisposable { public dispose() { traceCellMessage(this.cell, 'Execution disposed'); disposeAllDisposables(this.disposables); - this.prompts.forEach((item) => item.dispose()); - this.prompts.clear(); - } - private clearLastUsedStreamOutput() { - this.lastUsedStreamOutput = undefined; } private completedWithErrors(error: Partial) { traceWarning(`Cell completed with errors`, error); @@ -358,7 +254,7 @@ export class CellExecution implements IDisposable { let success: 'success' | 'failed' = 'success'; // If there are any errors in the cell, then change status to error. - if (this.cellHasErrorsInOutput) { + if (this.cellExecutionHandler?.hasErrorOutput) { success = 'failed'; runState = NotebookCellRunState.Error; } @@ -383,58 +279,12 @@ export class CellExecution implements IDisposable { // Undefined for not success or failures this.execution?.end(undefined); } - if (CellExecution.activeNotebookCellExecution.get(this.cell.notebook) === this.execution) { - CellExecution.activeNotebookCellExecution.set(this.cell.notebook, undefined); + if (activeNotebookCellExecution.get(this.cell.notebook) === this.execution) { + activeNotebookCellExecution.set(this.cell.notebook, undefined); } NotebookCellStateTracker.setCellState(this.cell, NotebookCellExecutionState.Idle); this.execution = undefined; } - /** - * Assume we run cell A - * Now run cell B, and this will update cell A. - * The way it works is, the request object created for cell A will get a message saying update your output. - * Cell A has completed, hence there's no execution task, we should create one or re-use an existing one. - * Creating one results in side effects such as execution order getting reset and timers starting. - * Hence where possible re-use an existing cell execution task associated with this document. - */ - private createTemporaryTask() { - if (this.cell.document.isClosed) { - return; - } - // If we have an active task, use that instead of creating a new task. - const existingTask = CellExecution.activeNotebookCellExecution.get(this.cell.notebook); - if (existingTask) { - return existingTask; - } - - // Create a temporary task. - this.previousResultsToRestore = { ...(this.cell.executionSummary || {}) }; - this.temporaryExecution = CellExecutionCreator.getOrCreate(this.cell, this.controller); - this.temporaryExecution?.start(); - if (this.previousResultsToRestore?.executionOrder && this.execution) { - this.execution.executionOrder = this.previousResultsToRestore.executionOrder; - } - return this.temporaryExecution; - } - private endTemporaryTask() { - if (this.previousResultsToRestore?.executionOrder && this.execution) { - this.execution.executionOrder = this.previousResultsToRestore.executionOrder; - } - if (this.previousResultsToRestore && this.temporaryExecution) { - if (this.previousResultsToRestore.executionOrder) { - this.temporaryExecution.executionOrder = this.previousResultsToRestore.executionOrder; - } - this.temporaryExecution.end( - this.previousResultsToRestore.success, - this.previousResultsToRestore.timing?.endTime - ); - } else { - // Undefined for not success or failure - this.temporaryExecution?.end(undefined); - } - this.previousResultsToRestore = undefined; - this.temporaryExecution = undefined; - } private completedDueToCancellation() { traceCellMessage(this.cell, 'Completed due to cancellation'); @@ -502,26 +352,22 @@ export class CellExecution implements IDisposable { traceError(`Cell execution failed without request, for cell Index ${this.cell.index}`, ex); return this.completedWithErrors(ex); } - // Listen to messages and update our cell execution state appropriately - // Keep track of our clear state - const clearState = new RefBool(false); - - const request = this.request; - request.onIOPub = (msg) => { - // Cell has been deleted or the like. - if (this.cell.document.isClosed) { - request.dispose(); - } - this.handleIOPub(clearState, msg); - }; - request.onReply = (msg) => { - // Cell has been deleted or the like. - if (this.cell.document.isClosed) { - request.dispose(); - } - this.handleReply(clearState, msg); - }; - request.onStdin = this.handleInputRequest.bind(this, session); + this.cellExecutionHandler = this.factory.create(this.cell, { + kernel: session.kernel!, + cellExecution: this.execution!, + request: this.request + }); + this.disposables.push(this.cellExecutionHandler); + this.cellExecutionHandler.onErrorHandlingExecuteRequestIOPubMessage( + ({ error }) => { + traceError(`Cell (index = ${this.cell.index}) execution completed with errors (2).`, error); + // If not a restart error, then tell the subscriber + // eslint-disable-next-line @typescript-eslint/no-explicit-any + this.completedWithErrors(error as any); + }, + this, + this.disposables + ); // WARNING: Do not dispose `request`. // Even after request.done & execute_reply is sent we could have more messages coming from iopub. @@ -532,7 +378,7 @@ export class CellExecution implements IDisposable { // request.done resolves even before all iopub messages have been sent through. // Solution is to wait for all messages to get processed. traceCellMessage(this.cell, 'Wait for jupyter execution'); - await request.done; + await this.request.done; traceCellMessage(this.cell, 'Jupyter execution completed'); this.completedSuccessfully(); traceCellMessage(this.cell, 'Executed successfully in executeCell'); @@ -552,379 +398,4 @@ export class CellExecution implements IDisposable { } } } - @swallowExceptions() - private handleIOPub(clearState: RefBool, msg: KernelMessage.IIOPubMessage) { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services'); - - try { - if (jupyterLab.KernelMessage.isExecuteResultMsg(msg)) { - this.handleExecuteResult(msg as KernelMessage.IExecuteResultMsg, clearState); - } else if (jupyterLab.KernelMessage.isExecuteInputMsg(msg)) { - this.handleExecuteInput(msg as KernelMessage.IExecuteInputMsg, clearState); - } else if (jupyterLab.KernelMessage.isStatusMsg(msg)) { - // Status is handled by the result promise. While it is running we are active. Otherwise we're stopped. - // So ignore status messages. - const statusMsg = msg as KernelMessage.IStatusMsg; - this.handleStatusMessage(statusMsg, clearState); - } else if (jupyterLab.KernelMessage.isStreamMsg(msg)) { - this.handleStreamMessage(msg as KernelMessage.IStreamMsg, clearState); - } else if (jupyterLab.KernelMessage.isDisplayDataMsg(msg)) { - this.handleDisplayData(msg as KernelMessage.IDisplayDataMsg, clearState); - } else if (jupyterLab.KernelMessage.isUpdateDisplayDataMsg(msg)) { - this.handleUpdateDisplayDataMessage(msg); - } else if (jupyterLab.KernelMessage.isClearOutputMsg(msg)) { - this.handleClearOutput(msg as KernelMessage.IClearOutputMsg, clearState); - } else if (jupyterLab.KernelMessage.isErrorMsg(msg)) { - this.handleError(msg as KernelMessage.IErrorMsg, clearState); - } else if (jupyterLab.KernelMessage.isCommOpenMsg(msg)) { - // Noop. - } else if (jupyterLab.KernelMessage.isCommMsgMsg(msg)) { - // Noop. - } else if (jupyterLab.KernelMessage.isCommCloseMsg(msg)) { - // Noop. - } else { - traceWarning(`Unknown message ${msg.header.msg_type} : hasData=${'data' in msg.content}`); - } - - // Set execution count, all messages should have it - if ('execution_count' in msg.content && typeof msg.content.execution_count === 'number' && this.execution) { - this.execution.executionOrder = msg.content.execution_count; - } - } catch (err) { - traceError(`Cell (index = ${this.cell.index}) execution completed with errors (2).`, err); - // If not a restart error, then tell the subscriber - // eslint-disable-next-line @typescript-eslint/no-explicit-any - this.completedWithErrors(err as any); - } - } - - private addToCellData( - output: ExecuteResult | DisplayData | nbformat.IStream | nbformat.IError | nbformat.IOutput, - clearState: RefBool - ) { - if ( - this.context.extensionMode === ExtensionMode.Test && - output.data && - typeof output.data === 'object' && - WIDGET_MIMETYPE in output.data - ) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (output.data[WIDGET_MIMETYPE] as any)['_vsc_test_cellIndex'] = this.cell.index; - } - const cellOutput = cellOutputToVSCCellOutput(output); - const displayId = - 'transient' in output && - typeof output.transient === 'object' && - output.transient && - 'display_id' in output.transient && - typeof output.transient?.display_id === 'string' - ? output.transient?.display_id - : undefined; - if (this.cell.document.isClosed) { - return; - } - traceCellMessage(this.cell, 'Update output'); - // Clear if necessary - if (clearState.value) { - this.clearLastUsedStreamOutput(); - this.execution?.clearOutput().then(noop, noop); - clearState.update(false); - } - // Keep track of the displa_id against the output item, we might need this to update this later. - if (displayId) { - this.outputDisplayIdTracker.trackOutputByDisplayId(this.cell, displayId, cellOutput); - } - - // Append to the data (we would push here but VS code requires a recreation of the array) - // Possible execution of cell has completed (the task would have been disposed). - // This message could have come from a background thread. - // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). - const task = this.execution || this.createTemporaryTask(); - this.clearLastUsedStreamOutput(); - traceCellMessage(this.cell, 'Append output in addToCellData'); - task?.appendOutput([cellOutput]).then(noop, noop); - this.endTemporaryTask(); - } - - private async handleInputRequest(session: IJupyterSession, msg: KernelMessage.IStdinMessage) { - // Ask the user for input - if (msg.content && 'prompt' in msg.content) { - const cancelToken = new CancellationTokenSource(); - this.prompts.add(cancelToken); - const hasPassword = msg.content.password !== null && (msg.content.password as boolean); - await this.applicationService - .showInputBox( - { - prompt: msg.content.prompt ? msg.content.prompt.toString() : '', - ignoreFocusOut: true, - password: hasPassword - }, - cancelToken.token - ) - .then((v) => { - session.sendInputReply({ value: v || '', status: 'ok' }); - }, noop); - - this.prompts.delete(cancelToken); - } - } - - // See this for docs on the messages: - // https://jupyter-client.readthedocs.io/en/latest/messaging.html#messaging-in-jupyter - private handleExecuteResult(msg: KernelMessage.IExecuteResultMsg, clearState: RefBool) { - this.addToCellData( - { - output_type: 'execute_result', - data: msg.content.data, - metadata: msg.content.metadata, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - transient: msg.content.transient as any, // NOSONAR - execution_count: msg.content.execution_count - }, - clearState - ); - } - - private handleExecuteReply(msg: KernelMessage.IExecuteReplyMsg, clearState: RefBool) { - const reply = msg.content as KernelMessage.IExecuteReply; - if (reply.payload) { - reply.payload.forEach((payload) => { - if ( - payload.source && - payload.source === 'set_next_input' && - 'text' in payload && - 'replace' in payload - ) { - this.handleSetNextInput(payload as unknown as ISetNextInputPayload); - } - if (payload.data && payload.data.hasOwnProperty('text/plain')) { - this.addToCellData( - { - // Mark as stream output so the text is formatted because it likely has ansi codes in it. - output_type: 'stream', - // eslint-disable-next-line @typescript-eslint/no-explicit-any - text: (payload.data as any)['text/plain'].toString(), - name: 'stdout', - metadata: {}, - execution_count: reply.execution_count - }, - clearState - ); - } - }); - } - } - - // Handle our set_next_input message, which can either replace or insert a new cell with text - private handleSetNextInput(payload: ISetNextInputPayload) { - const edit = new WorkspaceEdit(); - if (payload.replace) { - // Replace the contents of the current cell with text - edit.replace( - this.cell.document.uri, - new Range( - this.cell.document.lineAt(0).range.start, - this.cell.document.lineAt(this.cell.document.lineCount - 1).range.end - ), - payload.text - ); - } else { - // Add a new cell after the current with text - traceCellMessage(this.cell, 'Create new cell after current'); - const cellData = new NotebookCellData(NotebookCellKind.Code, payload.text, this.cell.document.languageId); - cellData.outputs = []; - cellData.metadata = {}; - const nbEdit = NotebookEdit.insertCells(this.cell.index + 1, [cellData]); - edit.set(this.cell.notebook.uri, [nbEdit]); - } - workspace.applyEdit(edit).then(noop, noop); - } - - private handleExecuteInput(msg: KernelMessage.IExecuteInputMsg, _clearState: RefBool) { - if (msg.content.execution_count && this.execution) { - this.execution.executionOrder = msg.content.execution_count; - } - } - - private handleStatusMessage(msg: KernelMessage.IStatusMsg, _clearState: RefBool) { - traceCellMessage(this.cell, `Kernel switching to ${msg.content.execution_state}`); - } - private handleStreamMessage(msg: KernelMessage.IStreamMsg, clearState: RefBool) { - // eslint-disable-next-line complexity - traceCellMessage(this.cell, 'Update streamed output'); - // Possible execution of cell has completed (the task would have been disposed). - // This message could have come from a background thread. - // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). - const task = this.execution || this.createTemporaryTask(); - - // Clear output if waiting for a clear - const clearOutput = clearState.value; - if (clearOutput) { - traceCellMessage(this.cell, 'Clear cell output'); - this.clearLastUsedStreamOutput(); - task?.clearOutput().then(noop, noop); - clearState.update(false); - } - // Ensure we append to previous output, only if the streams as the same & - // If the last output is the desired stream type. - if (this.lastUsedStreamOutput?.stream === msg.content.name) { - // Get the jupyter output from the vs code output (so we can concatenate the text ourselves). - let existingOutputText = this.lastUsedStreamOutput.text; - let newContent = msg.content.text; - // Look for the ansi code `[A`. (this means move up) - // Not going to support `[2A` (not for now). - const moveUpCode = `${String.fromCharCode(27)}[A`; - if (msg.content.text.startsWith(moveUpCode)) { - // Split message by lines & strip out the last n lines (where n = number of lines to move cursor up). - const existingOutputLines = existingOutputText.splitLines({ - trim: false, - removeEmptyEntries: false - }); - if (existingOutputLines.length) { - existingOutputLines.pop(); - } - existingOutputText = existingOutputLines.join('\n'); - newContent = newContent.substring(moveUpCode.length); - } - // Create a new output item with the concatenated string. - this.lastUsedStreamOutput.text = formatStreamText( - concatMultilineString(`${existingOutputText}${newContent}`) - ); - const output = cellOutputToVSCCellOutput({ - output_type: 'stream', - name: msg.content.name, - text: this.lastUsedStreamOutput.text - }); - traceCellMessage(this.cell, `Replace output items ${this.lastUsedStreamOutput.text.substring(0, 100)}`); - task?.replaceOutputItems(output.items, this.lastUsedStreamOutput.output).then(noop, noop); - } else if (clearOutput) { - // Replace the current outputs with a single new output. - const text = formatStreamText(concatMultilineString(msg.content.text)); - const output = cellOutputToVSCCellOutput({ - output_type: 'stream', - name: msg.content.name, - text - }); - this.lastUsedStreamOutput = { output, stream: msg.content.name, text }; - traceCellMessage(this.cell, `Replace output ${this.lastUsedStreamOutput.text.substring(0, 100)}`); - task?.replaceOutput([output]).then(noop, noop); - } else { - // Create a new output - const text = formatStreamText(concatMultilineString(msg.content.text)); - const output = cellOutputToVSCCellOutput({ - output_type: 'stream', - name: msg.content.name, - text - }); - this.lastUsedStreamOutput = { output, stream: msg.content.name, text }; - traceCellMessage(this.cell, `Append output ${this.lastUsedStreamOutput.text.substring(0, 100)}`); - task?.appendOutput([output]).then(noop, noop); - } - this.endTemporaryTask(); - } - - private handleDisplayData(msg: KernelMessage.IDisplayDataMsg, clearState: RefBool) { - const output = { - output_type: 'display_data', - data: handleTensorBoardDisplayDataOutput(msg.content.data), - metadata: msg.content.metadata, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - transient: msg.content.transient as any // NOSONAR - }; - this.addToCellData(output, clearState); - } - - private handleClearOutput(msg: KernelMessage.IClearOutputMsg, clearState: RefBool) { - // If the message says wait, add every message type to our clear state. This will - // make us wait for this type of output before we clear it. - if (msg && msg.content.wait) { - clearState.update(true); - } else { - // Possible execution of cell has completed (the task would have been disposed). - // This message could have come from a background thread. - // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). - // Clear all outputs and start over again. - const task = this.execution || this.createTemporaryTask(); - this.clearLastUsedStreamOutput(); - task?.clearOutput().then(noop, noop); - this.endTemporaryTask(); - } - } - - private handleError(msg: KernelMessage.IErrorMsg, clearState: RefBool) { - let traceback = msg.content.traceback; - this.formatters.forEach((formatter) => { - traceback = formatter.format(this.cell, traceback); - }); - const output: nbformat.IError = { - output_type: 'error', - ename: msg.content.ename, - evalue: msg.content.evalue, - traceback - }; - - this.addToCellData(output, clearState); - this.cellHasErrorsInOutput = true; - } - - @swallowExceptions() - private handleReply(clearState: RefBool, msg: KernelMessage.IShellControlMessage) { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services'); - - if (jupyterLab.KernelMessage.isExecuteReplyMsg(msg)) { - this.handleExecuteReply(msg, clearState); - - // Set execution count, all messages should have it - if ('execution_count' in msg.content && typeof msg.content.execution_count === 'number' && this.execution) { - this.execution.executionOrder = msg.content.execution_count; - } - } - } - /** - * Execution of Cell B could result in updates to output in Cell A. - */ - private handleUpdateDisplayDataMessage(msg: KernelMessage.IUpdateDisplayDataMsg) { - const displayId = msg.content.transient.display_id; - if (!displayId) { - return; - } - const outputToBeUpdated = this.outputDisplayIdTracker.getMappedOutput(this.cell.notebook, displayId); - if (!outputToBeUpdated) { - return; - } - const output = translateCellDisplayOutput(outputToBeUpdated); - const newOutput = cellOutputToVSCCellOutput({ - ...output, - data: msg.content.data, - metadata: msg.content.metadata - } as nbformat.IDisplayData); - // If there was no output and still no output, then nothing to do. - if (outputToBeUpdated.items.length === 0 && newOutput.items.length === 0) { - return; - } - // Compare each output item (at the end of the day everything is serializable). - // Hence this is a safe comparison. - if (outputToBeUpdated.items.length === newOutput.items.length) { - let allAllOutputItemsSame = true; - for (let index = 0; index < outputToBeUpdated.items.length; index++) { - if (!fastDeepEqual(outputToBeUpdated.items[index], newOutput.items[index])) { - allAllOutputItemsSame = false; - break; - } - } - if (allAllOutputItemsSame) { - // If everything is still the same, then there's nothing to update. - return; - } - } - // Possible execution of cell has completed (the task would have been disposed). - // This message could have come from a background thread. - // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). - const task = this.execution || this.createTemporaryTask(); - traceCellMessage(this.cell, `Replace output items in display data ${newOutput.items.length}`); - task?.replaceOutputItems(newOutput.items, outputToBeUpdated).then(noop, noop); - this.endTemporaryTask(); - } } diff --git a/src/notebooks/execution/cellExecutionMessageHandler.ts b/src/notebooks/execution/cellExecutionMessageHandler.ts new file mode 100644 index 00000000000..81a5789c2f6 --- /dev/null +++ b/src/notebooks/execution/cellExecutionMessageHandler.ts @@ -0,0 +1,574 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as fastDeepEqual from 'fast-deep-equal'; +import type * as nbformat from '@jupyterlab/nbformat'; +import type * as KernelMessage from '@jupyterlab/services/lib/kernel/messages'; +import { + NotebookCell, + NotebookCellExecution, + NotebookCellKind, + NotebookCellExecutionSummary, + NotebookDocument, + workspace, + NotebookController, + WorkspaceEdit, + NotebookCellData, + Range, + NotebookCellOutput, + CancellationTokenSource, + EventEmitter, + ExtensionMode, + NotebookEdit +} from 'vscode'; + +import { Kernel } from '@jupyterlab/services'; +import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; +import { CellExecutionCreator } from './cellExecutionCreator'; +import { IApplicationShell } from '../../platform/common/application/types'; +import { disposeAllDisposables } from '../../platform/common/helpers'; +import { traceWarning } from '../../platform/logging'; +import { RefBool } from '../../platform/common/refBool'; +import { IDisposable, IExtensionContext } from '../../platform/common/types'; +import { traceCellMessage, cellOutputToVSCCellOutput, translateCellDisplayOutput, isJupyterNotebook } from '../helpers'; +import { formatStreamText, concatMultilineString } from '../../webviews/webview-side/common'; +import { swallowExceptions } from '../../platform/common/utils/decorators'; +import { noop } from '../../platform/common/utils/misc'; +import { ITracebackFormatter } from '../../kernels/types'; +import { handleTensorBoardDisplayDataOutput } from './executionHelpers'; +import { WIDGET_MIMETYPE } from '../../kernels/ipywidgets-message-coordination/constants'; + +// Helper interface for the set_next_input execute reply payload +interface ISetNextInputPayload { + replace: boolean; + source: 'set_next_input'; + text: string; +} + +type ExecuteResult = nbformat.IExecuteResult & { + transient?: { display_id?: string }; +}; +type DisplayData = nbformat.IDisplayData & { + transient?: { display_id?: string }; +}; + +/** + * At any given point in time, we can only have one cell actively running. + * This will keep track of that task. + */ +export const activeNotebookCellExecution = new WeakMap(); + +/** + * Responsible for handling of jupyter messages as a result of execution of individual cells. + */ +export class CellExecutionMessageHandler implements IDisposable { + /** + * Listen to messages and update our cell execution state appropriately + * Keep track of our clear state + */ + private readonly clearState = new RefBool(false); + + private execution?: NotebookCellExecution; + private readonly _onErrorHandlingIOPubMessage = new EventEmitter<{ + error: Error; + msg: KernelMessage.IIOPubMessage; + }>(); + public readonly onErrorHandlingExecuteRequestIOPubMessage = this._onErrorHandlingIOPubMessage.event; + private temporaryExecution?: NotebookCellExecution; + private previousResultsToRestore?: NotebookCellExecutionSummary; + private cellHasErrorsInOutput?: boolean; + + public get hasErrorOutput() { + return this.cellHasErrorsInOutput === true; + } + /** + * We keep track of the last output that was used to store stream text. + * We need this so that we can update it later on (when we get new data for the same stream). + * If users clear outputs or if we have a new output other than stream, then clear this item. + * Because if after the stream we have an image, then the stream is not the last output item, hence its cleared. + */ + private lastUsedStreamOutput?: { stream: 'stdout' | 'stderr'; text: string; output: NotebookCellOutput }; + private readonly disposables: IDisposable[] = []; + private readonly prompts = new Set(); + constructor( + public readonly cell: NotebookCell, + private readonly applicationService: IApplicationShell, + private readonly controller: NotebookController, + private readonly outputDisplayIdTracker: CellOutputDisplayIdTracker, + private readonly context: IExtensionContext, + private readonly formatters: ITracebackFormatter[], + private readonly kernel: Kernel.IKernelConnection, + request: Kernel.IShellFuture, + cellExecution: NotebookCellExecution + ) { + workspace.onDidChangeNotebookDocument( + (e) => { + if (!isJupyterNotebook(e.notebook)) { + return; + } + const thisCellChange = e.cellChanges.find(({ cell }) => cell === this.cell); + if (!thisCellChange) { + return; + } + if (thisCellChange.outputs?.length === 0) { + // keep track of the fact that user has cleared the output. + this.clearLastUsedStreamOutput(); + this.cellHasErrorsInOutput = false; + } + }, + this, + this.disposables + ); + this.execution = cellExecution; + this.startHandlingExecutionMessages(request); + } + /** + * This method is called when all execution has been completed (successfully or failed). + * Or when execution has been cancelled. + */ + public dispose() { + traceCellMessage(this.cell, 'Execution disposed'); + disposeAllDisposables(this.disposables); + this.endCellExecution(); + } + private endCellExecution() { + this.prompts.forEach((item) => item.dispose()); + this.prompts.clear(); + this.clearLastUsedStreamOutput(); + this.execution = undefined; + } + private startHandlingExecutionMessages( + request: Kernel.IShellFuture + ) { + request.onIOPub = (msg) => { + // Cell has been deleted or the like. + if (this.cell.document.isClosed) { + request.dispose(); + } + try { + this.handleIOPub(msg); + } catch (ex) { + this._onErrorHandlingIOPubMessage.fire(ex); + } + }; + request.onReply = (msg) => { + // Cell has been deleted or the like. + if (this.cell.document.isClosed) { + request.dispose(); + } + this.handleReply(msg); + }; + request.onStdin = this.handleInputRequest.bind(this); + request.done.finally(() => this.endCellExecution()); + } + + private clearLastUsedStreamOutput() { + this.lastUsedStreamOutput = undefined; + } + /** + * Assume we run cell A + * Now run cell B, and this will update cell A. + * The way it works is, the request object created for cell A will get a message saying update your output. + * Cell A has completed, hence there's no execution task, we should create one or re-use an existing one. + * Creating one results in side effects such as execution order getting reset and timers starting. + * Hence where possible re-use an existing cell execution task associated with this document. + */ + private createTemporaryTask() { + if (this.cell.document.isClosed) { + return; + } + // If we have an active task, use that instead of creating a new task. + const existingTask = activeNotebookCellExecution.get(this.cell.notebook); + if (existingTask) { + return existingTask; + } + + // Create a temporary task. + this.previousResultsToRestore = { ...(this.cell.executionSummary || {}) }; + this.temporaryExecution = CellExecutionCreator.getOrCreate(this.cell, this.controller); + this.temporaryExecution?.start(); + if (this.previousResultsToRestore?.executionOrder && this.execution) { + this.execution.executionOrder = this.previousResultsToRestore.executionOrder; + } + return this.temporaryExecution; + } + private endTemporaryTask() { + if (this.previousResultsToRestore?.executionOrder && this.execution) { + this.execution.executionOrder = this.previousResultsToRestore.executionOrder; + } + if (this.previousResultsToRestore && this.temporaryExecution) { + if (this.previousResultsToRestore.executionOrder) { + this.temporaryExecution.executionOrder = this.previousResultsToRestore.executionOrder; + } + this.temporaryExecution.end( + this.previousResultsToRestore.success, + this.previousResultsToRestore.timing?.endTime + ); + } else { + // Undefined for not success or failure + this.temporaryExecution?.end(undefined); + } + this.previousResultsToRestore = undefined; + this.temporaryExecution = undefined; + } + private handleIOPub(msg: KernelMessage.IIOPubMessage) { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services'); + + if (jupyterLab.KernelMessage.isExecuteResultMsg(msg)) { + this.handleExecuteResult(msg as KernelMessage.IExecuteResultMsg); + } else if (jupyterLab.KernelMessage.isExecuteInputMsg(msg)) { + this.handleExecuteInput(msg as KernelMessage.IExecuteInputMsg); + } else if (jupyterLab.KernelMessage.isStatusMsg(msg)) { + // Status is handled by the result promise. While it is running we are active. Otherwise we're stopped. + // So ignore status messages. + const statusMsg = msg as KernelMessage.IStatusMsg; + this.handleStatusMessage(statusMsg); + } else if (jupyterLab.KernelMessage.isStreamMsg(msg)) { + this.handleStreamMessage(msg as KernelMessage.IStreamMsg); + } else if (jupyterLab.KernelMessage.isDisplayDataMsg(msg)) { + this.handleDisplayData(msg as KernelMessage.IDisplayDataMsg); + } else if (jupyterLab.KernelMessage.isUpdateDisplayDataMsg(msg)) { + this.handleUpdateDisplayDataMessage(msg); + } else if (jupyterLab.KernelMessage.isClearOutputMsg(msg)) { + this.handleClearOutput(msg as KernelMessage.IClearOutputMsg); + } else if (jupyterLab.KernelMessage.isErrorMsg(msg)) { + this.handleError(msg as KernelMessage.IErrorMsg); + } else if (jupyterLab.KernelMessage.isCommOpenMsg(msg)) { + // Noop. + } else if (jupyterLab.KernelMessage.isCommMsgMsg(msg)) { + // Noop. + } else if (jupyterLab.KernelMessage.isCommCloseMsg(msg)) { + // Noop. + } else { + traceWarning(`Unknown message ${msg.header.msg_type} : hasData=${'data' in msg.content}`); + } + + // Set execution count, all messages should have it + if ('execution_count' in msg.content && typeof msg.content.execution_count === 'number' && this.execution) { + this.execution.executionOrder = msg.content.execution_count; + } + } + + private addToCellData(output: ExecuteResult | DisplayData | nbformat.IStream | nbformat.IError | nbformat.IOutput) { + if ( + this.context.extensionMode === ExtensionMode.Test && + output.data && + typeof output.data === 'object' && + WIDGET_MIMETYPE in output.data + ) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (output.data[WIDGET_MIMETYPE] as any)['_vsc_test_cellIndex'] = this.cell.index; + } + const cellOutput = cellOutputToVSCCellOutput(output); + const displayId = + 'transient' in output && + typeof output.transient === 'object' && + output.transient && + 'display_id' in output.transient && + typeof output.transient?.display_id === 'string' + ? output.transient?.display_id + : undefined; + if (this.cell.document.isClosed) { + return; + } + traceCellMessage(this.cell, 'Update output'); + // Clear if necessary + if (this.clearState.value) { + this.clearLastUsedStreamOutput(); + this.execution?.clearOutput().then(noop, noop); + this.clearState.update(false); + } + // Keep track of the displa_id against the output item, we might need this to update this later. + if (displayId) { + this.outputDisplayIdTracker.trackOutputByDisplayId(this.cell, displayId, cellOutput); + } + + // Append to the data (we would push here but VS code requires a recreation of the array) + // Possible execution of cell has completed (the task would have been disposed). + // This message could have come from a background thread. + // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). + const task = this.execution || this.createTemporaryTask(); + this.clearLastUsedStreamOutput(); + traceCellMessage(this.cell, 'Append output in addToCellData'); + task?.appendOutput([cellOutput]).then(noop, noop); + this.endTemporaryTask(); + } + + private async handleInputRequest(msg: KernelMessage.IStdinMessage) { + // Ask the user for input + if (msg.content && 'prompt' in msg.content) { + const cancelToken = new CancellationTokenSource(); + this.prompts.add(cancelToken); + const hasPassword = msg.content.password !== null && (msg.content.password as boolean); + await this.applicationService + .showInputBox( + { + prompt: msg.content.prompt ? msg.content.prompt.toString() : '', + ignoreFocusOut: true, + password: hasPassword + }, + cancelToken.token + ) + .then((v) => { + this.kernel.sendInputReply({ value: v || '', status: 'ok' }); + }, noop); + + this.prompts.delete(cancelToken); + } + } + + // See this for docs on the messages: + // https://jupyter-client.readthedocs.io/en/latest/messaging.html#messaging-in-jupyter + private handleExecuteResult(msg: KernelMessage.IExecuteResultMsg) { + this.addToCellData({ + output_type: 'execute_result', + data: msg.content.data, + metadata: msg.content.metadata, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + transient: msg.content.transient as any, // NOSONAR + execution_count: msg.content.execution_count + }); + } + + private handleExecuteReply(msg: KernelMessage.IExecuteReplyMsg) { + const reply = msg.content as KernelMessage.IExecuteReply; + if (reply.payload) { + reply.payload.forEach((payload) => { + if ( + payload.source && + payload.source === 'set_next_input' && + 'text' in payload && + 'replace' in payload + ) { + this.handleSetNextInput(payload as unknown as ISetNextInputPayload); + } + if (payload.data && payload.data.hasOwnProperty('text/plain')) { + this.addToCellData({ + // Mark as stream output so the text is formatted because it likely has ansi codes in it. + output_type: 'stream', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + text: (payload.data as any)['text/plain'].toString(), + name: 'stdout', + metadata: {}, + execution_count: reply.execution_count + }); + } + }); + } + } + + // Handle our set_next_input message, which can either replace or insert a new cell with text + private handleSetNextInput(payload: ISetNextInputPayload) { + const edit = new WorkspaceEdit(); + if (payload.replace) { + // Replace the contents of the current cell with text + edit.replace( + this.cell.document.uri, + new Range( + this.cell.document.lineAt(0).range.start, + this.cell.document.lineAt(this.cell.document.lineCount - 1).range.end + ), + payload.text + ); + } else { + // Add a new cell after the current with text + traceCellMessage(this.cell, 'Create new cell after current'); + const cellData = new NotebookCellData(NotebookCellKind.Code, payload.text, this.cell.document.languageId); + cellData.outputs = []; + cellData.metadata = {}; + const nbEdit = NotebookEdit.insertCells(this.cell.index + 1, [cellData]); + edit.set(this.cell.notebook.uri, [nbEdit]); + } + workspace.applyEdit(edit).then(noop, noop); + } + + private handleExecuteInput(msg: KernelMessage.IExecuteInputMsg) { + if (msg.content.execution_count && this.execution) { + this.execution.executionOrder = msg.content.execution_count; + } + } + + private handleStatusMessage(msg: KernelMessage.IStatusMsg) { + traceCellMessage(this.cell, `Kernel switching to ${msg.content.execution_state}`); + } + private handleStreamMessage(msg: KernelMessage.IStreamMsg) { + // eslint-disable-next-line complexity + traceCellMessage(this.cell, 'Update streamed output'); + // Possible execution of cell has completed (the task would have been disposed). + // This message could have come from a background thread. + // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). + const task = this.execution || this.createTemporaryTask(); + + // Clear output if waiting for a clear + const clearOutput = this.clearState.value; + if (clearOutput) { + traceCellMessage(this.cell, 'Clear cell output'); + this.clearLastUsedStreamOutput(); + task?.clearOutput().then(noop, noop); + this.clearState.update(false); + } + // Ensure we append to previous output, only if the streams as the same & + // If the last output is the desired stream type. + if (this.lastUsedStreamOutput?.stream === msg.content.name) { + // Get the jupyter output from the vs code output (so we can concatenate the text ourselves). + let existingOutputText = this.lastUsedStreamOutput.text; + let newContent = msg.content.text; + // Look for the ansi code `[A`. (this means move up) + // Not going to support `[2A` (not for now). + const moveUpCode = `${String.fromCharCode(27)}[A`; + if (msg.content.text.startsWith(moveUpCode)) { + // Split message by lines & strip out the last n lines (where n = number of lines to move cursor up). + const existingOutputLines = existingOutputText.splitLines({ + trim: false, + removeEmptyEntries: false + }); + if (existingOutputLines.length) { + existingOutputLines.pop(); + } + existingOutputText = existingOutputLines.join('\n'); + newContent = newContent.substring(moveUpCode.length); + } + // Create a new output item with the concatenated string. + this.lastUsedStreamOutput.text = formatStreamText( + concatMultilineString(`${existingOutputText}${newContent}`) + ); + const output = cellOutputToVSCCellOutput({ + output_type: 'stream', + name: msg.content.name, + text: this.lastUsedStreamOutput.text + }); + traceCellMessage(this.cell, `Replace output items ${this.lastUsedStreamOutput.text.substring(0, 100)}`); + task?.replaceOutputItems(output.items, this.lastUsedStreamOutput.output).then(noop, noop); + } else if (clearOutput) { + // Replace the current outputs with a single new output. + const text = formatStreamText(concatMultilineString(msg.content.text)); + const output = cellOutputToVSCCellOutput({ + output_type: 'stream', + name: msg.content.name, + text + }); + this.lastUsedStreamOutput = { output, stream: msg.content.name, text }; + traceCellMessage(this.cell, `Replace output ${this.lastUsedStreamOutput.text.substring(0, 100)}`); + task?.replaceOutput([output]).then(noop, noop); + } else { + // Create a new output + const text = formatStreamText(concatMultilineString(msg.content.text)); + const output = cellOutputToVSCCellOutput({ + output_type: 'stream', + name: msg.content.name, + text + }); + this.lastUsedStreamOutput = { output, stream: msg.content.name, text }; + traceCellMessage(this.cell, `Append output ${this.lastUsedStreamOutput.text.substring(0, 100)}`); + task?.appendOutput([output]).then(noop, noop); + } + this.endTemporaryTask(); + } + + private handleDisplayData(msg: KernelMessage.IDisplayDataMsg) { + const output = { + output_type: 'display_data', + data: handleTensorBoardDisplayDataOutput(msg.content.data), + metadata: msg.content.metadata, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + transient: msg.content.transient as any // NOSONAR + }; + this.addToCellData(output); + } + + private handleClearOutput(msg: KernelMessage.IClearOutputMsg) { + // If the message says wait, add every message type to our clear state. This will + // make us wait for this type of output before we clear it. + if (msg && msg.content.wait) { + this.clearState.update(true); + } else { + // Possible execution of cell has completed (the task would have been disposed). + // This message could have come from a background thread. + // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). + // Clear all outputs and start over again. + const task = this.execution || this.createTemporaryTask(); + this.clearLastUsedStreamOutput(); + task?.clearOutput().then(noop, noop); + this.endTemporaryTask(); + } + } + + private handleError(msg: KernelMessage.IErrorMsg) { + let traceback = msg.content.traceback; + this.formatters.forEach((formatter) => { + traceback = formatter.format(this.cell, traceback); + }); + const output: nbformat.IError = { + output_type: 'error', + ename: msg.content.ename, + evalue: msg.content.evalue, + traceback + }; + + this.addToCellData(output); + this.cellHasErrorsInOutput = true; + } + + @swallowExceptions() + private handleReply(msg: KernelMessage.IShellControlMessage) { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services'); + + if (jupyterLab.KernelMessage.isExecuteReplyMsg(msg)) { + this.handleExecuteReply(msg); + + // Set execution count, all messages should have it + if ('execution_count' in msg.content && typeof msg.content.execution_count === 'number' && this.execution) { + this.execution.executionOrder = msg.content.execution_count; + } + } + } + /** + * Execution of Cell B could result in updates to output in Cell A. + */ + private handleUpdateDisplayDataMessage(msg: KernelMessage.IUpdateDisplayDataMsg) { + const displayId = msg.content.transient.display_id; + if (!displayId) { + return; + } + const outputToBeUpdated = this.outputDisplayIdTracker.getMappedOutput(this.cell.notebook, displayId); + if (!outputToBeUpdated) { + return; + } + const output = translateCellDisplayOutput(outputToBeUpdated); + const newOutput = cellOutputToVSCCellOutput({ + ...output, + data: msg.content.data, + metadata: msg.content.metadata + } as nbformat.IDisplayData); + // If there was no output and still no output, then nothing to do. + if (outputToBeUpdated.items.length === 0 && newOutput.items.length === 0) { + return; + } + // Compare each output item (at the end of the day everything is serializable). + // Hence this is a safe comparison. + if (outputToBeUpdated.items.length === newOutput.items.length) { + let allAllOutputItemsSame = true; + for (let index = 0; index < outputToBeUpdated.items.length; index++) { + if (!fastDeepEqual(outputToBeUpdated.items[index], newOutput.items[index])) { + allAllOutputItemsSame = false; + break; + } + } + if (allAllOutputItemsSame) { + // If everything is still the same, then there's nothing to update. + return; + } + } + // Possible execution of cell has completed (the task would have been disposed). + // This message could have come from a background thread. + // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). + const task = this.execution || this.createTemporaryTask(); + traceCellMessage(this.cell, `Replace output items in display data ${newOutput.items.length}`); + task?.replaceOutputItems(newOutput.items, outputToBeUpdated).then(noop, noop); + this.endTemporaryTask(); + } +} diff --git a/src/notebooks/execution/cellExecutionMessageHandlerFactory.ts b/src/notebooks/execution/cellExecutionMessageHandlerFactory.ts new file mode 100644 index 00000000000..8a78e2ad00d --- /dev/null +++ b/src/notebooks/execution/cellExecutionMessageHandlerFactory.ts @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Kernel, KernelMessage } from '@jupyterlab/services'; +import { NotebookCell, NotebookCellExecution, NotebookController } from 'vscode'; +import { ITracebackFormatter } from '../../kernels/types'; +import { IApplicationShell } from '../../platform/common/application/types'; +import { IExtensionContext } from '../../platform/common/types'; +import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; +import { CellExecutionMessageHandler } from './cellExecutionMessageHandler'; + +export class CellExecutionMessageHandlerFactory { + constructor( + private readonly appShell: IApplicationShell, + private readonly controller: NotebookController, + private readonly outputDisplayIdTracker: CellOutputDisplayIdTracker, + private readonly context: IExtensionContext, + private readonly formatters: ITracebackFormatter[] + ) {} + public create( + cell: NotebookCell, + options: { + kernel: Kernel.IKernelConnection; + request: Kernel.IShellFuture; + cellExecution: NotebookCellExecution; + } + ): CellExecutionMessageHandler { + return new CellExecutionMessageHandler( + cell, + this.appShell, + this.controller, + this.outputDisplayIdTracker, + this.context, + this.formatters, + options.kernel, + options.request, + options.cellExecution + ); + } +} diff --git a/src/notebooks/execution/kernelExecution.ts b/src/notebooks/execution/kernelExecution.ts index 27a1467b981..d7f67305143 100644 --- a/src/notebooks/execution/kernelExecution.ts +++ b/src/notebooks/execution/kernelExecution.ts @@ -27,6 +27,7 @@ import { import { traceCellMessage } from '../helpers'; import { getDisplayPath } from '../../platform/common/platform/fs-paths'; import { getAssociatedNotebookDocument } from '../controllers/kernelSelector'; +import { CellExecutionMessageHandlerFactory } from './cellExecutionMessageHandlerFactory'; /** * Separate class that deals just with kernel execution. @@ -49,7 +50,14 @@ export class KernelExecution implements IDisposable { context: IExtensionContext, formatters: ITracebackFormatter[] ) { - this.executionFactory = new CellExecutionFactory(appShell, controller, outputTracker, context, formatters); + const factory = new CellExecutionMessageHandlerFactory( + appShell, + controller, + outputTracker, + context, + formatters + ); + this.executionFactory = new CellExecutionFactory(controller, factory); } public get onPreExecute() { diff --git a/src/platform/common/refBool.node.ts b/src/platform/common/refBool.ts similarity index 67% rename from src/platform/common/refBool.node.ts rename to src/platform/common/refBool.ts index c0ef43a2077..22b0421b5e9 100644 --- a/src/platform/common/refBool.node.ts +++ b/src/platform/common/refBool.ts @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + export class RefBool { constructor(private val: boolean) {} From b33c2b2b67785ebdbe11b795e624324c1e1fce25 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 30 May 2022 13:47:10 +1000 Subject: [PATCH 3/3] Results for comm messages sent from widgets should get handled by the same cell (part 3) --- src/kernels/common/baseJupyterSession.ts | 22 ++- src/kernels/common/kernelSocketWrapper.ts | 4 + src/kernels/raw/session/rawKernel.node.ts | 13 +- src/kernels/raw/session/rawSocket.node.ts | 8 +- src/kernels/types.ts | 6 + src/notebooks/execution/cellExecution.ts | 30 ++-- .../execution/cellExecutionMessageHandler.ts | 130 ++++++++++++++---- .../cellExecutionMessageHandlerFactory.ts | 40 ------ .../cellExecutionMessageHandlerService.ts | 71 ++++++++++ src/notebooks/execution/kernelExecution.ts | 7 +- src/test/datascience/notebook/helper.ts | 5 +- .../notebooks/button_widget_comm_msg.ipynb | 39 ++++++ .../button_widget_comm_msg_matplotlib.ipynb | 47 +++++++ .../widgets/standard.vscode.common.ts | 25 ++++ 14 files changed, 346 insertions(+), 101 deletions(-) delete mode 100644 src/notebooks/execution/cellExecutionMessageHandlerFactory.ts create mode 100644 src/notebooks/execution/cellExecutionMessageHandlerService.ts create mode 100644 src/test/datascience/widgets/notebooks/button_widget_comm_msg.ipynb create mode 100644 src/test/datascience/widgets/notebooks/button_widget_comm_msg_matplotlib.ipynb diff --git a/src/kernels/common/baseJupyterSession.ts b/src/kernels/common/baseJupyterSession.ts index 172ccf3b343..972eff93540 100644 --- a/src/kernels/common/baseJupyterSession.ts +++ b/src/kernels/common/baseJupyterSession.ts @@ -127,7 +127,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { private _kernelSocket = new ReplaySubject(); private unhandledMessageHandler: Slot; private chainingExecute = new ChainingExecuteRequester(); - + private previousAnyMessageHandler?: IDisposable; constructor( public readonly kind: 'localRaw' | 'remoteJupyter' | 'localJupyter', protected resource: Resource, @@ -352,6 +352,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { // Changes the current session. protected setSession(session: ISessionWithSocket | undefined, forceUpdateKernelSocketInfo: boolean = false) { const oldSession = this._session; + this.previousAnyMessageHandler?.dispose(); if (oldSession) { if (this.unhandledMessageHandler) { oldSession.unhandledMessage.disconnect(this.unhandledMessageHandler); @@ -368,6 +369,24 @@ export abstract class BaseJupyterSession implements IJupyterSession { // Listen for session status changes session.statusChanged.connect(this.statusHandler); + if (session.kernelSocketInformation.socket?.onAnyMessage) { + // These messages are sent directly to the kernel bypassing the Jupyter lab npm libraries. + // As a result, we don't get any notification that messages were sent (on the anymessage signal). + // To ensure those signals can still be used to monitor such messages, send them via a callback so that we can emit these messages on the anymessage signal. + this.previousAnyMessageHandler = session.kernelSocketInformation.socket?.onAnyMessage((msg) => { + try { + if (this._wrappedKernel) { + const jupyterLabSerialize = + require('@jupyterlab/services/lib/kernel/serialize') as typeof import('@jupyterlab/services/lib/kernel/serialize'); // NOSONAR + const message = + typeof msg.msg === 'string' ? jupyterLabSerialize.deserialize(msg.msg) : msg.msg; + this._wrappedKernel.anyMessage.emit({ direction: msg.direction, msg: message }); + } + } catch (ex) { + traceWarning(`failed to deserialize message to broadcast anymessage signal`); + } + }); + } if (session.unhandledMessage) { session.unhandledMessage.connect(this.unhandledMessageHandler); } @@ -447,6 +466,7 @@ export abstract class BaseJupyterSession implements IJupyterSession { this._disposed.fire(); this._disposed.dispose(); this.onStatusChangedEvent.dispose(); + this.previousAnyMessageHandler?.dispose(); } disposeAllDisposables(this.disposables); traceVerbose('Shutdown session -- complete'); diff --git a/src/kernels/common/kernelSocketWrapper.ts b/src/kernels/common/kernelSocketWrapper.ts index f1034f83c11..0916f6e29dc 100644 --- a/src/kernels/common/kernelSocketWrapper.ts +++ b/src/kernels/common/kernelSocketWrapper.ts @@ -1,5 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { EventEmitter } from 'vscode'; import * as WebSocketWS from 'ws'; import { ClassType } from '../../platform/ioc/types'; import { traceError } from '../../platform/logging'; @@ -59,6 +60,8 @@ export function KernelSocketWrapper>(SuperCl this.receiveHooks = []; this.sendHooks = []; } + private _onAnyMessage = new EventEmitter<{ msg: string; direction: 'send' }>(); + public onAnyMessage = this._onAnyMessage.event; protected patchSuperEmit(patch: (event: string | symbol, ...args: any[]) => boolean) { super.emit = patch; @@ -67,6 +70,7 @@ export function KernelSocketWrapper>(SuperCl public sendToRealKernel(data: any, a2: any) { // This will skip the send hooks. It's coming from // the UI side. + this._onAnyMessage.fire({ msg: data, direction: 'send' }); super.send(data, a2); } diff --git a/src/kernels/raw/session/rawKernel.node.ts b/src/kernels/raw/session/rawKernel.node.ts index b335c21c2fd..4df68ec1e05 100644 --- a/src/kernels/raw/session/rawKernel.node.ts +++ b/src/kernels/raw/session/rawKernel.node.ts @@ -276,17 +276,9 @@ export function createRawKernel(kernelProcess: IKernelProcess, clientId: string) // Dummy websocket we give to the underlying real kernel let socketInstance: any; - let rawKernel: RawKernel; class RawSocketWrapper extends RawSocket { constructor() { - super(kernelProcess.connection, jupyterLabSerialize.serialize, jupyterLabSerialize.deserialize, (msg) => { - if (rawKernel) { - // These messages are sent directly to the kernel bypassing the Jupyter lab npm libraries. - // As a result, we don't get any notification that messages were sent (on the anymessage signal). - // To ensure those signals can still be used to monitor such messages, send them via a callback so that we can emit these messages on the anymessage signal. - rawKernel?.anyMessage.emit({ direction: 'send', msg }); - } - }); + super(kernelProcess.connection, jupyterLabSerialize.serialize, jupyterLabSerialize.deserialize); socketInstance = this; } } @@ -317,6 +309,5 @@ export function createRawKernel(kernelProcess: IKernelProcess, clientId: string) }); // Use this real kernel in result. - rawKernel = new RawKernel(realKernel, socketInstance, kernelProcess); - return rawKernel; + return new RawKernel(realKernel, socketInstance, kernelProcess); } diff --git a/src/kernels/raw/session/rawSocket.node.ts b/src/kernels/raw/session/rawSocket.node.ts index 1915e6c99df..e8a03eb7096 100644 --- a/src/kernels/raw/session/rawSocket.node.ts +++ b/src/kernels/raw/session/rawSocket.node.ts @@ -12,6 +12,7 @@ import { IWebSocketLike } from '../../common/kernelSocketWrapper'; import { IKernelSocket } from '../../types'; import { IKernelConnection } from '../types'; import type { Channel } from '@jupyterlab/services/lib/kernel/messages'; +import { EventEmitter } from 'vscode'; function formConnectionString(config: IKernelConnection, channel: string) { const portDelimiter = config.transport === 'tcp' ? ':' : '-'; @@ -34,6 +35,8 @@ interface IChannels { * it does all serialization/deserialization itself. */ export class RawSocket implements IWebSocketLike, IKernelSocket, IDisposable { + private _onAnyMessage = new EventEmitter<{ msg: string; direction: 'send' }>(); + public onAnyMessage = this._onAnyMessage.event; public onopen: (event: { target: any }) => void = noop; public onerror: (event: { error: any; message: string; type: string; target: any }) => void = noop; public onclose: (event: { wasClean: boolean; code: number; reason: string; target: any }) => void = noop; @@ -48,8 +51,7 @@ export class RawSocket implements IWebSocketLike, IKernelSocket, IDisposable { constructor( private connection: IKernelConnection, private serialize: (msg: KernelMessage.IMessage) => string | ArrayBuffer, - private deserialize: (data: ArrayBuffer | string) => KernelMessage.IMessage, - private readonly onAnyMessage: (msg: KernelMessage.IMessage) => void + private deserialize: (data: ArrayBuffer | string) => KernelMessage.IMessage ) { // Setup our ZMQ channels now this.channels = this.generateChannels(connection); @@ -104,7 +106,7 @@ export class RawSocket implements IWebSocketLike, IKernelSocket, IDisposable { // These messages are sent directly to the kernel bypassing the Jupyter lab npm libraries. // As a result, we don't get any notification that messages were sent (on the anymessage signal). // To ensure those signals can still be used to monitor such messages, send them via a callback so that we can emit these messages on the anymessage signal. - this.onAnyMessage(message as any); + this._onAnyMessage.fire({ msg: data, direction: 'send' }); // Send this directly (don't call back into the hooks) this.sendMessage(message, true); } diff --git a/src/kernels/types.ts b/src/kernels/types.ts index cbcccd76de5..92e746f7a5e 100644 --- a/src/kernels/types.ts +++ b/src/kernels/types.ts @@ -432,6 +432,12 @@ export interface INotebookProvider { } export interface IKernelSocket { + /** + * These messages are sent directly to the kernel bypassing the Jupyter lab npm libraries. + * As a result, we don't get any notification that messages were sent (on the anymessage signal). + * To ensure those signals can still be used to monitor such messages, send them via a callback so that we can emit these messages on the anymessage signal. + */ + onAnyMessage: Event<{ msg: string | KernelMessage.IMessage; direction: 'send' }>; // eslint-disable-next-line @typescript-eslint/no-explicit-any sendToRealKernel(data: any, cb?: (err?: Error) => void): void; /** diff --git a/src/notebooks/execution/cellExecution.ts b/src/notebooks/execution/cellExecution.ts index 06db387a21c..da758fbbd35 100644 --- a/src/notebooks/execution/cellExecution.ts +++ b/src/notebooks/execution/cellExecution.ts @@ -31,18 +31,18 @@ import { noop } from '../../platform/common/utils/misc'; import { getDisplayNameOrNameOfKernelConnection, isPythonKernelConnection } from '../../kernels/helpers'; import { isCancellationError } from '../../platform/common/cancellation'; import { activeNotebookCellExecution, CellExecutionMessageHandler } from './cellExecutionMessageHandler'; -import { CellExecutionMessageHandlerFactory } from './cellExecutionMessageHandlerFactory'; +import { CellExecutionMessageHandlerService } from './cellExecutionMessageHandlerService'; import { IJupyterSession, KernelConnectionMetadata, NotebookCellRunState } from '../../kernels/types'; export class CellExecutionFactory { constructor( private readonly controller: NotebookController, - private readonly factory: CellExecutionMessageHandlerFactory + private readonly requestListener: CellExecutionMessageHandlerService ) {} public create(cell: NotebookCell, code: string | undefined, metadata: Readonly) { // eslint-disable-next-line @typescript-eslint/no-use-before-define - return CellExecution.fromCell(cell, code, metadata, this.controller, this.factory); + return CellExecution.fromCell(cell, code, metadata, this.controller, this.requestListener); } } @@ -85,7 +85,7 @@ export class CellExecution implements IDisposable { private readonly codeOverride: string | undefined, private readonly kernelConnection: Readonly, private readonly controller: NotebookController, - private readonly factory: CellExecutionMessageHandlerFactory + private readonly requestListener: CellExecutionMessageHandlerService ) { workspace.onDidCloseTextDocument( (e) => { @@ -130,9 +130,9 @@ export class CellExecution implements IDisposable { code: string | undefined, metadata: Readonly, controller: NotebookController, - factory: CellExecutionMessageHandlerFactory + requestListener: CellExecutionMessageHandlerService ) { - return new CellExecution(cell, code, metadata, controller, factory); + return new CellExecution(cell, code, metadata, controller, requestListener); } public async start(session: IJupyterSession) { if (this.cancelHandled) { @@ -352,22 +352,16 @@ export class CellExecution implements IDisposable { traceError(`Cell execution failed without request, for cell Index ${this.cell.index}`, ex); return this.completedWithErrors(ex); } - this.cellExecutionHandler = this.factory.create(this.cell, { + this.cellExecutionHandler = this.requestListener.registerListener(this.cell, { kernel: session.kernel!, cellExecution: this.execution!, - request: this.request - }); - this.disposables.push(this.cellExecutionHandler); - this.cellExecutionHandler.onErrorHandlingExecuteRequestIOPubMessage( - ({ error }) => { + request: this.request, + onErrorHandlingExecuteRequestIOPubMessage: (error) => { traceError(`Cell (index = ${this.cell.index}) execution completed with errors (2).`, error); // If not a restart error, then tell the subscriber - // eslint-disable-next-line @typescript-eslint/no-explicit-any - this.completedWithErrors(error as any); - }, - this, - this.disposables - ); + this.completedWithErrors(error); + } + }); // WARNING: Do not dispose `request`. // Even after request.done & execute_reply is sent we could have more messages coming from iopub. diff --git a/src/notebooks/execution/cellExecutionMessageHandler.ts b/src/notebooks/execution/cellExecutionMessageHandler.ts index 81a5789c2f6..dcb2a4feddb 100644 --- a/src/notebooks/execution/cellExecutionMessageHandler.ts +++ b/src/notebooks/execution/cellExecutionMessageHandler.ts @@ -29,7 +29,7 @@ import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; import { CellExecutionCreator } from './cellExecutionCreator'; import { IApplicationShell } from '../../platform/common/application/types'; import { disposeAllDisposables } from '../../platform/common/helpers'; -import { traceWarning } from '../../platform/logging'; +import { traceError, traceWarning } from '../../platform/logging'; import { RefBool } from '../../platform/common/refBool'; import { IDisposable, IExtensionContext } from '../../platform/common/types'; import { traceCellMessage, cellOutputToVSCCellOutput, translateCellDisplayOutput, isJupyterNotebook } from '../helpers'; @@ -64,6 +64,14 @@ export const activeNotebookCellExecution = new WeakMap(); + /** + * List of comm_ids Jupyter sent back when this cell was first executed + * or for any subsequent requests as a result of outputs sending custom messages. + */ + private readonly ownedCommIds = new Set(); + /** + * List of msg_ids of requests sent either as part of request_execute + * or for any subsequent requests as a result of outputs sending custom messages. + */ + private readonly ownedRequestIds = new Set(); constructor( public readonly cell: NotebookCell, private readonly applicationService: IApplicationShell, @@ -103,6 +121,8 @@ export class CellExecutionMessageHandler implements IDisposable { request: Kernel.IShellFuture, cellExecution: NotebookCellExecution ) { + this.executeRequestMessageId = request.msg.header.msg_id; + this.ownedRequestIds.add(request.msg.header.msg_id); workspace.onDidChangeNotebookDocument( (e) => { if (!isJupyterNotebook(e.notebook)) { @@ -122,30 +142,11 @@ export class CellExecutionMessageHandler implements IDisposable { this.disposables ); this.execution = cellExecution; - this.startHandlingExecutionMessages(request); - } - /** - * This method is called when all execution has been completed (successfully or failed). - * Or when execution has been cancelled. - */ - public dispose() { - traceCellMessage(this.cell, 'Execution disposed'); - disposeAllDisposables(this.disposables); - this.endCellExecution(); - } - private endCellExecution() { - this.prompts.forEach((item) => item.dispose()); - this.prompts.clear(); - this.clearLastUsedStreamOutput(); - this.execution = undefined; - } - private startHandlingExecutionMessages( - request: Kernel.IShellFuture - ) { request.onIOPub = (msg) => { // Cell has been deleted or the like. if (this.cell.document.isClosed) { request.dispose(); + return; } try { this.handleIOPub(msg); @@ -157,13 +158,93 @@ export class CellExecutionMessageHandler implements IDisposable { // Cell has been deleted or the like. if (this.cell.document.isClosed) { request.dispose(); + return; } this.handleReply(msg); }; request.onStdin = this.handleInputRequest.bind(this); - request.done.finally(() => this.endCellExecution()); + request.done + .finally(() => { + this.completedExecution = true; + // We're only interested in messages after execution has completed. + // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. + this.kernel.anyMessage.connect(this.onKernelAnyMessage, this); + this.kernel.iopubMessage.connect(this.onKernelIOPubMessage, this); + + this.endCellExecution(); + }) + .catch(noop); + } + /** + * This method is called when all execution has been completed (successfully or failed). + * Or when execution has been cancelled. + */ + public dispose() { + traceCellMessage(this.cell, 'Execution disposed'); + disposeAllDisposables(this.disposables); + this.prompts.forEach((item) => item.dispose()); + this.prompts.clear(); + this.clearLastUsedStreamOutput(); + this.execution = undefined; + this.kernel.anyMessage.disconnect(this.onKernelAnyMessage, this); + this.kernel.iopubMessage.disconnect(this.onKernelIOPubMessage, this); } + /** + * This merely marks the end of the cell execution. + * However this class will still monitor iopub messages from the kernel. + * As its possible a widget from the output of this cell sends message to the kernel and + * as a result of the response we get some new output. + */ + private endCellExecution() { + this.prompts.forEach((item) => item.dispose()); + this.prompts.clear(); + this.clearLastUsedStreamOutput(); + this.execution = undefined; + if (this.ownedCommIds.size === 0 && this.completedExecution) { + // If no comms channels were opened as a result of any outputs of this cell, + // this means we don't have any widgets that can send comm message back to the kernel. + // Hence no point listening to any of the iopub messages & the like, i.e. we can stop listening to everything in this class. + this.dispose(); + } + } + private onKernelAnyMessage(_: unknown, { direction, msg }: Kernel.IAnyMessageArgs) { + // We're only interested in messages after execution has completed. + // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. + if (direction !== 'send' || !this.completedExecution) { + return; + } + // eslint-disable-next-line @typescript-eslint/no-require-imports + const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services'); + if (jupyterLab.KernelMessage.isCommMsgMsg(msg) && this.ownedCommIds.has(msg.content.comm_id)) { + // Looks like we have a comm msg request sent by some output or the like. + // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. + this.ownedRequestIds.add(msg.header.msg_id); + } + } + private onKernelIOPubMessage(_: unknown, msg: KernelMessage.IIOPubMessage) { + // We're only interested in messages after execution has completed. + // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. + + // Handle iopub messages that are sent from Jupyter in response to some + // comm message (requests) sent by an output widget. + // See https://github.com/microsoft/vscode-jupyter/issues/9503 for more information. + if ( + !this.completedExecution || + !msg.parent_header || + !('msg_id' in msg.parent_header) || + msg.parent_header.msg_id === this.executeRequestMessageId || + !this.ownedRequestIds.has(msg.parent_header.msg_id) || + msg.channel !== 'iopub' + ) { + return; + } + try { + this.handleIOPub(msg); + } catch (ex) { + traceError(`Failed to handle iopub message as a result of some comm message`, msg, ex); + } + } private clearLastUsedStreamOutput() { this.lastUsedStreamOutput = undefined; } @@ -216,8 +297,9 @@ export class CellExecutionMessageHandler implements IDisposable { private handleIOPub(msg: KernelMessage.IIOPubMessage) { // eslint-disable-next-line @typescript-eslint/no-require-imports const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services'); - - if (jupyterLab.KernelMessage.isExecuteResultMsg(msg)) { + if (jupyterLab.KernelMessage.isCommOpenMsg(msg)) { + this.ownedCommIds.add(msg.content.comm_id); + } else if (jupyterLab.KernelMessage.isExecuteResultMsg(msg)) { this.handleExecuteResult(msg as KernelMessage.IExecuteResultMsg); } else if (jupyterLab.KernelMessage.isExecuteInputMsg(msg)) { this.handleExecuteInput(msg as KernelMessage.IExecuteInputMsg); diff --git a/src/notebooks/execution/cellExecutionMessageHandlerFactory.ts b/src/notebooks/execution/cellExecutionMessageHandlerFactory.ts deleted file mode 100644 index 8a78e2ad00d..00000000000 --- a/src/notebooks/execution/cellExecutionMessageHandlerFactory.ts +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { Kernel, KernelMessage } from '@jupyterlab/services'; -import { NotebookCell, NotebookCellExecution, NotebookController } from 'vscode'; -import { ITracebackFormatter } from '../../kernels/types'; -import { IApplicationShell } from '../../platform/common/application/types'; -import { IExtensionContext } from '../../platform/common/types'; -import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; -import { CellExecutionMessageHandler } from './cellExecutionMessageHandler'; - -export class CellExecutionMessageHandlerFactory { - constructor( - private readonly appShell: IApplicationShell, - private readonly controller: NotebookController, - private readonly outputDisplayIdTracker: CellOutputDisplayIdTracker, - private readonly context: IExtensionContext, - private readonly formatters: ITracebackFormatter[] - ) {} - public create( - cell: NotebookCell, - options: { - kernel: Kernel.IKernelConnection; - request: Kernel.IShellFuture; - cellExecution: NotebookCellExecution; - } - ): CellExecutionMessageHandler { - return new CellExecutionMessageHandler( - cell, - this.appShell, - this.controller, - this.outputDisplayIdTracker, - this.context, - this.formatters, - options.kernel, - options.request, - options.cellExecution - ); - } -} diff --git a/src/notebooks/execution/cellExecutionMessageHandlerService.ts b/src/notebooks/execution/cellExecutionMessageHandlerService.ts new file mode 100644 index 00000000000..6d5c19bb2a4 --- /dev/null +++ b/src/notebooks/execution/cellExecutionMessageHandlerService.ts @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Kernel, KernelMessage } from '@jupyterlab/services'; +import { NotebookCell, NotebookCellExecution, NotebookController, NotebookDocument, workspace } from 'vscode'; +import { ITracebackFormatter } from '../../kernels/types'; +import { IApplicationShell } from '../../platform/common/application/types'; +import { disposeAllDisposables } from '../../platform/common/helpers'; +import { IDisposable, IExtensionContext } from '../../platform/common/types'; +import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker'; +import { CellExecutionMessageHandler } from './cellExecutionMessageHandler'; + +export class CellExecutionMessageHandlerService { + private readonly disposables: IDisposable[] = []; + private notebook?: NotebookDocument; + private readonly messageHandlers = new WeakMap(); + constructor( + private readonly appShell: IApplicationShell, + private readonly controller: NotebookController, + private readonly outputDisplayIdTracker: CellOutputDisplayIdTracker, + private readonly context: IExtensionContext, + private readonly formatters: ITracebackFormatter[] + ) { + workspace.onDidChangeNotebookDocument( + (e) => { + if (e.notebook !== this.notebook) { + return; + } + e.contentChanges.forEach((change) => + // If the cell is deleted, then dispose the corresponding handler. + change.removedCells.forEach((cell) => this.messageHandlers.get(cell)?.dispose()) + ); + }, + this, + this.disposables + ); + } + dispose() { + disposeAllDisposables(this.disposables); + if (this.notebook) { + this.notebook.getCells().forEach((cell) => this.messageHandlers.get(cell)?.dispose()); + } + } + public registerListener( + cell: NotebookCell, + options: { + kernel: Kernel.IKernelConnection; + request: Kernel.IShellFuture; + cellExecution: NotebookCellExecution; + onErrorHandlingExecuteRequestIOPubMessage: (error: Error) => void; + } + ): CellExecutionMessageHandler { + this.notebook = cell.notebook; + // Always dispose any previous handlers & create new ones. + this.messageHandlers.get(cell)?.dispose(); + const handler = new CellExecutionMessageHandler( + cell, + this.appShell, + this.controller, + this.outputDisplayIdTracker, + this.context, + this.formatters, + options.kernel, + options.request, + options.cellExecution + ); + // This object must be kept in memory has it monitors the kernel messages. + this.messageHandlers.set(cell, handler); + return handler; + } +} diff --git a/src/notebooks/execution/kernelExecution.ts b/src/notebooks/execution/kernelExecution.ts index d7f67305143..a2767ad2b04 100644 --- a/src/notebooks/execution/kernelExecution.ts +++ b/src/notebooks/execution/kernelExecution.ts @@ -27,7 +27,7 @@ import { import { traceCellMessage } from '../helpers'; import { getDisplayPath } from '../../platform/common/platform/fs-paths'; import { getAssociatedNotebookDocument } from '../controllers/kernelSelector'; -import { CellExecutionMessageHandlerFactory } from './cellExecutionMessageHandlerFactory'; +import { CellExecutionMessageHandlerService } from './cellExecutionMessageHandlerService'; /** * Separate class that deals just with kernel execution. @@ -50,14 +50,15 @@ export class KernelExecution implements IDisposable { context: IExtensionContext, formatters: ITracebackFormatter[] ) { - const factory = new CellExecutionMessageHandlerFactory( + const requestListener = new CellExecutionMessageHandlerService( appShell, controller, outputTracker, context, formatters ); - this.executionFactory = new CellExecutionFactory(controller, factory); + this.disposables.push(requestListener); + this.executionFactory = new CellExecutionFactory(controller, requestListener); } public get onPreExecute() { diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index 07b1d915a7a..353c58979c4 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -817,7 +817,10 @@ export async function waitForTextOutput( cell.index + 1 } in output index ${index}, it is ${cell.outputs .map( - (output, index) => `Output for Index "${index}" is "${output.items.map(getOutputText).join('\n')}"` + (output, index) => + `Output for Index "${index}" with total outputs ${output.items.length} is "${output.items + .map(getOutputText) + .join('\n')}"` ) .join('\n')}` ); diff --git a/src/test/datascience/widgets/notebooks/button_widget_comm_msg.ipynb b/src/test/datascience/widgets/notebooks/button_widget_comm_msg.ipynb new file mode 100644 index 00000000000..cf18cfd63f5 --- /dev/null +++ b/src/test/datascience/widgets/notebooks/button_widget_comm_msg.ipynb @@ -0,0 +1,39 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import ipywidgets as widgets\n", + "from IPython.display import display\n", + "button = widgets.Button(description=\"Click Me!\")\n", + "\n", + "display(button)\n", + "\n", + "def on_button_clicked(b):\n", + " print(\"Button clicked.\")\n", + "\n", + "button.on_click(on_button_clicked)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "display(button)" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "orig_nbformat": 4 + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/src/test/datascience/widgets/notebooks/button_widget_comm_msg_matplotlib.ipynb b/src/test/datascience/widgets/notebooks/button_widget_comm_msg_matplotlib.ipynb new file mode 100644 index 00000000000..49d4c350601 --- /dev/null +++ b/src/test/datascience/widgets/notebooks/button_widget_comm_msg_matplotlib.ipynb @@ -0,0 +1,47 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "%matplotlib widget\n", + "from IPython.display import display\n", + "from ipywidgets import widgets\n", + "import matplotlib.pyplot as plt\n", + "import numpy as np\n", + "\n", + "\n", + "button = widgets.Button(description=\"Click Me!\")\n", + "\n", + "def on_button_clicked(b):\n", + "\tX = np.linspace(0, 2*np.pi)\n", + "\tY = np.sin(X)\n", + "\n", + "\t_, ax = plt.subplots()\n", + "\tax.plot(X, Y)\n", + "\n", + "display(button)\n", + "button.on_click(on_button_clicked)\n" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "display(button)" + ] + } + ], + "metadata": { + "language_info": { + "name": "python" + }, + "orig_nbformat": 4 + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/src/test/datascience/widgets/standard.vscode.common.ts b/src/test/datascience/widgets/standard.vscode.common.ts index 0739639a4ad..e13f7fdc8ee 100644 --- a/src/test/datascience/widgets/standard.vscode.common.ts +++ b/src/test/datascience/widgets/standard.vscode.common.ts @@ -24,6 +24,7 @@ import { runCell, waitForExecutionCompletedSuccessfully, waitForKernelToGetAutoSelected, + waitForTextOutput, workAroundVSCodeNotebookStartPages } from '../notebook/helper'; import { initializeWidgetComms, Utils } from './commUtils'; @@ -214,6 +215,30 @@ export function sharedIPyWidgetsTests( await assertOutputContainsHtml(cell1, comms, ['Button clicked']); await assertOutputContainsHtml(cell2, comms, ['Button clicked']); }); + test('Button Widget with custom comm message', async () => { + const comms = await initializeNotebook({ templateFile: 'button_widget_comm_msg.ipynb' }); + const [cell0, cell1] = vscodeNotebook.activeNotebookEditor!.notebook.getCells(); + + await executeCellAndWaitForOutput(cell0, comms); + await executeCellAndWaitForOutput(cell1, comms); + await assertOutputContainsHtml(cell0, comms, ['Click Me!', ' { + const comms = await initializeNotebook({ templateFile: 'button_widget_comm_msg_matplotlib.ipynb' }); + const [cell0, cell1] = vscodeNotebook.activeNotebookEditor!.notebook.getCells(); + + await executeCellAndWaitForOutput(cell0, comms); + await executeCellAndWaitForOutput(cell1, comms); + await assertOutputContainsHtml(cell0, comms, ['Click Me!', 'Figure 1<', ' { const comms = await initializeNotebook({ templateFile: 'ipySheet_widgets.ipynb' }); const [, cell1] = vscodeNotebook.activeNotebookEditor!.notebook.getCells();