-
Notifications
You must be signed in to change notification settings - Fork 2
Gracefully handle error conditions #22
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors error handling across the codebase to use a structured diagnostic system instead of returning errors directly. The changes enable graceful handling of error conditions by collecting warnings and errors as diagnostics while continuing processing when possible.
- Introduces a new
resultpackage with structured diagnostic types and pretty-printing functionality - Converts schema generation functions to return
SchemaResultobjects containing both schema and diagnostics - Updates command-line interfaces to display diagnostics alongside schema output
Reviewed Changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/result/types.go | Defines new diagnostic types and result structures for schema generation |
| pkg/result/print.go | Implements pretty-printing methods for diagnostics and schema output |
| pkg/schema/types.go | Fixes field name from Comments to Comment for JSON schema compliance |
| pkg/opentofu/tofutoschema.go | Refactors to use diagnostic system instead of early error returns |
| pkg/helm/helmtoschema.go | Converts error handling to use structured diagnostics |
| pkg/bicep/biceptoschema.go | Updates to collect diagnostics instead of returning errors |
| cmd/*.go | Updates CLI commands to use new result types and display diagnostics |
| *_test.go | Updates tests to verify diagnostic collection alongside schema validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if err != nil { | ||
| return nil, fmt.Errorf("failed to convert variable %q to schema: %w", variable.Name, err) | ||
| variableSchema, diags := variableToSchema(variable, result.Diags) | ||
| result.Diags = diags |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The pattern of reassigning the Diags slice could lead to confusion. Consider passing a pointer to the result or using a different pattern to accumulate diagnostics.
| result.Diags = diags | |
| result.Diags = append(result.Diags, diags...) |
coryodaniel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets coordinate on cutting a release w/ the backend changes. There are a few dependent deploys I have to roll through.
Instead of returning errors, I created a
Diagnostictype which contains errors/warnings with context. This will allow users of the library to gracefully handle complications in translating schemas/IaC.For now this just works IaC -> Schema. Eventually this pattern will be also added to the reverse.