4df9f61347
* fix: stop generating empty CLAUDE.md files - Return empty string instead of "No recent activity" when no observations exist - Skip writing CLAUDE.md files when formatted content is empty - Remove redundant "auto-generated by claude-mem" HTML comment - Clean up 98 existing empty CLAUDE.md files across the codebase - Update tests to expect empty string for empty input Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * build assets * refactor: implement in-process worker architecture for hooks Replaces spawn-based worker startup with in-process architecture: - Hook processes now become the worker when port 37777 is free - Eliminates Windows spawn issues (NO SPAWN rule) - SessionStart chains: smart-install && stop && context Key changes: - worker-service.ts: hook case starts WorkerService in-process - hook-command.ts: skipExit option prevents process.exit() when hosting worker - hooks.json: single chained command replaces separate start/hook commands - worker-utils.ts: ensureWorkerRunning() returns boolean, doesn't block - handlers: graceful fallback when worker unavailable All 761 tests pass. Manual verification confirms hook stays alive as worker. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * context * a * MAESTRO: Mark PR #722 test verification task complete All 797 tests passed (3 skipped, 0 failed) after merge conflict resolution. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * MAESTRO: Mark PR #722 build verification task complete * MAESTRO: Mark PR #722 code review task complete Code review verified: - worker-service.ts hook case starts WorkerService in-process - hook-command.ts has skipExit option - hooks.json uses single chained command - worker-utils.ts ensureWorkerRunning() returns boolean Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * MAESTRO: Mark PR #722 conflict resolution push task complete Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
137 lines
4.0 KiB
Markdown
137 lines
4.0 KiB
Markdown
# Error Handling Anti-Pattern Rules
|
|
|
|
This folder contains `detect-error-handling-antipatterns.ts` - run it before committing any error handling changes.
|
|
|
|
## The Try-Catch Problem That Cost 10 Hours
|
|
|
|
A single overly-broad try-catch block wasted 10 hours of debugging time by silently swallowing errors.
|
|
**This pattern is BANNED.**
|
|
|
|
## BEFORE You Write Any Try-Catch
|
|
|
|
**RUN THIS TEST FIRST:**
|
|
```bash
|
|
bun run scripts/anti-pattern-test/detect-error-handling-antipatterns.ts
|
|
```
|
|
|
|
**You MUST answer these 5 questions to the user BEFORE writing try-catch:**
|
|
|
|
1. **What SPECIFIC error am I catching?** (Name the error type: `FileNotFoundError`, `NetworkTimeout`, `ValidationError`)
|
|
2. **Show documentation proving this error can occur** (Link to docs or show me the source code)
|
|
3. **Why can't this error be prevented?** (If it can be prevented, prevent it instead)
|
|
4. **What will the catch block DO?** (Must include logging + either rethrow OR explicit fallback)
|
|
5. **Why shouldn't this error propagate?** (Justify swallowing it rather than letting caller handle)
|
|
|
|
**If you cannot answer ALL 5 questions with specifics, DO NOT write the try-catch.**
|
|
|
|
## FORBIDDEN PATTERNS (Zero Tolerance)
|
|
|
|
### CRITICAL - Never Allowed
|
|
|
|
```typescript
|
|
// FORBIDDEN: Empty catch
|
|
try {
|
|
doSomething();
|
|
} catch {}
|
|
|
|
// FORBIDDEN: Catch without logging
|
|
try {
|
|
doSomething();
|
|
} catch (error) {
|
|
return null; // Silent failure!
|
|
}
|
|
|
|
// FORBIDDEN: Large try blocks (>10 lines)
|
|
try {
|
|
// 50 lines of code
|
|
// Multiple operations
|
|
// Different failure modes
|
|
} catch (error) {
|
|
logger.error('Something failed'); // Which thing?!
|
|
}
|
|
|
|
// FORBIDDEN: Promise empty catch
|
|
promise.catch(() => {}); // Error disappears into void
|
|
|
|
// FORBIDDEN: Try-catch to fix TypeScript errors
|
|
try {
|
|
// @ts-ignore
|
|
const value = response.propertyThatDoesntExist;
|
|
} catch {}
|
|
```
|
|
|
|
### ALLOWED Patterns
|
|
|
|
```typescript
|
|
// GOOD: Specific, logged, explicit handling
|
|
try {
|
|
await fetch(url);
|
|
} catch (error) {
|
|
if (error instanceof NetworkError) {
|
|
logger.warn('SYNC', 'Network request failed, will retry', { url }, error);
|
|
return null; // Explicit: null means "fetch failed"
|
|
}
|
|
throw error; // Unexpected errors propagate
|
|
}
|
|
|
|
// GOOD: Minimal scope, clear recovery
|
|
try {
|
|
JSON.parse(data);
|
|
} catch (error) {
|
|
logger.error('CONFIG', 'Corrupt settings file, using defaults', {}, error);
|
|
return DEFAULT_SETTINGS;
|
|
}
|
|
|
|
// GOOD: Fire-and-forget with logging
|
|
backgroundTask()
|
|
.catch(error => logger.warn('BACKGROUND', 'Task failed', {}, error));
|
|
|
|
// GOOD: Ignored anti-pattern for genuine hot paths only
|
|
try {
|
|
checkIfProcessAlive(pid);
|
|
} catch (error) {
|
|
// [ANTI-PATTERN IGNORED]: Tight loop checking 100s of PIDs during cleanup
|
|
return false;
|
|
}
|
|
```
|
|
|
|
## Ignoring Anti-Patterns (Rare)
|
|
|
|
**Only for genuine hot paths** where logging would cause performance problems:
|
|
|
|
```typescript
|
|
// [ANTI-PATTERN IGNORED]: Reason why logging is impossible
|
|
```
|
|
|
|
**Rules:**
|
|
- **Hot paths only** - code in tight loops called 1000s of times
|
|
- If you can add logging, ADD LOGGING - don't ignore
|
|
- Valid examples:
|
|
- "Tight loop checking process exit status during cleanup"
|
|
- "Health check polling every 100ms"
|
|
- Invalid examples:
|
|
- "Expected JSON parse failures" - Just add logger.debug
|
|
- "Common fallback path" - Just add logger.debug
|
|
|
|
## The Meta-Rule
|
|
|
|
**UNCERTAINTY TRIGGERS RESEARCH, NOT TRY-CATCH**
|
|
|
|
When you're unsure if a property exists or a method signature is correct:
|
|
1. **READ** the source code or documentation
|
|
2. **VERIFY** with the Read tool
|
|
3. **USE** TypeScript types to catch errors at compile time
|
|
4. **WRITE** code you KNOW is correct
|
|
|
|
Never use try-catch to paper over uncertainty. That wastes hours of debugging time later.
|
|
|
|
## Critical Path Protection
|
|
|
|
These files are **NEVER** allowed to have catch-and-continue:
|
|
- `SDKAgent.ts` - Errors must propagate, not hide
|
|
- `GeminiAgent.ts` - Must fail loud, not silent
|
|
- `OpenRouterAgent.ts` - Must fail loud, not silent
|
|
- `SessionStore.ts` - Database errors must propagate
|
|
- `worker-service.ts` - Core service errors must be visible
|
|
|
|
On critical paths, prefer **NO TRY-CATCH** and let errors propagate naturally. |