Skip to content

Commit cb2fab4

Browse files
committed
Preserve Conda cache on parse failures
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5aba47cf-0b43-48b9-bc22-75e625da70e8
1 parent 8558073 commit cb2fab4

4 files changed

Lines changed: 86 additions & 27 deletions

File tree

‎src/managers/conda/commands/list.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,10 @@ export class CondaListCommand extends ListCommand {
3939
try {
4040
parsed = JSON.parse(output);
4141
} catch (error) {
42-
this.log?.error('Failed to parse conda list output', error);
4342
throw new CondaListOutputError('Failed to parse conda list output', error);
4443
}
4544
if (!Array.isArray(parsed)) {
46-
const error = new CondaListOutputError('Invalid conda list output: expected a JSON array');
47-
this.log?.error(error.message);
48-
throw error;
45+
throw new CondaListOutputError('Invalid conda list output: expected a JSON array');
4946
}
5047

5148
const packages: PackageInfo[] = [];

‎src/managers/conda/condaPackageManager.ts‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,15 @@ export class CondaPackageManager implements PackageManager, Disposable {
101101
});
102102
}
103103

104-
await updatePackagesAndNotify(this, environment, this.packages.get(environment.envId.id), (changes) => {
105-
this._onDidChangePackages.fire({ environment, manager: this, changes });
106-
});
104+
await updatePackagesAndNotify(
105+
this,
106+
environment,
107+
this.packages.get(environment.envId.id),
108+
(changes) => {
109+
this._onDidChangePackages.fire({ environment, manager: this, changes });
110+
},
111+
() => this.fetchPackages(environment),
112+
);
107113
} catch (e) {
108114
if (e instanceof CancellationError) {
109115
throw e;
@@ -147,34 +153,40 @@ export class CondaPackageManager implements PackageManager, Disposable {
147153
(changes) => {
148154
this._onDidChangePackages.fire({ environment, manager: this, changes });
149155
},
156+
() => this.fetchPackages(environment),
150157
);
151-
this.packages.set(environment.envId.id, packages ?? []);
158+
if (packages !== undefined) {
159+
this.packages.set(environment.envId.id, packages);
160+
}
152161
},
153162
);
154163
}
155164

156165
async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise<Package[] | undefined> {
157166
if (options?.skipCache || !this.packages.has(environment.envId.id)) {
158-
const listCmd = new CondaListCommand({
159-
pythonExecutable: 'conda',
160-
condaEnvironmentPath: environment.environmentPath.fsPath,
161-
log: this.log,
162-
});
163-
let data;
164-
try {
165-
data = await listCmd.execute();
166-
} catch (error) {
167-
if (error instanceof CondaListOutputError) {
168-
this.log.error('Error parsing installed Conda packages', error);
169-
return [];
170-
}
171-
throw error;
172-
}
173-
const packages = (data ?? []).map((pkg) => this.api.createPackageItem(pkg, environment, this));
167+
return (await this.fetchPackages(environment)) ?? [];
168+
}
169+
return this.packages.get(environment.envId.id);
170+
}
171+
172+
private async fetchPackages(environment: PythonEnvironment): Promise<Package[] | undefined> {
173+
const listCmd = new CondaListCommand({
174+
pythonExecutable: 'conda',
175+
condaEnvironmentPath: environment.environmentPath.fsPath,
176+
log: this.log,
177+
});
178+
try {
179+
const data = await listCmd.execute();
180+
const packages = data.map((pkg) => this.api.createPackageItem(pkg, environment, this));
174181
this.packages.set(environment.envId.id, packages);
175182
return packages;
183+
} catch (error) {
184+
if (error instanceof CondaListOutputError) {
185+
this.log.error('Error parsing installed Conda packages', error);
186+
return undefined;
187+
}
188+
throw error;
176189
}
177-
return this.packages.get(environment.envId.id);
178190
}
179191

180192
formatInstallSpec(packageName: string, version: string): string {

‎src/test/managers/conda/commands.unit.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ suite('Conda commands', () => {
126126
});
127127

128128
await assert.rejects(() => command.execute(), CondaListOutputError);
129-
assert.ok((mockLog.error as sinon.SinonStub).calledOnce);
129+
assert.ok((mockLog.error as sinon.SinonStub).notCalled);
130130
});
131131

132132
test('CondaListCommand rejects non-array JSON', async () => {
@@ -138,7 +138,7 @@ suite('Conda commands', () => {
138138
});
139139

140140
await assert.rejects(() => command.execute(), /expected a JSON array/);
141-
assert.ok((mockLog.error as sinon.SinonStub).calledOnce);
141+
assert.ok((mockLog.error as sinon.SinonStub).notCalled);
142142
});
143143

144144
test('CondaVersionCommand parses the version', async () => {

‎src/test/managers/conda/condaPackageManager.unit.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,25 @@ suite('CondaPackageManager', () => {
4141
assert.ok(showErrorMessageWithLogs.notCalled);
4242
});
4343

44+
test('does not fail a successful mutation when follow-up list output is malformed', async () => {
45+
const environment = {
46+
envId: { id: 'test-environment', managerId: 'test-manager' },
47+
environmentPath: Uri.file('.'),
48+
} as PythonEnvironment;
49+
const logError = sinon.stub();
50+
const manager = new CondaPackageManager(
51+
{ createPackageItem: sinon.stub() } as unknown as PythonEnvironmentApi,
52+
{ error: logError } as unknown as LogOutputChannel,
53+
);
54+
sinon.stub(CondaInstallCommand.prototype, 'execute').resolves();
55+
const parseError = new CondaListOutputError('Failed to parse conda list output');
56+
sinon.stub(CondaListCommand.prototype, 'execute').rejects(parseError);
57+
58+
await manager.manage(environment, { install: ['requests'], runHeadless: true });
59+
60+
assert.ok(logError.calledOnceWithExactly('Error parsing installed Conda packages', parseError));
61+
});
62+
4463
test('propagates package version lookup failures', async () => {
4564
const environment = {
4665
envId: { id: 'test-environment', managerId: 'test-manager' },
@@ -105,4 +124,35 @@ suite('CondaPackageManager', () => {
105124

106125
await assert.rejects(manager.getPackages(environment), (error: unknown) => error === processError);
107126
});
127+
128+
test('does not report removals or replace the cache when refresh output is malformed', async () => {
129+
const environment = {
130+
envId: { id: 'test-environment', managerId: 'test-manager' },
131+
environmentPath: Uri.file('.'),
132+
} as PythonEnvironment;
133+
const cachedPackage = {
134+
name: 'requests',
135+
displayName: 'requests',
136+
version: '2.32.0',
137+
description: '2.32.0',
138+
};
139+
const api = {
140+
createPackageItem: sinon.stub().callsFake((pkg) => pkg),
141+
} as unknown as PythonEnvironmentApi;
142+
const manager = new CondaPackageManager(api, { error: sinon.stub() } as unknown as LogOutputChannel);
143+
const execute = sinon.stub(CondaListCommand.prototype, 'execute');
144+
execute.onFirstCall().resolves([cachedPackage]);
145+
execute.onSecondCall().rejects(new CondaListOutputError('Failed to parse conda list output'));
146+
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never));
147+
const onDidChangePackages = sinon.stub();
148+
manager.onDidChangePackages(onDidChangePackages);
149+
150+
await manager.getPackages(environment);
151+
await manager.refresh(environment);
152+
const packages = await manager.getPackages(environment);
153+
154+
assert.deepStrictEqual(packages?.map((pkg) => pkg.name), ['requests']);
155+
assert.ok(onDidChangePackages.notCalled);
156+
assert.strictEqual(execute.callCount, 2);
157+
});
108158
});

0 commit comments

Comments
 (0)