Skip to content

Commit e539741

Browse files
SivaParvathi3112SivaParvathi3112
committed
feat:Improved error management for config controller.
1 parent 40eef4b commit e539741

8 files changed

Lines changed: 131 additions & 109 deletions

File tree

‎src/catalog/cacheservice/main.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,12 @@ func NewCacheService(ctx basecontext.ApiContext) (*CacheService, error) {
9595
}
9696
svc.cacheFolder = cacheFolder
9797

98-
if freeDiskSpace, err := diskspaceservice.Get(ctx).GetCacheDiskSpace(ctx); err == nil {
98+
cacheDiag := errors.NewDiagnostics("CacheService")
99+
freeDiskSpace := diskspaceservice.Get(ctx).GetCacheDiskSpace(ctx, cacheDiag)
100+
if !cacheDiag.HasErrors() {
99101
svc.maxCacheSize = svc.cfg.CacheMaxSize(freeDiskSpace)
102+
} else {
103+
ctx.LogErrorf("Failed to get cache disk space: %v", cacheDiag.Errors[0].Message)
100104
}
101105

102106
return svc, nil

‎src/controllers/cache.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ func GetCatalogCacheHandler() restapi.ControllerHandler {
148148
}
149149

150150
var freeDiskSpace int64
151-
if ds, err := diskspace.Get(ctx).GetCacheDiskSpace(ctx); err == nil {
151+
localDiag := errors.NewDiagnostics("SilentDiskSpaceCheck")
152+
if ds := diskspace.Get(ctx).GetCacheDiskSpace(ctx, localDiag); !localDiag.HasErrors() {
152153
freeDiskSpace = ds
153154
} else if hwInfo, err := serviceprovider.Get().System.GetHardwareInfo(ctx); err == nil {
154155
freeDiskSpace = int64(hwInfo.FreeDiskSize)

‎src/controllers/config.go‎

Lines changed: 60 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ package controllers
22

33
import (
44
"encoding/json"
5-
"errors"
6-
"fmt"
75
"net/http"
86
"os"
7+
"strconv"
98

109
"github.com/Parallels/prl-devops-service/basecontext"
1110
"github.com/Parallels/prl-devops-service/config"
1211
"github.com/Parallels/prl-devops-service/constants"
12+
prlerrors "github.com/Parallels/prl-devops-service/errors"
1313
"github.com/Parallels/prl-devops-service/logs"
1414
"github.com/Parallels/prl-devops-service/mappers"
1515
"github.com/Parallels/prl-devops-service/models"
@@ -104,28 +104,27 @@ func registerConfigHandlers(ctx basecontext.ApiContext, version string) {
104104
// @Tags Config
105105
// @Produce json
106106
// @Success 200 {object} models.ParallelsDesktopLicense
107-
// @Failure 400 {object} models.ApiErrorResponse
107+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
108108
// @Failure 401 {object} models.OAuthErrorResponse
109109
// @Security ApiKeyAuth
110110
// @Security BearerAuth
111-
// @Router /v1/parallels_desktop/key [get]
111+
// @Router /v1/config/parallels-desktop/license [get]
112112
func GetParallelsDesktopLicenseHandler() restapi.ControllerHandler {
113113
return func(w http.ResponseWriter, r *http.Request) {
114114
defer r.Body.Close()
115115
ctx := GetBaseContext(r)
116116
defer Recover(ctx, r, w)
117+
getPDLicenseDiag := prlerrors.NewDiagnostics("/config/parallels-desktop/license")
117118
provider := serviceprovider.Get()
118119
if provider.ParallelsDesktopService == nil || !provider.ParallelsDesktopService.Installed() {
119-
ReturnApiError(ctx, w, models.ApiErrorResponse{
120-
Message: "Parallels Desktop is not installed",
121-
Code: http.StatusNotFound,
122-
})
120+
getPDLicenseDiag.AddError("404", "Parallels Desktop is not installed", "GetParallelsDesktopLicense")
121+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getPDLicenseDiag, http.StatusNotFound))
123122
return
124123
}
125124

126-
license, err := provider.ParallelsDesktopService.GetLicense()
127-
if err != nil {
128-
ReturnApiError(ctx, w, models.NewFromError(err))
125+
license := provider.ParallelsDesktopService.GetLicense(getPDLicenseDiag)
126+
if getPDLicenseDiag.HasErrors() {
127+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getPDLicenseDiag, http.StatusInternalServerError))
129128
return
130129
}
131130

@@ -141,7 +140,7 @@ func GetParallelsDesktopLicenseHandler() restapi.ControllerHandler {
141140
// @Produce json
142141
// @Param installToolsRequest body models.InstallToolsRequest true "Install Tools Request"
143142
// @Success 200 {object} models.InstallToolsResponse
144-
// @Failure 400 {object} models.ApiErrorResponse
143+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
145144
// @Failure 401 {object} models.OAuthErrorResponse
146145
// @Security ApiKeyAuth
147146
// @Security BearerAuth
@@ -151,19 +150,16 @@ func Install3rdPartyToolsHandler() restapi.ControllerHandler {
151150
defer r.Body.Close()
152151
ctx := GetBaseContext(r)
153152
defer Recover(ctx, r, w)
153+
installToolsDiag := prlerrors.NewDiagnostics("/config/tools/install")
154154
var request models.InstallToolsRequest
155155
if err := http_helper.MapRequestBody(r, &request); err != nil {
156-
ReturnApiError(ctx, w, models.ApiErrorResponse{
157-
Message: "Invalid request body: " + err.Error(),
158-
Code: http.StatusBadRequest,
159-
})
156+
installToolsDiag.AddError(strconv.Itoa(http.StatusBadRequest), "Invalid request body: "+err.Error(), "MapRequestBody")
157+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(installToolsDiag, http.StatusBadRequest))
160158
return
161159
}
162-
if err := request.Validate(); err != nil {
163-
ReturnApiError(ctx, w, models.ApiErrorResponse{
164-
Message: "Invalid request body: " + err.Error(),
165-
Code: http.StatusBadRequest,
166-
})
160+
request.Validate(installToolsDiag)
161+
if installToolsDiag.HasErrors() {
162+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(installToolsDiag, http.StatusBadRequest))
167163
return
168164
}
169165

@@ -205,7 +201,7 @@ func Install3rdPartyToolsHandler() restapi.ControllerHandler {
205201
// @Produce json
206202
// @Param uninstallToolsRequest body models.UninstallToolsRequest true "Uninstall Tools Request"
207203
// @Success 200 {object} models.InstallToolsResponse
208-
// @Failure 400 {object} models.ApiErrorResponse
204+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
209205
// @Failure 401 {object} models.OAuthErrorResponse
210206
// @Security ApiKeyAuth
211207
// @Security BearerAuth
@@ -215,19 +211,16 @@ func Uninstall3rdPartyToolsHandler() restapi.ControllerHandler {
215211
defer r.Body.Close()
216212
ctx := GetBaseContext(r)
217213
defer Recover(ctx, r, w)
214+
uninstallToolsDiag := prlerrors.NewDiagnostics("/config/tools/uninstall")
218215
var request models.UninstallToolsRequest
219216
if err := http_helper.MapRequestBody(r, &request); err != nil {
220-
ReturnApiError(ctx, w, models.ApiErrorResponse{
221-
Message: "Invalid request body: " + err.Error(),
222-
Code: http.StatusBadRequest,
223-
})
217+
uninstallToolsDiag.AddError(strconv.Itoa(http.StatusBadRequest), "Invalid request body: "+err.Error(), "MapRequestBody")
218+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(uninstallToolsDiag, http.StatusBadRequest))
224219
return
225220
}
226-
if err := request.Validate(); err != nil {
227-
ReturnApiError(ctx, w, models.ApiErrorResponse{
228-
Message: "Invalid request body: " + err.Error(),
229-
Code: http.StatusBadRequest,
230-
})
221+
request.Validate(uninstallToolsDiag)
222+
if uninstallToolsDiag.HasErrors() {
223+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(uninstallToolsDiag, http.StatusBadRequest))
231224
return
232225
}
233226

@@ -265,7 +258,7 @@ func Uninstall3rdPartyToolsHandler() restapi.ControllerHandler {
265258
// @Tags Config
266259
// @Produce json
267260
// @Success 202
268-
// @Failure 400 {object} models.ApiErrorResponse
261+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
269262
// @Failure 401 {object} models.OAuthErrorResponse
270263
// @Security ApiKeyAuth
271264
// @Security BearerAuth
@@ -286,7 +279,7 @@ func RestartApiHandler() restapi.ControllerHandler {
286279
// @Tags Config
287280
// @Produce json
288281
// @Success 200 {object} models.SystemUsageResponse
289-
// @Failure 400 {object} models.ApiErrorResponse
282+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
290283
// @Failure 401 {object} models.OAuthErrorResponse
291284
// @Security ApiKeyAuth
292285
// @Security BearerAuth
@@ -297,14 +290,25 @@ func GetHardwareInfo() restapi.ControllerHandler {
297290
ctx := GetBaseContext(r)
298291
cfg := config.Get()
299292
defer Recover(ctx, r, w)
293+
getHardwareInfoDiag := prlerrors.NewDiagnostics("/config/hardware")
300294
provider := serviceprovider.Get()
301295
os := system.Get().GetOperatingSystem()
302296
var hardwareInfo *models.SystemUsageResponse
303-
var err error
304297
if os == "macos" {
305-
hardwareInfo, err = provider.ParallelsDesktopService.GetHardwareUsage(ctx)
298+
hardwareInfo = provider.ParallelsDesktopService.GetHardwareUsage(ctx, getHardwareInfoDiag)
306299
} else {
307-
hardwareInfo, err = provider.System.GetHardwareUsage(ctx)
300+
hardwareInfo = provider.System.GetHardwareUsage(ctx, getHardwareInfoDiag)
301+
}
302+
// 1. First, safely check if the error is what failed
303+
if getHardwareInfoDiag.HasErrors() {
304+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getHardwareInfoDiag, http.StatusInternalServerError))
305+
return
306+
}
307+
// 2. ONLY if there was no error do we check if the data is missing!
308+
if hardwareInfo == nil {
309+
getHardwareInfoDiag.AddError("500", "Hardware info not found", "GetHardwareUsage")
310+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getHardwareInfoDiag, http.StatusInternalServerError))
311+
return
308312
}
309313

310314
hardwareInfo.EnabledModules = cfg.GetEnabledModules()
@@ -334,7 +338,7 @@ func GetHardwareInfo() restapi.ControllerHandler {
334338
}
335339

336340
var freeDiskSpace int64
337-
if ds, err := diskspace.Get(ctx).GetCacheDiskSpace(ctx); err == nil {
341+
if ds := diskspace.Get(ctx).GetCacheDiskSpace(ctx, getHardwareInfoDiag); !getHardwareInfoDiag.HasErrors() {
338342
freeDiskSpace = ds
339343
} else if hwInfo, err := provider.System.GetHardwareInfo(ctx); err == nil {
340344
freeDiskSpace = int64(hwInfo.FreeDiskSize)
@@ -353,11 +357,6 @@ func GetHardwareInfo() restapi.ControllerHandler {
353357
}
354358
}
355359

356-
if err != nil || hardwareInfo == nil {
357-
ReturnApiError(ctx, w, models.NewFromError(err))
358-
return
359-
}
360-
361360
w.WriteHeader(http.StatusOK)
362361
_ = json.NewEncoder(w).Encode(hardwareInfo)
363362
}
@@ -373,6 +372,8 @@ func GetHardwareInfo() restapi.ControllerHandler {
373372
func GetSystemHealth() restapi.ControllerHandler {
374373
return func(w http.ResponseWriter, r *http.Request) {
375374
defer r.Body.Close()
375+
ctx := GetBaseContext(r)
376+
defer Recover(ctx, r, w)
376377
provider := serviceprovider.Get()
377378
result := models.ApiHealthCheck{}
378379

@@ -445,7 +446,7 @@ func GetSystemHealth() restapi.ControllerHandler {
445446
// @Tags Config
446447
// @Produce plain
447448
// @Success 200
448-
// @Failure 400 {object} models.ApiErrorResponse
449+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
449450
// @Failure 401 {object} models.OAuthErrorResponse
450451
// @Security ApiKeyAuth
451452
// @Security BearerAuth
@@ -455,34 +456,28 @@ func GetSystemLogs() restapi.ControllerHandler {
455456
defer r.Body.Close()
456457
ctx := GetBaseContext(r)
457458
defer Recover(ctx, r, w)
458-
459+
getSystemLogsDiag := prlerrors.NewDiagnostics("/logs")
459460
cfg := config.Get()
460461

461462
// Checking if we have the logs to file enabled so we can read the logs
462463
if !cfg.GetBoolKey(constants.LOG_TO_FILE_ENV_VAR) && !cfg.GetBoolKey(constants.PRL_DEVOPS_LOG_TO_FILE_ENV_VAR) {
463-
err := errors.New("logs to file is not enabled, we cannot read the logs")
464-
ReturnApiError(ctx, w, models.ApiErrorResponse{
465-
Message: "Failed to read log file: " + err.Error(),
466-
Code: http.StatusBadRequest,
467-
})
464+
getSystemLogsDiag.AddError(strconv.Itoa(http.StatusBadRequest), "logs to file is not enabled, we cannot read the logs", "GetBoolKey")
465+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getSystemLogsDiag, http.StatusBadRequest))
468466
return
469467
}
470468

471469
logFile := logs.GetLogFilePath(ctx)
472470
if logFile == "" {
473-
ReturnApiError(ctx, w, models.ApiErrorResponse{
474-
Message: "Failed to read log file: log file path is empty",
475-
Code: http.StatusBadRequest,
476-
})
471+
getSystemLogsDiag.AddError(strconv.Itoa(http.StatusBadRequest), "Failed to read log file: log file path is empty", "GetLogFilePath")
472+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getSystemLogsDiag, http.StatusBadRequest))
477473
return
478474
}
479475

480476
content, err := os.ReadFile(logFile)
481477
if err != nil {
482-
ReturnApiError(ctx, w, models.ApiErrorResponse{
483-
Message: fmt.Sprintf("Failed to read log file %s: %v", logFile, err.Error()),
484-
Code: http.StatusBadRequest,
485-
})
478+
rsp := models.NewFromError(err)
479+
getSystemLogsDiag.AddError(strconv.Itoa(rsp.Code), rsp.Message, "Failed to read log file")
480+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getSystemLogsDiag, rsp.Code))
486481
return
487482
}
488483

@@ -497,7 +492,7 @@ func GetSystemLogs() restapi.ControllerHandler {
497492
// @Tags Config
498493
// @Produce json
499494
// @Success 101 "Switching Protocols to websocket"
500-
// @Failure 400 {object} models.ApiErrorResponse
495+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
501496
// @Failure 401 {object} models.OAuthErrorResponse
502497
// @Security ApiKeyAuth
503498
// @Security BearerAuth
@@ -569,7 +564,7 @@ func StreamSystemLogs() restapi.ControllerHandler {
569564
// @Produce json
570565
// @Param createRequest body models.DiskSpaceAvailableRequest false "Disk Space Available Request"
571566
// @Success 200 {object} models.DiskSpaceAvailable
572-
// @Failure 400 {object} models.ApiErrorResponse
567+
// @Failure 400 {object} models.ApiErrorDiagnosticsResponse
573568
// @Failure 401 {object} models.OAuthErrorResponse
574569
// @Security ApiKeyAuth
575570
// @Security BearerAuth
@@ -579,19 +574,17 @@ func GetParallelsDiskSpace() restapi.ControllerHandler {
579574
defer r.Body.Close()
580575
ctx := GetBaseContext(r)
581576
defer Recover(ctx, r, w)
582-
577+
getDiskSpaceAvailableDiag := prlerrors.NewDiagnostics("/config/diskspace")
583578
var request models.DiskSpaceAvailableRequest
584579
if err := http_helper.MapRequestBody(r, &request); err != nil {
585-
ReturnApiError(ctx, w, models.ApiErrorResponse{
586-
Message: "Invalid request body: " + err.Error(),
587-
Code: http.StatusBadRequest,
588-
})
580+
getDiskSpaceAvailableDiag.AddError(strconv.Itoa(http.StatusBadRequest), "Invalid request body: "+err.Error(), "MapRequestBody")
581+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getDiskSpaceAvailableDiag, http.StatusBadRequest))
589582
return
590583
}
591584

592-
response, err := diskspace.Get(ctx).GetDiskSpaceAvailable(ctx, request.UserName, request.FolderPath)
593-
if err != nil {
594-
ReturnApiError(ctx, w, models.NewFromError(err))
585+
response := diskspace.Get(ctx).GetDiskSpaceAvailable(ctx, request.UserName, request.FolderPath, getDiskSpaceAvailableDiag)
586+
if getDiskSpaceAvailableDiag.HasErrors() {
587+
ReturnApiErrorWithDiagnostics(ctx, w, models.NewDiagnosticsWithCode(getDiskSpaceAvailableDiag, http.StatusInternalServerError))
595588
return
596589
}
597590

‎src/models/tools.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
package models
22

3-
import "github.com/Parallels/prl-devops-service/errors"
3+
import (
4+
"net/http"
5+
"strconv"
6+
7+
"github.com/Parallels/prl-devops-service/errors"
8+
)
49

510
type InstallToolsRequest struct {
611
All bool `json:"all"`
@@ -13,9 +18,10 @@ type InstallToolsRequestItem struct {
1318
Flags map[string]string `json:"flags"`
1419
}
1520

16-
func (i *InstallToolsRequest) Validate() error {
21+
func (i *InstallToolsRequest) Validate(diag *errors.Diagnostics) {
1722
if i.Tools == nil && !i.All {
18-
return errors.New("tools cannot be empty")
23+
diag.AddError(strconv.Itoa(http.StatusBadRequest), "tools cannot be empty", "Validate")
24+
return
1925
}
2026
for _, tool := range i.Tools {
2127
if tool.Version == "" {
@@ -25,8 +31,6 @@ func (i *InstallToolsRequest) Validate() error {
2531
tool.Flags = make(map[string]string)
2632
}
2733
}
28-
29-
return nil
3034
}
3135

3236
type InstallToolsResponse struct {
@@ -52,17 +56,16 @@ type UninstallToolsRequestItem struct {
5256
Flags map[string]string `json:"flags"`
5357
}
5458

55-
func (u *UninstallToolsRequest) Validate() error {
59+
func (u *UninstallToolsRequest) Validate(diag *errors.Diagnostics) {
5660
if u.Tools == nil && !u.All {
57-
return errors.New("tools cannot be empty")
61+
diag.AddError(strconv.Itoa(http.StatusBadRequest), "tools cannot be empty", "Validate")
62+
return
5863
}
5964
for _, tool := range u.Tools {
6065
if tool.Flags == nil {
6166
tool.Flags = make(map[string]string)
6267
}
6368
}
64-
65-
return nil
6669
}
6770

6871
type UninstallToolsResponse struct {

0 commit comments

Comments
 (0)