Skip to content

Commit c6e4fe7

Browse files
Improve telemetry error classification to reduce 'unknown' error types (#1409)
## Problem The `classifyError()` function in the telemetry error classifier had limited coverage — only 6 error categories were recognized. Many errors from manager registration (`MANAGER_REGISTRATION_FAILED`) were falling into the `'unknown'` bucket, making it difficult to diagnose failures from telemetry data. After tracing every code path that leads to `MANAGER_REGISTRATION_FAILED` telemetry (pipenv, pyenv, poetry, conda, system, and shellStartupVars registration), we identified 6 common error patterns that were unclassified. ## Changes Added 6 new error types to `DiscoveryErrorType` and corresponding classification logic: | New Type | What It Catches | Detection Method | |---|---|---| | `tool_not_found` | "Conda not found", "Python extension not found", "Poetry executable not found" | Message pattern | | `command_failed` | `Failed to run "conda ..."`, `Failed to run poetry ...`, `Error spawning conda: ...` | Message pattern | | `connection_error` | PET process dies mid-request, JSON-RPC connection closed/disposed | `instanceof rpc.ConnectionError` | | `rpc_error` | PET returns a JSON-RPC error response (e.g., internal error, method not found) | `instanceof rpc.ResponseError` | | `process_crash` | PET restarting, failed after N restart attempts, failed to create stdio streams | Message pattern | | `already_registered` | Duplicate manager registration (race condition) | `instanceof BaseError` | Every message pattern is based on exact `throw new Error(...)` strings found in the codebase — no speculative matching. ## Ordering The check order is designed to avoid misclassification: 1. `instanceof` checks first (most reliable, no false positives) 2. `error.code` checks for Node.js errno codes 3. Message patterns from most specific to least specific (e.g., `'not found'` is after `'json'` to avoid catching JSON validation errors) 4. Error name fallback for cancellation variants 5. `'unknown'` as final fallback ## Testing Added 7 new test cases covering all new error types with real-world error messages from the codebase. All 15 tests pass.
1 parent eff0768 commit c6e4fe7

File tree

2 files changed

+104
-1
lines changed

2 files changed

+104
-1
lines changed

src/common/telemetry/errorClassifier.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
import { CancellationError } from 'vscode';
2+
import * as rpc from 'vscode-jsonrpc/node';
23
import { RpcTimeoutError } from '../../managers/common/nativePythonFinder';
4+
import { BaseError } from '../errors/types';
35

46
export type DiscoveryErrorType =
57
| 'spawn_timeout'
68
| 'spawn_enoent'
79
| 'permission_denied'
810
| 'canceled'
911
| 'parse_error'
12+
| 'tool_not_found'
13+
| 'command_failed'
14+
| 'connection_error'
15+
| 'rpc_error'
16+
| 'process_crash'
17+
| 'already_registered'
1018
| 'unknown';
1119

1220
/**
@@ -22,6 +30,21 @@ export function classifyError(ex: unknown): DiscoveryErrorType {
2230
return 'spawn_timeout';
2331
}
2432

33+
// JSON-RPC connection errors (e.g., PET process died mid-request, connection closed/disposed)
34+
if (ex instanceof rpc.ConnectionError) {
35+
return 'connection_error';
36+
}
37+
38+
// JSON-RPC response errors (PET returned an error response, e.g., internal error)
39+
if (ex instanceof rpc.ResponseError) {
40+
return 'rpc_error';
41+
}
42+
43+
// BaseError subclasses: EnvironmentManagerAlreadyRegisteredError, PackageManagerAlreadyRegisteredError
44+
if (ex instanceof BaseError) {
45+
return 'already_registered';
46+
}
47+
2548
if (!(ex instanceof Error)) {
2649
return 'unknown';
2750
}
@@ -35,15 +58,34 @@ export function classifyError(ex: unknown): DiscoveryErrorType {
3558
return 'permission_denied';
3659
}
3760

38-
// Check message patterns
61+
// Check message patterns (order matters — more specific patterns first)
3962
const msg = ex.message.toLowerCase();
4063
if (msg.includes('timed out') || msg.includes('timeout')) {
4164
return 'spawn_timeout';
4265
}
66+
67+
// CLI command execution failures — checked before parse_error because command args
68+
// may contain words like "json" (e.g., 'Failed to run "conda info --envs --json"')
69+
if (msg.includes('failed to run') || msg.includes('error spawning')) {
70+
return 'command_failed';
71+
}
72+
4373
if (msg.includes('parse') || msg.includes('unexpected token') || msg.includes('json')) {
4474
return 'parse_error';
4575
}
4676

77+
// Tool/executable not found — e.g., "Conda not found", "Python extension not found",
78+
// "Poetry executable not found"
79+
if (msg.includes('not found')) {
80+
return 'tool_not_found';
81+
}
82+
83+
// PET process crash/hang recovery failures — e.g., "PET is currently restarting",
84+
// "failed after 3 restart attempts", "Failed to create stdio streams for PET process"
85+
if (msg.includes('restart') || msg.includes('stdio stream')) {
86+
return 'process_crash';
87+
}
88+
4789
// Check error name for cancellation variants
4890
if (ex.name === 'CancellationError' || ex.name === 'AbortError') {
4991
return 'canceled';

src/test/common/telemetry/errorClassifier.unit.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import assert from 'node:assert';
22
import { CancellationError } from 'vscode';
3+
import * as rpc from 'vscode-jsonrpc/node';
4+
import { BaseError } from '../../../common/errors/types';
35
import { classifyError } from '../../../common/telemetry/errorClassifier';
46
import { RpcTimeoutError } from '../../../managers/common/nativePythonFinder';
57

@@ -64,5 +66,64 @@ suite('Error Classifier', () => {
6466
test('should classify unrecognized errors as unknown', () => {
6567
assert.strictEqual(classifyError(new Error('something went wrong')), 'unknown');
6668
});
69+
70+
test('should classify ConnectionError as connection_error', () => {
71+
assert.strictEqual(
72+
classifyError(new rpc.ConnectionError(rpc.ConnectionErrors.Closed, 'Connection closed')),
73+
'connection_error',
74+
);
75+
assert.strictEqual(
76+
classifyError(new rpc.ConnectionError(rpc.ConnectionErrors.Disposed, 'Connection disposed')),
77+
'connection_error',
78+
);
79+
});
80+
81+
test('should classify ResponseError as rpc_error', () => {
82+
assert.strictEqual(classifyError(new rpc.ResponseError(-32600, 'Invalid request')), 'rpc_error');
83+
assert.strictEqual(classifyError(new rpc.ResponseError(-32601, 'Method not found')), 'rpc_error');
84+
});
85+
86+
test('should classify BaseError subclasses as already_registered', () => {
87+
// Using a concrete subclass to test (BaseError is abstract)
88+
class TestRegisteredError extends BaseError {
89+
constructor(message: string) {
90+
super('InvalidArgument', message);
91+
}
92+
}
93+
assert.strictEqual(
94+
classifyError(new TestRegisteredError('Environment manager with id test already registered')),
95+
'already_registered',
96+
);
97+
});
98+
99+
test('should classify "not found" messages as tool_not_found', () => {
100+
assert.strictEqual(classifyError(new Error('Conda not found')), 'tool_not_found');
101+
assert.strictEqual(classifyError(new Error('Python extension not found')), 'tool_not_found');
102+
assert.strictEqual(classifyError(new Error('Poetry executable not found')), 'tool_not_found');
103+
});
104+
105+
test('should classify command execution failures as command_failed', () => {
106+
assert.strictEqual(
107+
classifyError(new Error('Failed to run "conda info --envs --json":\n some error')),
108+
'command_failed',
109+
);
110+
assert.strictEqual(classifyError(new Error('Failed to run poetry install')), 'command_failed');
111+
assert.strictEqual(classifyError(new Error('Error spawning conda: ENOENT')), 'command_failed');
112+
});
113+
114+
test('should classify PET process crash/restart errors as process_crash', () => {
115+
assert.strictEqual(
116+
classifyError(new Error('Python Environment Tools (PET) is currently restarting. Please try again.')),
117+
'process_crash',
118+
);
119+
assert.strictEqual(
120+
classifyError(new Error('Python Environment Tools (PET) failed after 3 restart attempts.')),
121+
'process_crash',
122+
);
123+
assert.strictEqual(
124+
classifyError(new Error('Failed to create stdio streams for PET process')),
125+
'process_crash',
126+
);
127+
});
67128
});
68129
});

0 commit comments

Comments
 (0)