Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to ipykernel detection #15561

Merged
merged 1 commit into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ Morbi molestie lacinia sapien nec porttitor. Nam at vestibulum nisi.
assert.equal(cells.length, 1, 'markdown cell multline failed');
assert.equal(cells[0].languageId, 'markdown', 'markdown cell not generated');
assert.equal(splitMarkdown(cells[0].value).length, 39, 'Lines for cell not emitted');
console.error(`"${cells[0].value}"`);
assert.equal(splitMarkdown(cells[0].value)[34], ' - Item 1-a-3-c', 'Lines for markdown not emitted');

// eslint-disable-next-line no-multi-str
Expand Down
20 changes: 10 additions & 10 deletions src/kernels/raw/launcher/kernelProcess.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
public get pid() {
return this._pid;
}
public get exited(): Event<{ exitCode?: number; reason?: string }> {
public get exited(): Event<{ exitCode?: number; reason?: string; stderr: string }> {
return this.exitEvent.event;
}
public get kernelConnectionMetadata(): Readonly<
Expand All @@ -110,11 +110,12 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
return true;
}
private _process?: ChildProcess;
private exitEvent = new EventEmitter<{ exitCode?: number; reason?: string }>();
private exitEvent = new EventEmitter<{ exitCode?: number; reason?: string; stderr: string }>();
private launchedOnce?: boolean;
private connectionFile?: Uri;
private _launchKernelSpec?: IJupyterKernelSpec;
private interrupter?: Interrupter;
private exitEventFired = false;
private readonly _kernelConnectionMetadata: Readonly<
LocalKernelSpecConnectionMetadata | PythonKernelConnectionMetadata
>;
Expand Down Expand Up @@ -182,7 +183,6 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
}
traceVerbose(`Kernel process ${proc?.pid}.`);
let stderr = '';
let exitEventFired = false;
let providedExitCode: number | null;
const deferred = createDeferred();
deferred.promise.catch(noop);
Expand All @@ -196,12 +196,13 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
return;
}
traceVerbose(`KernelProcess Exited ${pid}, Exit Code - ${exitCode}`, stderr);
if (!exitEventFired) {
if (!this.exitEventFired) {
this.exitEvent.fire({
exitCode: exitCode || undefined,
reason: getTelemetrySafeErrorMessageFromPythonTraceback(stderr) || stderr
reason: getTelemetrySafeErrorMessageFromPythonTraceback(stderr) || stderr,
stderr
});
exitEventFired = true;
this.exitEventFired = true;
}
if (!cancelToken.isCancellationRequested) {
traceInfoIfCI(`KernelProcessExitedError raised`, stderr);
Expand Down Expand Up @@ -289,8 +290,6 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
if (cancelToken.isCancellationRequested || deferred.rejected) {
return;
}
console.error('ex');
console.error(ex);
// Do not throw an error, ignore this.
// In the case of VPNs the port does not seem to get used.
// Possible we're blocking it.
Expand Down Expand Up @@ -326,7 +325,6 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
// If we have the python error message in std outputs, display that.
const errorMessage = getErrorMessageFromPythonTraceback(stdErrToLog) || stdErrToLog.substring(0, 100);
traceInfoIfCI(`KernelDiedError raised`, errorMessage, stderr + '\n' + stderr + '\n');
console.error(`KernelDiedError raised`, e);
throw new KernelDiedError(
DataScience.kernelDied(errorMessage),
// Include what ever we have as the stderr.
Expand Down Expand Up @@ -358,7 +356,9 @@ export class KernelProcess extends ObservableDisposable implements IKernelProces
try {
this.interrupter?.dispose().catch(noop);
this._process?.kill(); // NOSONAR
this.exitEvent.fire({});
if (!this.exitEventFired) {
this.exitEvent.fire({ stderr: '' });
}
} catch (ex) {
traceError(`Error disposing kernel process ${pid}`, ex);
}
Expand Down
24 changes: 23 additions & 1 deletion src/kernels/raw/session/rawKernelConnection.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import { StopWatch } from '../../../platform/common/utils/stopWatch';
import { dispose } from '../../../platform/common/utils/lifecycle';
import { KernelSocketMap } from '../../kernelSocket';
import { getNotebookTelemetryTracker } from '../../telemetry/notebookTelemetry';
import { KernelProcessExitedError } from '../../errors/kernelProcessExitedError';
import { once } from '../../../platform/common/utils/functional';

let nonSerializingKernel: typeof import('@jupyterlab/services/lib/kernel/default');

Expand Down Expand Up @@ -130,6 +132,7 @@ export class RawKernelConnection implements Kernel.IKernelConnection {
const disposables: IDisposable[] = [];
const postStartToken = wrapCancellationTokens(token);
disposables.push(postStartToken);
let kernelExitedError: KernelProcessExitedError | undefined = undefined;
try {
const oldKernelProcess = this.kernelProcess;
this.kernelProcess = undefined;
Expand Down Expand Up @@ -162,7 +165,18 @@ export class RawKernelConnection implements Kernel.IKernelConnection {
(info) => this.infoDeferred.resolve(info),
(ex) => this.infoDeferred.reject(ex)
);

once(this.kernelProcess.exited)(
(e) => {
kernelExitedError = new KernelProcessExitedError(
e.exitCode || -1,
e.stderr,
this.kernelConnectionMetadata
);
postStartToken.cancel();
},
this,
disposables
);
const timeout = setTimeout(() => postStartToken.cancel(), this.launchTimeout);
disposables.push({ dispose: () => clearTimeout(timeout) });
await KernelProgressReporter.wrapAndReportProgress(
Expand All @@ -179,6 +193,9 @@ export class RawKernelConnection implements Kernel.IKernelConnection {
).finally(() => tracker?.stop());
}
);
if (kernelExitedError) {
throw kernelExitedError;
}
if (token.isCancellationRequested) {
throw new CancellationError();
}
Expand All @@ -193,6 +210,9 @@ export class RawKernelConnection implements Kernel.IKernelConnection {
?.shutdown()
.catch((ex) => traceWarning(`Failed to shutdown kernel, ${this.kernelConnectionMetadata.id}`, ex))
]);
if (kernelExitedError) {
throw kernelExitedError;
}
if (
isCancellationError(error) &&
postStartToken.token.isCancellationRequested &&
Expand Down Expand Up @@ -488,6 +508,8 @@ async function postStartKernel(
const gotIoPubMessage = createDeferred<boolean>();
const kernelInfoRequestHandled = createDeferred<boolean>();
const iopubHandler = () => gotIoPubMessage.resolve(true);
gotIoPubMessage.promise.catch(noop);
kernelInfoRequestHandled.promise.catch(noop);
kernel.iopubMessage.connect(iopubHandler);
const sendKernelInfoRequestOnControlChannel = () => {
const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ suite('Raw Session & Raw Kernel Connection', () => {
let exitedEvent: EventEmitter<{
exitCode?: number | undefined;
reason?: string | undefined;
stderr: string;
}>;
const launchTimeout = 1_000;
let disposables: IDisposable[] = [];
Expand Down Expand Up @@ -178,6 +179,7 @@ suite('Raw Session & Raw Kernel Connection', () => {
exitedEvent = new EventEmitter<{
exitCode?: number | undefined;
reason?: string | undefined;
stderr: string;
}>();
nonSerializingKernel.KernelConnection = OldKernelConnectionClass;
const workspaceConfig = mock<WorkspaceConfiguration>();
Expand Down Expand Up @@ -275,7 +277,7 @@ suite('Raw Session & Raw Kernel Connection', () => {
session.kernel!.statusChanged.connect((_, s) => statuses.push(s));
session.kernel!.disposed.connect(() => (disposed = true));

exitedEvent.fire({ exitCode: 1, reason: 'Killed' });
exitedEvent.fire({ exitCode: 1, reason: 'Killed', stderr: '' });

await waitForCondition(
() => !disposed && statuses.join(',') === 'dead',
Expand Down
2 changes: 1 addition & 1 deletion src/kernels/raw/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface IKernelProcess extends IAsyncDisposable {
/**
* This event is triggered if the process is exited
*/
readonly exited: Event<{ exitCode?: number; reason?: string }>;
readonly exited: Event<{ exitCode?: number; reason?: string; stderr: string }>;
/**
* Whether we can interrupt this kernel process.
* If not possible, send a shell message to the underlying kernel.
Expand Down
4 changes: 2 additions & 2 deletions src/platform/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,11 @@ export namespace DataScience {
'Do not translate the text "command:jupyter.viewOutput", that is a command Id that will be used by VS Code to open the output panel'
]
});
export const kernelDied = (kernelName: string) =>
export const kernelDied = (message: string) =>
l10n.t({
message:
'The kernel died. Error: {0}... View Jupyter [log](command:jupyter.viewOutput) for further details.',
args: [kernelName],
args: [message],
comment: [
'Do not translate the text "command:jupyter.viewOutput", that is a command Id that will be used by VS Code to open the output panel'
]
Expand Down
Loading
Loading