Better gRPC error handling

arduino/cores/fqbn.go

@@ -35,7 +35,7 @@ func ParseFQBN(fqbnIn string) (*FQBN, error) {
 	// Split fqbn
 	fqbnParts := strings.Split(fqbnIn, ":")
 	if len(fqbnParts) < 3 || len(fqbnParts) > 4 {
-		return nil, fmt.Errorf("invalid fqbn: %s", fqbnIn)
+		return nil, fmt.Errorf("not an FQBN: %s", fqbnIn)
 	}
 
 	fqbn := &FQBN{
@@ -45,18 +45,18 @@ func ParseFQBN(fqbnIn string) (*FQBN, error) {
 		Configs:      properties.NewMap(),
 	}
 	if fqbn.BoardID == "" {
-		return nil, fmt.Errorf(tr("invalid fqbn: empty board identifier"))
+		return nil, fmt.Errorf(tr("empty board identifier"))
 	}
 	if len(fqbnParts) > 3 {
 		for _, pair := range strings.Split(fqbnParts[3], ",") {
 			parts := strings.SplitN(pair, "=", 2)
 			if len(parts) != 2 {
-				return nil, fmt.Errorf(tr("invalid fqbn config: %s"), pair)
+				return nil, fmt.Errorf(tr("invalid config option: %s"), pair)
 			}
 			k := strings.TrimSpace(parts[0])
 			v := strings.TrimSpace(parts[1])
 			if k == "" {
-				return nil, fmt.Errorf(tr("invalid fqbn config: %s"), pair)
+				return nil, fmt.Errorf(tr("invalid config option: %s"), pair)
 			}
 			fqbn.Configs.Set(k, v)
 		}

arduino/libraries/libraries.go

@@ -111,7 +111,7 @@ func (library *Library) ToRPCLibrary() (*rpc.Library, error) {
 		var err error
 		headers, err = library.SourceHeaders()
 		if err != nil {
-			return nil, fmt.Errorf(tr("gathering library headers: %w"), err)
+			return nil, fmt.Errorf(tr("reading library headers: %w"), err)
 		}
 	}
 

cli/compile/compile.go

@@ -178,20 +178,20 @@ func run(cmd *cobra.Command, args []string) {
 		SourceOverride:                overrides,
 		Library:                       library,
 	}
-	compileOut := new(bytes.Buffer)
-	compileErr := new(bytes.Buffer)
+	compileStdOut := new(bytes.Buffer)
+	compileStdErr := new(bytes.Buffer)
 	verboseCompile := configuration.Settings.GetString("logging.level") == "debug"
 	var compileRes *rpc.CompileResponse
-	var err error
+	var compileError error
 	if output.OutputFormat == "json" {
-		compileRes, err = compile.Compile(context.Background(), compileRequest, compileOut, compileErr, verboseCompile)
+		compileRes, compileError = compile.Compile(context.Background(), compileRequest, compileStdOut, compileStdErr, verboseCompile)
 	} else {
-		compileRes, err = compile.Compile(context.Background(), compileRequest, os.Stdout, os.Stderr, verboseCompile)
+		compileRes, compileError = compile.Compile(context.Background(), compileRequest, os.Stdout, os.Stderr, verboseCompile)
 	}
 
-	if err == nil && uploadAfterCompile {
+	if compileError == nil && uploadAfterCompile {
 		var sk *sketch.Sketch
-		sk, err = sketch.New(sketchPath)
+		sk, err := sketch.New(sketchPath)
 		if err != nil {
 			feedback.Errorf(tr("Error during Upload: %v"), err)
 			os.Exit(errorcodes.ErrGeneric)
@@ -230,28 +230,30 @@ func run(cmd *cobra.Command, args []string) {
 			Programmer: programmer,
 			UserFields: fields,
 		}
+
+		var uploadError error
 		if output.OutputFormat == "json" {
 			// TODO: do not print upload output in json mode
-			uploadOut := new(bytes.Buffer)
-			uploadErr := new(bytes.Buffer)
-			_, err = upload.Upload(context.Background(), uploadRequest, uploadOut, uploadErr)
+			uploadStdOut := new(bytes.Buffer)
+			uploadStdErr := new(bytes.Buffer)
+			_, uploadError = upload.Upload(context.Background(), uploadRequest, uploadStdOut, uploadStdErr)
 		} else {
-			_, err = upload.Upload(context.Background(), uploadRequest, os.Stdout, os.Stderr)
+			_, uploadError = upload.Upload(context.Background(), uploadRequest, os.Stdout, os.Stderr)
 		}
-		if err != nil {
-			feedback.Errorf(tr("Error during Upload: %v"), err)
+		if uploadError != nil {
+			feedback.Errorf(tr("Error during Upload: %v"), uploadError)
 			os.Exit(errorcodes.ErrGeneric)
 		}
 	}
 
 	feedback.PrintResult(&compileResult{
-		CompileOut:    compileOut.String(),
-		CompileErr:    compileErr.String(),
+		CompileOut:    compileStdOut.String(),
+		CompileErr:    compileStdErr.String(),
 		BuilderResult: compileRes,
-		Success:       err == nil,
+		Success:       compileError == nil,
 	})
-	if err != nil && output.OutputFormat != "json" {
-		feedback.Errorf(tr("Error during build: %v"), err)
+	if compileError != nil && output.OutputFormat != "json" {
+		feedback.Errorf(tr("Error during build: %v"), compileError)
 		os.Exit(errorcodes.ErrGeneric)
 	}
 }

cli/core/upgrade.go

@@ -17,6 +17,7 @@ package core
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 
@@ -25,6 +26,7 @@ import (
 	"github.com/arduino/arduino-cli/cli/feedback"
 	"github.com/arduino/arduino-cli/cli/instance"
 	"github.com/arduino/arduino-cli/cli/output"
+	"github.com/arduino/arduino-cli/commands"
 	"github.com/arduino/arduino-cli/commands/core"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/sirupsen/logrus"
@@ -94,11 +96,13 @@ func runUpgradeCommand(cmd *cobra.Command, args []string) {
 			SkipPostInstall: DetectSkipPostInstallValue(),
 		}
 
-		_, err := core.PlatformUpgrade(context.Background(), r, output.ProgressBar(), output.TaskProgress())
-		if err == core.ErrAlreadyLatest {
-			feedback.Printf(tr("Platform %s is already at the latest version"), platformRef)
-		} else if err != nil {
-			feedback.Errorf(tr("Error during upgrade: %v"), err)
+		if _, err := core.PlatformUpgrade(context.Background(), r, output.ProgressBar(), output.TaskProgress()); err != nil {
+			if errors.Is(err, &commands.PlatformAlreadyAtTheLatestVersionError{}) {
+				feedback.Print(err.Error())
+				continue
+			}
+
+			feedback.Errorf(tr("Error during upgrade: %v", err))
 			os.Exit(errorcodes.ErrGeneric)
 		}
 	}

cli/debug/debug.go

@@ -34,7 +34,6 @@ import (
 	"github.com/arduino/go-properties-orderedmap"
 	"github.com/fatih/color"
 	"github.com/spf13/cobra"
-	"google.golang.org/grpc/status"
 )
 
 var (
@@ -101,12 +100,8 @@ func run(command *cobra.Command, args []string) {
 	if printInfo {
 
 		if res, err := debug.GetDebugConfig(context.Background(), debugConfigRequested); err != nil {
-			if status, ok := status.FromError(err); ok {
-				feedback.Errorf(tr("Error getting Debug info: %v"), status.Message())
-				errorcodes.ExitWithGrpcStatus(status)
-			}
 			feedback.Errorf(tr("Error getting Debug info: %v"), err)
-			os.Exit(errorcodes.ErrGeneric)
+			os.Exit(errorcodes.ErrBadArgument)
 		} else {
 			feedback.PrintResult(&debugInfoResult{res})
 		}

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)
-	}
-}

cli/instance/instance.go

@@ -17,6 +17,7 @@ package instance
 
 import (
 	"context"
+	"errors"
 	"os"
 
 	"github.com/arduino/arduino-cli/cli/errorcodes"
@@ -27,8 +28,6 @@ import (
 	"github.com/arduino/arduino-cli/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 var tr = i18n.Tr
@@ -50,7 +49,7 @@ func CreateAndInit() *rpc.Instance {
 }
 
 // Create and return a new Instance.
-func Create() (*rpc.Instance, *status.Status) {
+func Create() (*rpc.Instance, error) {
 	res, err := commands.Create(&rpc.CreateRequest{})
 	if err != nil {
 		return nil, err
@@ -59,12 +58,12 @@ func Create() (*rpc.Instance, *status.Status) {
 }
 
 // Init initializes instance by loading installed libraries and platforms.
-// In case of loading failures return a list gRPC Status errors for each
+// In case of loading failures return a list of errors for each
 // platform or library that we failed to load.
 // Package and library indexes files are automatically updated if the
 // CLI is run for the first time.
-func Init(instance *rpc.Instance) []*status.Status {
-	errs := []*status.Status{}
+func Init(instance *rpc.Instance) []error {
+	errs := []error{}
 
 	// In case the CLI is executed for the first time
 	if err := FirstUpdate(instance); err != nil {
@@ -77,8 +76,8 @@ func Init(instance *rpc.Instance) []*status.Status {
 	err := commands.Init(&rpc.InitRequest{
 		Instance: instance,
 	}, func(res *rpc.InitResponse) {
-		if err := res.GetError(); err != nil {
-			errs = append(errs, status.FromProto(err))
+		if st := res.GetError(); st != nil {
+			errs = append(errs, errors.New(st.Message))
 		}
 
 		if progress := res.GetInitProgress(); progress != nil {
@@ -99,7 +98,7 @@ func Init(instance *rpc.Instance) []*status.Status {
 
 // FirstUpdate downloads libraries and packages indexes if they don't exist.
 // This ideally is only executed the first time the CLI is run.
-func FirstUpdate(instance *rpc.Instance) *status.Status {
+func FirstUpdate(instance *rpc.Instance) error {
 	// Gets the data directory to verify if library_index.json and package_index.json exist
 	dataDir := paths.New(configuration.Settings.GetString("directories.data"))
 
@@ -120,7 +119,7 @@ func FirstUpdate(instance *rpc.Instance) *status.Status {
 			output.ProgressBar(),
 		)
 		if err != nil {
-			return status.Newf(codes.FailedPrecondition, err.Error())
+			return err
 		}
 	}
 
@@ -135,7 +134,7 @@ func FirstUpdate(instance *rpc.Instance) *status.Status {
 			output.ProgressBar(),
 		)
 		if err != nil {
-			return status.Newf(codes.FailedPrecondition, err.Error())
+			return err
 		}
 	}
 

cli/lib/args.go

@@ -75,7 +75,7 @@ func ParseLibraryReferenceArgs(args []string) ([]*LibraryReferenceArg, error) {
 // ParseLibraryReferenceArgAndAdjustCase parse a command line argument that reference a
 // library and possibly adjust the case of the name to match a library in the index
 func ParseLibraryReferenceArgAndAdjustCase(instance *rpc.Instance, arg string) (*LibraryReferenceArg, error) {
-	libRef, err := ParseLibraryReferenceArg(arg)
+	libRef, _ := ParseLibraryReferenceArg(arg)
 	res, err := lib.LibrarySearch(context.Background(), &rpc.LibrarySearchRequest{
 		Instance: instance,
 		Query:    libRef.Name,

cli/lib/check_deps.go

@@ -57,7 +57,7 @@ func runDepsCommand(cmd *cobra.Command, args []string) {
 		Version:  libRef.Version,
 	})
 	if err != nil {
-		feedback.Errorf(tr("Error resolving dependencies for %[1]s: %[2]s"), libRef, err)
+		feedback.Errorf(tr("Error resolving dependencies for %[1]s: %[2]s", libRef, err))
 	}
 
 	feedback.PrintResult(&checkDepResult{deps: deps})

cli/lib/search.go

@@ -58,10 +58,11 @@ func runSearchCommand(cmd *cobra.Command, args []string) {
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
-	err := commands.UpdateLibrariesIndex(context.Background(), &rpc.UpdateLibrariesIndexRequest{
-		Instance: inst,
-	}, output.ProgressBar())
-	if err != nil {
+	if err := commands.UpdateLibrariesIndex(
+		context.Background(),
+		&rpc.UpdateLibrariesIndexRequest{Instance: inst},
+		output.ProgressBar(),
+	); err != nil {
 		feedback.Errorf(tr("Error updating library index: %v"), err)
 		os.Exit(errorcodes.ErrGeneric)
 	}

commands/board/attach.go

@@ -17,7 +17,6 @@ package board
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/url"
 	"strings"
@@ -39,15 +38,15 @@ var tr = i18n.Tr
 func Attach(ctx context.Context, req *rpc.BoardAttachRequest, taskCB commands.TaskProgressCB) (*rpc.BoardAttachResponse, error) {
 	pm := commands.GetPackageManager(req.GetInstance().GetId())
 	if pm == nil {
-		return nil, errors.New(tr("invalid instance"))
+		return nil, &commands.InvalidInstanceError{}
 	}
 	var sketchPath *paths.Path
 	if req.GetSketchPath() != "" {
 		sketchPath = paths.New(req.GetSketchPath())
 	}
 	sk, err := sketch.New(sketchPath)
 	if err != nil {
-		return nil, fmt.Errorf(tr("opening sketch: %s"), err)
+		return nil, &commands.CantOpenSketchError{Cause: err}
 	}
 
 	boardURI := req.GetBoardUri()
@@ -63,7 +62,7 @@ func Attach(ctx context.Context, req *rpc.BoardAttachRequest, taskCB commands.Ta
 	} else {
 		deviceURI, err := url.Parse(boardURI)
 		if err != nil {
-			return nil, fmt.Errorf(tr("invalid Device URL format: %s"), err)
+			return nil, &commands.InvalidArgumentError{Message: tr("Invalid Device URL format"), Cause: err}
 		}
 
 		var findBoardFunc func(*packagemanager.PackageManager, *discovery.Monitor, *url.URL) *cores.Board
@@ -73,7 +72,7 @@ func Attach(ctx context.Context, req *rpc.BoardAttachRequest, taskCB commands.Ta
 		case "http", "https", "tcp", "udp":
 			findBoardFunc = findNetworkConnectedBoard
 		default:
-			return nil, fmt.Errorf(tr("invalid device port type provided"))
+			return nil, &commands.InvalidArgumentError{Message: tr("Invalid device port type provided")}
 		}
 
 		duration, err := time.ParseDuration(req.GetSearchTimeout())
@@ -89,7 +88,7 @@ func Attach(ctx context.Context, req *rpc.BoardAttachRequest, taskCB commands.Ta
 		// TODO: Handle the case when no board is found.
 		board := findBoardFunc(pm, monitor, deviceURI)
 		if board == nil {
-			return nil, fmt.Errorf(tr("no supported board found at %s"), deviceURI.String())
+			return nil, &commands.InvalidArgumentError{Message: tr("No supported board found at %s", deviceURI)}
 		}
 		taskCB(&rpc.TaskProgress{Name: fmt.Sprintf(tr("Board found: %s"), board.Name())})
 
@@ -104,7 +103,7 @@ func Attach(ctx context.Context, req *rpc.BoardAttachRequest, taskCB commands.Ta
 
 	err = sk.ExportMetadata()
 	if err != nil {
-		return nil, fmt.Errorf(tr("cannot export sketch metadata: %s"), err)
+		return nil, &commands.PermissionDeniedError{Message: tr("Cannot export sketch metadata"), Cause: err}
 	}
 	taskCB(&rpc.TaskProgress{Name: fmt.Sprintf(tr("Selected fqbn: %s"), sk.Metadata.CPU.Fqbn), Completed: true})
 	return &rpc.BoardAttachResponse{}, nil