From 573cce0f31ac7ee10cd60c6146538ea047ca9515 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Wed, 24 Feb 2021 17:14:31 -0800 Subject: [PATCH 1/2] ensure a non-existant dotnet always returns something actionable --- .../.vscode/tasks.json | 38 +++++++++++-------- .../src/tests/unit/misc.test.ts | 11 +++++- .../src/utilities.ts | 37 ++++++++++++++++++ .../src/vscode/commands.ts | 29 +++----------- .../src/vscode/extension.ts | 8 ++-- 5 files changed, 78 insertions(+), 45 deletions(-) diff --git a/src/dotnet-interactive-vscode/.vscode/tasks.json b/src/dotnet-interactive-vscode/.vscode/tasks.json index 241aa6d996..93d46ff5e3 100644 --- a/src/dotnet-interactive-vscode/.vscode/tasks.json +++ b/src/dotnet-interactive-vscode/.vscode/tasks.json @@ -1,20 +1,26 @@ // See https://go.microsoft.com/fwlink/?LinkId=733558 // for the documentation about the tasks.json format { - "version": "2.0.0", - "tasks": [ - { - "type": "npm", - "script": "watch", - "problemMatcher": "$tsc-watch", - "isBackground": true, - "presentation": { - "reveal": "never" - }, - "group": { - "kind": "build", - "isDefault": true - } - } - ] + "version": "2.0.0", + "tasks": [ + { + "type": "npm", + "script": "watch", + "problemMatcher": "$tsc-watch", + "isBackground": true, + "presentation": { + "reveal": "never" + }, + "group": { + "kind": "build", + "isDefault": true + } + }, + { + "label": "run tests", + "type": "npm", + "script": "test", + "group": "test" + } + ] } \ No newline at end of file diff --git a/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts b/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts index 3fd7c0651a..f43384537a 100644 --- a/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts +++ b/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts @@ -5,7 +5,7 @@ import { expect } from 'chai'; import { NotebookCellDisplayOutput, NotebookCellErrorOutput, NotebookCellTextOutput } from 'dotnet-interactive-vscode-interfaces/out/contracts'; import { isDisplayOutput, isErrorOutput, isTextOutput, reshapeOutputValueForVsCode } from 'dotnet-interactive-vscode-interfaces/out/utilities'; import { requiredKernelspecData } from '../../ipynbUtilities'; -import { debounce, isDotNetKernelPreferred, parse, processArguments, stringify } from '../../utilities'; +import { debounce, executeSafe, isDotNetKernelPreferred, parse, processArguments, stringify } from '../../utilities'; import * as notebook from 'dotnet-interactive-vscode-interfaces/out/notebook'; @@ -296,4 +296,13 @@ describe('Miscellaneous tests', () => { expect(reshaped).to.equal('some error message'); }); }); + + it('executing a non-existant process still returns', async () => { + const result = await executeSafe('this-is-a-command-that-will-fail', []); + expect(result).to.deep.equal({ + code: -1, + output: '', + error: 'Error: spawn this-is-a-command-that-will-fail ENOENT', + }); + }); }); diff --git a/src/dotnet-interactive-vscode/src/utilities.ts b/src/dotnet-interactive-vscode/src/utilities.ts index 783580b302..99dcffc57d 100644 --- a/src/dotnet-interactive-vscode/src/utilities.ts +++ b/src/dotnet-interactive-vscode/src/utilities.ts @@ -1,10 +1,47 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +import * as cp from 'child_process'; import * as path from 'path'; import { InstallInteractiveArgs, ProcessStart } from "./interfaces"; import { Uri } from 'dotnet-interactive-vscode-interfaces/out/notebook'; +export function executeSafe(command: string, args: Array, workingDirectory?: string | undefined): Promise<{ code: number, output: string, error: string }> { + return new Promise<{ code: number, output: string, error: string }>(resolve => { + try { + let output = ''; + let error = ''; + + function exitOrClose(code: number, _signal: string) { + resolve({ + code, + output: output.trim(), + error: error.trim(), + }); + } + + let childProcess = cp.spawn(command, args, { cwd: workingDirectory }); + childProcess.stdout.on('data', data => output += data); + childProcess.stderr.on('data', data => error += data); + childProcess.on('error', err => { + resolve({ + code: -1, + output: '', + error: '' + err, + }); + }); + childProcess.on('close', exitOrClose); + childProcess.on('exit', exitOrClose); + } catch (err) { + resolve({ + code: -1, + output: '', + error: '' + err, + }); + } + }); +} + export function processArguments(template: { args: Array, workingDirectory: string }, notebookPath: string, fallbackWorkingDirectory: string, dotnetPath: string, globalStoragePath: string): ProcessStart { let workingDirectory = path.parse(notebookPath).dir; if (workingDirectory === '') { diff --git a/src/dotnet-interactive-vscode/src/vscode/commands.ts b/src/dotnet-interactive-vscode/src/vscode/commands.ts index 167404720f..2831e44140 100644 --- a/src/dotnet-interactive-vscode/src/vscode/commands.ts +++ b/src/dotnet-interactive-vscode/src/vscode/commands.ts @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. import * as vscode from 'vscode'; -import * as cp from 'child_process'; import * as path from 'path'; import { acquireDotnetInteractive } from '../acquisition'; import { InstallInteractiveArgs, InteractiveLaunchOptions } from '../interfaces'; @@ -11,7 +10,7 @@ import { getEol, isUnsavedNotebook } from './vscodeUtilities'; import { toNotebookDocument } from './notebookContentProvider'; import { KernelId, updateCellMetadata } from './notebookKernel'; import { setGlobalDotnetPath } from './extension'; -import { computeToolInstallArguments } from '../utilities'; +import { computeToolInstallArguments, executeSafe } from '../utilities'; export function registerAcquisitionCommands(context: vscode.ExtensionContext) { const config = vscode.workspace.getConfiguration('dotnet-interactive'); @@ -41,7 +40,7 @@ export function registerAcquisitionCommands(context: vscode.ExtensionContext) { 'uninstall', 'Microsoft.dotnet-interactive' ]; - await execute(args.dotnetPath, uninstallArgs, globalStoragePath); + await executeSafe(args.dotnetPath, uninstallArgs, globalStoragePath); let toolArgs = [ 'tool', @@ -56,7 +55,7 @@ export function registerAcquisitionCommands(context: vscode.ExtensionContext) { } return new Promise(async (resolve, reject) => { - const result = await execute(args.dotnetPath, toolArgs, globalStoragePath); + const result = await executeSafe(args.dotnetPath, toolArgs, globalStoragePath); if (result.code === 0) { resolve(); } else { @@ -204,7 +203,7 @@ async function switchToInteractiveKernel() { // callbacks used to install interactive tool async function getInteractiveVersion(dotnetPath: string, globalStoragePath: string): Promise { - const result = await execute(dotnetPath, ['tool', 'run', 'dotnet-interactive', '--', '--version'], globalStoragePath); + const result = await executeSafe(dotnetPath, ['tool', 'run', 'dotnet-interactive', '--', '--version'], globalStoragePath); if (result.code === 0) { return result.output; } @@ -213,26 +212,8 @@ async function getInteractiveVersion(dotnetPath: string, globalStoragePath: stri } async function createToolManifest(dotnetPath: string, globalStoragePath: string): Promise { - const result = await execute(dotnetPath, ['new', 'tool-manifest'], globalStoragePath); + const result = await executeSafe(dotnetPath, ['new', 'tool-manifest'], globalStoragePath); if (result.code !== 0) { throw new Error(`Unable to create local tool manifest. Command failed with code ${result.code}.\n\nSTDOUT:\n${result.output}\n\nSTDERR:\n${result.error}`); } } - -export function execute(command: string, args: Array, workingDirectory?: string | undefined): Promise<{ code: number, output: string, error: string }> { - return new Promise<{ code: number, output: string, error: string }>((resolve, reject) => { - let output = ''; - let error = ''; - - let childProcess = cp.spawn(command, args, { cwd: workingDirectory }); - childProcess.stdout.on('data', data => output += data); - childProcess.stderr.on('data', data => error += data); - childProcess.on('exit', (code: number, _signal: string) => { - resolve({ - code: code, - output: output.trim(), - error: error.trim() - }); - }); - }); -} diff --git a/src/dotnet-interactive-vscode/src/vscode/extension.ts b/src/dotnet-interactive-vscode/src/vscode/extension.ts index ade5043969..f7d9ae4c96 100644 --- a/src/dotnet-interactive-vscode/src/vscode/extension.ts +++ b/src/dotnet-interactive-vscode/src/vscode/extension.ts @@ -8,7 +8,7 @@ import { ClientMapper } from '../clientMapper'; import { DotNetInteractiveNotebookContentProvider } from './notebookContentProvider'; import { StdioKernelTransport } from '../stdioKernelTransport'; import { registerLanguageProviders } from './languageProvider'; -import { execute, registerAcquisitionCommands, registerKernelCommands, registerFileCommands } from './commands'; +import { registerAcquisitionCommands, registerKernelCommands, registerFileCommands } from './commands'; import { getSimpleLanguage, isDotnetInteractiveLanguage, notebookCellLanguages } from '../interactiveNotebook'; import { IDotnetAcquireResult } from 'dotnet-interactive-vscode-interfaces/out/dotnet'; @@ -16,7 +16,7 @@ import { InteractiveLaunchOptions, InstallInteractiveArgs } from '../interfaces' import compareVersions = require("compare-versions"); import { DotNetCellMetadata, withDotNetMetadata } from '../ipynbUtilities'; -import { processArguments } from '../utilities'; +import { executeSafe, processArguments } from '../utilities'; import { OutputChannelAdapter } from './OutputChannelAdapter'; import { KernelId, updateCellLanguages, updateDocumentKernelspecMetadata } from './notebookKernel'; import { DotNetInteractiveNotebookKernelProvider } from './notebookKernelProvider'; @@ -186,7 +186,7 @@ async function computeDotnetPath(outputChannel: OutputChannelAdapter): Promise('minimumDotNetSdkVersion'); let dotnetPath: string; if (await isDotnetUpToDate(minDotNetSdkVersion!)) { - dotnetPath = 'dotnet'; + dotnetPath = cachedDotnetPath; } else { const commandResult = await vscode.commands.executeCommand('dotnet.acquire', { version: minDotNetSdkVersion, requestingExtensionId: 'ms-dotnettools.dotnet-interactive-vscode' }); dotnetPath = commandResult!.dotnetPath; @@ -206,6 +206,6 @@ async function getInteractiveLaunchOptions(dotnetPath: string): Promise { - const result = await execute('dotnet', ['--version']); + const result = await executeSafe(cachedDotnetPath, ['--version']); return result.code === 0 && compareVersions.compare(result.output, minVersion, '>='); } From 42b45dbb3c742106cf9cfac651a297c3e05a10da Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Thu, 25 Feb 2021 09:13:05 -0800 Subject: [PATCH 2/2] Update src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts Co-authored-by: Jon Sequeira --- src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts b/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts index f43384537a..28c9767360 100644 --- a/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts +++ b/src/dotnet-interactive-vscode/src/tests/unit/misc.test.ts @@ -297,7 +297,7 @@ describe('Miscellaneous tests', () => { }); }); - it('executing a non-existant process still returns', async () => { + it('executing a non-existent process still returns', async () => { const result = await executeSafe('this-is-a-command-that-will-fail', []); expect(result).to.deep.equal({ code: -1,