fix: 500 fixes second iteration (#4464)

prakash100198 · ShashwatDadhich · web-flow · commit 7cff6793ee62 · 2024-01-22T00:39:42.000+05:30
* FetchChartNotes return 4xx when installed app or installed app versions not found

* GetDeploymentHistory and GetInstalledAppVersion to return 4xx when installed app or installed app versions not found

* fixes

* adding pg:",discard_unknown_columns" in tables where doesn't exist

* fix for Git Sensor response

* fix for inception

* fix for k8s resource delete

* fix for pipeline already exists

* fix for pipeline already exists

* fix

* fix

* fix

* fix

* go mod version changes

* merged

* common lib main version sync

* main merge

---------

Co-authored-by: ShashwatDadhich &lt;dadhichshashwat1808@gmail.com&gt;
diff --git a/api/appStore/deployment/CommonDeploymentRestHandler.go b/api/appStore/deployment/CommonDeploymentRestHandler.go
@@ -93,7 +93,7 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI
 		}
 		installedAppDto, err = handler.appStoreDeploymentServiceC.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName)
 		if err != nil {
-			err = &util.ApiError{HttpStatusCode: http.StatusInternalServerError, UserMessage: "unable to find app in database"}
+			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}
 			return appOfferingMode, installedAppDto, err
 		}
 		// this is the case when hyperion apps does not linked yet
@@ -119,7 +119,7 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI
 		}
 		installedAppDto, err = handler.appStoreDeploymentServiceC.GetInstalledAppByInstalledAppId(installedAppId)
 		if err != nil {
-			err = &util.ApiError{HttpStatusCode: http.StatusInternalServerError, UserMessage: "unable to find app in database"}
+			err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}
 			return appOfferingMode, installedAppDto, err
 		}
 	} else {
diff --git a/api/k8s/application/k8sApplicationRestHandler.go b/api/k8s/application/k8sApplicationRestHandler.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/devtron-labs/common-lib/utils"
 	"net/http"
 	"strconv"
 	"strings"
@@ -512,8 +513,17 @@ func (handler *K8sApplicationRestHandlerImpl) DeleteResource(w http.ResponseWrit
 
 	resource, err := handler.k8sApplicationService.DeleteResourceWithAudit(r.Context(), &request, userId)
 	if err != nil {
+		errCode := http.StatusInternalServerError
+		if apiErr, ok := err.(*utils.ApiError); ok {
+			errCode = apiErr.HttpStatusCode
+			switch errCode {
+			case http.StatusNotFound:
+				errorMessage := "resource not found"
+				err = fmt.Errorf("%s: %w", errorMessage, err)
+			}
+		}
 		handler.logger.Errorw("error in deleting resource", "err", err)
-		common.WriteJsonResp(w, err, resource, http.StatusInternalServerError)
+		common.WriteJsonResp(w, err, resource, errCode)
 		return
 	}
 	common.WriteJsonResp(w, nil, resource, http.StatusOK)
diff --git a/client/gitSensor/GitSensorGrpcClient.go b/client/gitSensor/GitSensorGrpcClient.go
@@ -335,10 +335,12 @@ func (client *GrpcApiClientImpl) GetCommitMetadataForPipelineMaterial(ctx contex
 		GitTag:             req.GitTag,
 		BranchName:         req.BranchName,
 	})
+
 	if err != nil {
 		return nil, err
 	}
-	if res == nil {
+
+	if res.Commit == "" {
 		return nil, nil
 	}
 
diff --git a/internal/sql/repository/NotificationSettingsRepository.go b/internal/sql/repository/NotificationSettingsRepository.go
@@ -73,7 +73,7 @@ type NotificationSettingsViewWithAppEnv struct {
 }
 
 type NotificationSettings struct {
-	tableName    struct{} `sql:"notification_settings"`
+	tableName    struct{} `sql:"notification_settings" pg:",discard_unknown_columns"`
 	Id           int      `sql:"id,pk"`
 	TeamId       *int     `sql:"team_id"`
 	AppId        *int     `sql:"app_id"`
diff --git a/internal/sql/repository/WebhookEventDataRepository.go b/internal/sql/repository/WebhookEventDataRepository.go
@@ -22,7 +22,7 @@ import (
 )
 
 type WebhookEventData struct {
-	tableName   struct{}  `sql:"webhook_event_data"`
+	tableName   struct{}  `sql:"webhook_event_data" pg:",discard_unknown_columns"`
 	Id          int       `sql:"id,pk"`
 	GitHostId   int       `sql:"git_host_id,notnull"`
 	EventType   string    `sql:"event_type,notnull"`
diff --git a/pkg/appStore/deployment/service/AppStoreDeploymentService.go b/pkg/appStore/deployment/service/AppStoreDeploymentService.go
@@ -1566,7 +1566,7 @@ func (impl AppStoreDeploymentServiceImpl) GetInstalledAppVersion(id int, userId
 	app, err := impl.installedAppRepository.GetInstalledAppVersion(id)
 	if err != nil {
 		if err == pg.ErrNoRows {
-			return nil, fmt.Errorf("values are outdated. please fetch the latest version and try again")
+			return nil, &util.ApiError{HttpStatusCode: http.StatusBadRequest, Code: "400", UserMessage: "values are outdated. please fetch the latest version and try again", InternalMessage: err.Error()}
 		}
 		impl.logger.Errorw("error while fetching from db", "error", err)
 		return nil, err
diff --git a/pkg/appStore/deployment/service/InstalledAppService.go b/pkg/appStore/deployment/service/InstalledAppService.go
@@ -919,11 +919,14 @@ func (impl *InstalledAppServiceImpl) FetchChartNotes(installedAppId int, envId i
 	installedApp, err := impl.installedAppRepository.FetchNotes(installedAppId)
 	if err != nil && err != pg.ErrNoRows {
 		return "", err
+	} else if err == pg.ErrNoRows {
+		impl.logger.Errorw("installed app not found or may have been deleted", "installedAppId", installedAppId, "envId", envId)
+		return "", &util.ApiError{HttpStatusCode: http.StatusBadRequest, Code: "400", UserMessage: "Installed app not found in database or may have been deleted", InternalMessage: err.Error()}
 	}
 	installedAppVerison, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppIdAndEnvId(installedAppId, envId)
 	if err != nil {
 		if err == pg.ErrNoRows {
-			return "", fmt.Errorf("values are outdated. please fetch the latest version and try again")
+			return "", &util.ApiError{HttpStatusCode: http.StatusBadRequest, Code: "400", UserMessage: "values are outdated. please fetch the latest version and try again", InternalMessage: err.Error()}
 		}
 		impl.logger.Errorw("error fetching installed  app version in installed app service", "err", err)
 		return "", err
diff --git a/pkg/appStore/deployment/tool/gitops/AppStoreDeploymentArgoCdService.go b/pkg/appStore/deployment/tool/gitops/AppStoreDeploymentArgoCdService.go
@@ -463,7 +463,7 @@ func (impl AppStoreDeploymentArgoCdServiceImpl) GetDeploymentHistory(ctx context
 	installedAppVersions, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppIdMeta(installedAppDto.InstalledAppId)
 	if err != nil {
 		if err == pg.ErrNoRows {
-			return nil, fmt.Errorf("values are outdated. please fetch the latest version and try again")
+			return nil, &util.ApiError{HttpStatusCode: http.StatusBadRequest, Code: "400", UserMessage: "values are outdated. please fetch the latest version and try again", InternalMessage: err.Error()}
 		}
 		impl.Logger.Errorw("error while fetching installed version", "error", err)
 		return result, err
diff --git a/pkg/chartRepo/repository/ChartRepoRepository.go b/pkg/chartRepo/repository/ChartRepoRepository.go
@@ -40,7 +40,7 @@ type ChartRepoFields struct {
 	AllowInsecureConnection bool                `sql:"allow_insecure_connection"`
 }
 type ChartRepo struct {
-	tableName struct{} `sql:"chart_repo"`
+	tableName struct{} `sql:"chart_repo" pg:",discard_unknown_columns"`
 	ChartRepoFields
 	sql.AuditLog
 }
diff --git a/pkg/externalLink/ExternalLinkIdentifierMappingRepository.go b/pkg/externalLink/ExternalLinkIdentifierMappingRepository.go
@@ -25,7 +25,7 @@ import (
 )
 
 type ExternalLinkIdentifierMapping struct {
-	tableName      struct{}      `sql:"external_link_identifier_mapping"`
+	tableName      struct{}      `sql:"external_link_identifier_mapping" pg:",discard_unknown_columns"`
 	Id             int           `sql:"id,pk"`
 	ExternalLinkId int           `sql:"external_link_id,notnull"`
 	Type           AppIdentifier `sql:"type,notnull"`
diff --git a/pkg/externalLink/ExternalLinkMonitoringToolRepository.go b/pkg/externalLink/ExternalLinkMonitoringToolRepository.go
@@ -23,7 +23,7 @@ import (
 )
 
 type ExternalLinkMonitoringTool struct {
-	tableName struct{} `sql:"external_link_monitoring_tool"`
+	tableName struct{} `sql:"external_link_monitoring_tool" pg:",discard_unknown_columns"`
 	Id        int      `sql:"id,pk"`
 	Name      string   `sql:"name,notnull"`
 	Icon      string   `sql:"icon,notnull"`
diff --git a/pkg/externalLink/ExternalLinkRepository.go b/pkg/externalLink/ExternalLinkRepository.go
@@ -23,7 +23,7 @@ import (
 )
 
 type ExternalLink struct {
-	tableName                    struct{} `sql:"external_link"`
+	tableName                    struct{} `sql:"external_link" pg:",discard_unknown_columns"`
 	Id                           int      `sql:"id,pk"`
 	ExternalLinkMonitoringToolId int      `sql:"external_link_monitoring_tool_id, notnull"`
 	Name                         string   `sql:"name,notnull"`
diff --git a/pkg/k8s/application/k8sApplicationService.go b/pkg/k8s/application/k8sApplicationService.go
@@ -14,6 +14,8 @@ import (
 	k8s2 "github.com/devtron-labs/common-lib/utils/k8s"
 	k8sCommonBean "github.com/devtron-labs/common-lib/utils/k8s/commonBean"
 	k8sObjectUtils "github.com/devtron-labs/common-lib/utils/k8sObjectsUtil"
+	"github.com/devtron-labs/devtron/internal/util"
+
 	yamlUtil "github.com/devtron-labs/common-lib/utils/yaml"
 	"github.com/devtron-labs/devtron/api/connector"
 	client "github.com/devtron-labs/devtron/api/helm-app"
@@ -425,6 +427,7 @@ func (impl *K8sApplicationServiceImpl) validateContainerNameIfReqd(valid bool, r
 func (impl *K8sApplicationServiceImpl) GetResourceInfo(ctx context.Context) (*bean3.ResourceInfo, error) {
 	pod, err := impl.K8sUtil.GetResourceInfoByLabelSelector(ctx, impl.aCDAuthConfig.ACDConfigMapNamespace, "app=inception")
 	if err != nil {
+		err = &util.ApiError{Code: "404", HttpStatusCode: 404, UserMessage: "error on getting resource from k8s"}
 		impl.logger.Errorw("error on getting resource from k8s, unable to fetch installer pod", "err", err)
 		return nil, err
 	}
diff --git a/pkg/pipeline/BuildPipelineConfigService.go b/pkg/pipeline/BuildPipelineConfigService.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"github.com/caarlos0/env"
+	"github.com/devtron-labs/common-lib/utils"
 	app2 "github.com/devtron-labs/devtron/internal/sql/repository/app"
 	"github.com/devtron-labs/devtron/internal/sql/repository/appWorkflow"
 	dockerRegistryRepository "github.com/devtron-labs/devtron/internal/sql/repository/dockerRegistry"
@@ -1236,6 +1237,7 @@ func (impl *CiPipelineConfigServiceImpl) handlePipelineCreate(request *bean.CiPa
 	}
 
 	if pipelineExists {
+		err = &utils.ApiError{Code: "400", HttpStatusCode: 400, UserMessage: "pipeline name already exist"}
 		impl.logger.Errorw("pipeline name already exist", "err", err, "patch cipipeline name", request.CiPipeline.Name)
 		return nil, fmt.Errorf(bean3.PIPELINE_NAME_ALREADY_EXISTS_ERROR)
 	}

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI`
`93`	`93`	`}`
`94`	`94`	`installedAppDto, err = handler.appStoreDeploymentServiceC.GetInstalledAppByClusterNamespaceAndName(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.ReleaseName)`
`95`	`95`	`if err != nil {`
`96`		`- err = &util.ApiError{HttpStatusCode: http.StatusInternalServerError, UserMessage: "unable to find app in database"}`
	`96`	`+ err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}`
`97`	`97`	`return appOfferingMode, installedAppDto, err`
`98`	`98`	`}`
`99`	`99`	`// this is the case when hyperion apps does not linked yet`
`@@ -119,7 +119,7 @@ func (handler *CommonDeploymentRestHandlerImpl) getAppOfferingMode(installedAppI`
`119`	`119`	`}`
`120`	`120`	`installedAppDto, err = handler.appStoreDeploymentServiceC.GetInstalledAppByInstalledAppId(installedAppId)`
`121`	`121`	`if err != nil {`
`122`		`- err = &util.ApiError{HttpStatusCode: http.StatusInternalServerError, UserMessage: "unable to find app in database"}`
	`122`	`+ err = &util.ApiError{HttpStatusCode: http.StatusBadRequest, UserMessage: "unable to find app in database"}`
`123`	`123`	`return appOfferingMode, installedAppDto, err`
`124`	`124`	`}`
`125`	`125`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -335,10 +335,12 @@ func (client *GrpcApiClientImpl) GetCommitMetadataForPipelineMaterial(ctx contex`
`335`	`335`	`GitTag: req.GitTag,`
`336`	`336`	`BranchName: req.BranchName,`
`337`	`337`	`})`
	`338`	`+`
`338`	`339`	`if err != nil {`
`339`	`340`	`return nil, err`
`340`	`341`	`}`
`341`		`- if res == nil {`
	`342`	`+`
	`343`	`+ if res.Commit == "" {`
`342`	`344`	`return nil, nil`
`343`	`345`	`}`
`344`	`346`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ type NotificationSettingsViewWithAppEnv struct {`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`type NotificationSettings struct {`
`76`		- tableName struct{} `sql:"notification_settings"`
	`76`	+ tableName struct{} `sql:"notification_settings" pg:",discard_unknown_columns"`
`77`	`77`	Id int `sql:"id,pk"`
`78`	`78`	TeamId *int `sql:"team_id"`
`79`	`79`	AppId *int `sql:"app_id"`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ import (`
`22`	`22`	`)`
`23`	`23`
`24`	`24`	`type WebhookEventData struct {`
`25`		- tableName struct{} `sql:"webhook_event_data"`
	`25`	+ tableName struct{} `sql:"webhook_event_data" pg:",discard_unknown_columns"`
`26`	`26`	Id int `sql:"id,pk"`
`27`	`27`	GitHostId int `sql:"git_host_id,notnull"`
`28`	`28`	EventType string `sql:"event_type,notnull"`
Original file line number	Diff line number	Diff line change
`@@ -1566,7 +1566,7 @@ func (impl AppStoreDeploymentServiceImpl) GetInstalledAppVersion(id int, userId`
`1566`	`1566`	`app, err := impl.installedAppRepository.GetInstalledAppVersion(id)`
`1567`	`1567`	`if err != nil {`
`1568`	`1568`	`if err == pg.ErrNoRows {`
`1569`		`- return nil, fmt.Errorf("values are outdated. please fetch the latest version and try again")`
	`1569`	`+ return nil, &util.ApiError{HttpStatusCode: http.StatusBadRequest, Code: "400", UserMessage: "values are outdated. please fetch the latest version and try again", InternalMessage: err.Error()}`
`1570`	`1570`	`}`
`1571`	`1571`	`impl.logger.Errorw("error while fetching from db", "error", err)`
`1572`	`1572`	`return nil, err`
Original file line number	Diff line number	Diff line change
`@@ -463,7 +463,7 @@ func (impl AppStoreDeploymentArgoCdServiceImpl) GetDeploymentHistory(ctx context`
`463`	`463`	`installedAppVersions, err := impl.installedAppRepository.GetInstalledAppVersionByInstalledAppIdMeta(installedAppDto.InstalledAppId)`
`464`	`464`	`if err != nil {`
`465`	`465`	`if err == pg.ErrNoRows {`
`466`		`- return nil, fmt.Errorf("values are outdated. please fetch the latest version and try again")`
	`466`	`+ return nil, &util.ApiError{HttpStatusCode: http.StatusBadRequest, Code: "400", UserMessage: "values are outdated. please fetch the latest version and try again", InternalMessage: err.Error()}`
`467`	`467`	`}`
`468`	`468`	`impl.Logger.Errorw("error while fetching installed version", "error", err)`
`469`	`469`	`return result, err`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ type ChartRepoFields struct {`
`40`	`40`	AllowInsecureConnection bool `sql:"allow_insecure_connection"`
`41`	`41`	`}`
`42`	`42`	`type ChartRepo struct {`
`43`		- tableName struct{} `sql:"chart_repo"`
	`43`	+ tableName struct{} `sql:"chart_repo" pg:",discard_unknown_columns"`
`44`	`44`	`ChartRepoFields`
`45`	`45`	`sql.AuditLog`
`46`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ import (`
`25`	`25`	`)`
`26`	`26`
`27`	`27`	`type ExternalLinkIdentifierMapping struct {`
`28`		- tableName struct{} `sql:"external_link_identifier_mapping"`
	`28`	+ tableName struct{} `sql:"external_link_identifier_mapping" pg:",discard_unknown_columns"`
`29`	`29`	Id int `sql:"id,pk"`
`30`	`30`	ExternalLinkId int `sql:"external_link_id,notnull"`
`31`	`31`	Type AppIdentifier `sql:"type,notnull"`