fix(update): return better errors

Author: lucarin91

cmd/arduino-app-cli/system/system.go

@@ -113,7 +113,13 @@ func newUpdateCmd() *cobra.Command {
 
 			events := updater.Subscribe()
 			for event := range events {
-				feedback.Printf("[%s] %s", event.Type.String(), event.Data)
+				if event.Type == update.ErrorEvent {
+					// TODO: add colors to error messages
+					err := event.GetError()
+					feedback.Printf("Error: %s [%s]", err.Error(), update.GetUpdateErrorCode(err))
+				} else {
+					feedback.Printf("[%s] %s", event.Type.String(), event.GetData())
+				}
 
 				if event.Type == update.DoneEvent {
 					break

internal/api/handlers/update.go

@@ -128,14 +128,19 @@ func HandleUpdateEvents(updater *update.Manager) http.HandlerFunc {
 					return
 				}
 				if event.Type == update.ErrorEvent {
+					err := event.GetError()
+					code := render.InternalServiceErr
+					if c := update.GetUpdateErrorCode(err); c != update.UnknownError {
+						code = render.SSEErrCode(string(c))
+					}
 					sseStream.SendError(render.SSEErrorData{
-						Code:    render.InternalServiceErr,
-						Message: event.Data,
+						Code:    code,
+						Message: err.Error(),
 					})
 				} else {
 					sseStream.Send(render.SSEEvent{
 						Type: event.Type.String(),
-						Data: event.Data,
+						Data: event.GetData(),
 					})
 				}
 

internal/update/apt/service.go

@@ -25,7 +25,6 @@ import (
 	"regexp"
 	"strings"
 	"sync"
-	"time"
 
 	"github.com/arduino/go-paths-helper"
 	"go.bug.st/f"
@@ -84,79 +83,55 @@ func (s *Service) UpgradePackages(ctx context.Context, names []string) (<-chan u
 		defer s.lock.Unlock()
 		defer close(eventsCh)
 
-		ctx, cancel := context.WithTimeout(ctx, 10*time.Minute)
-		defer cancel()
-
-		eventsCh <- update.Event{Type: update.StartEvent, Data: "Upgrade is starting"}
+		eventsCh <- update.NewDataEvent(update.StartEvent, "Upgrade is starting")
 		stream := runUpgradeCommand(ctx, names)
 		for line, err := range stream {
 			if err != nil {
-				eventsCh <- update.Event{
-					Type: update.ErrorEvent,
-					Err:  err,
-					Data: "Error running upgrade command",
-				}
-				slog.Error("error processing upgrade command output", "error", err)
+				eventsCh <- update.NewErrorEvent(fmt.Errorf("error running upgrade command: %w", err))
 				return
 			}
-			eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
+			eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
 		}
-		eventsCh <- update.Event{Type: update.StartEvent, Data: "apt cleaning cache is starting"}
+
+		eventsCh <- update.NewDataEvent(update.StartEvent, "apt cleaning cache is starting")
 		for line, err := range runAptCleanCommand(ctx) {
 			if err != nil {
-				eventsCh <- update.Event{
-					Type: update.ErrorEvent,
-					Err:  err,
-					Data: "Error running apt clean command",
-				}
-				slog.Error("error processing apt clean command output", "error", err)
+				eventsCh <- update.NewErrorEvent(fmt.Errorf("error running apt clean command: %w", err))
 				return
 			}
-			eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
+			eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
 		}
-		// TEMPORARY PATCH: stopping and destroying docker containers and images since IDE does not implement it yet.
-		// TODO: Remove this workaround once IDE implements it.
-		// Tracking issue: https://github.com/arduino/arduino-app-cli/issues/623
-		eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: "Stop and destroy docker containers and images ..."}
+
+		eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, "Apt upgrade completed successfully.")
 		streamCleanup := cleanupDockerContainers(ctx)
 		for line, err := range streamCleanup {
 			if err != nil {
 				// TODO: maybe we should retun an error or a better feedback to the user?
 				// currently, we just log the error and continue considenring not blocking
-				slog.Error("Error stopping and destroying docker containers", "error", err)
+				slog.Warn("Error stopping and destroying docker containers", "error", err)
+			} else {
+				eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
 			}
-			eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
 		}
 
-		// TEMPORARY PATCH: Install the latest docker images and show the logs to the users.
 		// TODO: Remove this workaround once docker image versions are no longer hardcoded in arduino-app-cli.
 		// Tracking issue: https://github.com/arduino/arduino-app-cli/issues/600
 		// Currently, we need to launch `arduino-app-cli system init` to pull the latest docker images because
 		// the version of the docker images are hardcoded in the (new downloaded) version of the arduino-app-cli.
-		eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: "Pulling the latest docker images ..."}
+		eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, "Pulling the latest docker images ...")
 		streamDocker := pullDockerImages(ctx)
 		for line, err := range streamDocker {
 			if err != nil {
-				eventsCh <- update.Event{
-					Type: update.ErrorEvent,
-					Err:  err,
-					Data: "Error upgrading docker images",
-				}
-				slog.Error("error upgrading docker images", "error", err)
+				eventsCh <- update.NewErrorEvent(fmt.Errorf("error pulling docker images: %w", err))
 				return
 			}
-			eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
+			eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
 		}
-		eventsCh <- update.Event{Type: update.RestartEvent, Data: "Upgrade completed. Restarting ..."}
+		eventsCh <- update.NewDataEvent(update.RestartEvent, "Upgrade completed. Restarting ...")
 
 		err := restartServices(ctx)
 		if err != nil {
-			eventsCh <- update.Event{
-				Type: update.ErrorEvent,
-				Err:  err,
-				Data: "Error restart services after upgrade",
-			}
-			slog.Error("failed to restart services", "error", err)
+			eventsCh <- update.NewErrorEvent(fmt.Errorf("error restarting services after upgrade: %w", err))
 			return
 		}
 	}()

internal/update/arduino/arduino.go

@@ -18,9 +18,9 @@ package arduino
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log/slog"
 	"sync"
-	"time"
 
 	"github.com/arduino/arduino-cli/commands"
 	"github.com/arduino/arduino-cli/commands/cmderrors"
@@ -134,42 +134,31 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
 	downloadProgressCB := func(curr *rpc.DownloadProgress) {
 		data := helpers.ArduinoCLIDownloadProgressToString(curr)
 		slog.Debug("Download progress", slog.String("download_progress", data))
-		eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: data}
+		eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, data)
 	}
 	taskProgressCB := func(msg *rpc.TaskProgress) {
 		data := helpers.ArduinoCLITaskProgressToString(msg)
 		slog.Debug("Task progress", slog.String("task_progress", data))
-		eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: data}
+		eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, data)
 	}
 
 	go func() {
 		defer a.lock.Unlock()
 		defer close(eventsCh)
 
-		ctx, cancel := context.WithTimeout(ctx, 10*time.Minute)
-		defer cancel()
-
-		eventsCh <- update.Event{Type: update.StartEvent, Data: "Upgrade is starting"}
+		eventsCh <- update.NewDataEvent(update.StartEvent, "Upgrade is starting")
 
 		logrus.SetLevel(logrus.ErrorLevel) // Reduce the log level of arduino-cli
 		srv := commands.NewArduinoCoreServer()
 
 		if err := setConfig(ctx, srv); err != nil {
-			eventsCh <- update.Event{
-				Type: update.ErrorEvent,
-				Err:  err,
-				Data: "Error setting additional URLs",
-			}
+			eventsCh <- update.NewErrorEvent(fmt.Errorf("error setting config: %w", err))
 			return
 		}
 
 		var inst *rpc.Instance
 		if resp, err := srv.Create(ctx, &rpc.CreateRequest{}); err != nil {
-			eventsCh <- update.Event{
-				Type: update.ErrorEvent,
-				Err:  err,
-				Data: "Error creating Arduino instance",
-			}
+			eventsCh <- update.NewErrorEvent(fmt.Errorf("error creating arduino-cli instance: %w", err))
 			return
 		} else {
 			inst = resp.GetInstance()
@@ -185,19 +174,11 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
 		{
 			stream, _ := commands.UpdateIndexStreamResponseToCallbackFunction(ctx, downloadProgressCB)
 			if err := srv.UpdateIndex(&rpc.UpdateIndexRequest{Instance: inst}, stream); err != nil {
-				eventsCh <- update.Event{
-					Type: update.ErrorEvent,
-					Err:  err,
-					Data: "Error updating index",
-				}
+				eventsCh <- update.NewErrorEvent(fmt.Errorf("error updating index: %w", err))
 				return
 			}
 			if err := srv.Init(&rpc.InitRequest{Instance: inst}, commands.InitStreamResponseToCallbackFunction(ctx, nil)); err != nil {
-				eventsCh <- update.Event{
-					Type: update.ErrorEvent,
-					Err:  err,
-					Data: "Error initializing Arduino instance",
-				}
+				eventsCh <- update.NewErrorEvent(fmt.Errorf("error initializing instance: %w", err))
 				return
 			}
 		}
@@ -219,17 +200,13 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
 		); err != nil {
 			var alreadyPresent *cmderrors.PlatformAlreadyAtTheLatestVersionError
 			if errors.As(err, &alreadyPresent) {
-				eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: alreadyPresent.Error()}
+				eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, alreadyPresent.Error())
 				return
 			}
 
 			var notFound *cmderrors.PlatformNotFoundError
 			if !errors.As(err, &notFound) {
-				eventsCh <- update.Event{
-					Type: update.ErrorEvent,
-					Err:  err,
-					Data: "Error upgrading platform",
-				}
+				eventsCh <- update.NewErrorEvent(fmt.Errorf("error upgrading platform: %w", err))
 				return
 			}
 			// If the platform is not found, we will try to install it
@@ -246,23 +223,16 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
 				),
 			)
 			if err != nil {
-				eventsCh <- update.Event{
-					Type: update.ErrorEvent,
-					Err:  err,
-					Data: "Error installing platform",
-				}
+				eventsCh <- update.NewErrorEvent(fmt.Errorf("error installing platform: %w", err))
 				return
 			}
 		} else if respCB().GetPlatform() == nil {
-			eventsCh <- update.Event{
-				Type: update.ErrorEvent,
-				Data: "platform upgrade failed",
-			}
+			eventsCh <- update.NewErrorEvent(fmt.Errorf("platform upgrade failed"))
 			return
 		}
 
 		cbw := orchestrator.NewCallbackWriter(func(line string) {
-			eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
+			eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
 		})
 
 		err := srv.BurnBootloader(
@@ -274,11 +244,7 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
 			commands.BurnBootloaderToServerStreams(ctx, cbw, cbw),
 		)
 		if err != nil {
-			eventsCh <- update.Event{
-				Type: update.ErrorEvent,
-				Err:  err,
-				Data: "Error burning bootloader",
-			}
+			eventsCh <- update.NewErrorEvent(fmt.Errorf("error burning bootloader: %w", err))
 			return
 		}
 	}()

internal/update/errors.go

@@ -0,0 +1,55 @@
+package update
+
+import "errors"
+
+type ErrorCode string
+
+const (
+	NoInternetConnection ErrorCode = "NO_INTERNET_CONNECTION"
+	OperationInProgress  ErrorCode = "OPERATION_IN_PROGRESS"
+	UnknownError         ErrorCode = "UNKNOWN_ERROR"
+)
+
+var (
+	ErrOperationAlreadyInProgress = &UpdateError{
+		Code:    OperationInProgress,
+		Details: " an operation is already in progress",
+	}
+	ErrNoInternetConnection = &UpdateError{
+		Code:    NoInternetConnection,
+		Details: "no internet connection available",
+	}
+)
+
+type UpdateError struct {
+	Code    ErrorCode `json:"code"`
+	Details string    `json:"details"`
+
+	err error
+}
+
+func (e *UpdateError) Error() string {
+	return e.Details
+}
+
+func (e *UpdateError) Unwrap() error {
+	return e.err
+}
+
+func NewUnkownError(err error) *UpdateError {
+	return &UpdateError{
+		Code:    "UNKNOWN_ERROR",
+		Details: err.Error(),
+		err:     err,
+	}
+}
+
+func GetUpdateErrorCode(err error) ErrorCode {
+	var updateError *UpdateError
+	if errors.As(err, &updateError) {
+		if updateError.Code != "" {
+			return updateError.Code
+		}
+	}
+	return UnknownError
+}

internal/update/event.go

@@ -15,6 +15,8 @@
 
 package update
 
+import "go.bug.st/f"
+
 // EventType defines the type of upgrade event.
 type EventType int
 
@@ -29,8 +31,9 @@ const (
 // Event represents a single event in the upgrade process.
 type Event struct {
 	Type EventType
-	Data string
-	Err  error // Optional error field for error events
+
+	data string
+	err  error // error field for error events
 }
 
 func (t EventType) String() string {
@@ -50,6 +53,30 @@ func (t EventType) String() string {
 	}
 }
 
+func NewDataEvent(t EventType, data string) Event {
+	return Event{
+		Type: t,
+		data: data,
+	}
+}
+
+func NewErrorEvent(err error) Event {
+	return Event{
+		Type: ErrorEvent,
+		err:  err,
+	}
+}
+
+func (e Event) GetData() string {
+	f.Assert(e.Type != ErrorEvent, "not a data event")
+	return e.data
+}
+
+func (e Event) GetError() error {
+	f.Assert(e.Type == ErrorEvent, "not an error event")
+	return e.err
+}
+
 type PackageType string
 
 const (