Skip to content

Commit aa550be

Browse files
fix(errors): implement proper error handling across codebase
- Add comprehensive error-utils.ts with safeExecute, extractError, toTypedError utilities - Replace catch (error: any) with catch (error: unknown) for type safety - Add proper logging in catch blocks instead of silently swallowing errors - Add debug/warn logging for expected failures (file not found, git not available) - Improve MCP server error handling with proper message extraction - Add 33 new tests for error handling utilities - Update SQLite adapter with improved error logging Addresses STA-186: Improve error handling patterns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 9748145 commit aa550be

6 files changed

Lines changed: 687 additions & 30 deletions

File tree

src/core/database/sqlite-adapter.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,11 @@ export class SQLiteAdapter extends FeatureAwareDatabaseAdapter {
112112
try {
113113
this.db.prepare('SELECT 1').get();
114114
return true;
115-
} catch {
115+
} catch (error: unknown) {
116+
// Database may be closed or corrupted
117+
logger.debug('Database ping failed', {
118+
error: error instanceof Error ? error.message : String(error),
119+
});
116120
return false;
117121
}
118122
}
@@ -306,7 +310,11 @@ export class SQLiteAdapter extends FeatureAwareDatabaseAdapter {
306310
.prepare('SELECT MAX(version) as version FROM schema_version')
307311
.get() as VersionResult;
308312
return result?.version || 0;
309-
} catch {
313+
} catch (error: unknown) {
314+
// Table may not exist yet in a fresh database
315+
logger.debug('Schema version table not found, returning 0', {
316+
error: error instanceof Error ? error.message : String(error),
317+
});
310318
return 0;
311319
}
312320
}
@@ -835,7 +843,13 @@ export class SQLiteAdapter extends FeatureAwareDatabaseAdapter {
835843
try {
836844
const fileStats = await fs.stat(this.dbPath);
837845
stats.diskUsage = fileStats.size;
838-
} catch {}
846+
} catch (error: unknown) {
847+
// File may not exist yet or be inaccessible - disk usage remains 0
848+
logger.debug('Failed to get database file size', {
849+
dbPath: this.dbPath,
850+
error: error instanceof Error ? error.message : String(error),
851+
});
852+
}
839853

840854
return stats;
841855
}
Lines changed: 302 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,320 @@
11
/**
2-
* Essential tests for error handling system
2+
* Tests for error handling system
33
*/
44

55
import { describe, it, expect } from 'vitest';
6-
import { StackMemoryError, ErrorCode } from '../index.js';
6+
import {
7+
StackMemoryError,
8+
ErrorCode,
9+
DatabaseError,
10+
IntegrationError,
11+
ValidationError,
12+
SystemError,
13+
isRetryableError,
14+
getErrorMessage,
15+
wrapError,
16+
getUserFriendlyMessage,
17+
createErrorHandler,
18+
} from '../index.js';
19+
import {
20+
safeExecute,
21+
safeExecuteSync,
22+
executeWithFallback,
23+
extractError,
24+
isNetworkError,
25+
toTypedError,
26+
assertCondition,
27+
assertDefined,
28+
} from '../error-utils.js';
729

830
describe('Error System', () => {
9-
it('should create StackMemoryError', () => {
31+
it('should create StackMemoryError with all properties', () => {
32+
const cause = new Error('Original error');
1033
const error = new StackMemoryError({
1134
message: 'Test error',
12-
code: ErrorCode.VALIDATION_FAILED
35+
code: ErrorCode.VALIDATION_FAILED,
36+
context: { field: 'test' },
37+
cause,
38+
isRetryable: true,
39+
httpStatus: 400,
1340
});
14-
41+
1542
expect(error).toBeInstanceOf(Error);
1643
expect(error.message).toBe('Test error');
1744
expect(error.name).toBe('StackMemoryError');
45+
expect(error.code).toBe(ErrorCode.VALIDATION_FAILED);
46+
expect(error.context).toEqual({ field: 'test' });
47+
expect(error.cause).toBe(cause);
48+
expect(error.isRetryable).toBe(true);
49+
expect(error.httpStatus).toBe(400);
50+
expect(error.timestamp).toBeInstanceOf(Date);
1851
});
1952

2053
it('should handle error codes', () => {
2154
expect(ErrorCode.VALIDATION_FAILED).toBe('VAL_001');
2255
expect(ErrorCode.DB_CONNECTION_FAILED).toBe('DB_001');
56+
expect(ErrorCode.LINEAR_AUTH_FAILED).toBe('LINEAR_001');
57+
});
58+
59+
it('should create specialized error types', () => {
60+
const dbError = new DatabaseError('DB failed', ErrorCode.DB_QUERY_FAILED);
61+
expect(dbError.httpStatus).toBe(503);
62+
63+
const integrationError = new IntegrationError('API failed');
64+
expect(integrationError.isRetryable).toBe(true);
65+
expect(integrationError.httpStatus).toBe(502);
66+
67+
const validationError = new ValidationError('Invalid input');
68+
expect(validationError.isRetryable).toBe(false);
69+
expect(validationError.httpStatus).toBe(400);
70+
71+
const systemError = new SystemError('Internal error');
72+
expect(systemError.httpStatus).toBe(500);
73+
});
74+
75+
it('should serialize error to JSON', () => {
76+
const error = new StackMemoryError({
77+
message: 'Test',
78+
code: ErrorCode.INTERNAL_ERROR,
79+
});
80+
81+
const json = error.toJSON();
82+
expect(json.name).toBe('StackMemoryError');
83+
expect(json.code).toBe(ErrorCode.INTERNAL_ERROR);
84+
expect(json.message).toBe('Test');
85+
expect(json.timestamp).toBeDefined();
86+
});
87+
});
88+
89+
describe('Error Utilities', () => {
90+
describe('isRetryableError', () => {
91+
it('should identify retryable StackMemoryError', () => {
92+
const retryable = new IntegrationError('API failed');
93+
expect(isRetryableError(retryable)).toBe(true);
94+
95+
const nonRetryable = new ValidationError('Invalid');
96+
expect(isRetryableError(nonRetryable)).toBe(false);
97+
});
98+
99+
it('should identify network errors as retryable', () => {
100+
expect(isRetryableError(new Error('ECONNREFUSED'))).toBe(true);
101+
expect(isRetryableError(new Error('Socket hang up'))).toBe(true);
102+
expect(isRetryableError(new Error('Timeout'))).toBe(true);
103+
});
104+
});
105+
106+
describe('getErrorMessage', () => {
107+
it('should extract message from Error', () => {
108+
expect(getErrorMessage(new Error('Test'))).toBe('Test');
109+
});
110+
111+
it('should handle string errors', () => {
112+
expect(getErrorMessage('String error')).toBe('String error');
113+
});
114+
115+
it('should handle unknown errors', () => {
116+
expect(getErrorMessage(null)).toBe('An unknown error occurred');
117+
expect(getErrorMessage(undefined)).toBe('An unknown error occurred');
118+
expect(getErrorMessage(42)).toBe('An unknown error occurred');
119+
});
120+
121+
it('should handle objects with message property', () => {
122+
expect(getErrorMessage({ message: 'Object error' })).toBe('Object error');
123+
});
124+
});
125+
126+
describe('wrapError', () => {
127+
it('should return StackMemoryError unchanged', () => {
128+
const original = new ValidationError('Test');
129+
expect(wrapError(original, 'Default')).toBe(original);
130+
});
131+
132+
it('should wrap regular Error', () => {
133+
const original = new Error('Original');
134+
const wrapped = wrapError(original, 'Default');
135+
136+
expect(wrapped).toBeInstanceOf(SystemError);
137+
expect(wrapped.message).toBe('Original');
138+
expect(wrapped.cause).toBe(original);
139+
});
140+
141+
it('should use default message for non-Error', () => {
142+
const wrapped = wrapError('string', 'Default message');
143+
expect(wrapped.message).toBe('Default message');
144+
});
145+
});
146+
147+
describe('getUserFriendlyMessage', () => {
148+
it('should return friendly messages for known codes', () => {
149+
expect(getUserFriendlyMessage(ErrorCode.AUTH_FAILED)).toContain(
150+
'Authentication'
151+
);
152+
expect(getUserFriendlyMessage(ErrorCode.FILE_NOT_FOUND)).toContain(
153+
'not found'
154+
);
155+
expect(getUserFriendlyMessage(ErrorCode.NETWORK_ERROR)).toContain(
156+
'internet'
157+
);
158+
});
159+
160+
it('should return generic message for unknown codes', () => {
161+
const msg = getUserFriendlyMessage('UNKNOWN_CODE' as ErrorCode);
162+
expect(msg).toContain('unexpected error');
163+
});
164+
});
165+
166+
describe('createErrorHandler', () => {
167+
it('should merge context', () => {
168+
const handler = createErrorHandler({ component: 'test' });
169+
const error = new ValidationError('Test');
170+
const handled = handler(error, { operation: 'validate' });
171+
172+
expect(handled.context?.component).toBe('test');
173+
expect(handled.context?.operation).toBe('validate');
174+
});
175+
});
176+
});
177+
178+
describe('Error Utils', () => {
179+
describe('safeExecute', () => {
180+
it('should return result on success', async () => {
181+
const result = await safeExecute(async () => 'success', {
182+
operation: 'test',
183+
component: 'test',
184+
});
185+
expect(result).toBe('success');
186+
});
187+
188+
it('should return undefined on failure', async () => {
189+
const result = await safeExecute(
190+
async () => {
191+
throw new Error('fail');
192+
},
193+
{ operation: 'test', component: 'test' }
194+
);
195+
expect(result).toBeUndefined();
196+
});
197+
198+
it('should return default value on failure', async () => {
199+
const result = await safeExecute(
200+
async () => {
201+
throw new Error('fail');
202+
},
203+
{ operation: 'test', component: 'test' },
204+
'default'
205+
);
206+
expect(result).toBe('default');
207+
});
208+
});
209+
210+
describe('safeExecuteSync', () => {
211+
it('should handle sync operations', () => {
212+
const result = safeExecuteSync(() => 'success', {
213+
operation: 'test',
214+
component: 'test',
215+
});
216+
expect(result).toBe('success');
217+
});
218+
219+
it('should return default on failure', () => {
220+
const result = safeExecuteSync(
221+
() => {
222+
throw new Error('fail');
223+
},
224+
{ operation: 'test', component: 'test' },
225+
'default'
226+
);
227+
expect(result).toBe('default');
228+
});
229+
});
230+
231+
describe('executeWithFallback', () => {
232+
it('should return null on failure', async () => {
233+
const result = await executeWithFallback(
234+
async () => {
235+
throw new Error('fail');
236+
},
237+
{ operation: 'test', component: 'test' }
238+
);
239+
expect(result).toBeNull();
240+
});
241+
});
242+
243+
describe('extractError', () => {
244+
it('should extract from StackMemoryError', () => {
245+
const error = new IntegrationError(
246+
'API failed',
247+
ErrorCode.LINEAR_API_ERROR
248+
);
249+
const extracted = extractError(error);
250+
251+
expect(extracted.message).toBe('API failed');
252+
expect(extracted.code).toBe(ErrorCode.LINEAR_API_ERROR);
253+
expect(extracted.isRetryable).toBe(true);
254+
});
255+
256+
it('should extract from regular Error', () => {
257+
const error = new Error('Regular error');
258+
const extracted = extractError(error);
259+
260+
expect(extracted.message).toBe('Regular error');
261+
expect(extracted.cause).toBe(error);
262+
});
263+
264+
it('should handle non-Error values', () => {
265+
expect(extractError('string').message).toBe('string');
266+
expect(extractError(null).message).toBe('Unknown error');
267+
});
268+
});
269+
270+
describe('isNetworkError', () => {
271+
it('should identify network errors', () => {
272+
expect(isNetworkError(new Error('ECONNREFUSED'))).toBe(true);
273+
expect(isNetworkError(new Error('ENOTFOUND'))).toBe(true);
274+
expect(isNetworkError(new Error('timeout'))).toBe(true);
275+
expect(isNetworkError(new Error('socket hang up'))).toBe(true);
276+
expect(isNetworkError(new Error('ECONNRESET'))).toBe(true);
277+
});
278+
279+
it('should return false for non-network errors', () => {
280+
expect(isNetworkError(new Error('Validation failed'))).toBe(false);
281+
expect(isNetworkError('string')).toBe(false);
282+
});
283+
});
284+
285+
describe('toTypedError', () => {
286+
it('should return StackMemoryError unchanged', () => {
287+
const error = new ValidationError('Test');
288+
expect(toTypedError(error)).toBe(error);
289+
});
290+
291+
it('should wrap unknown errors', () => {
292+
const wrapped = toTypedError(new Error('test'), ErrorCode.API_ERROR);
293+
expect(wrapped).toBeInstanceOf(SystemError);
294+
expect(wrapped.code).toBe(ErrorCode.API_ERROR);
295+
});
296+
});
297+
298+
describe('assertCondition', () => {
299+
it('should not throw when condition is true', () => {
300+
expect(() => assertCondition(true, 'Test')).not.toThrow();
301+
});
302+
303+
it('should throw when condition is false', () => {
304+
expect(() => assertCondition(false, 'Test')).toThrow(StackMemoryError);
305+
});
306+
});
307+
308+
describe('assertDefined', () => {
309+
it('should not throw for defined values', () => {
310+
expect(() => assertDefined('value', 'Test')).not.toThrow();
311+
expect(() => assertDefined(0, 'Test')).not.toThrow();
312+
expect(() => assertDefined('', 'Test')).not.toThrow();
313+
});
314+
315+
it('should throw for null or undefined', () => {
316+
expect(() => assertDefined(null, 'Test')).toThrow(StackMemoryError);
317+
expect(() => assertDefined(undefined, 'Test')).toThrow(StackMemoryError);
318+
});
23319
});
24-
});
320+
});

0 commit comments

Comments
 (0)