fix(update): return better errors

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

@@ -16,7 +16,6 @@
 package handlers
 
 import (
-	"errors"
 	"net/http"
 	"strings"
 
@@ -43,14 +42,20 @@ func HandleCheckUpgradable(updater *update.Manager) http.HandlerFunc {
 
 		pkgs, err := updater.ListUpgradablePackages(r.Context(), filterFunc)
 		if err != nil {
-			if errors.Is(err, update.ErrOperationAlreadyInProgress) {
-				render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()})
+			code := update.GetUpdateErrorCode(err)
+			if code == update.ErrOperationAlreadyInProgress.Code {
+				render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{
+					Code:    string(code),
+					Details: err.Error(),
+				})
 				return
 			}
-			render.EncodeResponse(w, http.StatusBadRequest, models.ErrorResponse{Details: "Error checking for upgradable packages: " + err.Error()})
+			render.EncodeResponse(w, http.StatusBadRequest, models.ErrorResponse{
+				Code:    string(code),
+				Details: err.Error(),
+			})
 			return
 		}
-
 		if len(pkgs) == 0 {
 			render.EncodeResponse(w, http.StatusNoContent, nil)
 			return
@@ -79,27 +84,40 @@ func HandleUpdateApply(updater *update.Manager) http.HandlerFunc {
 
 		pkgs, err := updater.ListUpgradablePackages(r.Context(), filterFunc)
 		if err != nil {
-			if errors.Is(err, update.ErrOperationAlreadyInProgress) {
-				render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()})
+			code := update.GetUpdateErrorCode(err)
+			if code == update.ErrOperationAlreadyInProgress.Code {
+				render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{
+					Code:    string(code),
+					Details: err.Error(),
+				})
 				return
 			}
 			slog.Error("Unable to get upgradable packages", slog.String("error", err.Error()))
-			render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "Error checking for upgradable packages"})
+			render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{
+				Code:    string(code),
+				Details: err.Error(),
+			})
 			return
 		}
-
 		if len(pkgs) == 0 {
-			render.EncodeResponse(w, http.StatusNoContent, models.ErrorResponse{Details: "System is up to date, no upgradable packages found"})
+			render.EncodeResponse(w, http.StatusNoContent, nil)
 			return
 		}
 
 		err = updater.UpgradePackages(r.Context(), pkgs)
 		if err != nil {
-			if errors.Is(err, update.ErrOperationAlreadyInProgress) {
-				render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()})
+			code := update.GetUpdateErrorCode(err)
+			if code == update.ErrOperationAlreadyInProgress.Code {
+				render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{
+					Code:    string(code),
+					Details: err.Error(),
+				})
 				return
 			}
-			render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "Error upgrading packages"})
+			render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{
+				Code:    string(code),
+				Details: err.Error(),
+			})
 			return
 		}
 
@@ -128,14 +146,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/api/models/errors.go

@@ -16,5 +16,6 @@
 package models
 
 type ErrorResponse struct {
+	Code    string `json:"code,omitempty"`
 	Details string `json:"details"`
 }

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