fix: comprehensive error handling improvements and architecture documentation (#522)
* Add enforceable anti-pattern detection for try-catch abuse PROBLEM: - Overly-broad try-catch blocks waste 10+ hours of debugging time - Empty catch blocks silently swallow errors - AI assistants use try-catch to paper over uncertainty instead of doing research SOLUTION: 1. Created detect-error-handling-antipatterns.ts test - Detects empty catch blocks (45 CRITICAL found) - Detects catch without logging (45 CRITICAL total) - Detects large try blocks (>10 lines) - Detects generic catch without type checking - Detects catch-and-continue on critical paths - Exit code 1 if critical issues found 2. Updated CLAUDE.md with MANDATORY ERROR HANDLING RULES - 5-question pre-flight checklist before any try-catch - FORBIDDEN patterns with examples - ALLOWED patterns with examples - Meta-rule: UNCERTAINTY TRIGGERS RESEARCH, NOT TRY-CATCH - Critical path protection list 3. Created comprehensive try-catch audit report - Documents all 96 try-catch blocks in worker service - Identifies critical issue at worker-service.ts:748-750 - Categorizes patterns and provides recommendations This is enforceable via test, not just instructions that can be ignored. Current state: 163 anti-patterns detected (45 critical, 47 high, 71 medium) Next: Fix critical issues identified by test 🤖 Generated with Claude Code Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: add logging to 5 critical empty catch blocks (Wave 1) Wave 1 of error handling cleanup - fixing empty catch blocks that silently swallow errors without any trace. Fixed files: - src/bin/import-xml-observations.ts:80 - Log skipped invalid JSON - src/utils/bun-path.ts:33 - Log when bun not in PATH - src/utils/cursor-utils.ts:44 - Log failed registry reads - src/utils/cursor-utils.ts:149 - Log corrupt MCP config - src/shared/worker-utils.ts:128 - Log failed health checks All catch blocks now have proper logging with context and error details. Progress: 41 → 39 CRITICAL issues remaining 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: add logging to promise catches on critical paths (Wave 2) Wave 2 of error handling cleanup - fixing empty promise catch handlers that silently swallow errors on critical code paths. These are the patterns that caused the 10-hour debugging session. Fixed empty promise catches: - worker-service.ts:642 - Background initialization failures - SDKAgent.ts:372,446 - Session processor errors - GeminiAgent.ts:408,475 - Finalization failures - OpenRouterAgent.ts:451,518 - Finalization failures - SessionManager.ts:289 - Generator promise failures Added justification comments to catch-and-continue blocks: - worker-service.ts:68 - PID file removal (cleanup, non-critical) - worker-service.ts:130 - Cursor context update (non-critical) All promise rejection handlers now log errors with context, preventing silent failures that were nearly impossible to debug. Note: The anti-pattern detector only tracks try-catch blocks, not standalone promise chains. These fixes address the root cause of the original 10-hour debugging session even though the detector count remains unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: add logging and documentation to error handling patterns (Wave 3) Wave 3 of error handling cleanup - comprehensive review and fixes for remaining critical issues identified by the anti-pattern detector. Changes organized by severity: **Wave 3.1: Fixed 2 EMPTY_CATCH blocks** - worker-service.ts:162 - Health check polling now logs failures - worker-service.ts:610 - Process cleanup logs failures **Wave 3.2: Reviewed 12 CATCH_AND_CONTINUE patterns** - Verified all are correct (log errors AND exit/return HTTP errors) - Added justification comment to session recovery (line 829) - All patterns properly notify callers of failures **Wave 3.3: Fixed 29 NO_LOGGING_IN_CATCH issues** Added logging to 16 catch blocks: - UI layer: useSettings.ts, useContextPreview.ts (console logging) - Servers: mcp-server.ts health checks and tool execution - Worker: version fetch, cleanup, config corruption - Routes: error handler, session recovery, settings validation - Services: branch checkout, timeline queries Documented 13 intentional exceptions with comments explaining why: - Hot paths (port checks, process checks in tight loops) - Error accumulation (transcript parser collects for batch retrieval) - Special cases (logger can't log its own failures) - Fallback parsing (JSON parse in optional data structures) All changes follow error handling guidelines from CLAUDE.md: - Appropriate log levels (error/warn/debug) - Context objects with relevant details - Descriptive messages explaining failures - Error extraction pattern for Error instances Progress: 41 → 29 detector warnings Remaining warnings are conservative flags on verified-correct patterns (catch-and-continue blocks that properly log + notify callers). Build verified successful. All error handling now provides visibility for debugging while avoiding excessive logging on hot paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * feat: add queue:clear command to remove failed messages Added functionality to clear failed messages from the observation queue: **Changes:** - PendingMessageStore: Added clearFailed() method to delete failed messages - DataRoutes: Added DELETE /api/pending-queue/failed endpoint - CLI: Created scripts/clear-failed-queue.ts for interactive queue clearing - package.json: Added npm run queue:clear script **Usage:** npm run queue:clear # Interactive - prompts for confirmation npm run queue:clear -- --force # Non-interactive - clears without prompt Failed messages are observations that exceeded max retry count. They remain in the queue for debugging but won't be processed. This command removes them to clean up the queue. Works alongside existing queue:check and queue:process commands to provide complete queue management capabilities. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * feat: add --all flag to queue:clear for complete queue reset Extended queue clearing functionality to support clearing all messages, not just failed ones. **Changes:** - PendingMessageStore: Added clearAll() method to clear pending, processing, and failed - DataRoutes: Added DELETE /api/pending-queue/all endpoint - clear-failed-queue.ts: Added --all flag to clear everything - Updated help text and UI to distinguish between failed-only and all-clear modes **Usage:** npm run queue:clear # Clear failed only (interactive) npm run queue:clear -- --all # Clear ALL messages (interactive) npm run queue:clear -- --all --force # Clear all without confirmation The --all flag provides a complete queue reset, removing pending, processing, and failed messages. Useful when you want a fresh start or need to cancel stuck sessions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * feat: add comprehensive documentation for session ID architecture and validation tests * feat: add logs viewer with clear functionality to UI - Add LogsRoutes API endpoint for fetching and clearing worker logs - Create LogsModal component with auto-refresh and clear button - Integrate logs viewer button into Header component - Add comprehensive CSS styling for logs modal - Logs accessible via new document icon button in header Logs viewer features: - Display last 1000 lines of current day's log file - Auto-refresh toggle (2s interval) - Clear logs button with confirmation - Monospace font for readable log output - Responsive modal design matching existing UI 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: redesign logs as Chrome DevTools-style console drawer Major UX improvements to match Chrome DevTools console: - Convert from modal to bottom drawer that slides up - Move toggle button to bottom-left corner (floating button) - Add draggable resize handle for height adjustment - Use plain monospace font (SF Mono/Monaco/Consolas) instead of Monaspace - Simplify controls with icon-only buttons - Add Console tab UI matching DevTools aesthetic Changes: - Renamed LogsModal to LogsDrawer with drawer implementation - Added resize functionality with mouse drag - Removed logs button from header - Added floating console toggle button in bottom-left - Updated all CSS to match Chrome console styling - Minimum height: 150px, maximum: window height - 100px 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: suppress /api/logs endpoint logging to reduce noise Skip logging GET /api/logs requests in HTTP middleware to prevent log spam from auto-refresh polling (every 2s). Keeps the auto-refresh feature functional while eliminating the repetitive log entries. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: enhance error handling guidelines with approved overrides for justified exceptions --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Alex Newman
Executable
+256
@@ -0,0 +1,256 @@
|
||||
#!/usr/bin/env bun
|
||||
/**
|
||||
* Clear messages from the queue
|
||||
*
|
||||
* Usage:
|
||||
* bun scripts/clear-failed-queue.ts # Clear failed messages (interactive)
|
||||
* bun scripts/clear-failed-queue.ts --all # Clear ALL messages (pending, processing, failed)
|
||||
* bun scripts/clear-failed-queue.ts --force # Non-interactive - clear without prompting
|
||||
*/
|
||||
|
||||
const WORKER_URL = 'http://localhost:37777';
|
||||
|
||||
interface QueueMessage {
|
||||
id: number;
|
||||
session_db_id: number;
|
||||
message_type: string;
|
||||
tool_name: string | null;
|
||||
status: 'pending' | 'processing' | 'failed';
|
||||
retry_count: number;
|
||||
created_at_epoch: number;
|
||||
project: string | null;
|
||||
}
|
||||
|
||||
interface QueueResponse {
|
||||
queue: {
|
||||
messages: QueueMessage[];
|
||||
totalPending: number;
|
||||
totalProcessing: number;
|
||||
totalFailed: number;
|
||||
stuckCount: number;
|
||||
};
|
||||
recentlyProcessed: QueueMessage[];
|
||||
sessionsWithPendingWork: number[];
|
||||
}
|
||||
|
||||
interface ClearResponse {
|
||||
success: boolean;
|
||||
clearedCount: number;
|
||||
}
|
||||
|
||||
async function checkWorkerHealth(): Promise<boolean> {
|
||||
try {
|
||||
const res = await fetch(`${WORKER_URL}/api/health`);
|
||||
return res.ok;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function getQueueStatus(): Promise<QueueResponse> {
|
||||
const res = await fetch(`${WORKER_URL}/api/pending-queue`);
|
||||
if (!res.ok) {
|
||||
throw new Error(`Failed to get queue status: ${res.status}`);
|
||||
}
|
||||
return res.json();
|
||||
}
|
||||
|
||||
async function clearFailedQueue(): Promise<ClearResponse> {
|
||||
const res = await fetch(`${WORKER_URL}/api/pending-queue/failed`, {
|
||||
method: 'DELETE'
|
||||
});
|
||||
if (!res.ok) {
|
||||
throw new Error(`Failed to clear failed queue: ${res.status}`);
|
||||
}
|
||||
return res.json();
|
||||
}
|
||||
|
||||
async function clearAllQueue(): Promise<ClearResponse> {
|
||||
const res = await fetch(`${WORKER_URL}/api/pending-queue/all`, {
|
||||
method: 'DELETE'
|
||||
});
|
||||
if (!res.ok) {
|
||||
throw new Error(`Failed to clear queue: ${res.status}`);
|
||||
}
|
||||
return res.json();
|
||||
}
|
||||
|
||||
function formatAge(epochMs: number): string {
|
||||
const ageMs = Date.now() - epochMs;
|
||||
const minutes = Math.floor(ageMs / 60000);
|
||||
const hours = Math.floor(minutes / 60);
|
||||
const days = Math.floor(hours / 24);
|
||||
|
||||
if (days > 0) return `${days}d ${hours % 24}h ago`;
|
||||
if (hours > 0) return `${hours}h ${minutes % 60}m ago`;
|
||||
return `${minutes}m ago`;
|
||||
}
|
||||
|
||||
async function prompt(question: string): Promise<string> {
|
||||
// Check if we have a TTY for interactive input
|
||||
if (!process.stdin.isTTY) {
|
||||
console.log(question + '(no TTY, use --force flag for non-interactive mode)');
|
||||
return 'n';
|
||||
}
|
||||
|
||||
return new Promise((resolve) => {
|
||||
process.stdout.write(question);
|
||||
process.stdin.setRawMode(false);
|
||||
process.stdin.resume();
|
||||
process.stdin.once('data', (data) => {
|
||||
process.stdin.pause();
|
||||
resolve(data.toString().trim());
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const args = process.argv.slice(2);
|
||||
|
||||
// Help flag
|
||||
if (args.includes('--help') || args.includes('-h')) {
|
||||
console.log(`
|
||||
Claude-Mem Queue Clearer
|
||||
|
||||
Clear messages from the observation queue.
|
||||
|
||||
Usage:
|
||||
bun scripts/clear-failed-queue.ts [options]
|
||||
|
||||
Options:
|
||||
--help, -h Show this help message
|
||||
--all Clear ALL messages (pending, processing, and failed)
|
||||
--force Clear without prompting for confirmation
|
||||
|
||||
Examples:
|
||||
# Clear failed messages interactively
|
||||
bun scripts/clear-failed-queue.ts
|
||||
|
||||
# Clear ALL messages (pending, processing, failed)
|
||||
bun scripts/clear-failed-queue.ts --all
|
||||
|
||||
# Clear without confirmation (non-interactive)
|
||||
bun scripts/clear-failed-queue.ts --force
|
||||
|
||||
# Clear all messages without confirmation
|
||||
bun scripts/clear-failed-queue.ts --all --force
|
||||
|
||||
What is this for?
|
||||
Failed messages are observations that exceeded the maximum retry count.
|
||||
Processing/pending messages may be stuck or unwanted.
|
||||
This command removes them to clean up the queue.
|
||||
|
||||
--all is useful for a complete reset when you want to start fresh.
|
||||
`);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const force = args.includes('--force');
|
||||
const clearAll = args.includes('--all');
|
||||
|
||||
console.log(clearAll
|
||||
? '\n=== Claude-Mem Queue Clearer (ALL) ===\n'
|
||||
: '\n=== Claude-Mem Queue Clearer (Failed) ===\n');
|
||||
|
||||
// Check worker health
|
||||
const healthy = await checkWorkerHealth();
|
||||
if (!healthy) {
|
||||
console.log('Worker is not running. Start it with:');
|
||||
console.log(' cd ~/.claude/plugins/marketplaces/thedotmack && npm run worker:start\n');
|
||||
process.exit(1);
|
||||
}
|
||||
console.log('Worker status: Running\n');
|
||||
|
||||
// Get queue status
|
||||
const status = await getQueueStatus();
|
||||
const { queue } = status;
|
||||
|
||||
console.log('Queue Summary:');
|
||||
console.log(` Pending: ${queue.totalPending}`);
|
||||
console.log(` Processing: ${queue.totalProcessing}`);
|
||||
console.log(` Failed: ${queue.totalFailed}`);
|
||||
console.log('');
|
||||
|
||||
// Check if there are messages to clear
|
||||
const totalToClear = clearAll
|
||||
? queue.totalPending + queue.totalProcessing + queue.totalFailed
|
||||
: queue.totalFailed;
|
||||
|
||||
if (totalToClear === 0) {
|
||||
console.log(clearAll
|
||||
? 'No messages in queue. Nothing to clear.\n'
|
||||
: 'No failed messages in queue. Nothing to clear.\n');
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Show details about messages to clear
|
||||
const messagesToShow = clearAll ? queue.messages : queue.messages.filter(m => m.status === 'failed');
|
||||
if (messagesToShow.length > 0) {
|
||||
console.log(clearAll ? 'Messages to Clear:' : 'Failed Messages:');
|
||||
console.log('─'.repeat(80));
|
||||
|
||||
// Group by session
|
||||
const bySession = new Map<number, QueueMessage[]>();
|
||||
for (const msg of messagesToShow) {
|
||||
const list = bySession.get(msg.session_db_id) || [];
|
||||
list.push(msg);
|
||||
bySession.set(msg.session_db_id, list);
|
||||
}
|
||||
|
||||
for (const [sessionId, messages] of bySession) {
|
||||
const project = messages[0].project || 'unknown';
|
||||
const oldest = Math.min(...messages.map(m => m.created_at_epoch));
|
||||
|
||||
if (clearAll) {
|
||||
const statuses = {
|
||||
pending: messages.filter(m => m.status === 'pending').length,
|
||||
processing: messages.filter(m => m.status === 'processing').length,
|
||||
failed: messages.filter(m => m.status === 'failed').length
|
||||
};
|
||||
console.log(` Session ${sessionId} (${project})`);
|
||||
console.log(` Messages: ${messages.length} total (${statuses.pending} pending, ${statuses.processing} processing, ${statuses.failed} failed)`);
|
||||
console.log(` Age: ${formatAge(oldest)}`);
|
||||
} else {
|
||||
console.log(` Session ${sessionId} (${project})`);
|
||||
console.log(` Messages: ${messages.length} failed`);
|
||||
console.log(` Age: ${formatAge(oldest)}`);
|
||||
}
|
||||
}
|
||||
console.log('─'.repeat(80));
|
||||
console.log('');
|
||||
}
|
||||
|
||||
// Confirm before clearing
|
||||
const clearMessage = clearAll
|
||||
? `Clear ${totalToClear} messages (pending, processing, and failed)?`
|
||||
: `Clear ${queue.totalFailed} failed messages?`;
|
||||
|
||||
if (force) {
|
||||
console.log(`${clearMessage.replace('?', '')}...\n`);
|
||||
} else {
|
||||
const answer = await prompt(`${clearMessage} [y/N]: `);
|
||||
if (answer.toLowerCase() !== 'y') {
|
||||
console.log('\nCancelled. Run with --force to skip confirmation.\n');
|
||||
process.exit(0);
|
||||
}
|
||||
console.log('');
|
||||
}
|
||||
|
||||
// Clear the queue
|
||||
const result = clearAll ? await clearAllQueue() : await clearFailedQueue();
|
||||
|
||||
console.log('Clearing Result:');
|
||||
console.log(` Messages cleared: ${result.clearedCount}`);
|
||||
console.log(` Status: ${result.success ? 'Success' : 'Failed'}\n`);
|
||||
|
||||
if (result.success && result.clearedCount > 0) {
|
||||
console.log(clearAll
|
||||
? 'All messages have been removed from the queue.\n'
|
||||
: 'Failed messages have been removed from the queue.\n');
|
||||
}
|
||||
}
|
||||
|
||||
main().catch(err => {
|
||||
console.error('Error:', err.message);
|
||||
process.exit(1);
|
||||
});
|
||||
@@ -0,0 +1,435 @@
|
||||
#!/usr/bin/env bun
|
||||
/**
|
||||
* Error Handling Anti-Pattern Detector
|
||||
*
|
||||
* Detects try-catch anti-patterns that cause silent failures and debugging nightmares.
|
||||
* Run this before committing code that touches error handling.
|
||||
*
|
||||
* Based on hard-learned lessons: defensive try-catch wastes 10+ hours of debugging time.
|
||||
*/
|
||||
|
||||
import { readFileSync, readdirSync, statSync } from 'fs';
|
||||
import { join, relative } from 'path';
|
||||
|
||||
interface AntiPattern {
|
||||
file: string;
|
||||
line: number;
|
||||
pattern: string;
|
||||
severity: 'CRITICAL' | 'HIGH' | 'MEDIUM' | 'APPROVED_OVERRIDE';
|
||||
description: string;
|
||||
code: string;
|
||||
overrideReason?: string;
|
||||
}
|
||||
|
||||
const CRITICAL_PATHS = [
|
||||
'SDKAgent.ts',
|
||||
'GeminiAgent.ts',
|
||||
'OpenRouterAgent.ts',
|
||||
'SessionStore.ts',
|
||||
'worker-service.ts'
|
||||
];
|
||||
|
||||
function findFilesRecursive(dir: string, pattern: RegExp): string[] {
|
||||
const files: string[] = [];
|
||||
|
||||
const items = readdirSync(dir);
|
||||
for (const item of items) {
|
||||
const fullPath = join(dir, item);
|
||||
const stat = statSync(fullPath);
|
||||
|
||||
if (stat.isDirectory()) {
|
||||
if (!item.startsWith('.') && item !== 'node_modules' && item !== 'dist' && item !== 'plugin') {
|
||||
files.push(...findFilesRecursive(fullPath, pattern));
|
||||
}
|
||||
} else if (pattern.test(item)) {
|
||||
files.push(fullPath);
|
||||
}
|
||||
}
|
||||
|
||||
return files;
|
||||
}
|
||||
|
||||
function detectAntiPatterns(filePath: string, projectRoot: string): AntiPattern[] {
|
||||
const content = readFileSync(filePath, 'utf-8');
|
||||
const lines = content.split('\n');
|
||||
const antiPatterns: AntiPattern[] = [];
|
||||
const relPath = relative(projectRoot, filePath);
|
||||
const isCriticalPath = CRITICAL_PATHS.some(cp => filePath.includes(cp));
|
||||
|
||||
// Track try-catch blocks
|
||||
let inTry = false;
|
||||
let tryStartLine = 0;
|
||||
let tryLines: string[] = [];
|
||||
let braceDepth = 0;
|
||||
let catchStartLine = 0;
|
||||
let catchLines: string[] = [];
|
||||
let inCatch = false;
|
||||
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
const trimmed = line.trim();
|
||||
|
||||
// Detect standalone promise empty catch: .catch(() => {})
|
||||
const emptyPromiseCatch = trimmed.match(/\.catch\s*\(\s*\(\s*\)\s*=>\s*\{\s*\}\s*\)/);
|
||||
if (emptyPromiseCatch) {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: i + 1,
|
||||
pattern: 'PROMISE_EMPTY_CATCH',
|
||||
severity: 'CRITICAL',
|
||||
description: 'Promise .catch() with empty handler - errors disappear into the void.',
|
||||
code: trimmed
|
||||
});
|
||||
}
|
||||
|
||||
// Detect standalone promise catch without logging: .catch(err => ...)
|
||||
const promiseCatchMatch = trimmed.match(/\.catch\s*\(\s*(?:\(\s*)?(\w+)(?:\s*\))?\s*=>/);
|
||||
if (promiseCatchMatch && !emptyPromiseCatch) {
|
||||
// Look ahead up to 10 lines to see if there's logging in the handler body
|
||||
let catchBody = trimmed.substring(promiseCatchMatch.index || 0);
|
||||
let braceCount = (catchBody.match(/{/g) || []).length - (catchBody.match(/}/g) || []).length;
|
||||
|
||||
// Collect subsequent lines if the handler spans multiple lines
|
||||
let lookAhead = 0;
|
||||
while (braceCount > 0 && lookAhead < 10 && i + lookAhead + 1 < lines.length) {
|
||||
lookAhead++;
|
||||
const nextLine = lines[i + lookAhead];
|
||||
catchBody += '\n' + nextLine;
|
||||
braceCount += (nextLine.match(/{/g) || []).length - (nextLine.match(/}/g) || []).length;
|
||||
}
|
||||
|
||||
const hasLogging = catchBody.match(/logger\.(error|warn|debug|info)/) ||
|
||||
catchBody.match(/console\.(error|warn)/);
|
||||
|
||||
if (!hasLogging && lookAhead > 0) { // Only flag if it's actually a multi-line handler
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: i + 1,
|
||||
pattern: 'PROMISE_CATCH_NO_LOGGING',
|
||||
severity: 'CRITICAL',
|
||||
description: 'Promise .catch() without logging - errors are silently swallowed.',
|
||||
code: catchBody.trim().split('\n').slice(0, 5).join('\n')
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Detect try block start
|
||||
if (trimmed.match(/^\s*try\s*{/) || trimmed.match(/}\s*try\s*{/)) {
|
||||
inTry = true;
|
||||
tryStartLine = i + 1;
|
||||
tryLines = [line];
|
||||
braceDepth = 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Track try block content
|
||||
if (inTry && !inCatch) {
|
||||
tryLines.push(line);
|
||||
|
||||
// Count braces to find try block end
|
||||
const openBraces = (line.match(/{/g) || []).length;
|
||||
const closeBraces = (line.match(/}/g) || []).length;
|
||||
braceDepth += openBraces - closeBraces;
|
||||
|
||||
// Found catch
|
||||
if (trimmed.match(/}\s*catch\s*(\(|{)/)) {
|
||||
inCatch = true;
|
||||
catchStartLine = i + 1;
|
||||
catchLines = [line];
|
||||
braceDepth = 1;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
// Track catch block
|
||||
if (inCatch) {
|
||||
catchLines.push(line);
|
||||
|
||||
const openBraces = (line.match(/{/g) || []).length;
|
||||
const closeBraces = (line.match(/}/g) || []).length;
|
||||
braceDepth += openBraces - closeBraces;
|
||||
|
||||
// Catch block ended
|
||||
if (braceDepth === 0) {
|
||||
// Analyze the try-catch block
|
||||
analyzeTryCatchBlock(
|
||||
filePath,
|
||||
relPath,
|
||||
tryStartLine,
|
||||
tryLines,
|
||||
catchStartLine,
|
||||
catchLines,
|
||||
isCriticalPath,
|
||||
antiPatterns
|
||||
);
|
||||
|
||||
// Reset
|
||||
inTry = false;
|
||||
inCatch = false;
|
||||
tryLines = [];
|
||||
catchLines = [];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return antiPatterns;
|
||||
}
|
||||
|
||||
function analyzeTryCatchBlock(
|
||||
filePath: string,
|
||||
relPath: string,
|
||||
tryStartLine: number,
|
||||
tryLines: string[],
|
||||
catchStartLine: number,
|
||||
catchLines: string[],
|
||||
isCriticalPath: boolean,
|
||||
antiPatterns: AntiPattern[]
|
||||
): void {
|
||||
const tryBlock = tryLines.join('\n');
|
||||
const catchBlock = catchLines.join('\n');
|
||||
|
||||
// CRITICAL: Empty catch block
|
||||
const catchContent = catchBlock
|
||||
.replace(/}\s*catch\s*\([^)]*\)\s*{/, '') // Remove catch signature
|
||||
.replace(/}\s*catch\s*{/, '') // Remove catch without param
|
||||
.replace(/}$/, '') // Remove closing brace
|
||||
.trim();
|
||||
|
||||
// Check for comment-only catch blocks
|
||||
const nonCommentContent = catchContent
|
||||
.split('\n')
|
||||
.filter(line => {
|
||||
const t = line.trim();
|
||||
return t && !t.startsWith('//') && !t.startsWith('/*') && !t.startsWith('*');
|
||||
})
|
||||
.join('\n')
|
||||
.trim();
|
||||
|
||||
if (!nonCommentContent || nonCommentContent === '') {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: catchStartLine,
|
||||
pattern: 'EMPTY_CATCH',
|
||||
severity: 'CRITICAL',
|
||||
description: 'Empty catch block - errors are silently swallowed. User will waste hours debugging.',
|
||||
code: catchBlock.trim()
|
||||
});
|
||||
}
|
||||
|
||||
// Check for [APPROVED OVERRIDE] marker
|
||||
const overrideMatch = catchContent.match(/\/\/\s*\[APPROVED OVERRIDE\]:\s*(.+)/i);
|
||||
const overrideReason = overrideMatch?.[1]?.trim();
|
||||
|
||||
// CRITICAL: No logging in catch block (unless explicitly approved)
|
||||
const hasLogging = catchContent.match(/logger\.(error|warn|debug|info)/);
|
||||
const hasConsoleError = catchContent.match(/console\.(error|warn)/);
|
||||
const hasStderr = catchContent.match(/process\.stderr\.write/);
|
||||
const hasThrow = catchContent.match(/throw/);
|
||||
|
||||
if (!hasLogging && !hasConsoleError && !hasStderr && !hasThrow && nonCommentContent) {
|
||||
if (overrideReason) {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: catchStartLine,
|
||||
pattern: 'NO_LOGGING_IN_CATCH',
|
||||
severity: 'APPROVED_OVERRIDE',
|
||||
description: 'Catch block has no logging - approved override.',
|
||||
code: catchBlock.trim(),
|
||||
overrideReason
|
||||
});
|
||||
} else {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: catchStartLine,
|
||||
pattern: 'NO_LOGGING_IN_CATCH',
|
||||
severity: 'CRITICAL',
|
||||
description: 'Catch block has no logging - errors occur invisibly.',
|
||||
code: catchBlock.trim()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// HIGH: Large try block (>10 lines)
|
||||
const significantTryLines = tryLines.filter(line => {
|
||||
const t = line.trim();
|
||||
return t && !t.startsWith('//') && t !== '{' && t !== '}';
|
||||
}).length;
|
||||
|
||||
if (significantTryLines > 10) {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: tryStartLine,
|
||||
pattern: 'LARGE_TRY_BLOCK',
|
||||
severity: 'HIGH',
|
||||
description: `Try block has ${significantTryLines} lines - too broad. Multiple errors lumped together.`,
|
||||
code: `${tryLines.slice(0, 3).join('\n')}\n... (${significantTryLines} lines) ...`
|
||||
});
|
||||
}
|
||||
|
||||
// HIGH: Generic catch without type checking
|
||||
const catchParam = catchBlock.match(/catch\s*\(([^)]+)\)/)?.[1]?.trim();
|
||||
const hasTypeCheck = catchContent.match(/instanceof\s+Error/) ||
|
||||
catchContent.match(/\.name\s*===/) ||
|
||||
catchContent.match(/typeof.*===\s*['"]object['"]/);
|
||||
|
||||
if (catchParam && !hasTypeCheck && nonCommentContent) {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: catchStartLine,
|
||||
pattern: 'GENERIC_CATCH',
|
||||
severity: 'MEDIUM',
|
||||
description: 'Catch block handles all errors identically - no error type discrimination.',
|
||||
code: catchBlock.trim()
|
||||
});
|
||||
}
|
||||
|
||||
// CRITICAL on critical paths: Catch-and-continue
|
||||
if (isCriticalPath && nonCommentContent && !hasThrow) {
|
||||
const hasReturn = catchContent.match(/return/);
|
||||
const continuesExecution = !hasReturn; // If no return/throw, execution continues
|
||||
|
||||
if (continuesExecution && hasLogging) {
|
||||
if (overrideReason) {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: catchStartLine,
|
||||
pattern: 'CATCH_AND_CONTINUE_CRITICAL_PATH',
|
||||
severity: 'APPROVED_OVERRIDE',
|
||||
description: 'Critical path continues after error - approved override.',
|
||||
code: catchBlock.trim(),
|
||||
overrideReason
|
||||
});
|
||||
} else {
|
||||
antiPatterns.push({
|
||||
file: relPath,
|
||||
line: catchStartLine,
|
||||
pattern: 'CATCH_AND_CONTINUE_CRITICAL_PATH',
|
||||
severity: 'CRITICAL',
|
||||
description: 'Critical path continues after error - may cause silent data corruption.',
|
||||
code: catchBlock.trim()
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
function formatReport(antiPatterns: AntiPattern[]): string {
|
||||
const critical = antiPatterns.filter(a => a.severity === 'CRITICAL');
|
||||
const high = antiPatterns.filter(a => a.severity === 'HIGH');
|
||||
const medium = antiPatterns.filter(a => a.severity === 'MEDIUM');
|
||||
const approved = antiPatterns.filter(a => a.severity === 'APPROVED_OVERRIDE');
|
||||
|
||||
if (antiPatterns.length === 0) {
|
||||
return '✅ No error handling anti-patterns detected!\n';
|
||||
}
|
||||
|
||||
let report = '\n';
|
||||
report += '═══════════════════════════════════════════════════════════════\n';
|
||||
report += ' ERROR HANDLING ANTI-PATTERNS DETECTED\n';
|
||||
report += '═══════════════════════════════════════════════════════════════\n\n';
|
||||
report += `Found ${critical.length + high.length + medium.length} anti-patterns:\n`;
|
||||
report += ` 🔴 CRITICAL: ${critical.length}\n`;
|
||||
report += ` 🟠 HIGH: ${high.length}\n`;
|
||||
report += ` 🟡 MEDIUM: ${medium.length}\n`;
|
||||
if (approved.length > 0) {
|
||||
report += ` ⚪ APPROVED OVERRIDES: ${approved.length}\n`;
|
||||
}
|
||||
report += '\n';
|
||||
|
||||
if (critical.length > 0) {
|
||||
report += '🔴 CRITICAL ISSUES (Fix immediately - these cause silent failures):\n';
|
||||
report += '─────────────────────────────────────────────────────────────\n\n';
|
||||
for (const ap of critical) {
|
||||
report += `📁 ${ap.file}:${ap.line}\n`;
|
||||
report += `❌ ${ap.pattern}\n`;
|
||||
report += ` ${ap.description}\n\n`;
|
||||
report += ` Code:\n`;
|
||||
const codeLines = ap.code.split('\n');
|
||||
for (const line of codeLines.slice(0, 5)) {
|
||||
report += ` ${line}\n`;
|
||||
}
|
||||
if (codeLines.length > 5) {
|
||||
report += ` ... (${codeLines.length - 5} more lines)\n`;
|
||||
}
|
||||
report += '\n';
|
||||
}
|
||||
}
|
||||
|
||||
if (high.length > 0) {
|
||||
report += '🟠 HIGH PRIORITY:\n';
|
||||
report += '─────────────────────────────────────────────────────────────\n\n';
|
||||
for (const ap of high) {
|
||||
report += `📁 ${ap.file}:${ap.line} - ${ap.pattern}\n`;
|
||||
report += ` ${ap.description}\n\n`;
|
||||
}
|
||||
}
|
||||
|
||||
if (medium.length > 0) {
|
||||
report += '🟡 MEDIUM PRIORITY:\n';
|
||||
report += '─────────────────────────────────────────────────────────────\n\n';
|
||||
for (const ap of medium) {
|
||||
report += `📁 ${ap.file}:${ap.line} - ${ap.pattern}\n`;
|
||||
report += ` ${ap.description}\n\n`;
|
||||
}
|
||||
}
|
||||
|
||||
if (approved.length > 0) {
|
||||
report += '⚪ APPROVED OVERRIDES (Review reasons for accuracy):\n';
|
||||
report += '─────────────────────────────────────────────────────────────\n\n';
|
||||
for (const ap of approved) {
|
||||
report += `📁 ${ap.file}:${ap.line} - ${ap.pattern}\n`;
|
||||
report += ` Reason: ${ap.overrideReason}\n`;
|
||||
report += ` Code:\n`;
|
||||
const codeLines = ap.code.split('\n');
|
||||
for (const line of codeLines.slice(0, 3)) {
|
||||
report += ` ${line}\n`;
|
||||
}
|
||||
if (codeLines.length > 3) {
|
||||
report += ` ... (${codeLines.length - 3} more lines)\n`;
|
||||
}
|
||||
report += '\n';
|
||||
}
|
||||
}
|
||||
|
||||
report += '═══════════════════════════════════════════════════════════════\n';
|
||||
report += 'REMINDER: Every try-catch must answer these questions:\n';
|
||||
report += '1. What SPECIFIC error am I catching? (Name it)\n';
|
||||
report += '2. Show me documentation proving this error can occur\n';
|
||||
report += '3. Why can\'t this error be prevented?\n';
|
||||
report += '4. What will the catch block DO? (Log + rethrow? Fallback?)\n';
|
||||
report += '5. Why shouldn\'t this error propagate to the caller?\n';
|
||||
report += '\n';
|
||||
report += 'To approve an anti-pattern, add: // [APPROVED OVERRIDE]: reason\n';
|
||||
report += '═══════════════════════════════════════════════════════════════\n\n';
|
||||
|
||||
return report;
|
||||
}
|
||||
|
||||
// Main execution
|
||||
const projectRoot = process.cwd();
|
||||
const srcDir = join(projectRoot, 'src');
|
||||
|
||||
console.log('🔍 Scanning for error handling anti-patterns...\n');
|
||||
|
||||
const tsFiles = findFilesRecursive(srcDir, /\.ts$/);
|
||||
console.log(`Found ${tsFiles.length} TypeScript files\n`);
|
||||
|
||||
let allAntiPatterns: AntiPattern[] = [];
|
||||
|
||||
for (const file of tsFiles) {
|
||||
const patterns = detectAntiPatterns(file, projectRoot);
|
||||
allAntiPatterns = allAntiPatterns.concat(patterns);
|
||||
}
|
||||
|
||||
const report = formatReport(allAntiPatterns);
|
||||
console.log(report);
|
||||
|
||||
// Exit with error code if critical issues found
|
||||
const critical = allAntiPatterns.filter(a => a.severity === 'CRITICAL');
|
||||
if (critical.length > 0) {
|
||||
console.error(`❌ FAILED: ${critical.length} critical error handling anti-patterns must be fixed.\n`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
process.exit(0);
|
||||
Reference in New Issue
Block a user