Refactoring 'debug' commands

arduino · cmaglie · Aug 30, 2021 · Feb 10, 2021 · Jun 25, 2021 · Jun 30, 2021
commit 01951114048c1f689fc8aa023bf851d1b478cc9f
diff --git a/cli/debug/debug.go b/cli/debug/debug.go
@@ -100,8 +100,8 @@ func run(command *cobra.Command, args []string) {
 	if printInfo {

 		if res, err := debug.GetDebugConfig(context.Background(), debugConfigRequested); err != nil {
-			feedback.Errorf(tr("Error getting Debug info: %v"), err.Message())
-			errorcodes.ExitWithGrpcStatus(err)
+			feedback.Errorf(tr("Error getting Debug info: %v"), err)
+			os.Exit(errorcodes.ErrBadArgument)
 		} else {
 			feedback.PrintResult(&debugInfoResult{res})
 		}

diff --git a/cli/errorcodes/errorcodes.go b/cli/errorcodes/errorcodes.go
@@ -15,13 +15,6 @@

 package errorcodes

-import (
-	"os"
-
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
-)
-
 // Error codes to be used for os.Exit().
 const (
 	_          = iota // 0 is not a valid exit error code
@@ -36,14 +29,3 @@ const (
 	ErrCoreConfig
 	ErrBadArgument
 )
-
-// ExitWithGrpcStatus will terminate the current process by returing the correct
-// error code closest matching the gRPC status.
-func ExitWithGrpcStatus(s *status.Status) {
-	switch s.Code() {
-	case codes.Unimplemented:
-		os.Exit(ErrBadArgument)
-	default:
-		os.Exit(ErrGeneric)
-	}
-}
diff --git a/commands/daemon/debug.go b/commands/daemon/debug.go
@@ -50,7 +50,7 @@ func (s *DebugService) Debug(stream dbg.DebugService_DebugServer) error {
 	// Launch debug recipe attaching stdin and out to grpc streaming
 	signalChan := make(chan os.Signal)
 	defer close(signalChan)
-	resp, stat := cmd.Debug(stream.Context(), req,
+	resp, debugErr := cmd.Debug(stream.Context(), req,
 		utils.ConsumeStreamFrom(func() ([]byte, error) {
 			command, err := stream.Recv()
 			if command.GetSendInterrupt() {
@@ -62,14 +62,13 @@ func (s *DebugService) Debug(stream dbg.DebugService_DebugServer) error {
 			stream.Send(&dbg.DebugResponse{Data: data})
 		}),
 		signalChan)
-	if stat != nil {
-		return stat.Err()
+	if debugErr != nil {
+		return debugErr
 	}
 	return stream.Send(resp)
 }

 // GetDebugConfig return metadata about a debug session
 func (s *DebugService) GetDebugConfig(ctx context.Context, req *dbg.DebugConfigRequest) (*dbg.GetDebugConfigResponse, error) {
-	resp, err := cmd.GetDebugConfig(ctx, req)
-	return resp, err.Err()
+	return cmd.GetDebugConfig(ctx, req)
 }
diff --git a/commands/debug/debug.go b/commands/debug/debug.go
@@ -30,10 +30,7 @@ import (
 	"github.com/arduino/arduino-cli/i18n"
 	dbg "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/debug/v1"
 	"github.com/arduino/go-paths-helper"
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )

 var tr = i18n.Tr
@@ -44,13 +41,13 @@ var tr = i18n.Tr
 // grpc Out <- tool stdOut
 // grpc Out <- tool stdErr
 // It also implements tool process lifecycle management
-func Debug(ctx context.Context, req *dbg.DebugConfigRequest, inStream io.Reader, out io.Writer, interrupt <-chan os.Signal) (*dbg.DebugResponse, *status.Status) {
+func Debug(ctx context.Context, req *dbg.DebugConfigRequest, inStream io.Reader, out io.Writer, interrupt <-chan os.Signal) (*dbg.DebugResponse, error) {

 	// Get debugging command line to run debugger
 	pm := commands.GetPackageManager(req.GetInstance().GetId())
 	commandLine, err := getCommandLine(req, pm)
 	if err != nil {
-		return nil, status.New(codes.FailedPrecondition, tr("Cannot get command line for tool: %s", err))
+		return nil, err
 	}

 	for i, arg := range commandLine {
@@ -66,7 +63,7 @@ func Debug(ctx context.Context, req *dbg.DebugConfigRequest, inStream io.Reader,

 	cmd, err := executils.NewProcess(commandLine...)
 	if err != nil {
-		return nil, status.New(codes.FailedPrecondition, tr("Cannot execute debug tool: %s", err))
+		return nil, &commands.FailedDebugError{Message: tr("Cannot execute debug tool"), Cause: err}
 	}

 	// Get stdIn pipe from tool
@@ -133,7 +130,7 @@ func getCommandLine(req *dbg.DebugConfigRequest, pm *packagemanager.PackageManag
 		}
 		gdbPath = paths.New(debugInfo.ToolchainPath).Join(gdbexecutable)
 	default:
-		return nil, errors.Errorf(tr("unsupported toolchain '%s'"), debugInfo.GetToolchain())
+		return nil, &commands.FailedDebugError{Message: tr("Toolchain '%s' is not supported", debugInfo.GetToolchain())}
 	}
 	add(gdbPath.String())

@@ -172,7 +169,7 @@ func getCommandLine(req *dbg.DebugConfigRequest, pm *packagemanager.PackageManag
 		add(serverCmd)

 	default:
-		return nil, errors.Errorf(tr("unsupported gdb server '%s'"), debugInfo.GetServer())
+		return nil, &commands.FailedDebugError{Message: tr("GDB server '%s' is not supported", debugInfo.GetServer())}
 	}

 	// Add executable

diff --git a/commands/debug/debug_info.go b/commands/debug/debug_info.go
@@ -17,7 +17,6 @@ package debug

 import (
 	"context"
-	"fmt"
 	"strings"

 	"github.com/arduino/arduino-cli/arduino/cores"
@@ -27,30 +26,25 @@ import (
 	"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/debug/v1"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )

 // GetDebugConfig returns metadata to start debugging with the specified board
-func GetDebugConfig(ctx context.Context, req *debug.DebugConfigRequest) (*debug.GetDebugConfigResponse, *status.Status) {
+func GetDebugConfig(ctx context.Context, req *debug.DebugConfigRequest) (*debug.GetDebugConfigResponse, error) {
 	pm := commands.GetPackageManager(req.GetInstance().GetId())
-
-	resp, err := getDebugProperties(req, pm)
-	return resp, status.Convert(err)
+	return getDebugProperties(req, pm)
 }

 func getDebugProperties(req *debug.DebugConfigRequest, pm *packagemanager.PackageManager) (*debug.GetDebugConfigResponse, error) {
 	// TODO: make a generic function to extract sketch from request
 	// and remove duplication in commands/compile.go
 	if req.GetSketchPath() == "" {
-		return nil, fmt.Errorf(tr("missing sketchPath"))
+		return nil, &commands.MissingSketchPathError{}
 	}
 	sketchPath := paths.New(req.GetSketchPath())
 	sk, err := sketch.New(sketchPath)
 	if err != nil {
-		return nil, errors.Wrap(err, tr("opening sketch"))
+		return nil, &commands.SketchNotFoundError{Cause: err}
 	}

 	// XXX Remove this code duplication!!
@@ -59,17 +53,17 @@ func getDebugProperties(req *debug.DebugConfigRequest, pm *packagemanager.Packag
 		fqbnIn = sk.Metadata.CPU.Fqbn
 	}
 	if fqbnIn == "" {
-		return nil, fmt.Errorf(tr("no Fully Qualified Board Name provided"))
+		return nil, &commands.MissingFQBNError{}
 	}
 	fqbn, err := cores.ParseFQBN(fqbnIn)
 	if err != nil {
-		return nil, errors.Wrap(err, tr("error parsing FQBN"))
+		return nil, &commands.InvalidFQBNError{Cause: err}
 	}

 	// Find target board and board properties
 	_, platformRelease, board, boardProperties, referencedPlatformRelease, err := pm.ResolveFQBN(fqbn)
 	if err != nil {
-		return nil, errors.Wrap(err, tr("error resolving FQBN"))
+		return nil, &commands.UnknownFQBNError{Cause: err}
 	}

 	// Build configuration for debug
@@ -112,7 +106,7 @@ func getDebugProperties(req *debug.DebugConfigRequest, pm *packagemanager.Packag
 		} else if refP, ok := referencedPlatformRelease.Programmers[req.GetProgrammer()]; ok {
 			toolProperties.Merge(refP.Properties)
 		} else {
-			return nil, fmt.Errorf(tr("programmer '%s' not found"), req.GetProgrammer())
+			return nil, &commands.ProgrammerNotFoundError{Programmer: req.GetProgrammer()}
 		}
 	}

@@ -121,10 +115,10 @@ func getDebugProperties(req *debug.DebugConfigRequest, pm *packagemanager.Packag
 		importPath = paths.New(importDir)
 	}
 	if !importPath.Exist() {
-		return nil, fmt.Errorf(tr("compiled sketch not found in %s"), importPath)
+		return nil, &commands.NotFoundError{Message: tr("Compiled sketch not found in %s", importPath)}
 	}
 	if !importPath.IsDir() {
-		return nil, fmt.Errorf(tr("expected compiled sketch in directory %s, but is a file instead"), importPath)
+		return nil, &commands.NotFoundError{Message: tr("Expected compiled sketch in directory %s, but is a file instead", importPath)}
 	}
 	toolProperties.SetPath("build.path", importPath)
 	toolProperties.Set("build.project_name", sk.Name+".ino")
@@ -144,7 +138,7 @@ func getDebugProperties(req *debug.DebugConfigRequest, pm *packagemanager.Packag
 	}

 	if !debugProperties.ContainsKey("executable") {
-		return nil, status.Error(codes.Unimplemented, fmt.Sprintf(tr("debugging not supported for board %s"), req.GetFqbn()))
+		return nil, &commands.FailedDebugError{Message: tr("Debugging not supported for board %s", req.GetFqbn())}
 	}

 	server := debugProperties.Get("server")

diff --git a/commands/errors.go b/commands/errors.go
@@ -144,20 +144,21 @@ func (e *ProgreammerRequiredForUploadError) ToRPCStatus() *status.Status {
 	return st
 }

-// UnknownProgrammerError is returned when the programmer is not found
-type UnknownProgrammerError struct {
-	Cause error
+// ProgrammerNotFoundError is returned when the programmer is not found
+type ProgrammerNotFoundError struct {
+	Programmer string
+	Cause      error
 }

-func (e *UnknownProgrammerError) Error() string {
-	return composeErrorMsg(tr("Unknown programmer"), e.Cause)
+func (e *ProgrammerNotFoundError) Error() string {
+	return composeErrorMsg(tr("Programmer '%s' not found", e.Programmer), e.Cause)
 }

-func (e *UnknownProgrammerError) Unwrap() error {
+func (e *ProgrammerNotFoundError) Unwrap() error {
 	return e.Cause
 }

-func (e *UnknownProgrammerError) ToRPCStatus() *status.Status {
+func (e *ProgrammerNotFoundError) ToRPCStatus() *status.Status {
 	return status.New(codes.NotFound, e.Error())
 }

@@ -212,7 +213,7 @@ type PlatformAlreadyAtTheLatestVersionError struct {
 }

 func (e *PlatformAlreadyAtTheLatestVersionError) Error() string {
-	return tr("Platform is already at the latest version")
+	return tr("Platform '%s' is already at the latest version", e.Platform)
 }

 func (e *PlatformAlreadyAtTheLatestVersionError) ToRPCStatus() *status.Status {
@@ -322,6 +323,24 @@ func (e *FailedUploadError) ToRPCStatus() *status.Status {
 	return status.New(codes.Internal, e.Error())
 }

+// FailedDebugError is returned when the debug fails
+type FailedDebugError struct {
+	Message string
+	Cause   error
+}
+
+func (e *FailedDebugError) Error() string {
+	return composeErrorMsg(e.Message, e.Cause)
+}
+
+func (e *FailedDebugError) Unwrap() error {
+	return e.Cause
+}
+
+func (e *FailedDebugError) ToRPCStatus() *status.Status {
+	return status.New(codes.Internal, e.Error())
+}
+
 // CompileFailedError is returned when the compile fails
 type CompileFailedError struct {
 	Message string

diff --git a/commands/upload/upload.go b/commands/upload/upload.go
@@ -218,7 +218,7 @@ func runProgramAction(pm *packagemanager.PackageManager,
 			programmer = buildPlatform.Programmers[programmerID]
 		}
 		if programmer == nil {
-			return &commands.UnknownProgrammerError{Cause: fmt.Errorf(tr("programmer '%s' not available"), programmerID)}
+			return &commands.ProgrammerNotFoundError{Programmer: programmerID}
 		}
 	}

@@ -352,13 +352,13 @@ func runProgramAction(pm *packagemanager.PackageManager,
 	if !burnBootloader {
 		importPath, sketchName, err := determineBuildPathAndSketchName(importFile, importDir, sk, fqbn)
 		if err != nil {
-			return &commands.FailedUploadError{Message: tr("Error finding build artifacts"), Cause: err}
+			return &commands.NotFoundError{Message: tr("Error finding build artifacts"), Cause: err}
 		}
 		if !importPath.Exist() {
-			return &commands.FailedUploadError{Message: tr("Compiled sketch not found in %s", importPath)}
+			return &commands.NotFoundError{Message: tr("Compiled sketch not found in %s", importPath)}
 		}
 		if !importPath.IsDir() {
-			return &commands.FailedUploadError{Message: tr("Expected compiled sketch in directory %s, but is a file instead", importPath)}
+			return &commands.NotFoundError{Message: tr("Expected compiled sketch in directory %s, but is a file instead", importPath)}
 		}
 		uploadProperties.SetPath("build.path", importPath)
 		uploadProperties.Set("build.project_name", sketchName)