Skip to content

Commit

Permalink
Rework error and debug handling
Browse files Browse the repository at this point in the history
- Do not silence Cobra errors
- Remove stdlib log use
- Make debug logging consistent with legacy CLI format
- Consistently use Cobra command output writers, rather than bypassing them with color.Error
  • Loading branch information
pjcdawkins committed Jan 3, 2025
1 parent 278d5ad commit c776605
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 74 deletions.
11 changes: 5 additions & 6 deletions cmd/platform/main.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package main

import (
"log"
"fmt"
"os"
"strings"

"github.com/fatih/color"
"github.com/spf13/cobra"
"github.com/spf13/viper"

Expand All @@ -14,16 +13,16 @@ import (
)

func main() {
log.SetOutput(color.Error)

// Load configuration.
cnfYAML, err := config.LoadYAML()
if err != nil {
log.Fatal(err)
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
cnf, err := config.FromYAML(cnfYAML)
if err != nil {
log.Fatal(err)
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}

// When Cobra starts, load Viper config from the environment.
Expand Down
13 changes: 7 additions & 6 deletions commands/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"path"
"strings"

"github.com/fatih/color"
"github.com/spf13/cobra"
"github.com/spf13/viper"

Expand All @@ -19,9 +18,10 @@ import (

func newCompletionCommand(cnf *config.Config) *cobra.Command {
return &cobra.Command{
Use: "completion",
Short: "Print the completion script for your shell",
Args: cobra.MaximumNArgs(1),
Use: "completion",
Short: "Print the completion script for your shell",
Args: cobra.MaximumNArgs(1),
SilenceErrors: true,
Run: func(cmd *cobra.Command, args []string) {
completionArgs := []string{"_completion", "-g", "--program", cnf.Application.Executable}
if len(args) > 0 {
Expand All @@ -33,20 +33,21 @@ func newCompletionCommand(cnf *config.Config) *cobra.Command {
Version: version,
CustomPharPath: viper.GetString("phar-path"),
Debug: viper.GetBool("debug"),
DebugLogFunc: debugLog,
DisableInteraction: viper.GetBool("no-interaction"),
Stdout: &b,
Stderr: cmd.ErrOrStderr(),
Stdin: cmd.InOrStdin(),
}

if err := c.Init(); err != nil {
debugLog("%s\n", color.RedString(err.Error()))
debugLog(err.Error())
os.Exit(1)
return
}

if err := c.Exec(cmd.Context(), completionArgs...); err != nil {
debugLog("%s\n", color.RedString(err.Error()))
debugLog(err.Error())
exitCode := 1
var execErr *exec.ExitError
if errors.As(err, &execErr) {
Expand Down
30 changes: 6 additions & 24 deletions commands/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@ package commands
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"os"
"os/exec"

"github.com/fatih/color"
"github.com/spf13/cobra"
"github.com/spf13/viper"

Expand All @@ -28,14 +24,14 @@ func newListCommand(cnf *config.Config) *cobra.Command {
Version: version,
CustomPharPath: viper.GetString("phar-path"),
Debug: viper.GetBool("debug"),
DebugLogFunc: debugLog,
DisableInteraction: viper.GetBool("no-interaction"),
Stdout: &b,
Stderr: cmd.ErrOrStderr(),
Stdin: cmd.InOrStdin(),
}
if err := c.Init(); err != nil {
debugLog("%s\n", color.RedString(err.Error()))
os.Exit(1)
exitWithError(cmd, err)
return
}

Expand All @@ -47,20 +43,13 @@ func newListCommand(cnf *config.Config) *cobra.Command {
arguments = append(arguments, args[0])
}
if err := c.Exec(cmd.Context(), arguments...); err != nil {
debugLog("%s\n", color.RedString(err.Error()))
exitCode := 1
var execErr *exec.ExitError
if errors.As(err, &execErr) {
exitCode = execErr.ExitCode()
}
os.Exit(exitCode)
exitWithError(cmd, err)
return
}

var list List
if err := json.Unmarshal(b.Bytes(), &list); err != nil {
debugLog("%s\n", color.RedString(err.Error()))
os.Exit(1)
exitWithError(cmd, err)
return
}

Expand Down Expand Up @@ -99,21 +88,14 @@ func newListCommand(cnf *config.Config) *cobra.Command {
c.Stdout = cmd.OutOrStdout()
arguments := []string{"list", "--format=" + format}
if err := c.Exec(cmd.Context(), arguments...); err != nil {
debugLog("%s\n", color.RedString(err.Error()))
exitCode := 1
var execErr *exec.ExitError
if errors.As(err, &execErr) {
exitCode = execErr.ExitCode()
}
os.Exit(exitCode)
exitWithError(cmd, err)
}
return
}

result, err := formatter.Format(&list, config.FromContext(cmd.Context()))
if err != nil {
debugLog("%s\n", color.RedString(err.Error()))
os.Exit(1)
exitWithError(cmd, err)
return
}

Expand Down
62 changes: 37 additions & 25 deletions commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"io"
"log"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -51,11 +50,19 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
DisableFlagParsing: false,
FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true},
SilenceUsage: true,
SilenceErrors: true,
SilenceErrors: false,
PersistentPreRun: func(cmd *cobra.Command, _ []string) {
if viper.GetBool("quiet") && !viper.GetBool("debug") && !viper.GetBool("verbose") {
viper.Set("no-interaction", true)
cmd.SetErr(io.Discard)
} else {
// Ensure the command's output writers can handle colors.
if cmd.OutOrStdout() == os.Stdout {
cmd.SetOut(color.Output)
}
if cmd.ErrOrStderr() == os.Stderr {
cmd.SetErr(color.Error)
}
}
if viper.GetBool("version") {
versionCommand.Run(cmd, []string{})
Expand All @@ -74,32 +81,26 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
Version: version,
CustomPharPath: viper.GetString("phar-path"),
Debug: viper.GetBool("debug"),
DebugLogFunc: debugLog,
DisableInteraction: viper.GetBool("no-interaction"),
Stdout: cmd.OutOrStdout(),
Stderr: cmd.ErrOrStderr(),
Stdin: cmd.InOrStdin(),
}
if err := c.Init(); err != nil {
log.Println(color.RedString(err.Error()))
os.Exit(1)
exitWithError(cmd, err)
return
}

if err := c.Exec(cmd.Context(), os.Args[1:]...); err != nil {
debugLog("%s\n", color.RedString(err.Error()))
exitCode := 1
var execErr *exec.ExitError
if errors.As(err, &execErr) {
exitCode = execErr.ExitCode()
}
os.Exit(exitCode)
exitWithError(cmd, err)
}
},
PersistentPostRun: func(_ *cobra.Command, _ []string) {
checkShellConfigLeftovers(cnf)
PersistentPostRun: func(cmd *cobra.Command, _ []string) {
checkShellConfigLeftovers(cmd.ErrOrStderr(), cnf)
select {
case rel := <-updateMessageChan:
printUpdateMessage(rel, cnf)
printUpdateMessage(cmd.ErrOrStderr(), rel, cnf)
default:
}
},
Expand Down Expand Up @@ -160,7 +161,7 @@ func newRootCommand(cnf *config.Config, assets *vendorization.VendorAssets) *cob
}

// checkShellConfigLeftovers checks .zshrc and .bashrc for any leftovers from the legacy CLI
func checkShellConfigLeftovers(cnf *config.Config) {
func checkShellConfigLeftovers(w io.Writer, cnf *config.Config) {
start := fmt.Sprintf("# BEGIN SNIPPET: %s configuration", cnf.Application.Name)
end := "# END SNIPPET"
shellConfigSnippet := regexp.MustCompile(regexp.QuoteMeta(start) + "(?s).+?" + regexp.QuoteMeta(end))
Expand All @@ -186,23 +187,23 @@ func checkShellConfigLeftovers(cnf *config.Config) {
}

if shellConfigSnippet.Match(shellConfig) {
fmt.Fprintf(color.Error, "%s Your %s file contains code that is no longer needed for the New %s\n",
color.YellowString("Warning:"),
fmt.Fprintf(w, "%s Your %s file contains code that is no longer needed for the New %s\n",
color.YellowString("Notice:"),
shellConfigFile,
cnf.Application.Name,
)
fmt.Fprintf(color.Error, "%s %s\n", color.YellowString("Please remove the following lines from:"), shellConfigFile)
fmt.Fprintf(color.Error, "\t%s\n", strings.ReplaceAll(string(shellConfigSnippet.Find(shellConfig)), "\n", "\n\t"))
fmt.Fprintf(w, "%s %s\n", color.YellowString("Please remove the following lines from:"), shellConfigFile)
fmt.Fprintf(w, "\t%s\n", strings.ReplaceAll(string(shellConfigSnippet.Find(shellConfig)), "\n", "\n\t"))
}
}
}

func printUpdateMessage(newRelease *internal.ReleaseInfo, cnf *config.Config) {
func printUpdateMessage(w io.Writer, newRelease *internal.ReleaseInfo, cnf *config.Config) {
if newRelease == nil {
return
}

fmt.Fprintf(color.Error, "\n\n%s %s → %s\n",
fmt.Fprintf(w, "\n\n%s %s → %s\n",
color.YellowString(fmt.Sprintf("A new release of the %s is available:", cnf.Application.Name)),
color.CyanString(version),
color.CyanString(newRelease.Version),
Expand All @@ -211,19 +212,19 @@ func printUpdateMessage(newRelease *internal.ReleaseInfo, cnf *config.Config) {
executable, err := os.Executable()
if err == nil && cnf.Wrapper.HomebrewTap != "" && isUnderHomebrew(executable) {
fmt.Fprintf(
color.Error,
w,
"To upgrade, run: brew update && brew upgrade %s\n",
color.YellowString(cnf.Wrapper.HomebrewTap),
)
} else if cnf.Wrapper.GitHubRepo != "" {
fmt.Fprintf(
color.Error,
w,
"To upgrade, follow the instructions at: https://github.com/%s#upgrade\n",
cnf.Wrapper.GitHubRepo,
)
}

fmt.Fprintf(color.Error, "%s\n\n", color.YellowString(newRelease.URL))
fmt.Fprintf(w, "%s\n\n", color.YellowString(newRelease.URL))
}

func isUnderHomebrew(binary string) bool {
Expand All @@ -246,5 +247,16 @@ func debugLog(format string, v ...any) {
return
}

log.Printf(format, v...)
prefix := color.New(color.ReverseVideo).Sprintf("DEBUG")
fmt.Fprintf(color.Error, prefix+" "+strings.TrimSpace(format)+"\n", v...)
}

func exitWithError(cmd *cobra.Command, err error) {
cmd.PrintErrln(color.RedString(err.Error()))
exitCode := 1
var execErr *exec.ExitError
if errors.As(err, &execErr) {
exitCode = execErr.ExitCode()
}
os.Exit(exitCode)
}
25 changes: 12 additions & 13 deletions internal/legacy/legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
_ "embed"
"fmt"
"io"
"log"
"os"
"os/exec"
"path"
Expand Down Expand Up @@ -77,6 +76,13 @@ type CLIWrapper struct {
CustomPharPath string
Debug bool
DisableInteraction bool
DebugLogFunc func(string, ...any)
}

func (c *CLIWrapper) debug(msg string, args ...any) {
if c.DebugLogFunc != nil {
c.DebugLogFunc(msg, args...)
}
}

func (c *CLIWrapper) cacheDir() string {
Expand All @@ -86,7 +92,7 @@ func (c *CLIWrapper) cacheDir() string {
// Init the CLI wrapper, creating a temporary directory and copying over files
func (c *CLIWrapper) Init() error {
if _, err := os.Stat(c.cacheDir()); os.IsNotExist(err) {
c.debugLog("cache directory does not exist, creating: %s", c.cacheDir())
c.debug("Cache directory does not exist, creating: %s", c.cacheDir())
if err := os.Mkdir(c.cacheDir(), 0o700); err != nil {
return fmt.Errorf("could not create temporary directory: %w", err)
}
Expand All @@ -95,7 +101,7 @@ func (c *CLIWrapper) Init() error {
if err := fileLock.Lock(); err != nil {
return fmt.Errorf("could not acquire lock: %w", err)
}
c.debugLog("lock acquired: %s", fileLock.Path())
c.debug("Lock acquired: %s", fileLock.Path())
//nolint:errcheck
defer fileLock.Unlock()

Expand All @@ -104,7 +110,7 @@ func (c *CLIWrapper) Init() error {
return fmt.Errorf("legacy CLI phar file not found: %w", err)
}

c.debugLog("phar file does not exist, copying: %s", c.PharPath())
c.debug("Phar file does not exist, copying: %s", c.PharPath())
if err := copyFile(c.PharPath(), phar); err != nil {
return fmt.Errorf("could not copy phar file: %w", err)
}
Expand All @@ -126,7 +132,7 @@ func (c *CLIWrapper) Init() error {
}

if _, err := os.Stat(c.PHPPath()); os.IsNotExist(err) {
c.debugLog("PHP binary does not exist, copying: %s", c.PHPPath())
c.debug("PHP binary does not exist, copying: %s", c.PHPPath())
if err := c.copyPHP(); err != nil {
return fmt.Errorf("could not copy files: %w", err)
}
Expand Down Expand Up @@ -183,7 +189,7 @@ func (c *CLIWrapper) Exec(ctx context.Context, args ...string) error {
))
if err := cmd.Run(); err != nil {
// Cleanup cache directory
c.debugLog("removing cache directory: %s", c.cacheDir())
c.debug("Removing cache directory: %s", c.cacheDir())
os.RemoveAll(c.cacheDir())
return fmt.Errorf("could not run legacy CLI command: %w", err)
}
Expand All @@ -204,10 +210,3 @@ func (c *CLIWrapper) PharPath() string {
func (c *CLIWrapper) ConfigPath() string {
return path.Join(c.cacheDir(), "config.yaml")
}

// debugLog logs a debugging message, if debug is enabled
func (c *CLIWrapper) debugLog(msg string, v ...any) {
if c.Debug {
log.Printf(msg, v...)
}
}

0 comments on commit c776605

Please sign in to comment.