Skip to content

Commit ade51a8

Browse files
committed
feat: Better error messages when installing the Deepnote kernel.
1 parent cddc017 commit ade51a8

File tree

4 files changed

+437
-11
lines changed

4 files changed

+437
-11
lines changed

src/kernels/deepnote/deepnoteServerStarter.node.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import * as fs from 'fs-extra';
1717
import * as os from 'os';
1818
import * as path from '../../platform/vscode-path/path';
1919
import { generateUuid } from '../../platform/common/uuid';
20+
import { DeepnoteServerStartupError, DeepnoteServerTimeoutError } from '../../platform/errors/deepnoteKernelErrors';
2021

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

4548
constructor(
4649
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
@@ -175,15 +178,27 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
175178
const disposables: IDisposable[] = [];
176179
this.disposablesByFile.set(fileKey, disposables);
177180

181+
// Initialize output tracking for error reporting
182+
this.serverOutputByFile.set(fileKey, { stdout: '', stderr: '' });
183+
178184
// Monitor server output
179185
serverProcess.out.onDidChange(
180186
(output) => {
187+
const outputTracking = this.serverOutputByFile.get(fileKey);
181188
if (output.source === 'stdout') {
182189
logger.trace(`Deepnote server (${fileKey}): ${output.out}`);
183190
this.outputChannel.appendLine(output.out);
191+
if (outputTracking) {
192+
// Keep last 5000 characters of output for error reporting
193+
outputTracking.stdout = (outputTracking.stdout + output.out).slice(-5000);
194+
}
184195
} else if (output.source === 'stderr') {
185196
logger.warn(`Deepnote server stderr (${fileKey}): ${output.out}`);
186197
this.outputChannel.appendLine(output.out);
198+
if (outputTracking) {
199+
// Keep last 5000 characters of error output for error reporting
200+
outputTracking.stderr = (outputTracking.stderr + output.out).slice(-5000);
201+
}
187202
}
188203
},
189204
this,
@@ -206,13 +221,30 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
206221
try {
207222
const serverReady = await this.waitForServer(serverInfo, 120000, token);
208223
if (!serverReady) {
224+
const output = this.serverOutputByFile.get(fileKey);
209225
await this.stopServerImpl(deepnoteFileUri);
210-
throw new Error('Deepnote server failed to start within timeout period');
226+
227+
throw new DeepnoteServerTimeoutError(serverInfo.url, 120000, output?.stderr || undefined);
211228
}
212229
} catch (error) {
213230
// Clean up leaked server before rethrowing
214231
await this.stopServerImpl(deepnoteFileUri);
215-
throw error;
232+
233+
// If this is already a DeepnoteKernelError, rethrow it
234+
if (error instanceof DeepnoteServerTimeoutError || error instanceof DeepnoteServerStartupError) {
235+
throw error;
236+
}
237+
238+
// Otherwise wrap in a generic server startup error
239+
const output = this.serverOutputByFile.get(fileKey);
240+
throw new DeepnoteServerStartupError(
241+
interpreter.uri.fsPath,
242+
port,
243+
'unknown',
244+
output?.stdout || '',
245+
output?.stderr || '',
246+
error instanceof Error ? error : undefined
247+
);
216248
}
217249

218250
logger.info(`Deepnote server started successfully at ${url} for ${fileKey}`);
@@ -261,6 +293,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
261293
serverProcess.proc?.kill();
262294
this.serverProcesses.delete(fileKey);
263295
this.serverInfos.delete(fileKey);
296+
this.serverOutputByFile.delete(fileKey);
264297
this.outputChannel.appendLine(`Deepnote server stopped for ${fileKey}`);
265298

266299
// Clean up lock file after stopping the server
@@ -381,6 +414,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
381414
this.serverInfos.clear();
382415
this.disposablesByFile.clear();
383416
this.pendingOperations.clear();
417+
this.serverOutputByFile.clear();
384418

385419
logger.info('DeepnoteServerStarter disposed successfully');
386420
}

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts

Lines changed: 30 additions & 3 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.
@@ -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 and wrap in a generic toolkit install error
256275
logger.error(`Failed to set up deepnote-toolkit: ${ex}`);
257276
this.outputChannel.appendLine(`Error setting up deepnote-toolkit: ${ex}`);
258-
return undefined;
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/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
import { inject, injectable, optional } from 'inversify';
4+
import { inject, injectable, optional, named } from 'inversify';
55
import {
66
CancellationToken,
77
NotebookDocument,
@@ -13,7 +13,8 @@ import {
1313
NotebookController,
1414
CancellationTokenSource,
1515
Disposable,
16-
l10n
16+
l10n,
17+
env
1718
} from 'vscode';
1819
import { IExtensionSyncActivationService } from '../../platform/activation/types';
1920
import { IDisposableRegistry } from '../../platform/common/types';
@@ -42,6 +43,9 @@ import { IDeepnoteNotebookManager } from '../types';
4243
import { IDeepnoteRequirementsHelper } from './deepnoteRequirementsHelper.node';
4344
import { DeepnoteProject } from './deepnoteTypes';
4445
import { IKernelProvider, IKernel } from '../../kernels/types';
46+
import { DeepnoteKernelError } from '../../platform/errors/deepnoteKernelErrors';
47+
import { STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants';
48+
import { IOutputChannel } from '../../platform/common/types';
4549

4650
/**
4751
* Automatically selects and starts Deepnote kernel for .deepnote notebooks
@@ -78,7 +82,8 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
7882
@inject(IDeepnoteInitNotebookRunner) private readonly initNotebookRunner: IDeepnoteInitNotebookRunner,
7983
@inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager,
8084
@inject(IKernelProvider) private readonly kernelProvider: IKernelProvider,
81-
@inject(IDeepnoteRequirementsHelper) private readonly requirementsHelper: IDeepnoteRequirementsHelper
85+
@inject(IDeepnoteRequirementsHelper) private readonly requirementsHelper: IDeepnoteRequirementsHelper,
86+
@inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel
8287
) {}
8388

8489
public activate() {
@@ -129,9 +134,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
129134
// Don't await - let it happen in background so notebook opens quickly
130135
void this.ensureKernelSelected(notebook).catch((error) => {
131136
logger.error(`Failed to auto-select Deepnote kernel for ${getDisplayPath(notebook.uri)}`, error);
132-
void window.showErrorMessage(
133-
l10n.t('Failed to load Deepnote kernel. Please check the output for details.')
134-
);
137+
void this.handleKernelSelectionError(error);
135138
});
136139
}
137140

@@ -553,4 +556,50 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
553556
this.loadingControllers.set(notebookKey, loadingController);
554557
logger.info(`Created loading controller for ${notebookKey}`);
555558
}
559+
560+
/**
561+
* Handle kernel selection errors with user-friendly messages and actions
562+
*/
563+
private async handleKernelSelectionError(error: unknown): Promise<void> {
564+
// Handle DeepnoteKernelError types with specific guidance
565+
if (error instanceof DeepnoteKernelError) {
566+
// Log the technical details
567+
logger.error(error.getErrorReport());
568+
569+
// Show user-friendly error with actions
570+
const actions: string[] = ['Show Output', 'Copy Error Details'];
571+
572+
const selectedAction = await window.showErrorMessage(
573+
`${error.userMessage}\n\nTroubleshooting:\n${error.troubleshootingSteps
574+
.slice(0, 3)
575+
.map((step, i) => `${i + 1}. ${step}`)
576+
.join('\n')}`,
577+
{ modal: false },
578+
...actions
579+
);
580+
581+
if (selectedAction === 'Show Output') {
582+
this.outputChannel.show();
583+
} else if (selectedAction === 'Copy Error Details') {
584+
await env.clipboard.writeText(error.getErrorReport());
585+
void window.showInformationMessage('Error details copied to clipboard');
586+
}
587+
588+
return;
589+
}
590+
591+
// Handle generic errors
592+
const errorMessage = error instanceof Error ? error.message : String(error);
593+
logger.error(`Deepnote kernel error: ${errorMessage}`);
594+
595+
const selectedAction = await window.showErrorMessage(
596+
l10n.t('Failed to load Deepnote kernel: {0}', errorMessage),
597+
{ modal: false },
598+
'Show Output'
599+
);
600+
601+
if (selectedAction === 'Show Output') {
602+
this.outputChannel.show();
603+
}
604+
}
556605
}

0 commit comments

Comments
 (0)