Skip to content

Commit ec13ed1

Browse files
lucarin91dido18
lucarin91
and
dido18
authored
fix(update): return better errors (#62)
* fix(update): return better errors * fix also list update function * add updateError test * add back a missing return * minor changes * fix details message * Update internal/update/apt/service.go Co-authored-by: Davide <davideneri18@gmail.com> * Update internal/update/errors.go Co-authored-by: Davide <davideneri18@gmail.com> * Update internal/api/handlers/update.go Co-authored-by: Davide <davideneri18@gmail.com> * Update internal/api/handlers/update.go Co-authored-by: Davide <davideneri18@gmail.com> * Update internal/api/handlers/update.go Co-authored-by: Davide <davideneri18@gmail.com> * fix after code review --------- Co-authored-by: Davide <davideneri18@gmail.com>
1 parent 8707370 commit ec13ed1

File tree

9 files changed

+202
-130
lines changed

9 files changed

+202
-130
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,13 @@ func newUpdateCmd() *cobra.Command {
113113

114114
events := updater.Subscribe()
115115
for event := range events {
116-
feedback.Printf("[%s] %s", event.Type.String(), event.Data)
116+
if event.Type == update.ErrorEvent {
117+
// TODO: add colors to error messages
118+
err := event.GetError()
119+
feedback.Printf("Error: %s [%s]", err.Error(), update.GetUpdateErrorCode(err))
120+
} else {
121+
feedback.Printf("[%s] %s", event.Type.String(), event.GetData())
122+
}
117123

118124
if event.Type == update.DoneEvent {
119125
break

‎internal/api/handlers/update.go‎

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package handlers
1717

1818
import (
19-
"errors"
2019
"net/http"
2120
"strings"
2221

@@ -43,14 +42,20 @@ func HandleCheckUpgradable(updater *update.Manager) http.HandlerFunc {
4342

4443
pkgs, err := updater.ListUpgradablePackages(r.Context(), filterFunc)
4544
if err != nil {
46-
if errors.Is(err, update.ErrOperationAlreadyInProgress) {
47-
render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()})
45+
code := update.GetUpdateErrorCode(err)
46+
if code == update.OperationInProgressCode {
47+
render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{
48+
Code: string(code),
49+
Details: err.Error(),
50+
})
4851
return
4952
}
50-
render.EncodeResponse(w, http.StatusBadRequest, models.ErrorResponse{Details: "Error checking for upgradable packages: " + err.Error()})
53+
render.EncodeResponse(w, http.StatusBadRequest, models.ErrorResponse{
54+
Code: string(code),
55+
Details: err.Error(),
56+
})
5157
return
5258
}
53-
5459
if len(pkgs) == 0 {
5560
render.EncodeResponse(w, http.StatusNoContent, nil)
5661
return
@@ -79,27 +84,40 @@ func HandleUpdateApply(updater *update.Manager) http.HandlerFunc {
7984

8085
pkgs, err := updater.ListUpgradablePackages(r.Context(), filterFunc)
8186
if err != nil {
82-
if errors.Is(err, update.ErrOperationAlreadyInProgress) {
83-
render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()})
87+
code := update.GetUpdateErrorCode(err)
88+
if code == update.OperationInProgressCode {
89+
render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{
90+
Code: string(code),
91+
Details: err.Error(),
92+
})
8493
return
8594
}
8695
slog.Error("Unable to get upgradable packages", slog.String("error", err.Error()))
87-
render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "Error checking for upgradable packages"})
96+
render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{
97+
Code: string(code),
98+
Details: err.Error(),
99+
})
88100
return
89101
}
90-
91102
if len(pkgs) == 0 {
92-
render.EncodeResponse(w, http.StatusNoContent, models.ErrorResponse{Details: "System is up to date, no upgradable packages found"})
103+
render.EncodeResponse(w, http.StatusNoContent, nil)
93104
return
94105
}
95106

96107
err = updater.UpgradePackages(r.Context(), pkgs)
97108
if err != nil {
98-
if errors.Is(err, update.ErrOperationAlreadyInProgress) {
99-
render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{Details: err.Error()})
109+
code := update.GetUpdateErrorCode(err)
110+
if code == update.OperationInProgressCode {
111+
render.EncodeResponse(w, http.StatusConflict, models.ErrorResponse{
112+
Code: string(code),
113+
Details: err.Error(),
114+
})
100115
return
101116
}
102-
render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "Error upgrading packages"})
117+
render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{
118+
Code: string(code),
119+
Details: err.Error(),
120+
})
103121
return
104122
}
105123

@@ -128,14 +146,19 @@ func HandleUpdateEvents(updater *update.Manager) http.HandlerFunc {
128146
return
129147
}
130148
if event.Type == update.ErrorEvent {
149+
err := event.GetError()
150+
code := render.InternalServiceErr
151+
if c := update.GetUpdateErrorCode(err); c != update.UnknownErrorCode {
152+
code = render.SSEErrCode(string(c))
153+
}
131154
sseStream.SendError(render.SSEErrorData{
132-
Code: render.InternalServiceErr,
133-
Message: event.Data,
155+
Code: code,
156+
Message: err.Error(),
134157
})
135158
} else {
136159
sseStream.Send(render.SSEEvent{
137160
Type: event.Type.String(),
138-
Data: event.Data,
161+
Data: event.GetData(),
139162
})
140163
}
141164

‎internal/api/models/errors.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,6 @@
1616
package models
1717

1818
type ErrorResponse struct {
19+
Code string `json:"code,omitempty"`
1920
Details string `json:"details"`
2021
}

‎internal/update/apt/service.go‎

Lines changed: 17 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"regexp"
2626
"strings"
2727
"sync"
28-
"time"
2928

3029
"github.com/arduino/go-paths-helper"
3130
"go.bug.st/f"
@@ -84,79 +83,55 @@ func (s *Service) UpgradePackages(ctx context.Context, names []string) (<-chan u
8483
defer s.lock.Unlock()
8584
defer close(eventsCh)
8685

87-
ctx, cancel := context.WithTimeout(ctx, 10*time.Minute)
88-
defer cancel()
89-
90-
eventsCh <- update.Event{Type: update.StartEvent, Data: "Upgrade is starting"}
86+
eventsCh <- update.NewDataEvent(update.StartEvent, "Upgrade is starting")
9187
stream := runUpgradeCommand(ctx, names)
9288
for line, err := range stream {
9389
if err != nil {
94-
eventsCh <- update.Event{
95-
Type: update.ErrorEvent,
96-
Err: err,
97-
Data: "Error running upgrade command",
98-
}
99-
slog.Error("error processing upgrade command output", "error", err)
90+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error running upgrade command: %w", err))
10091
return
10192
}
102-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
93+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
10394
}
104-
eventsCh <- update.Event{Type: update.StartEvent, Data: "apt cleaning cache is starting"}
95+
96+
eventsCh <- update.NewDataEvent(update.StartEvent, "apt cleaning cache is starting")
10597
for line, err := range runAptCleanCommand(ctx) {
10698
if err != nil {
107-
eventsCh <- update.Event{
108-
Type: update.ErrorEvent,
109-
Err: err,
110-
Data: "Error running apt clean command",
111-
}
112-
slog.Error("error processing apt clean command output", "error", err)
99+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error running apt clean command: %w", err))
113100
return
114101
}
115-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
102+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
116103
}
117-
// TEMPORARY PATCH: stopping and destroying docker containers and images since IDE does not implement it yet.
118-
// TODO: Remove this workaround once IDE implements it.
119-
// Tracking issue: https://github.com/arduino/arduino-app-cli/issues/623
120-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: "Stop and destroy docker containers and images ..."}
104+
105+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, "Stop and destroy docker containers and images ....")
121106
streamCleanup := cleanupDockerContainers(ctx)
122107
for line, err := range streamCleanup {
123108
if err != nil {
124109
// TODO: maybe we should retun an error or a better feedback to the user?
125110
// currently, we just log the error and continue considenring not blocking
126-
slog.Error("Error stopping and destroying docker containers", "error", err)
111+
slog.Warn("Error stopping and destroying docker containers", "error", err)
112+
} else {
113+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
127114
}
128-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
129115
}
130116

131-
// TEMPORARY PATCH: Install the latest docker images and show the logs to the users.
132117
// TODO: Remove this workaround once docker image versions are no longer hardcoded in arduino-app-cli.
133118
// Tracking issue: https://github.com/arduino/arduino-app-cli/issues/600
134119
// Currently, we need to launch `arduino-app-cli system init` to pull the latest docker images because
135120
// the version of the docker images are hardcoded in the (new downloaded) version of the arduino-app-cli.
136-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: "Pulling the latest docker images ..."}
121+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, "Pulling the latest docker images ...")
137122
streamDocker := pullDockerImages(ctx)
138123
for line, err := range streamDocker {
139124
if err != nil {
140-
eventsCh <- update.Event{
141-
Type: update.ErrorEvent,
142-
Err: err,
143-
Data: "Error upgrading docker images",
144-
}
145-
slog.Error("error upgrading docker images", "error", err)
125+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error pulling docker images: %w", err))
146126
return
147127
}
148-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
128+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
149129
}
150-
eventsCh <- update.Event{Type: update.RestartEvent, Data: "Upgrade completed. Restarting ..."}
130+
eventsCh <- update.NewDataEvent(update.RestartEvent, "Upgrade completed. Restarting ...")
151131

152132
err := restartServices(ctx)
153133
if err != nil {
154-
eventsCh <- update.Event{
155-
Type: update.ErrorEvent,
156-
Err: err,
157-
Data: "Error restart services after upgrade",
158-
}
159-
slog.Error("failed to restart services", "error", err)
134+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error restarting services after upgrade: %w", err))
160135
return
161136
}
162137
}()

‎internal/update/arduino/arduino.go‎

Lines changed: 14 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ package arduino
1818
import (
1919
"context"
2020
"errors"
21+
"fmt"
2122
"log/slog"
2223
"sync"
23-
"time"
2424

2525
"github.com/arduino/arduino-cli/commands"
2626
"github.com/arduino/arduino-cli/commands/cmderrors"
@@ -134,42 +134,31 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
134134
downloadProgressCB := func(curr *rpc.DownloadProgress) {
135135
data := helpers.ArduinoCLIDownloadProgressToString(curr)
136136
slog.Debug("Download progress", slog.String("download_progress", data))
137-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: data}
137+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, data)
138138
}
139139
taskProgressCB := func(msg *rpc.TaskProgress) {
140140
data := helpers.ArduinoCLITaskProgressToString(msg)
141141
slog.Debug("Task progress", slog.String("task_progress", data))
142-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: data}
142+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, data)
143143
}
144144

145145
go func() {
146146
defer a.lock.Unlock()
147147
defer close(eventsCh)
148148

149-
ctx, cancel := context.WithTimeout(ctx, 10*time.Minute)
150-
defer cancel()
151-
152-
eventsCh <- update.Event{Type: update.StartEvent, Data: "Upgrade is starting"}
149+
eventsCh <- update.NewDataEvent(update.StartEvent, "Upgrade is starting")
153150

154151
logrus.SetLevel(logrus.ErrorLevel) // Reduce the log level of arduino-cli
155152
srv := commands.NewArduinoCoreServer()
156153

157154
if err := setConfig(ctx, srv); err != nil {
158-
eventsCh <- update.Event{
159-
Type: update.ErrorEvent,
160-
Err: err,
161-
Data: "Error setting additional URLs",
162-
}
155+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error setting config: %w", err))
163156
return
164157
}
165158

166159
var inst *rpc.Instance
167160
if resp, err := srv.Create(ctx, &rpc.CreateRequest{}); err != nil {
168-
eventsCh <- update.Event{
169-
Type: update.ErrorEvent,
170-
Err: err,
171-
Data: "Error creating Arduino instance",
172-
}
161+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error creating arduino-cli instance: %w", err))
173162
return
174163
} else {
175164
inst = resp.GetInstance()
@@ -185,19 +174,11 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
185174
{
186175
stream, _ := commands.UpdateIndexStreamResponseToCallbackFunction(ctx, downloadProgressCB)
187176
if err := srv.UpdateIndex(&rpc.UpdateIndexRequest{Instance: inst}, stream); err != nil {
188-
eventsCh <- update.Event{
189-
Type: update.ErrorEvent,
190-
Err: err,
191-
Data: "Error updating index",
192-
}
177+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error updating index: %w", err))
193178
return
194179
}
195180
if err := srv.Init(&rpc.InitRequest{Instance: inst}, commands.InitStreamResponseToCallbackFunction(ctx, nil)); err != nil {
196-
eventsCh <- update.Event{
197-
Type: update.ErrorEvent,
198-
Err: err,
199-
Data: "Error initializing Arduino instance",
200-
}
181+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error initializing instance: %w", err))
201182
return
202183
}
203184
}
@@ -219,17 +200,13 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
219200
); err != nil {
220201
var alreadyPresent *cmderrors.PlatformAlreadyAtTheLatestVersionError
221202
if errors.As(err, &alreadyPresent) {
222-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: alreadyPresent.Error()}
203+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, alreadyPresent.Error())
223204
return
224205
}
225206

226207
var notFound *cmderrors.PlatformNotFoundError
227208
if !errors.As(err, &notFound) {
228-
eventsCh <- update.Event{
229-
Type: update.ErrorEvent,
230-
Err: err,
231-
Data: "Error upgrading platform",
232-
}
209+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error upgrading platform: %w", err))
233210
return
234211
}
235212
// 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
246223
),
247224
)
248225
if err != nil {
249-
eventsCh <- update.Event{
250-
Type: update.ErrorEvent,
251-
Err: err,
252-
Data: "Error installing platform",
253-
}
226+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error installing platform: %w", err))
254227
return
255228
}
256229
} else if respCB().GetPlatform() == nil {
257-
eventsCh <- update.Event{
258-
Type: update.ErrorEvent,
259-
Data: "platform upgrade failed",
260-
}
230+
eventsCh <- update.NewErrorEvent(fmt.Errorf("platform upgrade failed"))
261231
return
262232
}
263233

264234
cbw := orchestrator.NewCallbackWriter(func(line string) {
265-
eventsCh <- update.Event{Type: update.UpgradeLineEvent, Data: line}
235+
eventsCh <- update.NewDataEvent(update.UpgradeLineEvent, line)
266236
})
267237

268238
err := srv.BurnBootloader(
@@ -274,11 +244,7 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st
274244
commands.BurnBootloaderToServerStreams(ctx, cbw, cbw),
275245
)
276246
if err != nil {
277-
eventsCh <- update.Event{
278-
Type: update.ErrorEvent,
279-
Err: err,
280-
Data: "Error burning bootloader",
281-
}
247+
eventsCh <- update.NewErrorEvent(fmt.Errorf("error burning bootloader: %w", err))
282248
return
283249
}
284250
}()

0 commit comments

Comments
 (0)