WIP: Fix error handling #2972
WIP: Fix error handling #2972
Conversation
✅ Deploy Preview for zarf-docs canceled.
|
Codecov ReportAttention: Patch coverage is
|
| @@ -36,8 +36,11 @@ func yamlFormat(attr color.Attribute) string { | |||
| } | |||
|
|
|||
| // ColorPrintYAML pretty prints a yaml file to the console. | |||
| func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) { | |||
| text, _ := goyaml.Marshal(data) | |||
| func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) error { | |||
There was a problem hiding this comment.
Add error handling around input
| @@ -18,10 +18,13 @@ import ( | |||
| "github.com/zarf-dev/zarf/src/pkg/utils" | |||
| ) | |||
|
|
|||
| func (p *Packager) confirmAction(stage string, warnings []string, sbomViewFiles []string) bool { | |||
| func (p *Packager) confirmAction(stage string, warnings []string, sbomViewFiles []string) (bool, error) { | |||
There was a problem hiding this comment.
Needed to be updated to handle errors from ColorPrintYAML. Alternatively we might be able to just quietly error or debug log these if Packager has access to a logger and they're low impact. Very open to feedback here!
There was a problem hiding this comment.
Great question! This is low impact, but we're trying to get rid of all the quiet debugs, unless there is a good reason for it. This is a benign example, but there were plenty of places in the code where we continued because it might not cause a problem and that made implementations confusing and harder to read. If goyaml.Marshall errors there probably is something pretty bad going on, though 99.999% of the time it will be fine unless there's an actual issue in our code.
|
Closing this one out to avoid changes to packager for now. |
Description
Currently this WIP PR adds some error handling within
utils/yaml.go. The intent is to address one or part of the concerns in #2950 & #2951, after understanding the scope of the changes more. I just wanted to address some low hanging fruit that I found in the meantime.Related Issue
Relates to #2953
Checklist before merging