Skip to content

Standardize and enhance error handling across hooks and worker service #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
thedotmack merged 8 commits into from
Dec 14, 2025
Merged

Standardize and enhance error handling across hooks and worker service #295

thedotmack merged 8 commits into from
Dec 14, 2025

Conversation

@thedotmack
Copy link
Owner

@thedotmack thedotmack commented Dec 14, 2025

Summary

This PR comprehensively improves error handling quality across the claude-mem codebase:

  • Standardized hook error handling with shared error handlers for consistent, actionable error messages
  • Enhanced error context with platform-aware restart instructions and detailed debugging information
  • Fixed false error logging bug (issue PostToolUse hook missing cwd field - observations not being saved #260) where happy_path_error__with_fallback misuse caused spurious errors
  • Cleaned up database error logging to remove unnecessary console.error calls
  • Added comprehensive test coverage with 3 new test suites protecting against regression
  • Simplified SDKAgent error handling by removing unnecessary happy_path_error wrappers
  • Created audit documentation cataloging test coverage and code quality improvements

Key Changes

Hook Error Standardization

  • Created src/hooks/shared/error-handler.ts with handleFetchError() and handleWorkerError() utilities
  • Migrated context-hook, new-hook, save-hook, and summary-hook to use standardized handlers
  • Platform-aware error messages (macOS, Linux, Windows) with correct restart commands
  • Consistent error logging with actionable context before throwing restart instructions

ChromaSync Error Messages

  • Standardized client initialization checks across 4 methods (searchSimilar, addDocuments, updateDocument, deleteDocument)
  • Enhanced error messages with troubleshooting steps and restart instructions
  • All errors now include context about which operation failed

Worker Service Improvements

  • Enhanced version endpoint error logging with status codes and response text
  • Improved worker restart error messages with PM2 commands
  • Better context in all worker-related error scenarios

Bug Fixes

  • Issue PostToolUse hook missing cwd field - observations not being saved #260: Fixed happy_path_error__with_fallback misuse in save-hook causing false "Missing cwd" errors
  • Removed unnecessary happy_path_error calls from SDKAgent that were masking real error messages
  • Cleaned up migration logging to use console.log instead of console.error for non-error events

Test Coverage

  • tests/error-handling/hook-error-logging.test.ts - 12 tests validating hook error handler behavior
  • tests/services/chroma-sync-errors.test.ts - Tests for ChromaSync error message consistency
  • tests/integration/hook-execution-environments.test.ts - Tests for Bun PATH resolution across shells
  • docs/context/TEST_AUDIT_2025-12-13.md - Comprehensive audit report documenting coverage improvements

Testing

All 52 existing tests continue to pass. Added 12+ new tests specifically for error handling scenarios.

npm test

Files Changed

27 files changed: 1,435 additions, 200 deletions

Core changes:

  • Hook error handlers: src/hooks/shared/error-handler.ts (new)
  • Hook implementations: context-hook, new-hook, save-hook, summary-hook
  • Service improvements: ChromaSync, worker-service, SessionStore
  • Test additions: 3 new test files with comprehensive coverage

Related Issues

Fixes #260 - False "Missing cwd" errors from happy_path_error misuse

🤖 Generated with Claude Code

thedotmack and others added 8 commits December 13, 2025 21:31
@thedotmack
Enhance error logging in hooks
b26b77c 
- Added detailed error logging in context-hook, new-hook, save-hook, and summary-hook to capture status, project, port, and relevant session information on failures.
- Improved error messages thrown in save-hook and summary-hook to include specific context about the failure.
@thedotmack
Refactor migration logging to use console.log instead of console.error
981bc21 
- Updated SessionSearch and SessionStore classes to replace console.error with console.log for migration-related messages.
- Added notes in the documentation to clarify the use of console.log for migration messages due to the unavailability of the structured logger during constructor execution.
@thedotmack
Refactor SDKAgent and silent-debug utility to simplify error handling
aeade14 
- Updated SDKAgent to use direct defaults instead of happy_path_error__with_fallback for non-critical fields such as last_user_message, last_assistant_message, title, filesRead, filesModified, concepts, and summary.request.
- Enhanced silent-debug documentation to clarify appropriate use cases for happy_path_error__with_fallback, emphasizing its role in handling unexpected null/undefined values while discouraging its use for nullable fields with valid defaults.
@thedotmack @claude
fix: correct happy_path_error__with_fallback usage to prevent false e…
f47eb13 
…rrors

Fixes false "Missing cwd" and "Missing transcript_path" errors that were
flooding silent.log even when values were present.

Root cause: happy_path_error__with_fallback was being called unconditionally
instead of only when the value was actually missing.

Pattern changed from:
  value: happy_path_error__with_fallback('Missing', {}, value || '')

To correct usage:
  value: value || happy_path_error__with_fallback('Missing', {}, '')

Fixed in:
- src/hooks/save-hook.ts (PostToolUse hook)
- src/hooks/summary-hook.ts (Stop hook)
- src/services/worker/http/routes/SessionRoutes.ts (2 instances)

Impact: Eliminates false error noise, making actual errors visible.

Addresses issue #260 - users were seeing "Missing cwd" errors despite
Claude Code correctly passing all required fields.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@thedotmack
Enhance error logging and handling across services
b102d2f 
- Improved error messages in SessionStore to include project context when fetching boundary observations and timestamps.
- Updated ChromaSync error handling to provide more informative messages regarding client initialization failures, including the project context.
- Enhanced error logging in WorkerService to include the package path when reading version fails.
- Added detailed error logging in worker-utils to capture expected and running versions during health checks.
- Extended WorkerErrorMessageOptions to include actualError for more informative restart instructions.
@thedotmack
Refactor error handling in hooks to use standardized fetch error handler
7d75801 
- Introduced a new error handler `handleFetchError` in `shared/error-handler.ts` to standardize logging and user-facing error messages for fetch failures across hooks.
- Updated `context-hook.ts`, `new-hook.ts`, `save-hook.ts`, and `summary-hook.ts` to utilize the new error handler, improving consistency and maintainability.
- Removed redundant imports and error handling logic related to worker restart instructions from the hooks.
@thedotmack
feat: add comprehensive error handling tests for hooks and ChromaSync…
987a9fe 
… client
@thedotmack @claude
Merge main into feature/error-messages
6025be6 
Resolved conflicts by rebuilding plugin scripts from updated source.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@claude
Copy link

claude bot commented Dec 14, 2025

Code Review for PR 295

This is an excellent, comprehensive improvement to error handling across the codebase.

Strengths

  1. Standardized Error Handling Pattern - The new error-handler.ts provides consistent, platform-aware error messages with actionable restart instructions
  2. Bug Fix for Issue 260 - Correctly addresses false Missing cwd errors
  3. Comprehensive Test Coverage - 750+ lines of new regression tests across 3 test files
  4. SDKAgent Cleanup - Removing unnecessary error wrappers improves debugging
  5. Excellent Documentation - TEST_AUDIT_2025-12-13.md provides clear rationale

Areas for Improvement

  1. ChromaSync Duplication - Client initialization check appears 4 times (lines 140-145, 324-329, 504-509, 761-766). Consider extracting to ensureClientInitialized() method
  2. Error Context - The actualError parameter in getWorkerRestartInstructions() exists but is not being used. Including actual error text would improve debugging
  3. Empty Error Responses - handleFetchError should fall back to HTTP status text when errorText is empty

Security & Performance

  • Security: No concerns. No sensitive data exposed, proper input handling
  • Performance: No concerns. Fail-fast error handling, appropriate logging

Checklist Summary

Category Status
Bug Fixes Excellent
Code Quality Excellent
Best Practices Good
Test Coverage Excellent
Documentation Excellent

Final Verdict

APPROVED

This is production-ready and significantly improves codebase reliability. The standardized error handling and comprehensive tests provide excellent regression protection.

Minor suggestions above are optimizations, not blockers. This PR can be merged with confidence.

@thedotmack thedotmack merged commit 52d2f72 into main Dec 14, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PostToolUse hook missing cwd field - observations not being saved

2 participants

@thedotmack