Skip to content

Commit 3cbf2b1

Browse files
fix: 5xx 4.0 iter (#4620)
* bugs revert * 4th iter code changes * delete dag exec * initial poc (ongoing) of handling grpc errors * grpc error handling at install helm chart and template services * refactoring * refactor * code review changes with some refactoring * removed unused files * refactor
1 parent 4c59576 commit 3cbf2b1

File tree

9 files changed

+104
-19
lines changed

9 files changed

+104
-19
lines changed

api/helm-app/service/HelmAppService.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/devtron-labs/devtron/internal/constants"
1212
repository2 "github.com/devtron-labs/devtron/internal/sql/repository/dockerRegistry"
1313
"github.com/go-pg/pg"
14-
"google.golang.org/grpc/codes"
1514
"net/http"
1615
"reflect"
1716
"strconv"
@@ -389,7 +388,7 @@ func (impl *HelmAppServiceImpl) GetDeploymentHistory(ctx context.Context, app *A
389388
ReleaseName: app.ReleaseName,
390389
}
391390
history, err := impl.helmAppClient.GetDeploymentHistory(ctx, req)
392-
if util.GetGRPCErrorDetailedMessage(err) == bean.ErrReleaseNotFound {
391+
if util.GetClientErrorDetailedMessage(err) == bean.ErrReleaseNotFound {
393392
err = &util.ApiError{
394393
Code: constants.HelmReleaseNotFound,
395394
InternalMessage: bean.ErrReleaseNotFound,
@@ -552,16 +551,16 @@ func (impl *HelmAppServiceImpl) DeleteApplication(ctx context.Context, app *AppI
552551

553552
deleteApplicationResponse, err := impl.helmAppClient.DeleteApplication(ctx, req)
554553
if err != nil {
555-
code, message := util.GetGRPCDetailedError(err)
556-
if code == codes.NotFound {
554+
code, message := util.GetClientDetailedError(err)
555+
if code.IsNotFoundCode() {
557556
return nil, &util.ApiError{
558-
Code: "404",
559-
HttpStatusCode: 200,
557+
Code: strconv.Itoa(http.StatusNotFound),
558+
HttpStatusCode: 200, //need to revisit the status code
560559
UserMessage: message,
561560
}
562561
}
563562
impl.logger.Errorw("error in deleting helm application", "err", err)
564-
return nil, errors.New(util.GetGRPCErrorDetailedMessage(err))
563+
return nil, errors.New(util.GetClientErrorDetailedMessage(err))
565564
}
566565

567566
response := &openapi.UninstallReleaseResponse{
@@ -962,6 +961,10 @@ func (impl *HelmAppServiceImpl) TemplateChart(ctx context.Context, templateChart
962961
templateChartResponse, err := impl.helmAppClient.TemplateChart(ctx, installReleaseRequest)
963962
if err != nil {
964963
impl.logger.Errorw("error in templating chart", "err", err)
964+
clientErrCode, errMsg := util.GetClientDetailedError(err)
965+
if clientErrCode.IsInvalidArgumentCode() {
966+
return nil, &util.ApiError{HttpStatusCode: http.StatusConflict, Code: strconv.Itoa(http.StatusConflict), InternalMessage: errMsg, UserMessage: errMsg}
967+
}
965968
return nil, err
966969
}
967970

internal/errors/bean.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package errors
2+
3+
import "google.golang.org/grpc/codes"
4+
5+
type ClientStatusCode struct {
6+
Code codes.Code
7+
}
8+
9+
func (r *ClientStatusCode) IsInvalidArgumentCode() bool {
10+
return r.Code == codes.InvalidArgument
11+
}
12+
13+
func (r *ClientStatusCode) IsNotFoundCode() bool {
14+
return r.Code == codes.NotFound
15+
}
16+
17+
func (r *ClientStatusCode) IsFailedPreconditionCode() bool {
18+
return r.Code == codes.FailedPrecondition
19+
}

internal/util/ErrorUtil.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package util
1919

2020
import (
2121
"fmt"
22+
"github.com/devtron-labs/devtron/internal/errors"
2223
"github.com/go-pg/pg"
2324
"google.golang.org/grpc/codes"
2425
"google.golang.org/grpc/status"
@@ -50,16 +51,18 @@ func IsErrNoRows(err error) bool {
5051
return pg.ErrNoRows == err
5152
}
5253

53-
func GetGRPCErrorDetailedMessage(err error) string {
54+
func GetClientErrorDetailedMessage(err error) string {
5455
if errStatus, ok := status.FromError(err); ok {
5556
return errStatus.Message()
5657
}
5758
return err.Error()
5859
}
5960

60-
func GetGRPCDetailedError(err error) (codes.Code, string) {
61+
func GetClientDetailedError(err error) (*errors.ClientStatusCode, string) {
62+
grpcCode := &errors.ClientStatusCode{Code: codes.Unknown}
6163
if errStatus, ok := status.FromError(err); ok {
62-
return errStatus.Code(), errStatus.Message()
64+
grpcCode.Code = errStatus.Code()
65+
return grpcCode, errStatus.Message()
6366
}
64-
return codes.Unknown, err.Error()
67+
return grpcCode, err.Error()
6568
}

pkg/appStore/installedApp/service/FullMode/deployment/InstalledAppArgoCdService.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import (
1313
repository5 "github.com/devtron-labs/devtron/pkg/cluster/repository"
1414
commonBean "github.com/devtron-labs/devtron/pkg/deployment/gitOps/common/bean"
1515
"github.com/go-pg/pg"
16+
"net/http"
17+
"strconv"
1618
"strings"
1719
"time"
1820
)
@@ -108,6 +110,10 @@ func (impl *FullModeDeploymentServiceImpl) UpdateAndSyncACDApps(installAppVersio
108110
err = impl.argoClientWrapperService.SyncArgoCDApplicationIfNeededAndRefresh(ctx, acdAppName)
109111
if err != nil {
110112
impl.Logger.Errorw("error in getting argocd application with normal refresh", "err", err, "argoAppName", installAppVersionRequest.ACDAppName)
113+
clientErrCode, errMsg := util.GetClientDetailedError(err)
114+
if clientErrCode.IsFailedPreconditionCode() {
115+
return &util.ApiError{HttpStatusCode: http.StatusPreconditionFailed, Code: strconv.Itoa(http.StatusPreconditionFailed), InternalMessage: errMsg, UserMessage: errMsg}
116+
}
111117
return err
112118
}
113119
if !impl.acdConfig.ArgoCDAutoSyncEnabled {

pkg/deployment/trigger/devtronApps/TriggerService.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
bean5 "github.com/devtron-labs/devtron/pkg/deployment/manifest/deploymentTemplate/chartRef/bean"
3939
"github.com/devtron-labs/devtron/pkg/deployment/trigger/devtronApps/bean"
4040
"github.com/devtron-labs/devtron/pkg/deployment/trigger/devtronApps/helper"
41+
clientErrors "github.com/devtron-labs/devtron/pkg/errors"
4142
"github.com/devtron-labs/devtron/pkg/eventProcessor/out"
4243
"github.com/devtron-labs/devtron/pkg/imageDigestPolicy"
4344
"github.com/devtron-labs/devtron/pkg/pipeline"
@@ -992,7 +993,11 @@ func (impl *TriggerServiceImpl) createHelmAppForCdPipeline(overrideRequest *bean
992993
updateApplicationResponse, err := impl.helmAppClient.UpdateApplication(ctx, req)
993994
if err != nil {
994995
impl.logger.Errorw("error in updating helm application for cd pipeline", "err", err)
995-
if util.GetGRPCErrorDetailedMessage(err) == context.Canceled.Error() {
996+
apiError := clientErrors.ConvertToApiError(err)
997+
if apiError != nil {
998+
return false, apiError
999+
}
1000+
if util.GetClientErrorDetailedMessage(err) == context.Canceled.Error() {
9961001
err = errors.New(pipelineConfig.NEW_DEPLOYMENT_INITIATED)
9971002
}
9981003
return false, err
@@ -1009,7 +1014,7 @@ func (impl *TriggerServiceImpl) createHelmAppForCdPipeline(overrideRequest *bean
10091014
impl.logger.Errorw("error in helm install custom chart", "err", err)
10101015
return false, err
10111016
}
1012-
if util.GetGRPCErrorDetailedMessage(err) == context.Canceled.Error() {
1017+
if util.GetClientErrorDetailedMessage(err) == context.Canceled.Error() {
10131018
err = errors.New(pipelineConfig.NEW_DEPLOYMENT_INITIATED)
10141019
}
10151020

@@ -1020,10 +1025,13 @@ func (impl *TriggerServiceImpl) createHelmAppForCdPipeline(overrideRequest *bean
10201025

10211026
if err != nil {
10221027
impl.logger.Errorw("error in helm install custom chart", "err", err)
1023-
10241028
if pgErr != nil {
10251029
impl.logger.Errorw("failed to update deployment app created flag in pipeline table", "err", err)
10261030
}
1031+
apiError := clientErrors.ConvertToApiError(err)
1032+
if apiError != nil {
1033+
return false, apiError
1034+
}
10271035
return false, err
10281036
}
10291037

pkg/errors/utils.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package errors
2+
3+
import (
4+
util2 "github.com/devtron-labs/devtron/internal/util"
5+
"net/http"
6+
"strconv"
7+
"strings"
8+
)
9+
10+
// extract out this custom error messages in kubelink and send custom error messages from kubelink
11+
const (
12+
ClusterUnreachableErrorMsg = "cluster unreachable"
13+
CrdPreconditionErrorMsg = "ensure CRDs are installed first"
14+
ArrayStringMismatchErrorMsg = "got array expected string"
15+
NamespaceNotFoundErrorMsg = "namespace not found"
16+
InvalidValueErrorMsg = "invalid value in manifest"
17+
OperationInProgressErrorMsg = "another operation (install/upgrade/rollback) is in progress"
18+
)
19+
20+
var errorHttpStatusCodeMap = map[string]int{
21+
ClusterUnreachableErrorMsg: http.StatusUnprocessableEntity,
22+
CrdPreconditionErrorMsg: http.StatusPreconditionFailed,
23+
NamespaceNotFoundErrorMsg: http.StatusConflict,
24+
ArrayStringMismatchErrorMsg: http.StatusFailedDependency,
25+
InvalidValueErrorMsg: http.StatusFailedDependency,
26+
OperationInProgressErrorMsg: http.StatusConflict,
27+
}
28+
29+
func ConvertToApiError(err error) *util2.ApiError {
30+
for errMsg, statusCode := range errorHttpStatusCodeMap {
31+
if strings.Contains(err.Error(), errMsg) {
32+
return &util2.ApiError{
33+
InternalMessage: err.Error(),
34+
UserMessage: err.Error(),
35+
HttpStatusCode: statusCode,
36+
Code: strconv.Itoa(statusCode),
37+
}
38+
}
39+
}
40+
return nil
41+
}

pkg/generateManifest/DeployementTemplateService.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
util2 "github.com/devtron-labs/devtron/util"
2525
"github.com/go-pg/pg"
2626
"go.uber.org/zap"
27+
"net/http"
2728
"os"
2829
"strconv"
2930
"time"
@@ -364,6 +365,10 @@ func (impl DeploymentTemplateServiceImpl) GenerateManifest(ctx context.Context,
364365
templateChartResponse, err := impl.helmAppClient.TemplateChart(ctx, installReleaseRequest)
365366
if err != nil {
366367
impl.Logger.Errorw("error in templating chart", "err", err)
368+
clientErrCode, errMsg := util.GetClientDetailedError(err)
369+
if clientErrCode.IsInvalidArgumentCode() {
370+
return nil, &util.ApiError{HttpStatusCode: http.StatusConflict, Code: strconv.Itoa(http.StatusConflict), InternalMessage: errMsg, UserMessage: errMsg}
371+
}
367372
return nil, err
368373
}
369374
response := &openapi2.TemplateChartResponse{

pkg/workflow/cd/CdWorkflowCommonService.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func (impl *CdWorkflowCommonServiceImpl) MarkCurrentDeploymentFailed(runner *pip
140140
//update current WF with error status
141141
impl.logger.Errorw("error in triggering cd WF, setting wf status as fail ", "wfId", runner.Id, "err", releaseErr)
142142
runner.Status = pipelineConfig.WorkflowFailed
143-
runner.Message = util.GetGRPCErrorDetailedMessage(releaseErr)
143+
runner.Message = util.GetClientErrorDetailedMessage(releaseErr)
144144
runner.FinishedOn = time.Now()
145145
runner.UpdatedOn = time.Now()
146146
runner.UpdatedBy = triggeredBy
@@ -239,7 +239,7 @@ func (impl *CdWorkflowCommonServiceImpl) UpdateCDWorkflowRunnerStatus(ctx contex
239239
}
240240

241241
func extractTimelineFailedStatusDetails(err error) string {
242-
errorString := util.GetGRPCErrorDetailedMessage(err)
242+
errorString := util.GetClientErrorDetailedMessage(err)
243243
switch errorString {
244244
case pipelineConfig.FOUND_VULNERABILITY:
245245
return pipelineConfig.TIMELINE_DESCRIPTION_VULNERABLE_IMAGE

pkg/workflow/dag/WorkflowDagExecutor.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,14 +278,14 @@ func (impl *WorkflowDagExecutorImpl) UpdateWorkflowRunnerStatusForDeployment(app
278278
if err != nil {
279279
impl.logger.Errorw("error in getting helm app release status", "appIdentifier", appIdentifier, "err", err)
280280
// Handle release not found errors
281-
if skipReleaseNotFound && util.GetGRPCErrorDetailedMessage(err) != bean6.ErrReleaseNotFound {
281+
if skipReleaseNotFound && util.GetClientErrorDetailedMessage(err) != bean6.ErrReleaseNotFound {
282282
// skip this error and continue for next workflow status
283283
impl.logger.Warnw("found error, skipping helm apps status update for this trigger", "appIdentifier", appIdentifier, "err", err)
284284
return false
285285
}
286286
// If release not found, mark the deployment as failure
287287
wfr.Status = pipelineConfig.WorkflowFailed
288-
wfr.Message = util.GetGRPCErrorDetailedMessage(err)
288+
wfr.Message = util.GetClientErrorDetailedMessage(err)
289289
wfr.FinishedOn = time.Now()
290290
return true
291291
}
@@ -325,7 +325,7 @@ func (impl *WorkflowDagExecutorImpl) UpdateWorkflowRunnerStatusForDeployment(app
325325
}
326326

327327
func (impl *WorkflowDagExecutorImpl) handleAsyncTriggerReleaseError(releaseErr error, cdWfr *pipelineConfig.CdWorkflowRunner, overrideRequest *bean.ValuesOverrideRequest, appIdentifier *client2.AppIdentifier) {
328-
releaseErrString := util.GetGRPCErrorDetailedMessage(releaseErr)
328+
releaseErrString := util.GetClientErrorDetailedMessage(releaseErr)
329329
switch releaseErrString {
330330
case context.DeadlineExceeded.Error():
331331
// if context deadline is exceeded fetch release status and UpdateWorkflowRunnerStatusForDeployment

0 commit comments

Comments
 (0)