From 13b232e061f0d33d365beb9dc28ca72656051552 Mon Sep 17 00:00:00 2001 From: Jesse Simpson Date: Sat, 22 Feb 2025 11:58:11 -0500 Subject: [PATCH] refactor: make use of regexs for err parsing Signed-off-by: Jesse Simpson --- pkg/engine/engine.go | 132 ++++++++++++++------------------------ pkg/engine/engine_test.go | 6 +- 2 files changed, 50 insertions(+), 88 deletions(-) diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index d70e9fdad..48020a521 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -339,67 +339,22 @@ type TraceableError struct { func (t TraceableError) String() string { return t.location + "\n " + t.executedFunction + "\n " + t.message + "\n" } - -func (t TraceableError) ExtractExecutedFunction() (TraceableError, error) { - executionLocationRegex, regexFindErr := regexp.Compile(`executing "[^\"]*" at <[^\<\>]*>:?\s*`) - if regexFindErr != nil { - return t, regexFindErr - } - byteArrayMsg := []byte(t.message) - executionLocations := executionLocationRegex.FindAll(byteArrayMsg, -1) - if len(executionLocations) == 0 { - return t, nil - } - t.executedFunction = string(executionLocations[0]) - t.message = strings.ReplaceAll(t.message, t.executedFunction, "") - return t, nil -} - -func (t TraceableError) FilterLocation() TraceableError { - if strings.Contains(t.message, t.location) { - t.message = strings.ReplaceAll(t.message, t.location, "") - } - return t -} - -func (t TraceableError) FilterUnnecessaryWords() TraceableError { - if strings.Contains(t.message, "template:") { - t.message = strings.TrimSpace(strings.ReplaceAll(t.message, "template:", "")) - } - if strings.HasPrefix(t.message, ": ") { - t.message = strings.TrimSpace(strings.TrimPrefix(t.message, ": ")) - } - return t -} - -// In the process of formatting the error, we want to ensure that the formatted version of the error -// is not losing any necessary information. This function will tokenize and compare the two strings -// and if the formatted error doesn't meet the threshold, it will fallback to the originalErr -func determineIfFormattedErrorIsAcceptable(formattedErr error, originalErr error) error { - formattedErrTokens := strings.Fields(formattedErr.Error()) - originalErrTokens := strings.Fields(originalErr.Error()) - - tokenSet := make(map[string]struct{}) - for _, token := range originalErrTokens { - tokenSet[token] = struct{}{} +func cleanupExecError(filename string, err error) error { + if _, isExecError := err.(template.ExecError); !isExecError { + return err } - matchCount := 0 - for _, token := range formattedErrTokens { - if _, exists := tokenSet[token]; exists { - matchCount++ - } - } + // taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 + // > "template: %s: %s" + // taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=141 + // > "template: %s: executing %q at <%s>: %s" - equivalenceRating := (float64(matchCount) / float64(len(formattedErrTokens))) * 100 - if equivalenceRating >= 80 { - return formattedErr + execErrFmt, compileErr := regexp.Compile(`^template: (?P(?U).+): executing (?P(?U).+) at (?P(?U).+): (?P(?U).+)(?P( template:.*)?)$`) + if compileErr != nil { + return err } - return originalErr -} - -func cleanupExecError(filename string, err error) error { - if _, isExecError := err.(template.ExecError); !isExecError { + execErrFmtWithoutTemplate, compileErr := regexp.Compile(`^template: (?P(?U).+): (?P.*)(?P( template:.*)?)$`) + if compileErr != nil { return err } @@ -424,16 +379,35 @@ func cleanupExecError(filename string, err error) error { if current == nil { break } - tokens = strings.SplitN(current.Error(), ": ", 3) - if len(tokens) == 1 { - // For cases where the error message doesn't contain a colon - location = tokens[0] + + var traceable TraceableError + if execErrFmt.MatchString(current.Error()) { + matches := execErrFmt.FindStringSubmatch(current.Error()) + templateIndex := execErrFmt.SubexpIndex("templateName") + templateName := matches[templateIndex] + functionNameIndex := execErrFmt.SubexpIndex("functionName") + functionName := matches[functionNameIndex] + locationNameIndex := execErrFmt.SubexpIndex("location") + locationName := matches[locationNameIndex] + errMsgIndex := execErrFmt.SubexpIndex("errMsg") + errMsg := matches[errMsgIndex] + traceable = TraceableError{ + location: templateName, + message: errMsg, + executedFunction: "executing " + functionName + " at " + locationName + ":", + } + } else if execErrFmtWithoutTemplate.MatchString(current.Error()) { + matches := execErrFmt.FindStringSubmatch(current.Error()) + templateIndex := execErrFmt.SubexpIndex("templateName") + templateName := matches[templateIndex] + errMsgIndex := execErrFmt.SubexpIndex("errMsg") + errMsg := matches[errMsgIndex] + traceable = TraceableError{ + location: templateName, + message: errMsg, + } } else { - location = tokens[1] - } - traceable := TraceableError{ - location: location, - message: current.Error(), + return err } fileLocations = append(fileLocations, traceable) current = errors.Unwrap(current) @@ -442,30 +416,18 @@ func cleanupExecError(filename string, err error) error { return fmt.Errorf("%s", err.Error()) } - prevMessage := "" + var prev TraceableError for i := len(fileLocations) - 1; i >= 0; i-- { - currentMsg := fileLocations[i].message + current := fileLocations[i] if i == len(fileLocations)-1 { - prevMessage = currentMsg + prev = current continue } - if strings.Contains(currentMsg, prevMessage) { - fileLocations[i].message = strings.ReplaceAll(fileLocations[i].message, prevMessage, "") - } - prevMessage = currentMsg - } - - for i, fileLocation := range fileLocations { - fileLocation = fileLocation.FilterLocation().FilterUnnecessaryWords() - if fileLocation.message == "" { - continue - } - t, extractionErr := fileLocation.ExtractExecutedFunction() - if extractionErr != nil { - continue + if current.message == prev.message && current.location == prev.location && current.executedFunction == prev.executedFunction { + fileLocations[i].message = "" } - fileLocations[i] = t + prev = current } finalErrorString := "" @@ -480,7 +442,7 @@ func cleanupExecError(filename string, err error) error { return fmt.Errorf("%s", err.Error()) } - return determineIfFormattedErrorIsAcceptable(fmt.Errorf("%s", finalErrorString), err) + return fmt.Errorf("%s", finalErrorString) } func sortTemplates(tpls map[string]renderable) []string { diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index a34082e5f..58d8588ab 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1314,13 +1314,13 @@ func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { } expectedErrorMessage := `NestedHelperFunctions/templates/svc.yaml:1:9 - executing "NestedHelperFunctions/templates/svc.yaml" at : + executing "NestedHelperFunctions/templates/svc.yaml" at : error calling include: NestedHelperFunctions/templates/_helpers_1.tpl:1:39 - executing "nested_helper.name" at : + executing "nested_helper.name" at : error calling include: NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:50 - executing "common.names.get_name" at <.Release.Name>: + executing "common.names.get_name" at <.Release.Name>: nil pointer evaluating interface {}.Name `