Skip to content
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

DO NOT MERGE (needs review and testing). Break error into parts #715

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
7 changes: 6 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ func Execute() error {
if err != nil && !errors.Is(err, cfg.NotFound) {
u.LogErrorAndExit(schema.CliConfiguration{}, err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
u.LogErrorAndExit(schema.CliConfiguration{}, err)
u.LogErrorAndExit(cliConfig, err)

We have cliConfig, let's use it (I missed this line in the previous PR)

}

// Set the log level for ExtendedError interface . must be set on root level to avoid missing log level
if cliConfig.Logs.Level != "" {
if err := u.SetPrintDebugPart(cliConfig.Logs.Level); err != nil {
u.LogErrorAndExit(schema.CliConfiguration{}, err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
u.LogErrorAndExit(schema.CliConfiguration{}, err)
u.LogErrorAndExit(cliConfig, err)

We already have cliConfig, let's use it

}
}
// If CLI configuration was found, process its custom commands and command aliases
if err == nil {
err = processCustomCommands(cliConfig, cliConfig.Commands, RootCmd, true)
Expand Down
1 change: 1 addition & 0 deletions pkg/config/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,5 @@ const (
AtmosProTokenEnvVarName = "ATMOS_PRO_TOKEN"
AtmosProDefaultBaseUrl = "https://app.cloudposse.com"
AtmosProDefaultEndpoint = "api"
CliConfigPrefix = "CLI config:"
)
11 changes: 9 additions & 2 deletions pkg/config/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ func FindAllStackConfigsInPathsForStack(
matches, err = u.GetGlobMatches(pathWithExt)
if err != nil {
y, _ := u.ConvertToYAML(cliConfig)
return nil, nil, false, fmt.Errorf("%v\n\n\nCLI config:\n\n%v", err, y)
return nil, nil, false, &u.ExtendedError{
Message: err.Error(),
DebugInfo: fmt.Sprintf(CliConfigPrefix+"\n\n%v", y),
}
aknysh marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -119,7 +122,11 @@ func FindAllStackConfigsInPaths(
matches, err = u.GetGlobMatches(pathWithExt)
if err != nil {
y, _ := u.ConvertToYAML(cliConfig)
return nil, nil, fmt.Errorf("%v\n\n\nCLI config:\n\n%v", err, y)
// Return an error with the debug info
return nil, nil, &u.ExtendedError{
Message: err.Error(),
DebugInfo: fmt.Sprintf(CliConfigPrefix+"\n\n%v", y),
}
aknysh marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
42 changes: 42 additions & 0 deletions pkg/utils/log_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"runtime/debug"
"sync"

"github.com/fatih/color"

Expand All @@ -19,6 +20,41 @@ const (
LogLevelWarning = "Warning"
)

var (
PrintDebugPart bool = false
mu sync.Mutex
)

// set PrintDebugPart to true if log level is Debug or Trace
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// set PrintDebugPart to true if log level is Debug or Trace
// SetPrintDebugPart .....

In Go, comments must start with the function name

func SetPrintDebugPart(val string) error {
mu.Lock()
haitham911 marked this conversation as resolved.
Show resolved Hide resolved
defer mu.Unlock()
if val == "" {
return errors.New("log level not set")
}
if val == LogLevelDebug || val == LogLevelTrace {
PrintDebugPart = true
} else {
PrintDebugPart = false
}
return nil
}
aknysh marked this conversation as resolved.
Show resolved Hide resolved

// ExtendedError is an error type that includes a message and debug info.
type ExtendedError struct {
Message string // Error message to be printed with log level Error , Warning or Info
aknysh marked this conversation as resolved.
Show resolved Hide resolved
DebugInfo string // Debug info to be printed with log level Debug or Trace
}

// Error returns the error message . If PrintDebugPart is true, it returns the error message and debug info
aknysh marked this conversation as resolved.
Show resolved Hide resolved
func (e *ExtendedError) Error() string {
// Print debug info if PrintDebugPart is true
if PrintDebugPart {
return fmt.Sprintf("%s\n%s", e.Message, e.DebugInfo)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use fmt.Sprintf, Atmos has many helper functions to print info, warning and error in diff colors.

These two parts e.Message and e.DebugInfo should be printed in diff colors (this is the main point of this PR)

}
return e.Message
}
aknysh marked this conversation as resolved.
Show resolved Hide resolved

aknysh marked this conversation as resolved.
Show resolved Hide resolved
// PrintMessage prints the message to the console
func PrintMessage(message string) {
fmt.Println(message)
Expand Down Expand Up @@ -48,6 +84,12 @@ func LogErrorAndExit(cliConfig schema.CliConfiguration, err error) {
// LogError logs errors to std.Error
func LogError(cliConfig schema.CliConfiguration, err error) {
if err != nil {
// set PrintDebugPart to true if log level is Debug or Trace
if cliConfig.Logs.Level != "" {
if setErr := SetPrintDebugPart(cliConfig.Logs.Level); setErr != nil {
color.Red("%s\n", setErr)
}
}
aknysh marked this conversation as resolved.
Show resolved Hide resolved
c := color.New(color.FgRed)
_, printErr := c.Fprintln(color.Error, err.Error()+"\n")
if printErr != nil {
Expand Down
Loading