Skip to content

Commit 34bedc9

Browse files
committed
Handle ipykernel 7.0.1 failure to send background output updates
1 parent 4efd36f commit 34bedc9

File tree

1 file changed

+69
-21
lines changed

1 file changed

+69
-21
lines changed

src/standalone/api/kernels/backgroundExecution.ts

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

4-
import { CancellationToken } from 'vscode';
4+
import { CancellationToken, Disposable } from 'vscode';
55
import { IKernel } from '../../../kernels/types';
66
import { JVSC_EXTENSION_ID } from '../../../platform/common/constants';
77
import { createKernelApiForExtension } from './kernel';
88
import { DisposableStore } from '../../../platform/common/utils/lifecycle';
9-
import { raceCancellation } from '../../../platform/common/cancellation';
10-
import { getNotebookCellOutputMetadata } from '../../../kernels/execution/helpers';
9+
import { raceCancellation, wrapCancellationTokens } from '../../../platform/common/cancellation';
10+
import { CellOutputMimeTypes, getNotebookCellOutputMetadata } from '../../../kernels/execution/helpers';
1111
import { unTrackDisplayDataForExtension } from '../../../kernels/execution/extensionDisplayDataTracker';
1212
import { logger } from '../../../platform/logging';
13+
import { Delayer } from '../../../platform/common/utils/async';
1314

1415
export const executionCounters = new WeakMap<IKernel, number>();
1516
export async function execCodeInBackgroundThread<T>(
@@ -28,14 +29,15 @@ export async function execCodeInBackgroundThread<T>(
2829
const codeToSend = `
2930
def __jupyter_exec_background__():
3031
from IPython.display import display
32+
from ipykernel import __version__ as ipykernel_version
3133
from threading import Thread
3234
from traceback import format_exc
3335
3436
# First send a dummy response to get the display id.
3537
# Later we'll send the real response with the actual data.
3638
# And that can happen much later even after the execution completes,
3739
# as that response will be sent from a bg thread.
38-
output = display({"${mime}": ""}, raw=True, display_id=True)
40+
output = display({"${mime}": ipykernel_version}, raw=True, display_id=True)
3941
4042
def do_implementation():
4143
${codeWithReturnStatement.map((l, i) => (i === 0 ? l : ` ${l}`)).join('\n')}
@@ -53,14 +55,22 @@ def __jupyter_exec_background__():
5355
__jupyter_exec_background__()
5456
del __jupyter_exec_background__
5557
`.trim();
58+
let lastStdError = '';
5659
const disposables = new DisposableStore();
57-
disposables.add(token.onCancellationRequested(() => disposables.dispose()));
60+
disposables.add(
61+
new Disposable(() => {
62+
// We no longer need to track any more outputs from the kernel that are related to this output.
63+
kernel.session && unTrackDisplayDataForExtension(kernel.session, displayId);
64+
})
65+
);
66+
const wrappedCancellation = disposables.add(wrapCancellationTokens(token));
67+
disposables.add(wrappedCancellation.token.onCancellationRequested(() => disposables.dispose()));
5868
const promise = raceCancellation(
59-
token,
69+
wrappedCancellation.token,
6070
new Promise<T | undefined>((resolve, reject) => {
6171
disposables.add(
6272
api.onDidReceiveDisplayUpdate(async (output) => {
63-
if (token.isCancellationRequested) {
73+
if (wrappedCancellation.token.isCancellationRequested) {
6474
return resolve(undefined);
6575
}
6676
const metadata = getNotebookCellOutputMetadata(output);
@@ -80,23 +90,47 @@ del __jupyter_exec_background__
8090
})
8191
);
8292
})
83-
// We no longer need to track any more outputs from the kernel that are related to this output.
84-
).finally(() => {
85-
kernel.session && unTrackDisplayDataForExtension(kernel.session, displayId);
86-
disposables.dispose();
87-
});
93+
).finally(() => disposables.dispose());
94+
95+
let ipyKernelVersion = '';
96+
const exitIfFailuresFound = new Delayer(5_000);
8897

89-
for await (const output of api.executeCode(codeToSend, token)) {
98+
for await (const output of api.executeCode(codeToSend, wrappedCancellation.token)) {
9099
if (token.isCancellationRequested) {
91100
return;
92101
}
93102
const metadata = getNotebookCellOutputMetadata(output);
94103
if (!metadata?.transient?.display_id) {
104+
if (
105+
output.metadata?.outputType === 'stream' &&
106+
output.items.length &&
107+
output.items[0].mime === CellOutputMimeTypes.stderr
108+
) {
109+
lastStdError += new TextDecoder().decode(output.items[0].data);
110+
if (lastStdError && ipyKernelVersion.startsWith('7.0.1')) {
111+
// ipykernel 7.0.1 has a bug where background thread errors are printed to stderr
112+
// https://github.com/ipython/ipykernel/issues/1450
113+
wrappedCancellation.cancel();
114+
} else {
115+
logger.trace('Background execution stderr:', lastStdError);
116+
}
117+
}
95118
continue;
96119
}
97120
const dummyMessage = output.items.find((item) => item.mime === mime);
98121
if (dummyMessage) {
99122
displayId = metadata.transient.display_id;
123+
exitIfFailuresFound.cancel();
124+
125+
try {
126+
ipyKernelVersion = new TextDecoder().decode(dummyMessage.data).trim();
127+
// Check if ipykernel version matches the pattern d.d.d<anything>
128+
if (!ipyKernelVersion.match(/^\d+\.\d+\.\d+/)) {
129+
ipyKernelVersion = '';
130+
}
131+
} catch {
132+
// Ignore errors in decoding
133+
}
100134
continue;
101135
}
102136

@@ -110,13 +144,27 @@ del __jupyter_exec_background__
110144
}
111145
}
112146
}
113-
if (token.isCancellationRequested) {
114-
return;
115-
}
116-
if (!displayId) {
117-
logger.warn('Failed to get display id for completions');
118-
return;
147+
try {
148+
if (wrappedCancellation.token.isCancellationRequested) {
149+
if (!token.isCancellationRequested && lastStdError && ipyKernelVersion.startsWith('7.0.1')) {
150+
throw new Error(lastStdError);
151+
}
152+
return;
153+
}
154+
if (!displayId) {
155+
logger.warn('Failed to get display id for completions');
156+
return;
157+
}
158+
const result = await raceCancellation(wrappedCancellation.token, promise);
159+
if (result) {
160+
return result;
161+
}
162+
if (wrappedCancellation.token.isCancellationRequested && !token.isCancellationRequested && lastStdError) {
163+
throw new Error(lastStdError);
164+
}
165+
} finally {
166+
if (lastStdError) {
167+
logger.error('Error in background execution:\n', lastStdError);
168+
}
119169
}
120-
121-
return promise;
122170
}

0 commit comments

Comments
 (0)