fix: API token generation api responses refactoring

Author: SATYAsasini

api/apiToken/ApiTokenRestHandler.go

@@ -18,11 +18,6 @@ package apiToken
 
 import (
 	"encoding/json"
-	"net/http"
-	"strconv"
-	"strings"
-	"time"
-
 	openapi "github.com/devtron-labs/devtron/api/openapi/openapiClient"
 	"github.com/devtron-labs/devtron/api/restHandler/common"
 	"github.com/devtron-labs/devtron/pkg/apiToken"
@@ -32,6 +27,8 @@ import (
 	"github.com/juju/errors"
 	"go.uber.org/zap"
 	"gopkg.in/go-playground/validator.v9"
+	"net/http"
+	"strconv"
 )
 
 type ApiTokenRestHandler interface {
@@ -105,70 +102,23 @@ func (impl ApiTokenRestHandlerImpl) CreateApiToken(w http.ResponseWriter, r *htt
 	err = decoder.Decode(&request)
 	if err != nil {
 		impl.logger.Errorw("err in decoding request in CreateApiToken", "err", err)
-		common.WriteJsonResp(w, errors.New("invalid JSON payload"), nil, http.StatusBadRequest)
+		common.WriteJsonResp(w, errors.New("invalid JSON payload "+err.Error()), nil, http.StatusBadRequest)
 		return
 	}
 
 	// validate request structure
 	err = impl.validator.Struct(request)
 	if err != nil {
-		impl.logger.Errorw("validation err in CreateApiToken", "err", err, "request", request)
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
-		return
-	}
-
-	// Comprehensive validation with specific error messages
-	if request.Name == nil || *request.Name == "" {
-		common.WriteJsonResp(w, errors.New("name field is required and cannot be empty"), nil, http.StatusBadRequest)
-		return
-	}
-
-	// Check name length (max 100 characters)
-	if len(*request.Name) > 100 {
-		common.WriteJsonResp(w, errors.New("name field cannot exceed 100 characters"), nil, http.StatusBadRequest)
+		impl.logger.Errorw("validation err in CreateApiToken ", "err", err, "request", request)
+		common.HandleValidationErrors(w, r, err)
 		return
 	}
 
-	// Check for invalid characters in name (spaces, commas)
-	if strings.Contains(*request.Name, " ") || strings.Contains(*request.Name, ",") {
-		common.WriteJsonResp(w, errors.New("name field cannot contain spaces or commas"), nil, http.StatusBadRequest)
-		return
-	}
-
-	// Check description length (max 350 characters as per UI)
-	if request.Description != nil && len(*request.Description) > 350 {
-		common.WriteJsonResp(w, errors.New("description field cannot exceed 350 characters"), nil, http.StatusBadRequest)
-		return
-	}
-
-	// Validate expireAtInMs field
-	if request.ExpireAtInMs != nil {
-		// Check if it's a valid positive timestamp
-		if *request.ExpireAtInMs <= 0 {
-			common.WriteJsonResp(w, errors.New("expireAtInMs must be a positive timestamp in milliseconds"), nil, http.StatusBadRequest)
-			return
-		}
-
-		// Check if it's not in the past (allow 1 minute buffer for clock skew)
-		currentTime := time.Now().UnixMilli()
-		if *request.ExpireAtInMs < (currentTime - 60000) {
-			common.WriteJsonResp(w, errors.New("expireAtInMs cannot be in the past"), nil, http.StatusBadRequest)
-			return
-		}
-
-		// Check if it's not too far in the future (max 10 years)
-		maxFutureTime := currentTime + (10 * 365 * 24 * 60 * 60 * 1000)
-		if *request.ExpireAtInMs > maxFutureTime {
-			common.WriteJsonResp(w, errors.New("expireAtInMs cannot be more than 10 years in the future"), nil, http.StatusBadRequest)
-			return
-		}
-	}
-
 	// service call
 	res, err := impl.apiTokenService.CreateApiToken(request, userId, impl.checkManagerAuth)
 	if err != nil {
 		impl.logger.Errorw("service err, CreateApiToken", "err", err, "payload", request)
-		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
+		common.WriteJsonRespV2(w, err, nil, http.StatusInternalServerError)
 		return
 	}
 	common.WriteJsonResp(w, err, res, http.StatusOK)

api/openapi/openapiClient/model_create_api_token_request.go

@@ -17,9 +17,11 @@
 package common
 
 import (
+	"errors"
 	"fmt"
 	"github.com/devtron-labs/devtron/api/middleware"
 	"github.com/devtron-labs/devtron/internal/util"
+	"gopkg.in/go-playground/validator.v9"
 	"net/http"
 	"strconv"
 )
@@ -226,3 +228,41 @@ func HandleValidationError(w http.ResponseWriter, r *http.Request, fieldName, me
 	apiErr := util.NewValidationErrorForField(fieldName, message)
 	WriteJsonResp(w, apiErr, nil, apiErr.HttpStatusCode)
 }
+
+// HandleValidationErrors handles multiple validation errors
+func HandleValidationErrors(w http.ResponseWriter, r *http.Request, err error) {
+	// validator.ValidationErrors is a slice
+	var vErrs validator.ValidationErrors
+	if errors.As(err, &vErrs) {
+		for _, fe := range vErrs {
+			field := fe.Field()
+			message := validationMessage(fe)
+			HandleValidationError(w, r, field, message)
+			return
+		}
+	}
+
+	// fallback: generic
+	HandleValidationError(w, r, "request", "invalid request payload")
+}
+func validationMessage(fe validator.FieldError) string {
+	switch fe.Tag() {
+	// validation tag for api token name
+	case "validate-api-token-name":
+		return fmt.Sprintf(
+			"%s must start and end with a lowercase letter or digit; may only contain lowercase letters, digits, '_' or '-' (no spaces or commas)",
+			fe.Field(),
+		)
+
+	// if a certain validator tag is not included in switch case then,
+	// we will parse the error as generic validator error,
+	// and further divide them on basis of parametric and non-parametric validation tags
+	default:
+		if fe.Param() != "" {
+			// generic parametric fallback (e.g., min=3, max=50)
+			return fmt.Sprintf("%s failed validation rule '%s=%s'", fe.Field(), fe.Tag(), fe.Param())
+		}
+		// generic non-parametric fallback (e.g., required, email, uuid)
+		return fmt.Sprintf("%s failed validation rule '%s'", fe.Field(), fe.Tag())
+	}
+}

api/restHandler/common/EnhancedErrorResponse.go

@@ -127,6 +127,120 @@ func WriteJsonResp(w http.ResponseWriter, err error, respBody interface{}, statu
 
 }
 
+// This version will have more cases handled for like for 409(till now)
+// can be used in multiple places for suitable status code in response
+// Made a new version for backwards compatibility
+func WriteJsonRespV2(w http.ResponseWriter, err error, respBody interface{}, status int) {
+	response := Response{}
+	if err == nil {
+		response.Result = respBody
+	} else if apiErr, ok := err.(*util.ApiError); ok {
+		response.Errors = []*util.ApiError{apiErr}
+		if apiErr.HttpStatusCode != 0 {
+			status = apiErr.HttpStatusCode
+		}
+	} else if validationErrs, ok := err.(validator.ValidationErrors); ok {
+		var valErrors []*util.ApiError
+		for _, validationErr := range validationErrs {
+			//validationErr
+			valErr := &util.ApiError{
+				UserMessage:     fmt.Sprint(validationErr),
+				InternalMessage: fmt.Sprint(validationErr),
+			}
+			valErrors = append(valErrors, valErr)
+		}
+		response.Errors = valErrors
+	} else if util.IsErrNoRows(err) {
+		status = http.StatusNotFound
+		// Try to extract resource context from respBody for better error messages
+		resourceType, resourceId := extractResourceContext(respBody)
+		if resourceType != "" && resourceId != "" {
+			// Create context-aware resource not found error
+			apiErr := util.NewResourceNotFoundError(resourceType, resourceId)
+			response.Errors = []*util.ApiError{apiErr}
+		} else {
+			// Fallback to generic not found error (no more "pg: no rows in result set")
+			apiErr := util.NewGenericResourceNotFoundError()
+			response.Errors = []*util.ApiError{apiErr}
+		}
+	} else if util.IsResourceConflictError(err) {
+		// Handles response for error due to resource with the same identifier already exists.
+		status = http.StatusConflict
+		// Try to extract resource context from respBody for better error messages
+		resourceType, resourceId := extractResourceContext(respBody)
+		if resourceType != "" && resourceId != "" {
+			// Create context-aware resource duplicate error
+			apiErr := util.NewDuplicateResourceError(resourceType, resourceId)
+			response.Errors = []*util.ApiError{apiErr}
+		} else {
+			// Fallback to generic resource duplicate error
+			apiErr := util.NewGenericDuplicateResourceError()
+			response.Errors = []*util.ApiError{apiErr}
+		}
+	} else if multiErr, ok := err.(*multierror.Error); ok {
+		var errorsResp []*util.ApiError
+		for _, e := range multiErr.Errors {
+			errorResp := &util.ApiError{
+				UserMessage:     e.Error(),
+				InternalMessage: e.Error(),
+			}
+			errorsResp = append(errorsResp, errorResp)
+		}
+		response.Errors = errorsResp
+	} else if errStatus, ok := err.(*errors2.StatusError); ok {
+		apiErr := &util.ApiError{}
+		apiErr.Code = strconv.Itoa(int(errStatus.ErrStatus.Code))
+		apiErr.InternalMessage = errStatus.Error()
+		apiErr.UserMessage = errStatus.Error()
+		response.Errors = []*util.ApiError{apiErr}
+	} else if unexpectedObjectError, ok := err.(*errors2.UnexpectedObjectError); ok {
+		apiErr := &util.ApiError{}
+		apiErr.InternalMessage = unexpectedObjectError.Error()
+		apiErr.UserMessage = unexpectedObjectError.Error()
+		response.Errors = []*util.ApiError{apiErr}
+	} else {
+		apiErr := &util.ApiError{}
+		apiErr.Code = "000" // 000=unknown
+		apiErr.InternalMessage = errors.Details(err)
+		if respBody != nil {
+			apiErr.UserMessage = respBody
+		} else {
+			apiErr.UserMessage = err.Error()
+		}
+		response.Errors = []*util.ApiError{apiErr}
+	}
+	response.Code = status //TODO : discuss with prashant about http status header
+	response.Status = http.StatusText(status)
+
+	b, err := json.Marshal(response)
+	if err != nil {
+		util.GetLogger().Errorw("error in marshaling err object", "err", err)
+		status = 500
+
+		response := Response{}
+		apiErr := &util.ApiError{}
+		apiErr.Code = "0000" // 000=unknown
+		apiErr.InternalMessage = errors.Details(err)
+		apiErr.UserMessage = "response marshaling error"
+		response.Errors = []*util.ApiError{apiErr}
+		b, err = json.Marshal(response)
+		if err != nil {
+			b = []byte("response marshaling error")
+			util.GetLogger().Errorw("Unexpected error in apiError", "err", err)
+		}
+	}
+	if status > 299 || err != nil {
+		util.GetLogger().Infow("ERROR RES", "TYPE", "API-ERROR", "RES", response.Code, "err", err)
+	}
+	w.Header().Set(CONTENT_TYPE, APPLICATION_JSON)
+	w.WriteHeader(status)
+	_, err = w.Write(b)
+	if err != nil {
+		util.GetLogger().Error(err)
+	}
+
+}
+
 // global response body used across api
 type Response struct {
 	Code   int              `json:"code,omitempty"`

api/restHandler/common/apiError.go

@@ -27,6 +27,7 @@ import (
 	"google.golang.org/grpc/status"
 	"net/http"
 	"strconv"
+	"strings"
 )
 
 type ApiError struct {
@@ -89,7 +90,19 @@ func (e *ApiError) ErrorfUser(format string, a ...interface{}) error {
 func IsErrNoRows(err error) bool {
 	return pg.ErrNoRows == err
 }
-
+func IsResourceConflictError(err error) bool {
+	var resourceConflictPhrases = []string{
+		"already exists",
+		"already used",
+	}
+	msg := err.Error()
+	for _, phrase := range resourceConflictPhrases {
+		if strings.Contains(msg, phrase) {
+			return true
+		}
+	}
+	return false
+}
 func GetClientErrorDetailedMessage(err error) string {
 	if errStatus, ok := status.FromError(err); ok {
 		return errStatus.Message()

internal/util/ErrorUtil.go

@@ -89,3 +89,13 @@ func NewGenericResourceNotFoundError() *ApiError {
 	).WithCode(constants.ResourceNotFound).
 		WithUserDetailMessage("The requested resource does not exist or has been deleted.")
 }
+
+// NewGenericDuplicateResourceError creates a generic conflict error when a resource already exists
+func NewGenericDuplicateResourceError() *ApiError {
+	return NewApiError(
+		http.StatusConflict,
+		"Resource already exists",
+		"resource already exists",
+	).WithCode(constants.DuplicateResource).
+		WithUserDetailMessage("The resource you are trying to create already exists. Please use a different name or identifier.")
+}