Log all errors at handling point with contextual information#321
Log all errors at handling point with contextual information#321VioletCorvin merged 7 commits intodevfrom
Conversation
This change ensures all errors are logged at the point where they are handled, including contextual information such as operations and logical stack traces. Key changes: - Added `StackTrace` and `FormatStackTrace` to `pkg/log/errors.go`. - Enhanced `Logger.log` in `pkg/log/log.go` to automatically extract and log `op` and `stack` keys when an error is passed in keyvals. - Updated CLI logging and output helpers to support structured logging. - Updated CLI fatal error handlers to log errors before exiting. - Audited and updated error logging in MCP service (tool handlers and TCP transport), CLI background services (signal and health), and Agentic task handlers.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
This change ensures all errors are logged at the point where they are handled, including contextual information such as operations and logical stack traces. Key changes: - Added `StackTrace` and `FormatStackTrace` to `pkg/log/errors.go`. - Enhanced `Logger.log` in `pkg/log/log.go` to automatically extract and log `op` and `stack` keys when an error is passed in keyvals. - Updated CLI logging and output helpers to support structured logging. - Updated CLI fatal error handlers to log errors before exiting. - Audited and updated error logging in MCP service (tool handlers and TCP transport), CLI background services (signal and health), and Agentic task handlers. - Fixed formatting in `pkg/mcp/mcp.go` and `pkg/io/local/client.go`. - Removed unused `fmt` import in `pkg/cli/runtime.go`.
This change ensures all errors are logged at the point where they are handled, including contextual information such as operations and logical stack traces. Key changes: - Added `StackTrace` and `FormatStackTrace` to `pkg/log/errors.go`. - Enhanced `Logger.log` in `pkg/log/log.go` to automatically extract and log `op` and `stack` keys when an error is passed in keyvals. - Updated CLI logging and output helpers to support structured logging. - Updated CLI fatal error handlers to log errors before exiting. - Audited and updated error logging in MCP service (tool handlers and TCP transport), CLI background services (signal and health), and Agentic task handlers. - Fixed formatting in `pkg/mcp/mcp.go` and `pkg/io/local/client.go`. - Removed unused `fmt` import in `pkg/cli/runtime.go`. - Fixed CI failure in `auto-merge` workflow by providing explicit repository context to the GitHub CLI.
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive error logging at handling points with contextual information, addressing issue #229 from the Error Handling and Recovery Audit. The changes add automatic error context extraction to the core logger and update error handling throughout the codebase to log errors with relevant context such as operation name, file paths, and stack traces.
Changes:
- Added automatic error context extraction to the logger that extracts operation name and stack trace from structured errors
- Added
StackTraceandFormatStackTracehelper functions to build logical operation stack traces from error chains - Updated error logging across MCP, agentic, and CLI packages to log errors at handling points with contextual information
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/log/errors.go | Added StackTrace and FormatStackTrace functions for extracting and formatting logical stack traces from error chains |
| pkg/log/log.go | Implemented automatic error context extraction that adds "op" and "stack" fields when logging errors |
| pkg/log/log_test.go | Added test coverage for automatic error context extraction feature |
| pkg/mcp/transport_tcp.go | Replaced fmt.Fprintf error output with structured log.Error calls including error context |
| pkg/mcp/mcp.go | Added structured error logging for all file operations with relevant context (paths, operations) |
| pkg/agentic/service.go | Added error logging for task failures with relevant context (task type, paths) |
| pkg/cli/runtime.go | Updated reload error logging to use structured logging with error object |
| pkg/cli/output.go | Added logging calls to Error and Warn display functions |
| pkg/cli/log.go | Updated log helper functions to accept variadic key-value pairs |
| pkg/cli/errors.go | Added error logging to all Fatal functions before exiting |
| pkg/cli/daemon.go | Updated health server error logging to use structured logging |
| pkg/io/local/client.go | Minor whitespace cleanup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Can you fix the code reviews? |
Addressed feedback from PR review: - Improved `Fatalf` and other fatal functions in `pkg/cli/errors.go` to use structured logging for the formatted message. - Added direct unit tests for `StackTrace` and `FormatStackTrace` in `pkg/log/errors_test.go`, covering edge cases like plain errors, nil errors, and mixed error chains. - Optimized the automatic context extraction loop in `pkg/log/log.go` by capturing the original length of keyvals. - Fixed a bug in `StackTrace` where operations were duplicated when the error chain included non-`*log.Err` errors. - Fixed formatting and unused imports from previous commits.
Merged changes from dev branch while preserving error logging functionality: - pkg/cli/errors.go: Keep LogError calls and use stderr for output - pkg/cli/output.go: Add new ErrorWrap functions from dev, keep logging - pkg/mcp/transport_tcp.go: Keep descriptive "mcp:" prefixed log messages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Can you fix the code reviews? |
- Simplify Fatalf logging by removing redundant format parameter (the formatted message is already logged as "msg") - Tests for StackTrace/FormatStackTrace edge cases already exist - Loop optimization in pkg/log/log.go already implemented Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Ensure all errors are logged at the handling point with contextual information (operation, stack trace) using structured logging. Added automatic error context extraction to the core logger.
Fixes #229
PR created automatically by Jules for task 15835560617830587069 started by @Snider