Skip to content

Commit c8bb80b

Browse files
fix: skip package refresh for deleted environments (#1833)
Fixes #1824 ## Problem Deleting and recreating a virtual environment (for example `make clean-venv` followed by `make venv`) surfaced an **"Error refreshing packages"** notification. ### Explanation The package watcher observes `site-packages`. Deleting the environment fires delete events, which schedule a debounced refresh. That refresh then ran `pip list` against the interpreter that had just been removed, so the failure was an expected consequence of the deletion rather than an actionable problem — but it was reported to the user as an error. ## Fix `watchPackageChangesForEnvironment` now verifies the environment executable exists before refreshing. - If the executable is missing, the refresh is not attempted - Because a deletion is usually followed by a recreation, the watcher retries on the existing 500ms debounce interval for up to 30 seconds - If the executable never returns, the watcher stops retrying and logs at debug level. - Genuine stat failures and genuine refresh failures are still logged as errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f214db8 commit c8bb80b

5 files changed

Lines changed: 141 additions & 15 deletions

File tree

‎src/common/utils/filesystem.ts‎

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

4+
/**
5+
* Determines whether an error indicates that a file system entry does not exist.
6+
*
7+
* Supports Node.js and VS Code file system error codes.
8+
*
9+
* @param error The error to inspect.
10+
* @returns Whether the error represents a missing file system entry.
11+
*/
412
export function isFileNotFoundError(error: unknown): error is NodeJS.ErrnoException {
513
return (
614
typeof error === 'object' &&
715
error !== null &&
8-
'code' in error &&
9-
(error as NodeJS.ErrnoException).code === 'ENOENT'
16+
(('code' in error &&
17+
((error as NodeJS.ErrnoException).code === 'ENOENT' ||
18+
(error as NodeJS.ErrnoException).code === 'FileNotFound')) ||
19+
('name' in error && String(error.name).startsWith('EntryNotFound')))
1020
);
1121
}

‎src/common/workspace.fs.apis.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { FileStat, Uri, workspace } from 'vscode';
2+
import { isFileNotFoundError } from './utils/filesystem';
23

34
export function readFile(uri: Uri): Thenable<Uint8Array> {
45
return workspace.fs.readFile(uri);
@@ -7,3 +8,21 @@ export function readFile(uri: Uri): Thenable<Uint8Array> {
78
export function stat(uri: Uri): Thenable<FileStat> {
89
return workspace.fs.stat(uri);
910
}
11+
12+
/**
13+
* Checks whether a workspace file system entry exists.
14+
*
15+
* @param uri The URI of the entry to check.
16+
* @returns Whether the entry exists.
17+
*/
18+
export async function pathExists(uri: Uri): Promise<boolean> {
19+
try {
20+
await workspace.fs.stat(uri);
21+
return true;
22+
} catch (error) {
23+
if (isFileNotFoundError(error)) {
24+
return false;
25+
}
26+
throw error;
27+
}
28+
}

‎src/managers/common/packageWatcher.ts‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Disposable, Event, LogOutputChannel, RelativePattern, Terminal, Uri } f
33
import { PackageManager, PythonEnvironment } from '../../api';
44
import { createSimpleDebounce } from '../../common/utils/debounce';
55
import { onDidCloseTerminal } from '../../common/window.apis';
6+
import { pathExists } from '../../common/workspace.fs.apis';
67
import { createFileSystemWatcher, getConfiguration, onDidChangeConfiguration } from '../../common/workspace.apis';
78
import type { EnvironmentManagers } from '../../features/envManagers';
89

@@ -14,6 +15,8 @@ export interface PackageWatcherTerminalActivation {
1415
}>;
1516
}
1617

18+
const MAX_MISSING_EXECUTABLE_REFRESH_RETRIES = 60;
19+
1720
/**
1821
* Derives the file system watch targets for a given Python environment.
1922
*
@@ -63,18 +66,43 @@ export function watchPackageChangesForEnvironment(
6366
return new Disposable(() => undefined);
6467
}
6568

69+
let missingExecutableRefreshRetries = 0;
6670
const debouncedRefresh = createSimpleDebounce(500, () => {
67-
log.debug(`Package change detected for environment ${env.envId.id}, refreshing packages.`);
68-
const currentPackageManager = resolvePackageManager();
69-
if (!currentPackageManager) {
70-
log.debug(`No current package manager found for environment ${env.envId.id}`);
71-
return;
72-
}
73-
void currentPackageManager.refresh(env).catch((ex) => {
74-
log.error(
75-
`Failed to refresh packages for environment ${env.envId.id}: ${ex instanceof Error ? ex.message : String(ex)}`,
76-
);
77-
});
71+
void (async () => {
72+
try {
73+
if (!(await pathExists(Uri.file(env.execInfo.run.executable)))) {
74+
if (missingExecutableRefreshRetries < MAX_MISSING_EXECUTABLE_REFRESH_RETRIES) {
75+
missingExecutableRefreshRetries += 1;
76+
debouncedRefresh.trigger();
77+
} else {
78+
log.debug(
79+
`Package change detected for environment ${env.envId.id}, but its executable did not reappear. Skipping package refresh.`,
80+
);
81+
}
82+
return;
83+
}
84+
} catch (ex) {
85+
log.error(
86+
`Failed to check the executable for environment ${env.envId.id}: ${ex instanceof Error ? ex.message : String(ex)}`,
87+
);
88+
return;
89+
}
90+
91+
missingExecutableRefreshRetries = 0;
92+
log.debug(`Package change detected for environment ${env.envId.id}, refreshing packages.`);
93+
const currentPackageManager = resolvePackageManager();
94+
if (!currentPackageManager) {
95+
log.debug(`No current package manager found for environment ${env.envId.id}`);
96+
return;
97+
}
98+
try {
99+
await currentPackageManager.refresh(env);
100+
} catch (ex) {
101+
log.error(
102+
`Failed to refresh packages for environment ${env.envId.id}: ${ex instanceof Error ? ex.message : String(ex)}`,
103+
);
104+
}
105+
})();
78106
});
79107
const disposables: Disposable[] = [debouncedRefresh];
80108
const trigger = debouncedRefresh.trigger.bind(debouncedRefresh);

‎src/test/common/filesystem.unit.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,19 @@ suite('filesystem utilities', () => {
1111
assert.strictEqual(isFileNotFoundError(error), true);
1212
});
1313

14+
test('returns true for VS Code missing file errors', () => {
15+
assert.strictEqual(isFileNotFoundError(Object.assign(new Error('missing'), { code: 'FileNotFound' })), true);
16+
assert.strictEqual(
17+
isFileNotFoundError(Object.assign(new Error('missing'), { name: 'EntryNotFound (FileSystemError)' })),
18+
true,
19+
);
20+
});
21+
1422
test('rejects other errors and non-errors', () => {
15-
assert.strictEqual(isFileNotFoundError(Object.assign(new Error('not a directory'), { code: 'ENOTDIR' })), false);
23+
assert.strictEqual(
24+
isFileNotFoundError(Object.assign(new Error('not a directory'), { code: 'ENOTDIR' })),
25+
false,
26+
);
1627
assert.strictEqual(isFileNotFoundError(new Error('missing code')), false);
1728
assert.strictEqual(isFileNotFoundError(undefined), false);
1829
assert.strictEqual(isFileNotFoundError('ENOENT'), false);

‎src/test/managers/common/packageWatcher.unit.test.ts‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
PythonProject,
1313
} from '../../../api';
1414
import * as windowApis from '../../../common/window.apis';
15+
import * as workspaceFsApis from '../../../common/workspace.fs.apis';
1516
import * as workspaceApis from '../../../common/workspace.apis';
1617
import type { EnvironmentManagers } from '../../../features/envManagers';
1718
import { InternalPackageManager } from '../../../managers/common/registeredManagers';
@@ -41,6 +42,7 @@ suite('Package Watcher', () => {
4142
debug: sandbox.stub(),
4243
};
4344
createFileSystemWatcherStub = sandbox.stub(workspaceApis, 'createFileSystemWatcher');
45+
sandbox.stub(workspaceFsApis, 'pathExists').resolves(true);
4446
sandbox.stub(workspaceApis, 'getConfiguration').returns({
4547
get: (_key: string, defaultValue?: unknown) => defaultValue ?? true,
4648
} as ReturnType<typeof workspaceApis.getConfiguration>);
@@ -300,6 +302,60 @@ suite('Package Watcher', () => {
300302
clock.restore();
301303
});
302304

305+
test('should not refresh packages when the environment executable was deleted', async () => {
306+
const clock = sandbox.useFakeTimers();
307+
const mockWatcher = createMockWatcher();
308+
createFileSystemWatcherStub.returns(mockWatcher);
309+
(workspaceFsApis.pathExists as sinon.SinonStub).resolves(false);
310+
311+
const env = createMockEnvironment();
312+
const packageManager = createMockPackageManager();
313+
314+
watchPackageChangesForEnvironment(
315+
env,
316+
packageManager as PackageManager,
317+
mockLogOutputChannel as LogOutputChannel,
318+
);
319+
320+
mockWatcher._deleteEmitter.fire(Uri.file('/path/to/pkg.dist-info/METADATA'));
321+
await clock.tickAsync(600);
322+
323+
assert.strictEqual(
324+
(packageManager.refresh as sinon.SinonStub).callCount,
325+
0,
326+
'Should not refresh packages for a deleted environment',
327+
);
328+
329+
clock.restore();
330+
});
331+
332+
test('should refresh packages when a deleted environment executable reappears', async () => {
333+
const clock = sandbox.useFakeTimers();
334+
const mockWatcher = createMockWatcher();
335+
createFileSystemWatcherStub.returns(mockWatcher);
336+
(workspaceFsApis.pathExists as sinon.SinonStub).onFirstCall().resolves(false).resolves(true);
337+
338+
const env = createMockEnvironment();
339+
const packageManager = createMockPackageManager();
340+
341+
watchPackageChangesForEnvironment(
342+
env,
343+
packageManager as PackageManager,
344+
mockLogOutputChannel as LogOutputChannel,
345+
);
346+
347+
mockWatcher._deleteEmitter.fire(Uri.file('/path/to/pkg.dist-info/METADATA'));
348+
await clock.tickAsync(1_200);
349+
350+
assert.strictEqual(
351+
(packageManager.refresh as sinon.SinonStub).callCount,
352+
1,
353+
'Should refresh packages after the environment is recreated',
354+
);
355+
356+
clock.restore();
357+
});
358+
303359
test('should debounce multiple rapid file events', () => {
304360
const mockWatcher = createMockWatcher();
305361
createFileSystemWatcherStub.returns(mockWatcher);
@@ -528,7 +584,9 @@ suite('Package Watcher', () => {
528584
const terminal = { name: 'terminal' } as Terminal;
529585
const envManagers = {
530586
onDidChangeActiveEnvironment: environmentChanges.event,
531-
getPackageManager: sandbox.stub().callsFake((context) => (context === env ? packageManager : undefined)),
587+
getPackageManager: sandbox
588+
.stub()
589+
.callsFake((context) => (context === env ? packageManager : undefined)),
532590
} as unknown as EnvironmentManagers;
533591

534592
registerPackageWatchers(envManagers, mockTerminalActivation, mockLogOutputChannel as LogOutputChannel);

0 commit comments

Comments
 (0)