Skip to content

Commit 8b3dc52

Browse files
committed
Proper gRPC error handling
1 parent 65b2ea6 commit 8b3dc52

File tree

8 files changed

+201
-141
lines changed

8 files changed

+201
-141
lines changed

cli/compile/compile.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/arduino/arduino-cli/cli/feedback"
2626
"github.com/arduino/arduino-cli/cli/output"
2727
"github.com/arduino/arduino-cli/configuration"
28+
"google.golang.org/grpc/status"
2829

2930
"github.com/arduino/arduino-cli/cli/errorcodes"
3031
"github.com/arduino/arduino-cli/cli/instance"
@@ -197,7 +198,7 @@ func run(cmd *cobra.Command, args []string) {
197198
ImportDir: buildPath,
198199
Programmer: programmer,
199200
}
200-
var err error
201+
var err *status.Status
201202
if output.OutputFormat == "json" {
202203
// TODO: do not print upload output in json mode
203204
uploadOut := new(bytes.Buffer)

client_example/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
dbg "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/debug/v1"
3333
"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/settings/v1"
3434
"google.golang.org/grpc"
35+
"google.golang.org/grpc/status"
3536
)
3637

3738
var (

commands/daemon/daemon.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ func (s *ArduinoCoreServerImpl) Upload(req *rpc.UploadRequest, stream rpc.Arduin
314314
utils.FeedStreamTo(func(data []byte) { stream.Send(&rpc.UploadResponse{ErrStream: data}) }),
315315
)
316316
if err != nil {
317-
return err
317+
return err.Err()
318318
}
319319
return stream.Send(resp)
320320
}
@@ -327,7 +327,7 @@ func (s *ArduinoCoreServerImpl) UploadUsingProgrammer(req *rpc.UploadUsingProgra
327327
utils.FeedStreamTo(func(data []byte) { stream.Send(&rpc.UploadUsingProgrammerResponse{ErrStream: data}) }),
328328
)
329329
if err != nil {
330-
return err
330+
return err.Err()
331331
}
332332
return stream.Send(resp)
333333
}
@@ -340,7 +340,7 @@ func (s *ArduinoCoreServerImpl) BurnBootloader(req *rpc.BurnBootloaderRequest, s
340340
utils.FeedStreamTo(func(data []byte) { stream.Send(&rpc.BurnBootloaderResponse{ErrStream: data}) }),
341341
)
342342
if err != nil {
343-
return err
343+
return err.Err()
344344
}
345345
return stream.Send(resp)
346346
}

commands/upload/burnbootloader.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ import (
2222
"github.com/arduino/arduino-cli/commands"
2323
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
2424
"github.com/sirupsen/logrus"
25+
"google.golang.org/grpc/status"
2526
)
2627

2728
// BurnBootloader FIXMEDOC
28-
func BurnBootloader(ctx context.Context, req *rpc.BurnBootloaderRequest, outStream io.Writer, errStream io.Writer) (*rpc.BurnBootloaderResponse, error) {
29+
func BurnBootloader(ctx context.Context, req *rpc.BurnBootloaderRequest, outStream io.Writer, errStream io.Writer) (*rpc.BurnBootloaderResponse, *status.Status) {
2930
logrus.
3031
WithField("fqbn", req.GetFqbn()).
3132
WithField("port", req.GetPort()).

commands/upload/upload.go

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,25 @@ import (
3636
properties "github.com/arduino/go-properties-orderedmap"
3737
"github.com/pkg/errors"
3838
"github.com/sirupsen/logrus"
39+
"google.golang.org/grpc/codes"
40+
"google.golang.org/grpc/status"
3941
)
4042

4143
// Upload FIXMEDOC
42-
func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, errStream io.Writer) (*rpc.UploadResponse, error) {
44+
func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, errStream io.Writer) (*rpc.UploadResponse, *status.Status) {
4345
logrus.Tracef("Upload %s on %s started", req.GetSketchPath(), req.GetFqbn())
4446

4547
// TODO: make a generic function to extract sketch from request
4648
// and remove duplication in commands/compile.go
4749
sketchPath := paths.New(req.GetSketchPath())
4850
sketch, err := sketches.NewSketchFromPath(sketchPath)
4951
if err != nil && req.GetImportDir() == "" && req.GetImportFile() == "" {
50-
return nil, fmt.Errorf("opening sketch: %s", err)
52+
return nil, status.Newf(codes.InvalidArgument, "Sketch not found: %s", err)
5153
}
5254

5355
pm := commands.GetPackageManager(req.GetInstance().GetId())
5456

55-
err = runProgramAction(
57+
return &rpc.UploadResponse{}, runProgramAction(
5658
pm,
5759
sketch,
5860
req.GetImportFile(),
@@ -67,18 +69,14 @@ func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, er
6769
errStream,
6870
req.GetDryRun(),
6971
)
70-
if err != nil {
71-
return nil, err
72-
}
73-
return &rpc.UploadResponse{}, nil
7472
}
7573

7674
// UsingProgrammer FIXMEDOC
77-
func UsingProgrammer(ctx context.Context, req *rpc.UploadUsingProgrammerRequest, outStream io.Writer, errStream io.Writer) (*rpc.UploadUsingProgrammerResponse, error) {
75+
func UsingProgrammer(ctx context.Context, req *rpc.UploadUsingProgrammerRequest, outStream io.Writer, errStream io.Writer) (*rpc.UploadUsingProgrammerResponse, *status.Status) {
7876
logrus.Tracef("Upload using programmer %s on %s started", req.GetSketchPath(), req.GetFqbn())
7977

8078
if req.GetProgrammer() == "" {
81-
return nil, errors.New("programmer not specified")
79+
return nil, status.New(codes.InvalidArgument, "Programmer not specified")
8280
}
8381
_, err := Upload(ctx, &rpc.UploadRequest{
8482
Instance: req.GetInstance(),
@@ -100,17 +98,17 @@ func runProgramAction(pm *packagemanager.PackageManager,
10098
programmerID string,
10199
verbose, verify, burnBootloader bool,
102100
outStream, errStream io.Writer,
103-
dryRun bool) error {
101+
dryRun bool) *status.Status {
104102

105103
if burnBootloader && programmerID == "" {
106-
return fmt.Errorf("no programmer specified for burning bootloader")
104+
return status.New(codes.InvalidArgument, "Programmer not specified")
107105
}
108106

109107
// FIXME: make a specification on how a port is specified via command line
110108
if port == "" && sketch != nil && sketch.Metadata != nil {
111109
deviceURI, err := url.Parse(sketch.Metadata.CPU.Port)
112110
if err != nil {
113-
return fmt.Errorf("invalid Device URL format: %s", err)
111+
return status.Newf(codes.InvalidArgument, "Invalid Device URL format: %s", err)
114112
}
115113
if deviceURI.Scheme == "serial" {
116114
port = deviceURI.Host + deviceURI.Path
@@ -122,18 +120,18 @@ func runProgramAction(pm *packagemanager.PackageManager,
122120
fqbnIn = sketch.Metadata.CPU.Fqbn
123121
}
124122
if fqbnIn == "" {
125-
return fmt.Errorf("no Fully Qualified Board Name provided")
123+
return status.New(codes.InvalidArgument, "No FQBN (Fully Qualified Board Name) provided")
126124
}
127125
fqbn, err := cores.ParseFQBN(fqbnIn)
128126
if err != nil {
129-
return fmt.Errorf("incorrect FQBN: %s", err)
127+
return status.Newf(codes.InvalidArgument, fmt.Sprintf("Invalid FQBN: %s", err))
130128
}
131129
logrus.WithField("fqbn", fqbn).Tracef("Detected FQBN")
132130

133131
// Find target board and board properties
134132
_, boardPlatform, board, boardProperties, buildPlatform, err := pm.ResolveFQBN(fqbn)
135133
if err != nil {
136-
return fmt.Errorf("incorrect FQBN: %s", err)
134+
return status.Newf(codes.InvalidArgument, "Could not resolve FQBN: %s", err)
137135
}
138136
logrus.
139137
WithField("boardPlatform", boardPlatform).
@@ -150,7 +148,7 @@ func runProgramAction(pm *packagemanager.PackageManager,
150148
programmer = buildPlatform.Programmers[programmerID]
151149
}
152150
if programmer == nil {
153-
return fmt.Errorf("programmer '%s' not available", programmerID)
151+
return status.Newf(codes.InvalidArgument, "Programmer '%s' not available", programmerID)
154152
}
155153
}
156154

@@ -175,7 +173,7 @@ func runProgramAction(pm *packagemanager.PackageManager,
175173
if t, ok := props.GetOk(toolProperty); ok {
176174
uploadToolID = t
177175
} else {
178-
return fmt.Errorf("cannot get programmer tool: undefined '%s' property", toolProperty)
176+
return status.Newf(codes.FailedPrecondition, "Cannot get programmer tool: undefined '%s' property", toolProperty)
179177
}
180178
}
181179

@@ -191,7 +189,7 @@ func runProgramAction(pm *packagemanager.PackageManager,
191189
Trace("Upload tool")
192190

193191
if split := strings.Split(uploadToolID, ":"); len(split) > 2 {
194-
return fmt.Errorf("invalid 'upload.tool' property: %s", uploadToolID)
192+
return status.Newf(codes.FailedPrecondition, "Invalid 'upload.tool' property: %s", uploadToolID)
195193
} else if len(split) == 2 {
196194
uploadToolID = split[1]
197195
uploadToolPlatform = pm.GetInstalledPlatformRelease(
@@ -230,7 +228,10 @@ func runProgramAction(pm *packagemanager.PackageManager,
230228
}
231229

232230
if !uploadProperties.ContainsKey("upload.protocol") && programmer == nil {
233-
return fmt.Errorf("a programmer is required to upload for this board")
231+
err, _ := status.
232+
Newf(codes.InvalidArgument, "A programmer is required to upload on this board").
233+
WithDetails(&rpc.ProgrammerIsRequiredForUploadError{})
234+
return err
234235
}
235236

236237
// Set properties for verbose upload
@@ -278,13 +279,13 @@ func runProgramAction(pm *packagemanager.PackageManager,
278279
if !burnBootloader {
279280
importPath, sketchName, err := determineBuildPathAndSketchName(importFile, importDir, sketch, fqbn)
280281
if err != nil {
281-
return errors.Errorf("retrieving build artifacts: %s", err)
282+
return status.Newf(codes.Internal, "Error finding build artifacts: %s", err)
282283
}
283284
if !importPath.Exist() {
284-
return fmt.Errorf("compiled sketch not found in %s", importPath)
285+
return status.Newf(codes.Internal, "Compiled sketch not found in %s", importPath)
285286
}
286287
if !importPath.IsDir() {
287-
return fmt.Errorf("expected compiled sketch in directory %s, but is a file instead", importPath)
288+
return status.Newf(codes.Internal, "Expected compiled sketch in directory %s, but is a file instead", importPath)
288289
}
289290
uploadProperties.SetPath("build.path", importPath)
290291
uploadProperties.Set("build.project_name", sketchName)
@@ -368,18 +369,18 @@ func runProgramAction(pm *packagemanager.PackageManager,
368369
// Run recipes for upload
369370
if burnBootloader {
370371
if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
371-
return fmt.Errorf("chip erase error: %s", err)
372+
return status.Newf(codes.Internal, "Failed chip erase: %s", err)
372373
}
373374
if err := runTool("bootloader.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
374-
return fmt.Errorf("burn bootloader error: %s", err)
375+
return status.Newf(codes.Internal, "Failed to burn bootloader: %s", err)
375376
}
376377
} else if programmer != nil {
377378
if err := runTool("program.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
378-
return fmt.Errorf("programming error: %s", err)
379+
return status.Newf(codes.Internal, "Failed programming: %s", err)
379380
}
380381
} else {
381382
if err := runTool("upload.pattern", uploadProperties, outStream, errStream, verbose, dryRun); err != nil {
382-
return fmt.Errorf("uploading error: %s", err)
383+
return status.Newf(codes.Internal, "Failed uploading: %s", err)
383384
}
384385
}
385386

commands/upload/upload_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func TestUploadPropertiesComposition(t *testing.T) {
178178
testRunner := func(t *testing.T, test test, verboseVerify bool) {
179179
outStream := &bytes.Buffer{}
180180
errStream := &bytes.Buffer{}
181-
err := runProgramAction(
181+
status := runProgramAction(
182182
pm,
183183
nil, // sketch
184184
"", // importFile
@@ -198,9 +198,9 @@ func TestUploadPropertiesComposition(t *testing.T) {
198198
verboseVerifyOutput = "quiet noverify"
199199
}
200200
if test.expectedOutput == "FAIL" {
201-
require.Error(t, err)
201+
require.NotNil(t, status)
202202
} else {
203-
require.NoError(t, err)
203+
require.Nil(t, status)
204204
outFiltered := strings.ReplaceAll(outStream.String(), "\r", "")
205205
outFiltered = strings.ReplaceAll(outFiltered, "\\", "/")
206206
require.Contains(t, outFiltered, strings.ReplaceAll(test.expectedOutput, "$$VERBOSE-VERIFY$$", verboseVerifyOutput))

0 commit comments

Comments
 (0)