Skip to content

Commit d7a192e

Browse files
committed
fix: graduate PET configure timeout to avoid killing process prematurely (Fixes #1274)
1 parent 6703525 commit d7a192e

File tree

1 file changed

+45
-13
lines changed

1 file changed

+45
-13
lines changed

src/managers/common/nativePythonFinder.ts

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const RESOLVE_TIMEOUT_MS = 30_000; // 30 seconds for single resolve
2323
// Restart/recovery constants
2424
const MAX_RESTART_ATTEMPTS = 3;
2525
const RESTART_BACKOFF_BASE_MS = 1_000; // 1 second base, exponential: 1s, 2s, 4s
26+
const MAX_CONFIGURE_TIMEOUTS_BEFORE_KILL = 2; // Kill on the 2nd consecutive timeout
2627

2728
export async function getNativePythonToolsPath(): Promise<string> {
2829
const envsExt = getExtension(ENVS_EXTENSION_ID);
@@ -119,14 +120,26 @@ interface RefreshOptions {
119120
searchPaths?: string[];
120121
}
121122

123+
/**
124+
* Error thrown when a JSON-RPC request times out.
125+
*/
126+
class RpcTimeoutError extends Error {
127+
constructor(
128+
public readonly method: string,
129+
timeoutMs: number,
130+
) {
131+
super(`Request '${method}' timed out after ${timeoutMs}ms`);
132+
}
133+
}
134+
122135
/**
123136
* Wraps a JSON-RPC sendRequest call with a timeout.
124137
* @param connection The JSON-RPC connection
125138
* @param method The RPC method name
126139
* @param params The parameters to send
127140
* @param timeoutMs Timeout in milliseconds
128141
* @returns The result of the request
129-
* @throws Error if the request times out
142+
* @throws RpcTimeoutError if the request times out
130143
*/
131144
async function sendRequestWithTimeout<T>(
132145
connection: rpc.MessageConnection,
@@ -138,7 +151,7 @@ async function sendRequestWithTimeout<T>(
138151
const timeoutPromise = new Promise<never>((_, reject) => {
139152
const timer = setTimeout(() => {
140153
cts.cancel();
141-
reject(new Error(`Request '${method}' timed out after ${timeoutMs}ms`));
154+
reject(new RpcTimeoutError(method, timeoutMs));
142155
}, timeoutMs);
143156
// Clear timeout if the CancellationTokenSource is disposed
144157
cts.token.onCancellationRequested(() => clearTimeout(timer));
@@ -161,6 +174,7 @@ class NativePythonFinderImpl implements NativePythonFinder {
161174
private startFailed: boolean = false;
162175
private restartAttempts: number = 0;
163176
private isRestarting: boolean = false;
177+
private configureTimeoutCount: number = 0;
164178

165179
constructor(
166180
private readonly outputChannel: LogOutputChannel,
@@ -192,8 +206,9 @@ class NativePythonFinderImpl implements NativePythonFinder {
192206
this.restartAttempts = 0;
193207
return environment;
194208
} catch (ex) {
195-
// On timeout, kill the hung process so next request triggers restart
196-
if (ex instanceof Error && ex.message.includes('timed out')) {
209+
// On resolve timeout (not configure — configure handles its own timeout),
210+
// kill the hung process so next request triggers restart
211+
if (ex instanceof RpcTimeoutError && ex.method !== 'configure') {
197212
this.outputChannel.warn('[pet] Resolve request timed out, killing hung process for restart');
198213
this.killProcess();
199214
this.processExited = true;
@@ -260,6 +275,7 @@ class NativePythonFinderImpl implements NativePythonFinder {
260275
this.processExited = false;
261276
this.startFailed = false;
262277
this.lastConfiguration = undefined; // Force reconfiguration
278+
this.configureTimeoutCount = 0;
263279

264280
// Start fresh
265281
this.connection = this.start();
@@ -544,8 +560,9 @@ class NativePythonFinderImpl implements NativePythonFinder {
544560
// Reset restart attempts on successful refresh
545561
this.restartAttempts = 0;
546562
} catch (ex) {
547-
// On timeout, kill the hung process so next request triggers restart
548-
if (ex instanceof Error && ex.message.includes('timed out')) {
563+
// On refresh timeout (not configure — configure handles its own timeout),
564+
// kill the hung process so next request triggers restart
565+
if (ex instanceof RpcTimeoutError && ex.method !== 'configure') {
549566
this.outputChannel.warn('[pet] Request timed out, killing hung process for restart');
550567
this.killProcess();
551568
this.processExited = true;
@@ -584,16 +601,31 @@ class NativePythonFinderImpl implements NativePythonFinder {
584601
}
585602
this.outputChannel.info('[pet] configure: Sending configuration update:', JSON.stringify(options));
586603
try {
587-
this.lastConfiguration = options;
588604
await sendRequestWithTimeout(this.connection, 'configure', options, CONFIGURE_TIMEOUT_MS);
605+
// Only cache after success so failed/timed-out calls will retry
606+
this.lastConfiguration = options;
607+
this.configureTimeoutCount = 0;
589608
} catch (ex) {
590-
// On timeout, kill the hung process so next request triggers restart
591-
if (ex instanceof Error && ex.message.includes('timed out')) {
592-
this.outputChannel.warn('[pet] Configure request timed out, killing hung process for restart');
593-
this.killProcess();
594-
this.processExited = true;
609+
if (ex instanceof RpcTimeoutError) {
610+
this.configureTimeoutCount++;
611+
if (this.configureTimeoutCount >= MAX_CONFIGURE_TIMEOUTS_BEFORE_KILL) {
612+
// Repeated configure timeouts suggest PET is truly hung — kill and restart
613+
this.outputChannel.error(
614+
`[pet] Configure timed out ${this.configureTimeoutCount} consecutive times, killing hung process for restart`,
615+
);
616+
this.killProcess();
617+
this.processExited = true;
618+
this.configureTimeoutCount = 0;
619+
} else {
620+
// First timeout — PET may still be working. Let it continue and retry next call.
621+
this.outputChannel.warn(
622+
`[pet] Configure request timed out (attempt ${this.configureTimeoutCount}/${MAX_CONFIGURE_TIMEOUTS_BEFORE_KILL}), ` +
623+
'will retry on next request without killing process',
624+
);
625+
}
626+
} else {
627+
this.outputChannel.error('[pet] configure: Configuration error', ex);
595628
}
596-
this.outputChannel.error('[pet] configure: Configuration error', ex);
597629
throw ex;
598630
}
599631
}

0 commit comments

Comments
 (0)