feat: Display ErrorBuilder enrichments in early startup errors#1892
feat: Display ErrorBuilder enrichments in early startup errors#1892
Conversation
…r enrichments Add structured plain text error formatting to display ErrorBuilder enrichments (hints, explanations, context) even when markdown renderer is not initialized. This fixes profile not found errors during config loading to show which profile is missing and how to configure it. Before: 'Error: profile not found' After: Displays profile name, search paths, hints, and context
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
📝 WalkthroughWalkthroughAdds structured plain error printing functions to render error explanations, hints, and context when the Markdown renderer is unavailable or fails. Updates fallback logic and adds tests covering enriched and simple error outputs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
🧠 Learnings (18)📓 Common learnings📚 Learning: 2025-06-02T14:12:02.710ZApplied to files:
📚 Learning: 2025-06-02T14:12:02.710ZApplied to files:
📚 Learning: 2025-11-24T17:35:37.209ZApplied to files:
📚 Learning: 2025-11-24T17:35:37.209ZApplied to files:
📚 Learning: 2025-02-03T06:00:11.419ZApplied to files:
📚 Learning: 2024-10-12T18:38:28.458ZApplied to files:
📚 Learning: 2025-12-16T18:20:55.630ZApplied to files:
📚 Learning: 2024-11-30T22:07:08.610ZApplied to files:
📚 Learning: 2024-12-02T21:26:32.337ZApplied to files:
📚 Learning: 2025-01-07T20:38:09.618ZApplied to files:
📚 Learning: 2025-09-30T19:03:50.738ZApplied to files:
📚 Learning: 2024-10-23T21:36:40.262ZApplied to files:
📚 Learning: 2024-12-17T07:08:41.288ZApplied to files:
📚 Learning: 2025-12-13T06:10:13.688ZApplied to files:
📚 Learning: 2025-12-16T18:20:55.630ZApplied to files:
📚 Learning: 2025-11-24T17:35:37.209ZApplied to files:
📚 Learning: 2025-02-03T06:00:11.419ZApplied to files:
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
errors/error_funcs.go(5 hunks)errors/error_funcs_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: NEVER use fmt.Fprintf(os.Stdout/Stderr) or fmt.Println(); use data.* or ui.* functions instead
All comments must end with periods (enforced by godot linter)
Organize imports in three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages; maintain aliases: cfg, log, u, errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions for performance tracking; use nil if no atmosConfig param
All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation; never create manual mocks
Keep files small...
Files:
errors/error_funcs_test.goerrors/error_funcs.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Prefer unit tests with mocks over integration tests; use interfaces + dependency injection for testability; generate mocks with go.uber.org/mock/mockgen; use table-driven tests; target >80% coverage
Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Files:
errors/error_funcs_test.go
🧠 Learnings (12)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning the error instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions.
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*_test.go : Test behavior, not implementation; never test stub functions; avoid tautological tests; make code testable via DI; no coverage theater; remove always-skipped tests; use errors.Is() for error checking
Applied to files:
errors/error_funcs_test.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning the error instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions.
Applied to files:
errors/error_funcs_test.goerrors/error_funcs.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
errors/error_funcs_test.goerrors/error_funcs.go
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: Commands should use `PrintErrorMarkdownAndExit` with empty title and suggestion (`"", err, ""`) for general error handling. Specific titles like "Invalid Usage" or "File Not Found" should only be used for validation or specific error scenarios.
Applied to files:
errors/error_funcs_test.goerrors/error_funcs.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
errors/error_funcs_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errors
Applied to files:
errors/error_funcs_test.goerrors/error_funcs.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
Applied to files:
errors/error_funcs.go
📚 Learning: 2024-10-12T18:38:28.458Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 715
File: pkg/logger/logger.go:21-32
Timestamp: 2024-10-12T18:38:28.458Z
Learning: In Go code, avoid prefixing struct names with 'I' (e.g., `IError`) as it can be confusing, suggesting an interface. Use descriptive names for structs to improve code clarity.
Applied to files:
errors/error_funcs.go
📚 Learning: 2025-12-16T18:20:55.630Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to **/*.go : All errors MUST be wrapped using static errors defined in errors/errors.go; use errors.Join for combining multiple errors; use fmt.Errorf with %w for adding string context; use error builder for complex errors; use errors.Is() for error checking; NEVER use dynamic errors directly
Applied to files:
errors/error_funcs.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
errors/error_funcs.go
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: The `describe config` command should use `PrintErrorMarkdownAndExit` with empty title and suggestion for consistency with other commands.
Applied to files:
errors/error_funcs.go
🧬 Code graph analysis (1)
errors/error_funcs_test.go (3)
errors/builder.go (1)
Build(24-37)errors/errors.go (1)
ErrProfileNotFound(208-208)errors/exit_code.go (1)
WithExitCode(40-48)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Summary
🔇 Additional comments (5)
errors/error_funcs.go (4)
9-9: LGTM - Import addition necessary for enrichments extraction.The
github.com/cockroachdb/errorsimport is required forGetAllDetails,GetAllHints, andGetAllSafeDetailsused in the new structured plain error helpers.
93-118: LGTM - Structured plain error formatting looks solid.The function correctly defaults the title to "Error", delegates to helper functions for each enrichment type, and preserves backward compatibility with the legacy suggestion parameter. The structured output will provide much better visibility for early startup errors.
120-148: LGTM - Error details and hints extraction is well-structured.Both
printErrorDetailsandprintErrorHintscorrectly check for empty results before printing section headers. The filtering of internal prefixes (TITLE:, EXAMPLE:) in hints is appropriate.
283-289: LGTM - Fallback behavior correctly updated.The updated calls to
printStructuredPlainErrorensure ErrorBuilder enrichments are displayed even when the markdown renderer is unavailable. This addresses the PR objective of surfacing rich error information during early startup.Also applies to: 297-299
errors/error_funcs_test.go (1)
305-413: LGTM - Comprehensive test coverage for structured plain error output.The test suite validates all critical paths:
- ErrorBuilder enrichments (explanations, hints, context) are correctly extracted and displayed
- Simple errors without enrichments fall back gracefully with title and suggestion
- Default title behavior works when title is empty
- Proper stderr capture/restore pattern throughout
The assertions check both positive cases (enrichment sections present) and negative cases (sections absent for simple errors), which is exactly right.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1892 +/- ##
==========================================
+ Coverage 73.19% 73.20% +0.01%
==========================================
Files 609 609
Lines 56883 56933 +50
==========================================
+ Hits 41634 41680 +46
- Misses 12312 12315 +3
- Partials 2937 2938 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Address CodeRabbit feedback about fragile space-based parsing in extractContextPairs. The SafeDetails format is established by ErrorBuilder.WithContext() which uses space-separated key=value pairs. This is a known limitation - values containing spaces won't parse correctly. Changes: - Document the space limitation in function comments - Add TrimSpace for cleaner parsing - Add empty string check for robustness
|
These changes were released in v1.203.0-test.1. |
what
why
references
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.