Skip to content

Commit e6c3ad6

Browse files
authored
Merge branch 'main' into jk/feat/sql-block-variable-input
2 parents 984f071 + a626bb3 commit e6c3ad6

File tree

13 files changed

+637
-58
lines changed

13 files changed

+637
-58
lines changed

.eslintrc.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ module.exports = {
7474
'import/no-unresolved': [
7575
'error',
7676
{
77-
ignore: ['monaco-editor', 'vscode']
77+
ignore: ['monaco-editor', 'vscode', 'react-error-boundary']
7878
}
7979
],
8080
'import/prefer-default-export': 'off',

.nvmrc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
22.15.1
1+
22.21.0

package-lock.json

Lines changed: 27 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2197,6 +2197,7 @@
21972197
"react": "^16.5.2",
21982198
"react-data-grid": "^6.0.2-0",
21992199
"react-dom": "^16.5.2",
2200+
"react-error-boundary": "^6.0.0",
22002201
"react-redux": "^7.1.1",
22012202
"react-svg-pan-zoom": "3.9.0",
22022203
"react-svgmt": "1.1.11",

src/kernels/deepnote/deepnoteServerStarter.node.ts

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import * as fs from 'fs-extra';
1818
import * as os from 'os';
1919
import * as path from '../../platform/vscode-path/path';
2020
import { generateUuid } from '../../platform/common/uuid';
21+
import { DeepnoteServerStartupError, DeepnoteServerTimeoutError } from '../../platform/errors/deepnoteKernelErrors';
2122

2223
/**
2324
* Lock file data structure for tracking server ownership
@@ -42,6 +43,8 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
4243
private readonly sessionId: string = generateUuid();
4344
// Directory for lock files
4445
private readonly lockFileDir: string = path.join(os.tmpdir(), 'vscode-deepnote-locks');
46+
// Track server output for error reporting
47+
private readonly serverOutputByFile: Map<string, { stdout: string; stderr: string }> = new Map();
4548

4649
constructor(
4750
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
@@ -118,12 +121,9 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
118121

119122
Cancellation.throwIfCanceled(token);
120123

121-
// Ensure toolkit is installed
124+
// Ensure toolkit is installed (will throw typed errors on failure)
122125
logger.info(`Ensuring deepnote-toolkit is installed for ${fileKey}...`);
123-
const installed = await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token);
124-
if (!installed) {
125-
throw new Error('Failed to install deepnote-toolkit. Please check the output for details.');
126-
}
126+
await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token);
127127

128128
Cancellation.throwIfCanceled(token);
129129

@@ -197,15 +197,27 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
197197
const disposables: IDisposable[] = [];
198198
this.disposablesByFile.set(fileKey, disposables);
199199

200+
// Initialize output tracking for error reporting
201+
this.serverOutputByFile.set(fileKey, { stdout: '', stderr: '' });
202+
200203
// Monitor server output
201204
serverProcess.out.onDidChange(
202205
(output) => {
206+
const outputTracking = this.serverOutputByFile.get(fileKey);
203207
if (output.source === 'stdout') {
204208
logger.trace(`Deepnote server (${fileKey}): ${output.out}`);
205209
this.outputChannel.appendLine(output.out);
210+
if (outputTracking) {
211+
// Keep last 5000 characters of output for error reporting
212+
outputTracking.stdout = (outputTracking.stdout + output.out).slice(-5000);
213+
}
206214
} else if (output.source === 'stderr') {
207215
logger.warn(`Deepnote server stderr (${fileKey}): ${output.out}`);
208216
this.outputChannel.appendLine(output.out);
217+
if (outputTracking) {
218+
// Keep last 5000 characters of error output for error reporting
219+
outputTracking.stderr = (outputTracking.stderr + output.out).slice(-5000);
220+
}
209221
}
210222
},
211223
this,
@@ -228,13 +240,35 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
228240
try {
229241
const serverReady = await this.waitForServer(serverInfo, 120000, token);
230242
if (!serverReady) {
243+
const output = this.serverOutputByFile.get(fileKey);
231244
await this.stopServerImpl(deepnoteFileUri);
232-
throw new Error('Deepnote server failed to start within timeout period');
245+
246+
throw new DeepnoteServerTimeoutError(serverInfo.url, 120000, output?.stderr || undefined);
233247
}
234248
} catch (error) {
235-
// Clean up leaked server before rethrowing
249+
// If this is already a DeepnoteKernelError, clean up and rethrow it
250+
if (error instanceof DeepnoteServerTimeoutError || error instanceof DeepnoteServerStartupError) {
251+
await this.stopServerImpl(deepnoteFileUri);
252+
throw error;
253+
}
254+
255+
// Capture output BEFORE cleaning up (stopServerImpl deletes it)
256+
const output = this.serverOutputByFile.get(fileKey);
257+
const capturedStdout = output?.stdout || '';
258+
const capturedStderr = output?.stderr || '';
259+
260+
// Clean up leaked server after capturing output
236261
await this.stopServerImpl(deepnoteFileUri);
237-
throw error;
262+
263+
// Wrap in a generic server startup error with captured output
264+
throw new DeepnoteServerStartupError(
265+
interpreter.uri.fsPath,
266+
port,
267+
'unknown',
268+
capturedStdout,
269+
capturedStderr,
270+
error instanceof Error ? error : undefined
271+
);
238272
}
239273

240274
logger.info(`Deepnote server started successfully at ${url} for ${fileKey}`);
@@ -283,6 +317,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
283317
serverProcess.proc?.kill();
284318
this.serverProcesses.delete(fileKey);
285319
this.serverInfos.delete(fileKey);
320+
this.serverOutputByFile.delete(fileKey);
286321
this.outputChannel.appendLine(`Deepnote server stopped for ${fileKey}`);
287322

288323
// Clean up lock file after stopping the server
@@ -403,6 +438,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
403438
this.serverInfos.clear();
404439
this.disposablesByFile.clear();
405440
this.pendingOperations.clear();
441+
this.serverOutputByFile.clear();
406442

407443
logger.info('DeepnoteServerStarter disposed successfully');
408444
}

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { IOutputChannel, IExtensionContext } from '../../platform/common/types';
1111
import { STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants';
1212
import { IFileSystem } from '../../platform/common/platform/types';
1313
import { Cancellation } from '../../platform/common/cancellation';
14+
import { DeepnoteVenvCreationError, DeepnoteToolkitInstallError } from '../../platform/errors/deepnoteKernelErrors';
1415

1516
/**
1617
* Handles installation of the deepnote-toolkit Python package.
@@ -19,7 +20,7 @@ import { Cancellation } from '../../platform/common/cancellation';
1920
export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
2021
private readonly venvPythonPaths: Map<string, Uri> = new Map();
2122
// Track in-flight installations per venv path to prevent concurrent installs
22-
private readonly pendingInstallations: Map<string, Promise<PythonEnvironment | undefined>> = new Map();
23+
private readonly pendingInstallations: Map<string, Promise<PythonEnvironment>> = new Map();
2324

2425
constructor(
2526
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
@@ -62,7 +63,7 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
6263
baseInterpreter: PythonEnvironment,
6364
deepnoteFileUri: Uri,
6465
token?: CancellationToken
65-
): Promise<PythonEnvironment | undefined> {
66+
): Promise<PythonEnvironment> {
6667
const venvPath = this.getVenvPath(deepnoteFileUri);
6768
const venvKey = venvPath.fsPath;
6869

@@ -119,7 +120,7 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
119120
deepnoteFileUri: Uri,
120121
venvPath: Uri,
121122
token?: CancellationToken
122-
): Promise<PythonEnvironment | undefined> {
123+
): Promise<PythonEnvironment> {
123124
try {
124125
Cancellation.throwIfCanceled(token);
125126

@@ -158,7 +159,12 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
158159
logger.error(`venv stderr: ${venvResult.stderr}`);
159160
}
160161
this.outputChannel.appendLine('Error: Failed to create virtual environment');
161-
return undefined;
162+
163+
throw new DeepnoteVenvCreationError(
164+
baseInterpreter.uri.fsPath,
165+
venvPath.fsPath,
166+
venvResult.stderr || 'Virtual environment was created but Python interpreter not found'
167+
);
162168
}
163169

164170
// Use undefined as resource to get full system environment (including git in PATH)
@@ -250,12 +256,33 @@ export class DeepnoteToolkitInstaller implements IDeepnoteToolkitInstaller {
250256
} else {
251257
logger.error('deepnote-toolkit installation failed');
252258
this.outputChannel.appendLine('✗ deepnote-toolkit installation failed');
253-
return undefined;
259+
260+
throw new DeepnoteToolkitInstallError(
261+
venvInterpreter.uri.fsPath,
262+
venvPath.fsPath,
263+
DEEPNOTE_TOOLKIT_WHEEL_URL,
264+
installResult.stdout || '',
265+
installResult.stderr || 'Package installation completed but verification failed'
266+
);
254267
}
255268
} catch (ex) {
269+
// If this is already a DeepnoteKernelError, rethrow it without wrapping
270+
if (ex instanceof DeepnoteVenvCreationError || ex instanceof DeepnoteToolkitInstallError) {
271+
throw ex;
272+
}
273+
274+
// Otherwise, log full details and wrap in a generic toolkit install error
256275
logger.error(`Failed to set up deepnote-toolkit: ${ex}`);
257-
this.outputChannel.appendLine(`Error setting up deepnote-toolkit: ${ex}`);
258-
return undefined;
276+
this.outputChannel.appendLine('Failed to set up deepnote-toolkit; see logs for details');
277+
278+
throw new DeepnoteToolkitInstallError(
279+
baseInterpreter.uri.fsPath,
280+
venvPath.fsPath,
281+
DEEPNOTE_TOOLKIT_WHEEL_URL,
282+
'',
283+
ex instanceof Error ? ex.message : String(ex),
284+
ex instanceof Error ? ex : undefined
285+
);
259286
}
260287
}
261288

src/kernels/deepnote/types.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,15 @@ export interface IDeepnoteToolkitInstaller {
7272
* @param baseInterpreter The base Python interpreter to use for creating the venv
7373
* @param deepnoteFileUri The URI of the .deepnote file (used to create a unique venv per file)
7474
* @param token Cancellation token to cancel the operation
75-
* @returns The Python interpreter from the venv if installed successfully, undefined otherwise
75+
* @returns The Python interpreter from the venv
76+
* @throws {DeepnoteVenvCreationError} If venv creation fails
77+
* @throws {DeepnoteToolkitInstallError} If toolkit installation fails
7678
*/
7779
ensureInstalled(
7880
baseInterpreter: PythonEnvironment,
7981
deepnoteFileUri: vscode.Uri,
8082
token?: vscode.CancellationToken
81-
): Promise<PythonEnvironment | undefined>;
83+
): Promise<PythonEnvironment>;
8284

8385
/**
8486
* Gets the venv Python interpreter if toolkit is installed, undefined otherwise.

0 commit comments

Comments
 (0)