fix: resolve TypeScript strict mode errors in providerErrorHandler.ts (#720)
* fix: resolve TypeScript strict mode errors in providerErrorHandler.ts - Add proper type guards for error object property access - Create ErrorWithStatus and ErrorWithMessage interfaces - Implement hasStatusProperty() and hasMessageProperty() type guards - Replace unsafe object property access with type-safe checks - All 8 TypeScript strict mode errors now resolved - Maintains existing functionality for LLM provider error handling Fixes #686 * fix: apply biome linting improvements to providerErrorHandler.ts - Use optional chaining instead of logical AND for property access - Improve formatting for better readability - Maintain all existing functionality while addressing linter warnings * chore: remove .claude-flow directory - Remove unnecessary .claude-flow metrics files - Clean up repository structure * Add comprehensive test coverage for providerErrorHandler TypeScript strict mode fixes - Added 24 comprehensive tests for parseProviderError and getProviderErrorMessage - Tests cover all error scenarios: basic errors, status codes, structured provider errors, malformed JSON, null/undefined handling, and TypeScript strict mode compliance - Fixed null/undefined handling in parseProviderError to properly return fallback messages - All tests passing (24/24) ensuring TypeScript strict mode fixes work correctly - Validates error handling for OpenAI, Google AI, Anthropic, and other LLM providers Related to PR #720 TypeScript strict mode compliance --------- Co-authored-by: OmniNode CI <noreply@omninode.ai>
This commit is contained in:
Jonah Gray
GitHub
parent
394ac1befa
commit
7a4c67cf90
@ -10,23 +10,55 @@ export interface ProviderError extends Error {
|
||||
isProviderError?: boolean;
|
||||
}
|
||||
|
||||
// Type guards for error object properties
|
||||
interface ErrorWithStatus {
|
||||
statusCode?: number;
|
||||
status?: number;
|
||||
}
|
||||
|
||||
interface ErrorWithMessage {
|
||||
message?: string;
|
||||
}
|
||||
|
||||
// Type guard functions
|
||||
function hasStatusProperty(obj: unknown): obj is ErrorWithStatus {
|
||||
return typeof obj === "object" && obj !== null && ("statusCode" in obj || "status" in obj);
|
||||
}
|
||||
|
||||
function hasMessageProperty(obj: unknown): obj is ErrorWithMessage {
|
||||
return typeof obj === "object" && obj !== null && "message" in obj;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse backend error responses into provider-aware error objects
|
||||
*/
|
||||
export function parseProviderError(error: unknown): ProviderError {
|
||||
// Handle null, undefined, and non-object types
|
||||
if (!error || typeof error !== "object") {
|
||||
const result: ProviderError = {
|
||||
name: "Error",
|
||||
} as ProviderError;
|
||||
|
||||
// Only set message for non-null/undefined values
|
||||
if (error) {
|
||||
result.message = String(error);
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
const providerError = error as ProviderError;
|
||||
|
||||
// Check if this is a structured provider error from backend
|
||||
if (error && typeof error === "object") {
|
||||
if (error.statusCode || error.status) {
|
||||
// Type-safe status code extraction
|
||||
if (hasStatusProperty(error)) {
|
||||
providerError.statusCode = error.statusCode || error.status;
|
||||
}
|
||||
|
||||
// Parse backend error structure
|
||||
if (error.message && error.message.includes("detail")) {
|
||||
// Parse backend error structure with type safety
|
||||
if (hasMessageProperty(error) && error.message && error.message.includes("detail")) {
|
||||
try {
|
||||
const parsed = JSON.parse(error.message);
|
||||
if (parsed.detail && parsed.detail.error_type) {
|
||||
if (parsed.detail?.error_type) {
|
||||
providerError.isProviderError = true;
|
||||
providerError.provider = parsed.detail.provider || "LLM";
|
||||
providerError.errorType = parsed.detail.error_type;
|
||||
@ -36,7 +68,6 @@ export function parseProviderError(error: unknown): ProviderError {
|
||||
// If parsing fails, use message as-is
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return providerError;
|
||||
}
|
||||
|
||||
@ -0,0 +1,281 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { parseProviderError, getProviderErrorMessage, type ProviderError } from '../providerErrorHandler';
|
||||
|
||||
describe('providerErrorHandler', () => {
|
||||
describe('parseProviderError', () => {
|
||||
it('should handle basic Error objects', () => {
|
||||
const error = new Error('Basic error message');
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.message).toBe('Basic error message');
|
||||
expect(result.isProviderError).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should handle errors with statusCode property', () => {
|
||||
const error = { statusCode: 401, message: 'Unauthorized' };
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.statusCode).toBe(401);
|
||||
expect(result.message).toBe('Unauthorized');
|
||||
});
|
||||
|
||||
it('should handle errors with status property', () => {
|
||||
const error = { status: 429, message: 'Rate limited' };
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.statusCode).toBe(429);
|
||||
expect(result.message).toBe('Rate limited');
|
||||
});
|
||||
|
||||
it('should prioritize statusCode over status when both are present', () => {
|
||||
const error = { statusCode: 401, status: 429, message: 'Auth error' };
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.statusCode).toBe(401);
|
||||
});
|
||||
|
||||
it('should parse structured provider errors from backend', () => {
|
||||
const error = {
|
||||
message: JSON.stringify({
|
||||
detail: {
|
||||
error_type: 'authentication_failed',
|
||||
provider: 'OpenAI',
|
||||
message: 'Invalid API key'
|
||||
}
|
||||
})
|
||||
};
|
||||
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.isProviderError).toBe(true);
|
||||
expect(result.provider).toBe('OpenAI');
|
||||
expect(result.errorType).toBe('authentication_failed');
|
||||
expect(result.message).toBe('Invalid API key');
|
||||
});
|
||||
|
||||
it('should handle malformed JSON in message gracefully', () => {
|
||||
const error = {
|
||||
message: 'invalid json { detail'
|
||||
};
|
||||
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.isProviderError).toBeUndefined();
|
||||
expect(result.message).toBe('invalid json { detail');
|
||||
});
|
||||
|
||||
it('should handle null and undefined inputs safely', () => {
|
||||
expect(() => parseProviderError(null)).not.toThrow();
|
||||
expect(() => parseProviderError(undefined)).not.toThrow();
|
||||
|
||||
const nullResult = parseProviderError(null);
|
||||
const undefinedResult = parseProviderError(undefined);
|
||||
|
||||
expect(nullResult).toBeDefined();
|
||||
expect(undefinedResult).toBeDefined();
|
||||
});
|
||||
|
||||
it('should handle empty objects', () => {
|
||||
const result = parseProviderError({});
|
||||
|
||||
expect(result).toBeDefined();
|
||||
expect(result.statusCode).toBeUndefined();
|
||||
expect(result.isProviderError).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should handle primitive values', () => {
|
||||
expect(() => parseProviderError('string error')).not.toThrow();
|
||||
expect(() => parseProviderError(42)).not.toThrow();
|
||||
expect(() => parseProviderError(true)).not.toThrow();
|
||||
});
|
||||
|
||||
it('should handle structured errors without provider field', () => {
|
||||
const error = {
|
||||
message: JSON.stringify({
|
||||
detail: {
|
||||
error_type: 'quota_exhausted',
|
||||
message: 'Usage limit exceeded'
|
||||
}
|
||||
})
|
||||
};
|
||||
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.isProviderError).toBe(true);
|
||||
expect(result.provider).toBe('LLM'); // Default fallback
|
||||
expect(result.errorType).toBe('quota_exhausted');
|
||||
expect(result.message).toBe('Usage limit exceeded');
|
||||
});
|
||||
|
||||
it('should handle partial structured errors', () => {
|
||||
const error = {
|
||||
message: JSON.stringify({
|
||||
detail: {
|
||||
error_type: 'rate_limit'
|
||||
// Missing message field
|
||||
}
|
||||
})
|
||||
};
|
||||
|
||||
const result = parseProviderError(error);
|
||||
|
||||
expect(result.isProviderError).toBe(true);
|
||||
expect(result.errorType).toBe('rate_limit');
|
||||
expect(result.message).toBe(error.message); // Falls back to original message
|
||||
});
|
||||
});
|
||||
|
||||
describe('getProviderErrorMessage', () => {
|
||||
it('should return user-friendly message for authentication_failed', () => {
|
||||
const error: ProviderError = {
|
||||
name: 'Error',
|
||||
message: 'Auth failed',
|
||||
isProviderError: true,
|
||||
provider: 'OpenAI',
|
||||
errorType: 'authentication_failed'
|
||||
};
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('Please verify your OpenAI API key in Settings.');
|
||||
});
|
||||
|
||||
it('should return user-friendly message for quota_exhausted', () => {
|
||||
const error: ProviderError = {
|
||||
name: 'Error',
|
||||
message: 'Quota exceeded',
|
||||
isProviderError: true,
|
||||
provider: 'Google AI',
|
||||
errorType: 'quota_exhausted'
|
||||
};
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('Google AI quota exhausted. Please check your billing settings.');
|
||||
});
|
||||
|
||||
it('should return user-friendly message for rate_limit', () => {
|
||||
const error: ProviderError = {
|
||||
name: 'Error',
|
||||
message: 'Rate limited',
|
||||
isProviderError: true,
|
||||
provider: 'Anthropic',
|
||||
errorType: 'rate_limit'
|
||||
};
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('Anthropic rate limit exceeded. Please wait and try again.');
|
||||
});
|
||||
|
||||
it('should return generic provider message for unknown error types', () => {
|
||||
const error: ProviderError = {
|
||||
name: 'Error',
|
||||
message: 'Unknown error',
|
||||
isProviderError: true,
|
||||
provider: 'OpenAI',
|
||||
errorType: 'unknown_error'
|
||||
};
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('OpenAI API error. Please check your configuration.');
|
||||
});
|
||||
|
||||
it('should use default provider when provider is missing', () => {
|
||||
const error: ProviderError = {
|
||||
name: 'Error',
|
||||
message: 'Auth failed',
|
||||
isProviderError: true,
|
||||
errorType: 'authentication_failed'
|
||||
};
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('Please verify your LLM API key in Settings.');
|
||||
});
|
||||
|
||||
it('should handle 401 status code for non-provider errors', () => {
|
||||
const error = { statusCode: 401, message: 'Unauthorized' };
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('Please verify your API key in Settings.');
|
||||
});
|
||||
|
||||
it('should return original message for non-provider errors', () => {
|
||||
const error = new Error('Network connection failed');
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('Network connection failed');
|
||||
});
|
||||
|
||||
it('should return default message when no message is available', () => {
|
||||
const error = {};
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('An error occurred.');
|
||||
});
|
||||
|
||||
it('should handle complex error objects with structured backend response', () => {
|
||||
const backendError = {
|
||||
statusCode: 400,
|
||||
message: JSON.stringify({
|
||||
detail: {
|
||||
error_type: 'authentication_failed',
|
||||
provider: 'OpenAI',
|
||||
message: 'API key invalid or expired'
|
||||
}
|
||||
})
|
||||
};
|
||||
|
||||
const result = getProviderErrorMessage(backendError);
|
||||
expect(result).toBe('Please verify your OpenAI API key in Settings.');
|
||||
});
|
||||
|
||||
it('should handle edge case: message contains "detail" but is not JSON', () => {
|
||||
const error = {
|
||||
message: 'Error detail: something went wrong'
|
||||
};
|
||||
|
||||
const result = getProviderErrorMessage(error);
|
||||
expect(result).toBe('Error detail: something went wrong');
|
||||
});
|
||||
|
||||
it('should handle null and undefined gracefully', () => {
|
||||
expect(getProviderErrorMessage(null)).toBe('An error occurred.');
|
||||
expect(getProviderErrorMessage(undefined)).toBe('An error occurred.');
|
||||
});
|
||||
});
|
||||
|
||||
describe('TypeScript strict mode compliance', () => {
|
||||
it('should handle type-safe property access', () => {
|
||||
// Test that our type guards work properly
|
||||
const errorWithStatus = { statusCode: 500 };
|
||||
const errorWithMessage = { message: 'test' };
|
||||
const errorWithBoth = { statusCode: 401, message: 'unauthorized' };
|
||||
|
||||
// These should not throw TypeScript errors and should work correctly
|
||||
expect(() => parseProviderError(errorWithStatus)).not.toThrow();
|
||||
expect(() => parseProviderError(errorWithMessage)).not.toThrow();
|
||||
expect(() => parseProviderError(errorWithBoth)).not.toThrow();
|
||||
|
||||
const result1 = parseProviderError(errorWithStatus);
|
||||
const result2 = parseProviderError(errorWithMessage);
|
||||
const result3 = parseProviderError(errorWithBoth);
|
||||
|
||||
expect(result1.statusCode).toBe(500);
|
||||
expect(result2.message).toBe('test');
|
||||
expect(result3.statusCode).toBe(401);
|
||||
expect(result3.message).toBe('unauthorized');
|
||||
});
|
||||
|
||||
it('should handle objects without expected properties safely', () => {
|
||||
const objectWithoutStatus = { someOtherProperty: 'value' };
|
||||
const objectWithoutMessage = { anotherProperty: 42 };
|
||||
|
||||
expect(() => parseProviderError(objectWithoutStatus)).not.toThrow();
|
||||
expect(() => parseProviderError(objectWithoutMessage)).not.toThrow();
|
||||
|
||||
const result1 = parseProviderError(objectWithoutStatus);
|
||||
const result2 = parseProviderError(objectWithoutMessage);
|
||||
|
||||
expect(result1.statusCode).toBeUndefined();
|
||||
expect(result2.message).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user